WIP: Associate PlAssessmentQuestions with Concepts (#116)

[pconrad]

Summary

• Backend half of Add database table to associate plAssessmentQuestions with Concepts #116: new PlAssessmentQuestionConcept join entity/repository/migration, and a PLAssessmentQuestionController with GET /{id}/concepts, POST /addConcept, DELETE /deleteConcept (422 on PL-instance mismatch per the issue's spec).
• Removes the now-superseded QuestionController stub (GET /api/questions/{questionId}/concepts, always returned []) that this issue was blocking on.
• Frontend not started yet (toggle button, graph highlighting wiring, tests/stories).

Status

This is a work-in-progress checkpoint, paused mid-implementation. Full context for resuming — what's done, what's left, and the key design decisions (e.g. why the new table's FKs use ON DELETE CASCADE instead of this codebase's usual manual-cleanup convention) — is written up in docs/issue118.md.

Test plan

• mvn compile / mvn test-compile pass
• mvn test -Dtest=PLAssessmentQuestionControllerTests,AssessmentControllerTests passes
• Full backend suite (mvn test) — not yet run, do this first when resuming
• Frontend implementation, tests, and manual verification — not started

🤖 Generated with Claude Code

[pconrad]

Use this prompt with Claude when we return:

Resume work on issue #116 (https://github.com/ucsb-cs156/proj-scaffold/issues/116), continuing from draft PR #117 on branch pc-Claude-issue116.

Start by checking out that branch and reading docs/issue118.md in full — it's a handoff doc I had a previous Claude session write, with everything needed to pick this up: what's done (backend: entity, repository, migration, controller, tests), what's not started (all of the frontend), and the key design decisions and why they were made.

First run the full backend test suite (mvn test) to confirm nothing broke — that was the one thing the previous session ran out of time to do. Then continue with the frontend work as scoped in the handoff doc. 

# Issue #116: Associate PlAssessmentQuestions with Concepts

## Context

Issue #116 (https://github.com/ucsb-cs156/proj-scaffold/issues/116) asks for a one-to-many
association between `PlAssessmentQuestion` (the join row that places a `PlQuestion` at a
specific ordinal within a `PlAssessment`) and `Concept`, exposed via `POST
/api/plAssessmentQuestion/addConcept` and a DELETE counterpart, with server-side validation
that the concept's course and the assessment's PrairieLearn instance match. On the frontend,
when a staff member has editing enabled and a question is selected in `ConceptGraphPage`, a
new "Assign Concepts" toggle should grey out the concept graph (reusing the existing
selection/ancestor-highlight mechanism) and let clicks tag/untag concepts for that question.

Research confirmed the UI scaffolding for this already exists and is already wired to live PL
data: `ConceptGraphPage.tsx`'s `QuestionSearch` dropdown is backed by `PlAssessmentQuestion`/
`PlQuestion` end-to-end (via `AssessmentController.getQuestions`), not the legacy `Question`
entity. The only missing piece on the frontend is the toggle + the tagging mutations. The one
open backend piece that already exists as an intentional stub is
`QuestionController.getQuestionConcepts` (`GET /api/questions/{questionId}/concepts`, always
returns `[]`) — its own comment says it's dark "until PL questions have their own concept
tagging." This issue is exactly that follow-up, so that stub (and its test) will be removed and
replaced by a real endpoint under the new `/api/plAssessmentQuestion` namespace.

## Backend

### New entity, repository, migration

Follow the `ConceptEdge`/`ConceptEdgeRepository`/migration-016 pattern
(`src/main/java/edu/ucsb/cs/scaffold/entity/ConceptEdge.java`,
`.../repository/ConceptEdgeRepository.java`,
`.../resources/db/migration/changes/016-create-concept-edges-table.json`), adapted to bridge
the raw-`Long`-FK `pl_*` family with the `@ManyToOne`-relation `Concept` world:

- **`PlAssessmentQuestionConcept`** entity (`.../entity/PlAssessmentQuestionConcept.java`),
  table `pl_assessment_question_concept`:
  - `id` (PK, identity)
  - `plAssessmentQuestionId` (raw `Long`, `nullable = false`) — matches the `pl_*` convention
    used by `PlAssessmentQuestion` itself (raw FK columns, no JPA relation), since this row is
    keyed off a sync-job-populated table
  - `concept` (`@ManyToOne Concept`, `JoinColumn concept_id`, `nullable = false`) — a real JPA
    relation, matching `Concept`/`ConceptEdge`'s own style, so `concept.getCourse()` is directly
    available for the validation logic below
  - unique constraint on `(pl_assessment_question_id, concept_id)`

- **`PlAssessmentQuestionConceptRepository`**:
  `Optional<PlAssessmentQuestionConcept> findByPlAssessmentQuestionIdAndConceptId(Long, Long)`,
  `List<PlAssessmentQuestionConcept> findByPlAssessmentQuestionId(Long)`.

- **Migration** `042-create-pl-assessment-question-concept-table.json`, same shape as
  migration 016: `createTable` (id, pl_assessment_question_id BIGINT NOT NULL, concept_id
  BIGINT NOT NULL), `addUniqueConstraint`, and two `addForeignKeyConstraint`s — to
  `pl_assessment_question(id)` and `concepts(id)` — **both with `"onDelete": "CASCADE"`**.

  This is a deliberate, called-out deviation from the codebase's usual convention (seen in
  `ConceptsController.deleteConceptArtifacts` and `SyncCourseWithPlRepoJob`/`PLRepoController`)
  of manually deleting dependent rows in Java before the parent, in FK-safe order. Doing that
  here would mean touching four existing, already-tested call sites (`PLRepoController
  .deletePlRepo`, and three cleanup spots in `SyncCourseWithPlRepoJob` — stale-question,
  stale-assessment, and per-assessment question-list reconciliation) purely to avoid FK
  violations on a table those methods have no other reason to know about. `ON DELETE CASCADE`
  keeps that risk (a forgotten call site crashing the sync job) off the table entirely, at the
  cost of one new pattern in this migration file. Net effect either way is the same: resyncing
  an assessment's question list deletes and re-inserts its `pl_assessment_question` rows, so any
  concept tags on those questions are lost on resync — this is an inherent consequence of how
  `SyncCourseWithPlRepoJob` already reconciles questions (delete-all-then-reinsert with fresh
  ids), not something introduced by this choice.

### New controller: `PLAssessmentQuestionController`

New file `.../controller/PLAssessmentQuestionController.java`, `@RequestMapping
("/api/plAssessmentQuestion")`, following `ConceptsController.postConceptEdge`/`deleteConceptEdge`'s
validate-then-act style:

- `GET /{plAssessmentQuestionId}/concepts` → `List<TaggedConceptDTO>`. No `@PreAuthorize`
  (matches the sibling read endpoints `AssessmentController.getAssessments`/`getQuestions` and
  the stub it replaces — visible to any authenticated user, since this drives the
  always-on highlighting, not just the editing-only assign mode). Returns `[]` if the
  `plAssessmentQuestionId` doesn't exist (mirrors `getAssessments`'s "not configured yet, not an
  error" style) rather than 404, since the frontend fires this query opportunistically whenever
  a question is selected.

- `POST /addConcept?plAssessmentQuestionId=&conceptId=` and `DELETE
  /deleteConcept?plAssessmentQuestionId=&conceptId=`, both `@PreAuthorize
  ("@CourseSecurity.hasConceptManagementPermissions(#root, #conceptId)")` — reusing the
  existing `CourseSecurity` method as-is (it already resolves the course from a concept id), so
  **no new `CourseSecurity` method or `TestCourseSecurity` stub is needed**.

  Validation logic in `addConcept` (per the issue's spec):
  ```java
  Concept concept = conceptRepository.findById(conceptId)
      .orElseThrow(() -> new EntityNotFoundException(Concept.class, conceptId));
  PlAssessmentQuestion paq = plAssessmentQuestionRepository.findById(plAssessmentQuestionId)
      .orElseThrow(() -> new EntityNotFoundException(PlAssessmentQuestion.class, plAssessmentQuestionId));
  PlAssessment assessment = plAssessmentRepository.findById(paq.getPlAssessmentId())
      .orElseThrow(() -> new EntityNotFoundException(PlAssessment.class, paq.getPlAssessmentId()));

  if (!Objects.equals(concept.getCourse().getPlInstanceId(), assessment.getPlInstanceId())) {
    throw new IllegalArgumentException(
        "concept's course PL instance does not match the assessment's PL instance");
  }

deleteConcept looks the join row up via findByPlAssessmentQuestionIdAndConceptId,
orElseThrow(EntityNotFoundException), then deletes it (no instance-mismatch check needed —
deleting an already-inconsistent tag is harmless).

422 status code: the issue explicitly asks for IllegalArgumentException → 422, but
ApiController (the base class every controller extends) already maps
IllegalArgumentException → 400 globally. Spring resolves a handler declared on the concrete
controller before one inherited from the base class, so PLAssessmentQuestionController
declares its own local override:

@ExceptionHandler(IllegalArgumentException.class)
@ResponseStatus(HttpStatus.UNPROCESSABLE_ENTITY)
public Object handleIllegalArgument(Throwable e) {
  return Map.of("type", e.getClass().getSimpleName(), "message", e.getMessage());
}

This only changes behavior for this one controller; every other controller keeps 400.

• TaggedConceptDTO reuses the existing frontend QuestionConcept shape (id, question_id,
  concept_id, subconcept_label) unchanged, so no frontend type changes are needed for the
  consuming side — question_id is populated with the plAssessmentQuestionId (as a string,
  field name is legacy/generic but unused by computeScaffoldSubgraph), and
  subconcept_label is always null (this issue only tags top-level concepts).

Remove the superseded stub

Delete QuestionController.java and QuestionControllerTests.java — confirmed via grep that
GET /api/questions/{questionId}/concepts has no other backend or frontend consumers once
ConceptGraphPage.tsx is repointed at the new endpoint (LegacyQuestionController is separate
and untouched).

Backend tests

New PLAssessmentQuestionControllerTests.java modeled on ConceptsControllerTests's edge
section: @WebMvcTest(controllers = PLAssessmentQuestionController.class), mocked
repositories, @WithInstructorCoursePermissions for the happy path, plain @WithMockUser for
the 403 case, anonymous for the other 403 case. Cases: GET returns tagged concepts / empty list
for unknown id; POST succeeds when instances match, 422 when they don't, 404 for unknown
concept/plAssessmentQuestion id, 403 without permissions; DELETE succeeds, 404 for an untagged
pair, 403 without permissions. Also a PlAssessmentQuestionConceptRepositoryTests if this
codebase has @DataJpaTest-style repository tests for ConceptEdgeRepository to mirror (will
check during implementation; skip if edges don't have one).

Frontend

Expose plAssessmentQuestionId to the client

AssessmentController.toQuestionDTO/QuestionDTO gains a field carrying the join row's own id
(join.getId()), JSON property pl_assessment_question_id. frontend/src/main/types /conceptGraph.ts's Question interface gains pl_assessment_question_id: string. This is the
only reason ConceptGraphPage.tsx needs to change what it fetches for questions — everything
else about that query is untouched.

Repoint the concepts-for-question query

In ConceptGraphPage.tsx, derive the selected question's join id and use it instead of
selectedQuestionId (a PlQuestion id) for the concepts query, since tagging is keyed by
plAssessmentQuestionId per the issue:

const selectedQuestion = questions.find((q) => q.id === selectedQuestionId);
const plAssessmentQuestionId = selectedQuestion?.pl_assessment_question_id;

const { data: questionConcepts = [] } = useBackend<QuestionConcept[]>(
  ["/api/plAssessmentQuestion", plAssessmentQuestionId, "concepts"],
  { method: "GET", url: `/api/plAssessmentQuestion/${plAssessmentQuestionId}/concepts` },
  [], false, { enabled: !!plAssessmentQuestionId },
);

The existing useEffect that turns questionConcepts into highlightedIds via
computeScaffoldSubgraph (ConceptGraphPage.tsx:789-811) is untouched — this is the exact
"grey out all concepts except selected + ancestors" mechanism the issue asks to reuse, and it
already reruns whenever questionConcepts changes (i.e., after a tag is added/removed and the
query is invalidated).

New component: AssignConceptsToggle

New file frontend/src/main/components/Scaffold/AssignConceptsToggle.tsx, styled like its
neighbors (AssessmentSelect, QuestionSearch — plain <button>, not react-bootstrap, per
existing ScaffoldTopBar convention), props active: boolean, disabled: boolean, onClick: () => void. Rendered in ScaffoldTopBar.tsx immediately after the QuestionSearch block,
gated on enableEditing (already available via useStaffTools() in that file) — visible
whenever editing is enabled and a question is selected, matching "whenever a specific
assessment question is selected, put a toggle button immediately to the right." Gets its own
test file and Storybook story (frontend/src/tests/components/Scaffold /AssignConceptsToggle.test.tsx, frontend/src/stories/components/Scaffold /AssignConceptsToggle.stories.tsx), per this repo's convention of one small top-bar component
per file with dedicated coverage.

Wiring in ConceptGraphPage.tsx

• New state const [assignConceptsMode, setAssignConceptsMode] = useState(false);, reset to
  false alongside the existing resets in the selectedAssessmentId/selectedQuestionId
  change effects (:768-776, :778-783) and in handleReset (:851-863).
• Two mutations via useBackendMutation (matching the createConceptMutation/
  deleteConceptMutation pattern), each invalidating the concepts query key on success:

  const addConceptToQuestionMutation = useBackendMutation<{ conceptId: string }>(
    ({ conceptId }) => ({
      method: "POST",
      url: "/api/plAssessmentQuestion/addConcept",
      params: { plAssessmentQuestionId, conceptId },
    }),
    { onSuccess: () => queryClient.invalidateQueries({ queryKey: ["/api/plAssessmentQuestion", plAssessmentQuestionId, "concepts"] }) },
  );
  // removeConceptFromQuestionMutation: same shape, method: "DELETE", url: ".../deleteConcept"

• assignedConceptIds = useMemo(() => new Set(questionConcepts.map((c) => c.concept_id)), [questionConcepts])
  — the set of directly tagged concepts, distinct from the ancestor-expanded highlightedIds,
  needed so a click on an ancestor-only-highlighted node (included via computeScaffoldSubgraph
  but not itself tagged) correctly POSTs rather than mistakenly attempting a DELETE.
• handleConceptClick branches at the top: if assignConceptsMode && plAssessmentQuestionId,
  call add or remove based on assignedConceptIds.has(id) and return — skipping the normal
  setSelectedConceptId/detail-toolbar behavior, since assign mode is a distinct interaction
  mode per the issue text ("any concept the user clicks on will result in a POST").
• Pass active={assignConceptsMode}, disabled={!selectedQuestionId} (editing-gate already
  handled by not rendering the button at all when !enableEditing), and
  onClick={() => setAssignConceptsMode((v) => !v)} from ScaffoldTopBar down to the new
  toggle.

Frontend tests

Extend frontend/src/tests/pages/ConceptGraphPage.test.tsx (existing mocked-ScaffoldConceptGraph
harness) with cases: toggling assign mode in editing mode with a question selected; clicking an
untagged concept POSTs and it becomes highlighted; clicking a tagged concept DELETEs and it
stops being highlighted; toggle is absent/disabled without editing enabled or without a
selected question. Update ScaffoldTopBar.test.tsx for the new button's presence/absence.

Verification

1. Backend: mvn test (or repo's equivalent) — specifically the new
   PLAssessmentQuestionControllerTests, and confirm QuestionControllerTests deletion doesn't
   leave dangling references (ScaffoldApplicationTests context-load test still passes with
   QuestionController removed).
2. Frontend: npm test for the updated/new test files; npm run storybook (or existing story
   check) to confirm AssignConceptsToggle.stories.tsx renders.
3. Manual: run the app (/run skill), open a course's ConceptGraphPage with editing enabled,
   select an assessment + question, toggle "Assign Concepts," click a few concept nodes to tag
   them (confirm graph greys out non-tagged concepts and POST calls succeed via Network tab),
   click a tagged node again to untag (confirm DELETE + graph updates), and confirm the toggle
   is hidden/disabled outside editing mode or with no question selected.