test(V2): remove model-level AEF tests from aef-t1..t5 specs

[TheLastCicada]

Summary

Removes 57 model-level tests (1,548 lines) from the five AEF V2 integration specs. These tests called AefTnXV2.create / findByPk directly and never reached the HTTP API — the same category removed from other V2 resource specs in #1743, which only took the blocks literally titled "CRUD Operations" and left Validation Tests, Foreign Key Tests, Association Tests, Edge Cases, and Business Logic Tests behind.

This is a pure deletion. No test was rewritten or replaced, and nothing under src/ changed. Migrating the surviving 27 API-level tests onto the runCrudStagingSuite factory from #1754 is a follow-up.

File	Lines before	Lines after	Tests before	Tests after
aef-t1-submission-v2.spec.js	537	324	19	8
aef-t2-authorizations-v2.spec.js	661	339	17	5
aef-t3-actions-v2.spec.js	840	421	17	5
aef-t4-holdings-v2.spec.js	736	400	16	5
aef-t5-authorized-entities-v2.spec.js	587	332	16	5
total	3,361	1,816	85	28

Why these tests were not pulling their weight

Most of them tested Sequelize, not CADT. should handle various date formats, should handle long text fields, should handle different party types, should handle different version formats and friends assert that Sequelize returns a value it was just handed. They pass regardless of anything CADT does.

20 of them could not fail. They use expect.fail() inside a try whose catch asserts only expect(error).to.exist. expect.fail throws an AssertionError, which the catch then accepts as proof of correct behavior, so the test passed whether the operation threw or succeeded. Reproduced against this repo's chai build:

create() succeeds (constraint not enforced) -> PASSED
create() throws  (constraint enforced)      -> PASSED

This set includes the entire model-level foreign-key rejection battery — 16 tests, 4 FKs each across t2/t3/t4/t5. Those were the tests that looked like the most valuable per-table coverage in the file, and they were proving nothing.

FK enforcement is genuinely covered at the layer that matters. Each of t2–t5 retains a real should reject … with invalid foreign key (non-existent) test that POSTs a bad cadTrustAefT1SubmissionId and asserts 400 plus an error containing does not exist (t2 :184, t3 :202, t4 :199, t5 :179 post-change).

One test was kept rather than deleted

aef-t1-submission-v2.spec.js keeps should reject AEF-T1-Submission with invalid date format. Unlike the rest, it exercises CADT code: aefT1SubmissionSubmissionDate has a hand-written setter that rejects non-ISO input with a bespoke message before Sequelize parses the value, and that message is asserted nowhere else in the repo.

// src/models/v2/aef-t1-submission-v2.modeltypes.js
set(value) {
  const isoDateRegex = /^\d{4}-\d{2}-\d{2}$/;
  if (!isoDateRegex.test(value)) {
    throw new Error('aefT1SubmissionSubmissionDate must be in ISO format (YYYY-MM-DD)');

The API layer does not reach it — Joi.date().iso() is a separate check with a different message — so the setter only guards the non-API write paths used by datalayer sync. Confirmed to be a real regression guard by relaxing the regex to /.*/, watching the test fail, then restoring it and watching it pass.

Fixture trimmed

The before hook in aef-t2-authorizations-v2.spec.js built an AefT5AuthorizedEntitiesV2 row that only the deleted FK and association tests consumed. With those gone the variable was write-only, so the fixture, the let, and the AefT5AuthorizedEntitiesV2 import are removed. Every other fixture in the Program → Project → Methodology → Verification → ProjectMethodology → Issuance → Unit → AefT1Submission chain is still read by the surviving POST/PUT/DELETE tests and is untouched.

Verification

• npm run test:v2: 1758 → 1701 passing, exactly the 57 removed, 5 pending unchanged, exit 0. Baseline re-measured on a clean tree to rule out overlap with the edits.
• npm run test:v1: 165 passing, 5 pending, exit 0 (unaffected; no v1 files touched).
• Title diff via mocha --dry-run --reporter json before and after: 57 titles removed, 0 added, 0 renamed. Every removed title belongs to one of the five targeted describe blocks; no API-level title changed.
• No HTTP-level assertion was touched: the per-file count of supertest(app) calls is identical before and after (9 / 7 / 7 / 7 / 7), and the deleted lines contain zero occurrences.
• npx eslint clean on all five files.
• Mutation check on the one retained test, as described above.

Coverage gaps this creates

Recording these as backlog rather than fixing them here, per the deletions-before-replacements split. Nothing below was covered by a test that could actually fail, except where noted.

Worth re-adding at the API layer:

• Required-field rejection for t2–t5. The deleted should reject … with missing required fields tests asserted SequelizeValidationError / notNull Violation and were non-vacuous. t1 already has an API-level equivalent; t2–t5 now have none at any layer. The factory migration adds a requiredFields battery, which covers this at the better layer.
• FK rejection for Unit / Project / T2 references. The surviving API test only covers the T1-submission FK.
• Association loading. The four deleted should load … with associations tests were the only consumers of the belongsTo aliases on the AEF models; no AEF controller currently builds a Sequelize include.
• Null-optional-field acceptance. The should accept … with optional fields null tests asserted every nullable column round-trips null. The live-API specs cover this via generate*Minimal() payloads, but only in the live suite.

Full list of the 57 removed test titles

AEF-T1-Submission — Business Logic Tests

- `should handle different party types` - `should handle different version formats` - `should handle NDC year ranges correctly`

AEF-T1-Submission — Edge Cases

- `should handle long text fields` - `should handle valid URL formats` - `should handle various date formats` - `should handle various year values`

AEF-T1-Submission — Validation Tests

- `should accept AEF-T1-Submission with optional fields null` - `should reject AEF-T1-Submission with invalid URL format` - `should reject AEF-T1-Submission with invalid year values` - `should reject AEF-T1-Submission with missing required fields`

AEF-T2-Authorizations — Association Tests

- `should load AEF-T2-Authorizations with associations`

AEF-T2-Authorizations — Edge Cases

- `should handle different picklist values` - `should handle long text fields` - `should handle various date formats`

AEF-T2-Authorizations — Foreign Key Tests

- `should accept AEF-T2-Authorizations with valid foreign keys` - `should reject AEF-T2-Authorizations with non-existent AEF-T1-Submission ID` - `should reject AEF-T2-Authorizations with non-existent AEF-T5-Authorized-Entities ID` - `should reject AEF-T2-Authorizations with non-existent Project ID` - `should reject AEF-T2-Authorizations with non-existent Unit ID`

AEF-T2-Authorizations — Validation Tests

- `should accept AEF-T2-Authorizations with optional fields null` - `should reject AEF-T2-Authorizations with invalid date format` - `should reject AEF-T2-Authorizations with missing required fields`

AEF-T3-Actions — Association Tests

- `should load AEF-T3-Actions with associations`

AEF-T3-Actions — Edge Cases

- `should handle different picklist values` - `should handle different year values` - `should handle various date formats`

AEF-T3-Actions — Foreign Key Tests

- `should accept AEF-T3-Actions with valid foreign keys` - `should reject AEF-T3-Actions with non-existent AEF-T1-Submission ID` - `should reject AEF-T3-Actions with non-existent AEF-T2-Authorizations ID` - `should reject AEF-T3-Actions with non-existent Project ID` - `should reject AEF-T3-Actions with non-existent Unit ID`

AEF-T3-Actions — Validation Tests

- `should accept AEF-T3-Actions with optional fields null` - `should reject AEF-T3-Actions with invalid date format` - `should reject AEF-T3-Actions with missing required fields`

AEF-T4-Holdings — Association Tests

- `should load AEF-T4-Holdings with associations`

AEF-T4-Holdings — Edge Cases

- `should handle different decimal quantities` - `should handle different picklist values` - `should handle different year values`

AEF-T4-Holdings — Foreign Key Tests

- `should accept AEF-T4-Holdings with valid foreign keys` - `should reject AEF-T4-Holdings with non-existent AEF-T1-Submission ID` - `should reject AEF-T4-Holdings with non-existent AEF-T2-Authorizations ID` - `should reject AEF-T4-Holdings with non-existent Project ID` - `should reject AEF-T4-Holdings with non-existent Unit ID`

AEF-T4-Holdings — Validation Tests

- `should accept AEF-T4-Holdings with optional fields null` - `should reject AEF-T4-Holdings with missing required fields`

AEF-T5-Authorized-Entities — Association Tests

- `should load AEF-T5-Authorized-Entities with associations`

AEF-T5-Authorized-Entities — Edge Cases

- `should handle different country values` - `should handle long text fields` - `should handle various date formats`

AEF-T5-Authorized-Entities — Foreign Key Tests

- `should accept AEF-T5-Authorized-Entities with valid foreign keys` - `should reject AEF-T5-Authorized-Entities with non-existent AEF-T1-Submission ID` - `should reject AEF-T5-Authorized-Entities with non-existent Project ID` - `should reject AEF-T5-Authorized-Entities with non-existent Unit ID`

AEF-T5-Authorized-Entities — Validation Tests

- `should accept AEF-T5-Authorized-Entities with optional fields null` - `should reject AEF-T5-Authorized-Entities with invalid date format` - `should reject AEF-T5-Authorized-Entities with missing required fields`

Not included

aef-cooperative-approach-migration-v2.spec.js (101 lines, 1 test) is left alone. Sharing the ~120-line fixture chain that t2–t5 duplicate is a separate change.

---

Note

Low Risk
Test-only deletions with no production code changes; API integration tests are preserved.

Overview
This PR removes ~1,548 lines and 57 model-level tests from the five AEF V2 integration specs (aef-t1-submission through aef-t5-authorized-entities). Those tests exercised Sequelize via direct Model.create / findByPk (validation, FK, association, edge-case, and business-logic blocks) and did not hit the HTTP API—aligned with earlier V2 spec cleanup in #1743.

What remains: API-level POST/PUT/DELETE (and t1 GET/list) coverage via supertest is unchanged. One model test stays on t1: invalid aefT1SubmissionSubmissionDate is rejected by the model setter’s ISO check, which Joi on the API path does not cover.

Fixture trim: aef-t2-authorizations-v2.spec.js drops unused AefT5AuthorizedEntitiesV2 setup and import after FK/association tests were removed.

No src/ changes; migrating survivors to runCrudStagingSuite is noted as follow-up.

^{Reviewed by Cursor Bugbot for commit 4856086. Bugbot is set up for automated code reviews on this repo. Configure here.}

[TheLastCicada]

Note on NOT NULL coverage for t2–t5

Flagging this explicitly since the gap backlog above mentions required-field rejection, and NOT NULL completeness on staged records is called out in .cursor/rules/v2.mdc:42-56 as sync-halting.

The deleted should reject … with missing required fields tests were not covering that invariant. They called AefTnXV2.create() directly, which never enters the staging path — they asserted Sequelize's own notNull enforcement on a direct model write, and no production code writes to these models outside the controllers.

The staging invariant is enforced by validateStagedRecord in src/utils/v2-staging-validation.js, which is table-generic: it resolves the Joi schema by model name from modelSchemaRegistry and derives NOT NULL and FK checks from Sequelize metadata, with no per-table branching. All five AEF models are registered there (:64-68), so they flow through the identical, already-tested code path that v2-staging-validation.spec.js exercises via project and program. StagingV2.assertChangeListNotNullCompleteness remains the commit-time backstop.

What is genuinely missing after this PR is an AEF-specific assertion that the right schema is wired up per table — registry wiring, not enforcement logic. That is worth having, and the factory migration adds it as a requiredFields battery at the API layer, which is the layer that matters. I would rather add it there once than hand-write four tests now that the migration would immediately replace.