Skip to content

Remove ordinalPosition from curriculum subject criteria — Design

Date: 2026-06-22 Status: Approved (Fabio, 2026-06-22) Type: Field removal (exempt from the formal Design Gate — no new entity/scope/RBAC primitive/cross-cutting concern/integration/chapter; matches the "add a field to a scope" recipe in reverse).

Problem

CurriculumSubjectCriterion.ordinalPosition forces the FE to send an explicit 1-based position on every criterion and adds a server-side contiguity rule. It is the only curriculum-config input DTO that carries a position — subjects, tracks, option blocks, and rules all derive their order server-side from the submitted array index. Criteria are the inconsistent outlier, and the field is the source of two avoidable validation errors:

  • Position: must be at least 1@Min(1) on the input field (VALIDATION_FAILED).
  • EVALUATION_CRITERIA_INVALID_ORDINAL_SEQUENCE — the 1..n contiguity check in the sync service.

We do not need ordering on criteria.

Decision

Criteria become an unordered set:

  • The client never sends a position. Criterion in = { id?, name }, out = { id, name }.
  • The ordinalPosition column is dropped (not kept as an internal substrate).
  • Reads return criteria sorted by name (orderBy: { name: 'asc' }) — deterministic, stable.
  • The duplicate-name rule (EVALUATION_CRITERIA_DUPLICATE_NAME) and the not-found rule (EVALUATION_CRITERION_NOT_FOUND) are kept.
  • The contiguity rule and its error code EVALUATION_CRITERIA_INVALID_ORDINAL_SEQUENCE are removed.

Migration strategy: single release (option A)

One migration drops the column; the code stops referencing it in the same PR.

This is chapter-12 Hazard #3 (DROP COLUMN): during a rolling deploy, old pods still SELECT ordinal_position until they recycle. Accepted because: the column lives on an admin-only curriculum-config read path (not a student-facing hot path), this is beta, and the strict two-release dance is disproportionate here. The column is NOT NULL with no default, so the two-release alternative would have required a DROP NOT NULL release first anyway — extra ceremony for a low-traffic surface.

A new migration is created (not folded into the uncommitted, unrelated 20260622120000_align_teacher_staff_diet_type_enum). The generated SQL is audited against the ch.12 hazard checklist before commit; expected to be a single ALTER TABLE "curriculum_subject_criteria" DROP COLUMN "ordinal_position";.

Blast radius

Area File Change
Schema prisma/schema.prisma (CurriculumSubjectCriterion) drop ordinalPosition Int @map("ordinal_position"); new migration
Input DTO src/curriculum/dto/scopes/subject-criterion.dto.ts drop field on SubjectCriterionInputDto + SubjectCriterionResponseDto; prune now-unused IsInt/Min imports
Struct types src/curriculum/interfaces/bulk-sync.interfaces.ts drop ordinalPosition from SubjectCriterionInput
Sync src/curriculum/curriculum-structure-sync.ts CriterionRow + CriterionRef types; toCriterionRefs sort by name, drop field from output; assertCriteriaShape drop the contiguity block (keep dup-name); syncSubjectCriteria onCreate/onUpdate drop ordinalPosition
Queries src/curriculum/curriculum.queries.ts orderBy: { ordinalPosition }{ name: 'asc' }; drop ordinalPosition: true from the criteria select
Errors src/common/constants/error-codes.ts, error-examples.ts remove EVALUATION_CRITERIA_INVALID_ORDINAL_SEQUENCE enum member + its params shape + its Swagger example (drift guard stays balanced)
Unit tests curriculum-configuration.dto.spec.ts, curriculum-structure-sync.spec.ts, curriculum.service.spec.ts drop ordinalPosition from criterion factories; delete below-1 / non-integer / contiguity tests; flip criteria-ordering assertions to by-name
E2E test/curricula.e2e-spec.ts drop ordinalPosition from defaultCriteria / grade1Override; asNameOrdinalPairs → name-only comparator
Docs docs/14-homerooms-subject-groups.md, docs/REFERENCE.md (if it references the field), evaluation-criteria spec note scrub criteria-ordinal references

Untouched: subject / track / option-block / rule ordinalPosition (they keep server-derived ordering). The curriculum.service.spec.ts subject-position test (~L411-426) stays. No scope/action/RBAC change. No setup-handler change (presets do not define criteria — they are added post-expansion via PATCH).

TDD approach

Red → green (test-driven-development skill):

  1. Update the spec layer first to assert the new contract — criterion shape {id,name}, name-sorted reads, contiguity rejection gone, dup-name rejection kept. Watch them fail.
  2. Make the source edits (DTOs → struct types → sync → queries → errors) until green.
  3. Migration last, after green: npx prisma migrate dev, then audit the generated SQL.

Verification commands (npm run build / npm test / npm run lint) are run by the user before commit, per the project verification-discipline rule.