fix(web): keep an assignment's row in place on write

web/src/domain/assignments.test.ts

@@ -36,6 +36,15 @@ import type { Assignment } from "@/types/classroom"
 import { REPO_PERMISSIONS, SUBMISSION_MODES } from "@/types/classroom"
 import type { SubmissionMode } from "@/types/classroom"
 
+// A row the write under test never touches, so a test can assert the touched
+// row keeps its position instead of moving to the end of the array.
+const neighborRow = (slug: string): Assignment => ({
+  slug,
+  name: `Homework ${slug}`,
+  mode: "individual",
+  autograder: "default",
+})
+
 const fullSource: Assignment = {
   slug: "hw1",
   name: "Homework 1",
@@ -491,13 +500,16 @@ describe("editAssignment (preserved-entry integration)", () => {
   // on the template-less path: ref read, commit read, assignments.json contents
   // read, then tree/commit/ref writes. classroom.json is absent (404) so the
   // archive guard reads the classroom as active.
-  function makeClient(entry: Assignment = existingEntry): {
+  function makeClient(
+    entry: Assignment = existingEntry,
+    ...neighbors: Assignment[]
+  ): {
     client: GitHubClient
     committedContent: () => string
   } {
     const assignmentsFile = {
       schema: "classroom50/assignments/v1",
-      assignments: [entry],
+      assignments: [entry, ...neighbors],
     }
     const b64 = (s: string) => Buffer.from(s, "utf-8").toString("base64")
 
@@ -627,6 +639,38 @@ describe("editAssignment (preserved-entry integration)", () => {
     expect(written.assignments[0].name).toBe("Homework 1 (edited)")
   })
 
+  it("edits the entry in place instead of moving it to the end", async () => {
+    const { client, committedContent } = makeClient(
+      existingEntry,
+      neighborRow("hw2"),
+    )
+
+    await editAssignment(client, editInput())
+
+    const written = JSON.parse(committedContent()) as {
+      assignments: Assignment[]
+    }
+    expect(written.assignments.map((a) => a.slug)).toEqual([SLUG, "hw2"])
+  })
+
+  it("rewrites only the first row when a manifest holds a duplicate slug", async () => {
+    // A hand-edited config repo can carry two rows for one slug. The edit is
+    // built from the first (`find`), so the second must be left alone rather
+    // than overwritten with a copy of its twin.
+    const duplicate: Assignment = { ...existingEntry, name: "Homework 1 (dup)" }
+    const { client, committedContent } = makeClient(existingEntry, duplicate)
+
+    await editAssignment(client, editInput())
+
+    const written = JSON.parse(committedContent()) as {
+      assignments: Assignment[]
+    }
+    expect(written.assignments.map((a) => a.name)).toEqual([
+      "Homework 1 (edited)",
+      "Homework 1 (dup)",
+    ])
+  })
+
   it.each([
     ["omitted", undefined, undefined],
     ["zero", 0, undefined],
@@ -4199,6 +4243,7 @@ describe("setAssignmentLock", () => {
     const assignmentsFile = {
       schema: "classroom50/assignments/v1",
       assignments: [
+        neighborRow("hw0"),
         {
           slug: SLUG,
           name: "Homework 1",
@@ -4207,6 +4252,7 @@ describe("setAssignmentLock", () => {
           ...(opts.locked ? { locked: true } : {}),
           ...(template ? { template } : {}),
         },
+        neighborRow("hw2"),
       ],
     }
     const classroomJson: Record<string, unknown> = {
@@ -4349,6 +4395,19 @@ describe("setAssignmentLock", () => {
     expect(grants()).toContain("classroom50-cs50")
   })
 
+  it("flips the flag in place instead of moving the row to the end", async () => {
+    const { client, committed } = makeLockClient({ locked: false })
+    await setAssignmentLock(client, {
+      org: ORG,
+      classroom: CLASSROOM,
+      slug: SLUG,
+      locked: true,
+    })
+    const written = JSON.parse(committed()!) as { assignments: Assignment[] }
+    // Matches the CLI's lock, which assigns back into the same index.
+    expect(written.assignments.map((a) => a.slug)).toEqual(["hw0", SLUG, "hw2"])
+  })
+
   it("makes a public template a UX-gate-only lock (no access change)", async () => {
     const { client, revokes } = makeLockClient({
       locked: false,
@@ -4417,6 +4476,7 @@ describe("setAssignmentClosed", () => {
     const assignmentsFile = {
       schema: "classroom50/assignments/v1",
       assignments: [
+        neighborRow("hw0"),
         {
           slug: SLUG,
           name: "Homework 1",
@@ -4425,6 +4485,7 @@ describe("setAssignmentClosed", () => {
           template: { owner: ORG, repo: "tmpl", branch: "main" },
           ...(opts.closed ? { closed: true } : {}),
         },
+        neighborRow("hw2"),
       ],
     }
     const classroomJson: Record<string, unknown> = {
@@ -4506,6 +4567,18 @@ describe("setAssignmentClosed", () => {
     expect(committed()).not.toContain(`"closed"`)
   })
 
+  it("flips the flag in place instead of moving the row to the end", async () => {
+    const { client, committed } = makeClosedClient({ closed: false })
+    await setAssignmentClosed(client, {
+      org: ORG,
+      classroom: CLASSROOM,
+      slug: SLUG,
+      closed: true,
+    })
+    const written = JSON.parse(committed()!) as { assignments: Assignment[] }
+    expect(written.assignments.map((a) => a.slug)).toEqual(["hw0", SLUG, "hw2"])
+  })
+
   it("no-ops when already in the requested state (no commit)", async () => {
     const { client, committed } = makeClosedClient({ closed: true })
     const result = await setAssignmentClosed(client, {

web/src/domain/assignments/createEdit.ts

@@ -159,6 +159,29 @@ const EDIT_MANAGED_ASSIGNMENT_KEYS = new Set<string>(
     .map(([key]) => key),
 )
 
+// Replace an existing entry by slug, keeping it at its position in the array.
+// The CLI's UpsertAssignment does the same ("Position preserved on replace"),
+// and `gh teacher assignment list` prints entries in file order — so a writer
+// that dropped the row and re-appended it would reorder the teacher's listing
+// and turn every one-field edit into a whole-row diff in the config repo.
+//
+// First match only, like UpsertAssignment: a hand-edited manifest can hold two
+// rows with one slug, and every caller built `entry` from the first of them
+// (`find`), so replacing both would overwrite the second row's own content.
+// Callers look the slug up and throw before calling this, so a miss returns the
+// array untouched rather than inventing a row.
+function replaceAssignmentEntry(
+  entries: Assignment[],
+  slug: string,
+  entry: Assignment,
+): Assignment[] {
+  const index = entries.findIndex((a) => a.slug === slug)
+  if (index === -1) return entries
+  const next = [...entries]
+  next[index] = entry
+  return next
+}
+
 // Copy forward entry-level keys the edit form doesn't manage (e.g.
 // `migrated_from`, unknown future keys) onto the rebuilt edit, without
 // overwriting managed keys. Mirrors the CLI's AssignmentEntry.Extra round-trip.
@@ -245,10 +268,11 @@ export async function editAssignment(
 
   const nextAssignments = {
     ...currentAssignments,
-    assignments: [
-      ...currentAssignments.assignments.filter((a) => a.slug !== slug),
+    assignments: replaceAssignmentEntry(
+      currentAssignments.assignments,
+      slug,
       preservedEntry,
-    ],
+    ),
   }
 
   const tree = await createGitTree(client, {
@@ -1311,10 +1335,11 @@ export async function setAssignmentLock(
 
     const nextAssignments: AssignmentsFile = {
       ...currentAssignments,
-      assignments: [
-        ...currentAssignments.assignments.filter((a) => a.slug !== slug),
+      assignments: replaceAssignmentEntry(
+        currentAssignments.assignments,
+        slug,
         updatedEntry,
-      ],
+      ),
     }
 
     const tree = await createGitTree(client, {
@@ -1452,10 +1477,11 @@ export async function setAssignmentClosed(
 
     const nextAssignments: AssignmentsFile = {
       ...currentAssignments,
-      assignments: [
-        ...currentAssignments.assignments.filter((a) => a.slug !== slug),
+      assignments: replaceAssignmentEntry(
+        currentAssignments.assignments,
+        slug,
         updatedEntry,
-      ],
+      ),
     }
 
     const tree = await createGitTree(client, {

web/src/domain/assignments/rename.ts

@@ -283,8 +283,9 @@ async function commitRenameConfig(
       })
     }
 
-    // Map in place so the entry keeps its position in the array (unlike the
-    // lock flip's filter+push, a rename shouldn't reorder the manifest).
+    // Map in place so the entry keeps its position in the array: a rename
+    // shouldn't reorder the manifest, same as the edit/lock/close writers in
+    // createEdit.ts and the CLI's UpsertAssignment.
     // Lock for the fan-out window: an accept mid-rename would mint a fresh
     // empty repo at the NEW name and 422 the real repo's rename.
     const nextAssignments: AssignmentsFile = {