Skip to content

feat(cv-sections): add best-effort bulk-create endpoint for education/experience #174

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
datvt243 merged 1 commit into from
Sep 28, 2026
Merged

feat(cv-sections): add best-effort bulk-create endpoint for education/experience #174

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
145 changes: 145 additions & 0 deletions agent-hub/evidence/implementer/2026-09-28/add-bulk-import-cv-sections-plan.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,145 @@
# 2026-09-28 — add-bulk-import-cv-sections (implementer plan)

- Worker: implementer
- Version: 0.1.0
- Node: `add-bulk-import-cv-sections` (`haven/diagrams/dev-loop.prime-mermaid.md`)
- Task (verbatim): Bulk import endpoint for CV sections (issue #161): Add a
bulk-create endpoint per CV section (e.g. POST /api/v1/education/bulk,
POST /api/v1/experience/bulk, ...), accepting an array of entries and
creating them in one request under the authenticated candidate's
candidateId (never client-supplied, same IDOR-safe pattern as every other
write path). Natural pairing with the LinkedIn-export-parse flow: parse ->
review client-side -> bulk-save. Open questions to resolve during
implementation: which sections need it first (likely education/experience
only, matching what LinkedIn export currently parses), partial-failure
behavior (all-or-nothing vs best-effort per-item report), and whether to
reuse createCrudService() (BaseService.ts) with a new handlerBulkCreate or
a dedicated bulk-only path. Acceptance criteria: bulk endpoint(s) create
multiple entries under the authenticated candidate only; validation
applies per-entry (same Joi schemas as existing single-create routes);
response reports what succeeded/failed per item (if best-effort) or a
single success (if transactional).

## Hub bytes before: 97923

## Open questions resolved
- Scope: education + experience only (issue's own recommendation — matches
what `parseLinkedInExport.service.ts`/#141 currently parses). Not applied
to the other 5 CV-section-like collections (award/certificate/project/
reference/generalInformation) or application/profile — own follow-up node
if a section beyond LinkedIn's scope needs it.
- Partial-failure behavior: best-effort, per-item report — matches the
acceptance criteria's "if best-effort" branch and how `baseGetAll`
already reports partial states (pagination) rather than an all-or-nothing
transaction, which Mongoose (no multi-document ACID transaction already
wired anywhere in this codebase) would add real new complexity for.
- Reuse vs dedicated path: reused `createCrudController()`
(`BaseController.ts`) with a new `fnBulkCreate`, calling the SAME
`service.handlerCreate` each existing single-create route already calls
and validating each item against the SAME Joi `schema` each section
already passes in. No changes needed to `BaseService.ts` or
`services/index.ts` — `baseCreateDocument` already validates+creates one
document at a time correctly, looping over it per item was sufficient.

## Diff
| File | Why |
|---|---|
| `src/candidate_profile/BaseController.ts` | New `MAX_BULK_ITEMS = 100` cap + new `fnBulkCreate` returned from `createCrudController()`. Forces `candidateId` from `(req as any).user?._id` onto every array item before validation (verifyToken only forces `req.body.candidateId` at the top level, never touching entries nested inside `req.body.items` — same IDOR-safe pattern as every other write path, applied at the per-item level here). Validates each item with the section's own Joi `schema`, calls the section's own `service.handlerCreate` per valid item, collects `{index, ...result}` per item, returns `{results, summary: {total, succeeded, failed}}`. |
| `src/candidate_profile/education/education.controller.ts` | Destructure+export the new `fnBulkCreate` alongside the existing `fnCreate`/`fnUpdate`. |
| `src/candidate_profile/experience/experience.controller.ts` | Same. |
| `src/routers/api/v1/education.route.ts` | New `POST /bulk` route + swagger doc, mounted before `/update` (no path collision with existing routes). |
| `src/routers/api/v1/experience.route.ts` | Same. |
| `src/locales/en.ts`, `src/locales/vi.ts` | 2 new `common.*` i18n keys: `bulkNoItems`, `bulkTooManyItems` (for the 2 new 400 rejection paths — no items sent / more than 100 items in one request). |
| `src/__tests__/candidate_profile/BaseController.test.ts` | 5 new tests for `fnBulkCreate` (new `describe` block, existing `baseGetAll` tests untouched). |
| `agent-hub/haven/diagrams/dev-loop.prime-mermaid.md` | New PENDING row for this node, appended at the end of the PM status table (AppendOnly). |

## Bug found during implementation
`src/utils/helper.ts`'s `formatResponse()` (lines ~178-186) nulls out
`data` whenever the response envelope's `success` is `false`:
```ts
const getData = (() => {
if (!success) return null;
...
return data;
})();
```
A naive `fnBulkCreate` that set the envelope `success: summary.failed === 0`
would have silently dropped `results`/`summary` from the response body on
every partial failure — exactly the one case the acceptance criteria
("response reports what succeeded/failed per item") needs it most. Fixed
in this diff by keeping the envelope `success: true` always (the bulk
request itself was processed successfully; per-item pass/fail lives in
`results[].success` and `summary`, not the envelope) — commented in both
the implementation and the regression test that asserts it
(`BaseController.test.ts`, "is best-effort: one invalid item does not
block the others, and results/summary are still returned on partial
failure"). Not added to `doctrine/domains/PROJECT.md`'s Traps table — this
is pre-existing infra behavior every OTHER caller already works around by
never setting `success: false` with a real `data` payload; flagging it
here as a footgun for any future non-bulk caller that tries to do the same
is a documentation call for the operator, not fixed in this diff (would be
a behavior change to `formatResponse()` itself, out of scope for #161).

## Command
`npm test` (from `/Users/_david/Workspace/Project/resume/resume-nodejs-api`)

## Output (verbatim, tail)
```
PASS src/__tests__/candidate_profile/BaseController.test.ts
baseGetAll
✓ passes page/limit/sort through as numbers/string when present (1 ms)
✓ omits page/limit/sort when the query string has none (backward compatible)
✓ silently drops a sort value that could smuggle a Mongo operator (1 ms)
✓ accepts a leading "-" in sort for descending order
createCrudController -> fnBulkCreate (issue #161)
✓ rejects with 400 when items is missing or not an array
✓ rejects with 400 when items exceeds the 100-item cap
✓ forces candidateId from the authenticated user onto every item, ignoring a client-supplied value (IDOR-safe) (1 ms)
✓ is best-effort: one invalid item does not block the others, and results/summary are still returned on partial failure (2 ms)
✓ reports summary.failed: 0 when every item succeeds

A worker process has failed to exit gracefully and has been force exited. This is likely caused by tests leaking due to improper teardown. Try running with --detectOpenHandles to find leaks. Active timers can also cause this, ensure that .unref() was called on them.
Test Suites: 29 passed, 29 total
Tests: 155 passed, 155 total
Snapshots: 0 total
Time: 9.312 s
Ran all test suites.
```
(The "worker process failed to exit gracefully" warning is a pre-existing
Jest/open-handle notice unrelated to this diff — every suite still passed,
0 failures.)

Also ran `npm run build` (tsc && copy) — clean, no output beyond the copy
step, no typecheck errors.

## Acceptance
| Criterion | Evidence |
|---|---|
| Bulk endpoint(s) create multiple entries under the authenticated candidate only | `fnBulkCreate` forces `candidateId` from `(req as any).user?._id` onto every item before validation, never trusting a client-supplied value — `BaseController.ts` lines in the `fnBulkCreate` block; regression test "forces candidateId from the authenticated user onto every item, ignoring a client-supplied value (IDOR-safe)" — `Tests: 155 passed, 155 total` includes this test |
| Validation applies per-entry (same Joi schemas as existing single-create routes) | `fnBulkCreate` calls `validateSchema({ schema, item: {...items[index], candidateId}, lang })` per item using the SAME `schema` param already passed to `createCrudController()` for `/create` — no new schema. Regression test "is best-effort: one invalid item does not block the others..." proves an invalid item (`{name: 'x'}`, fails `min(2)`) is rejected per-item while the valid sibling item still reaches `service.handlerCreate` |
| Response reports what succeeded/failed per item (best-effort) | `data: { results, summary }` where `results[i]` carries `{index, success, message, data/errors}` per item and `summary = {total, succeeded, failed}`. Regression tests "is best-effort..." (`summary: {total:2, succeeded:1, failed:1}`) and "reports summary.failed: 0 when every item succeeds" (`summary: {total:2, succeeded:2, failed:0}`) both pass |

## Noticed, not done
- Scope limited to education/experience — the other CV-section-like
collections (award/certificate/project/reference/generalInformation,
application, profile) don't have a bulk-create route. Matches the
issue's own open question guidance ("likely education/experience only");
own follow-up node if a wider section needs it.
- No transactional (all-or-nothing) mode — best-effort only. The issue's
acceptance criteria explicitly allows either; best-effort was chosen
since no Mongo multi-document transaction wiring exists anywhere else in
this codebase (would be new infrastructure, not proportionate to this
task).
- `formatResponse()`'s success-nulls-data behavior (see "Bug found during
implementation" above) is pre-existing, not touched — flagged for the
operator as a possible footgun for a future caller, not a regression
introduced here.

## Seal gate
Not applicable to this recipe step — no outward-facing action (commit,
push, delete, external API call) has happened yet. All changes are in the
local working tree on branch `161-bulk-import-endpoint`, unstaged/staged
but not committed. The `src/` diff itself was shown in full in the session
transcript, matching the seal-gate spirit even though nothing outward-
facing has been requested yet — commit/push waits for `/ship` (not invoked
this round; `/todo` was run without `--ship`).
152 changes: 152 additions & 0 deletions agent-hub/evidence/verifier/2026-09-28/add-bulk-import-cv-sections-seal.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,152 @@
# 2026-09-28 — add-bulk-import-cv-sections (verifier verdict)

- Worker: verifier (subagent, dispatched via Agent tool)
- Node: `add-bulk-import-cv-sections` (`haven/diagrams/dev-loop.prime-mermaid.md`)
- New PM status: SEALED

## Isolation proof
Dispatched as a fresh Agent-tool subagent with the task description "You
are the `verifier` worker in this repo's agent-hub... Run `verify_seal`
for node `add-bulk-import-cv-sections`" — a spawn string the implementer
session never saw. No conversation history with the implementer pass;
every fact below was re-derived by reading the working tree, the note,
and re-running `npm test`/`npm run build` in this session, not carried
over from any prior context.

## Reasoning
1. **Diff scope** — `git status --short` on branch `161-bulk-import-endpoint`
showed exactly the 9 files the note claims (`agent-hub/haven/diagrams/
dev-loop.prime-mermaid.md`, `src/__tests__/candidate_profile/
BaseController.test.ts`, `src/candidate_profile/BaseController.ts`,
`src/candidate_profile/education/education.controller.ts`,
`src/candidate_profile/experience/experience.controller.ts`,
`src/locales/en.ts`, `src/locales/vi.ts`, `src/routers/api/v1/
education.route.ts`, `src/routers/api/v1/experience.route.ts`) plus
the untracked evidence note. Nothing else touched; no commit/push.

2. **`BaseController.ts` — `fnBulkCreate` (read in full)**:
- `const candidateId = (req as any).user?._id;` then every item is
validated as `{ ...items[index], candidateId }` — client-supplied
`candidateId` inside an array item is overwritten before
`validateSchema` ever sees it. Confirmed this is genuinely necessary
(not redundant with `verifyToken`) by reading `verifyToken.
middleware.ts` — it only assigns `req.body.candidateId = req.user._id`
at the top level, never touches nested array entries.
- Uses the SAME `schema` param already passed into
`createCrudController()` for `fnCreate`/`fnUpdate` — no second/looser
schema introduced.
- Calls `service.handlerCreate` per item — the identical service call
`fnCreate` uses. No bypass of `BaseService.ts`/`services/index.ts`.
Read `baseCreateDocument` in `services/index.ts`: it already returns
`{ success, message, data, errors }`, which is exactly the shape
`fnBulkCreate` spreads into `results[]` and filters on
`r.success` — consistent, not assumed.
- `MAX_BULK_ITEMS = 100` hard cap; `!items || !items.length` and
`items.length > MAX_BULK_ITEMS` both return 400 via `t('common.
bulkNoItems'|'bulkTooManyItems', lang)`.
- **Bug-avoidance claim independently verified, not trusted from the
note**: read `utils/helper.ts`'s `formatResponse()` end-to-end —
`const getData = (() => { if (!success) return null; ... })()`
really does null `data` whenever the envelope's `success` is falsy.
Read `fnBulkCreate`'s final `formatReturn` call — `success: true`
is hardcoded, never `summary.failed === 0`, with the comment
explaining why. Traced `formatReturn` → `formatResponse` to confirm
the real call chain, not just the test mock. Had `fnBulkCreate` used
`summary.failed === 0` instead, a partial-failure response would
have `data: null`, breaking the "response reports what succeeded/
failed per item" acceptance criterion — this was genuinely avoided.

3. **Controllers** — `education.controller.ts` and `experience.
controller.ts` both now `export const { fnCreate, fnUpdate,
fnBulkCreate } = createCrudController({...})` — the `createCrudController`
call itself is otherwise unchanged (same `schema`/`service`/
`booleanDefaultField` args), only the destructured export set grew.

4. **Routes** — both `education.route.ts` and `experience.route.ts` add
`router.post('/bulk', fnBulkCreate)` (plus a swagger block), positioned
between `/create` and `/update` — no collision with `/`, `/create`,
`/update`, `/delete/:id`, `/restore/:id`. Confirmed in `routers/api/v1/
index.ts` that both routers are mounted `router.use('/education',
verifyToken, routeEducation)` / `router.use('/experience', verifyToken,
routeExperience)` — `req.user._id` is genuinely populated by the time
`fnBulkCreate` runs.

5. **i18n** — `common.bulkNoItems`/`common.bulkTooManyItems` exist in both
`src/locales/en.ts` and `src/locales/vi.ts` (grep-confirmed at line
47-48 in both files) and are the exact keys referenced by
`fnBulkCreate`'s two 400 branches.

6. **Tests** — read `src/__tests__/candidate_profile/BaseController.test.ts`
in full (143 lines). The 4 pre-existing `baseGetAll` tests are present
and unchanged. The new `describe('createCrudController -> fnBulkCreate
(issue #161)')` block has exactly 5 tests, each doing real assertion
work, not just "was called":
- missing/non-array `items` -> 400, `handlerCreate` never called.
- 101 items -> 400, `handlerCreate` never called.
- a client-supplied `candidateId: 'someone-elses-id'` on the one item
is overwritten — `expect(handlerCreate).toHaveBeenCalledWith(
expect.objectContaining({ candidateId: 'real-user' }), 'en')` is a
concrete, non-vacuous IDOR assertion.
- one invalid item (`name: 'x'`, fails `min(2)`) + one valid item ->
`handlerCreate` called exactly once, `res.status(201)`,
`payload.data.summary` equals `{ total: 2, succeeded: 1, failed: 1 }`,
`results[0]`/`results[1]` assert `success: true`/`false` respectively.
Note this test does NOT mock `@/utils` — only `@/services` is
`jest.mock`ed — so `payload.success === true` on this partial-failure
path is exercising the REAL `formatReturn`/`formatResponse` chain,
genuinely proving the bug-avoidance claim in criterion 2 above, not
just a mocked assertion.
- all-success case: `summary` equals `{ total: 2, succeeded: 2,
failed: 0 }`.

7. **`npm test` — independently re-run** (not audit-only; see Re-run
below) from `/Users/_david/Workspace/Project/resume/resume-nodejs-api`:
```
Test Suites: 29 passed, 29 total
Tests: 155 passed, 155 total
Snapshots: 0 total
Time: 6.273 s, estimated 9 s
Ran all test suites.
```
Matches the note's claimed `29 passed, 29 total` / `155 passed, 155
total` exactly. (Same pre-existing "worker process failed to exit
gracefully" Jest open-handle notice as the note describes, unrelated
to this diff.)

8. **`npm run build` — independently re-run**: `tsc && npm run copy`
completed with no output beyond the `cp -R ./src/views ./src/public
./dist/` copy step — clean, no typecheck errors.

9. **Traps/invariants** (`doctrine/domains/PROJECT.md`) — this diff does
not touch `QuerySafe`, bcrypt, the Chrome path, CORS, or body-size
limit traps. It still goes through Joi validation (`schema` param,
unchanged), still forces `candidateId` server-side (now per-item, on
top of the existing top-level force), and never trusts raw
`req.body.items[i].candidateId`. No new trap introduced; none of the
existing 7 Traps table rows are reintroduced.

10. **Diagram AppendOnly** — before this verdict, the `add-bulk-import-
cv-sections` row was the LAST row of the PM status table, immediately
before the "Any regression must be a new node" closing note —
confirmed by reading the file directly (not inferred), so the
implementer appended correctly, not mid-table.

## Re-run
`full` — re-ran both `npm test` and `npm run build` independently from
scratch (not audit-only), because the task explicitly directed
independent verification of the real source (not just the note's prose)
for a change that alters the codebase's IDOR-safety surface (per-item
`candidateId` forcing on a new write path) — the same class of
security-sensitive diff this hub has previously chosen full re-run for
(e.g. `add-csrf-protection-auth-cookies`, `add-cv-profile-selection`).
Also independently read every file in the diff plus the files the note's
claims depend on (`verifyToken.middleware.ts`, `routers/api/v1/index.ts`,
`utils/helper.ts`, `services/index.ts`) rather than only auditing the
note's prose.

## Hub bytes
`hub_bytes_before: 97923` (from the implementer note)
`hub_bytes_after: 100436` (measured after updating PM status to SEALED,
same 5-category `/hub-tokens` per-session-total formula: root .md files +
doctrine/ + active `haven/diagrams/` file + implementer worker bundle +
verifier worker bundle, raw byte counts summed)
Loading
Loading