RollNote/rollnote-api
Watch
1
0
Fork
You've already forked rollnote-api
 0
Code Issues Pull requests Projects Releases 10 Packages Wiki Activity Actions

chore: simplify codebase reuse, quality, and efficiency #35

Merged
eliott merged 13 commits from chore/simplify-cleanup into develop 2026-05-07 16:04:27 +02:00
Conversation 0 Commits 13 Files changed 84 +5830 -1137
eliott commented 2026-05-06 23:02:54 +02:00
Owner

Summary

Codebase-wide simplify pass: extract shared utilities, fix duplicated patterns, address efficiency wins. No behavior changes intended.

What changed

New shared utility src/common/db-helpers.ts: `ownerOrGlobal`, `ilikeContains`, `isUniqueViolation`, `awaitAllOrThrowFirst`.

Reuse

  • Replaced `or(eq(createdBy), eq(isGlobal))` pattern in 5 services (iso, weather, film-format, shutter-speed, film) with `ownerOrGlobal()`.
  • Replaced `ilike(col, '%search%')` in 5 services (filter, film, camera, lens, shoot) with `ilikeContains()`.
  • Pulled `PG_UNIQUE_VIOLATION` / `isUniqueViolation` out of frame.service into the shared helper.
  • Replaced 8-line `Promise.allSettled` boilerplate in frame + shoot with `awaitAllOrThrowFirst()`.

Efficiency

  • Fixed O(N*M) filtering in changelog.service: `compose` now receives pre-grouped items via new `fetchItemsByEntry` (Map<entryId, items[]>).

Quality

  • Collapsed export.service 5-way ternary chain (52 lines -> 16) with a local `ownedOrReferenced` helper.
  • Deduplicated frame-location upsert `set` clause; added `.limit(1)` to ownership query.
  • Replaced em-dashes with regular hyphens (project rule).

Deferred

See `docs/simplify-followups.md` for items intentionally not addressed - notably batching the per-row inserts in `import.service.commitData` (~10-15x faster on large imports) and removing the duplicate `computeCounts` call from `import.commit`. Both need targeted review.

Test plan

  • `pnpm exec tsc --noEmit` clean
  • `pnpm lint` clean
  • `pnpm build` clean
  • `pnpm test` (4/4) pass
  • Smoke-test list endpoints for the 10 services touched (films, filters, cameras, lenses, isos, weathers, film-formats, shutter-speeds, shoots, frames) - verify search + pagination still work
  • Smoke-test changelog list/listSince/findOne endpoints - verify items still attached to the right entries
  • Smoke-test export endpoint - verify reference tables still scoped correctly (own + referenced globals)
  • Smoke-test frame-location create/update endpoints (upsert path)
  • Smoke-test frame create endpoint - verify unique-constraint 409 path still triggers correctly

🤖 Generated with Claude Code

## Summary Codebase-wide simplify pass: extract shared utilities, fix duplicated patterns, address efficiency wins. No behavior changes intended. ## What changed **New shared utility** `src/common/db-helpers.ts`: \`ownerOrGlobal\`, \`ilikeContains\`, \`isUniqueViolation\`, \`awaitAllOrThrowFirst\`. ### Reuse - Replaced \`or(eq(createdBy), eq(isGlobal))\` pattern in 5 services (iso, weather, film-format, shutter-speed, film) with \`ownerOrGlobal()\`. - Replaced \`ilike(col, '%search%')\` in 5 services (filter, film, camera, lens, shoot) with \`ilikeContains()\`. - Pulled \`PG_UNIQUE_VIOLATION\` / \`isUniqueViolation\` out of frame.service into the shared helper. - Replaced 8-line \`Promise.allSettled\` boilerplate in frame + shoot with \`awaitAllOrThrowFirst()\`. ### Efficiency - Fixed O(N\*M) filtering in changelog.service: \`compose\` now receives pre-grouped items via new \`fetchItemsByEntry\` (Map<entryId, items[]>). ### Quality - Collapsed export.service 5-way ternary chain (52 lines -> 16) with a local \`ownedOrReferenced\` helper. - Deduplicated frame-location upsert \`set\` clause; added \`.limit(1)\` to ownership query. - Replaced em-dashes with regular hyphens (project rule). ### Deferred See \`docs/simplify-followups.md\` for items intentionally not addressed - notably batching the per-row inserts in \`import.service.commitData\` (~10-15x faster on large imports) and removing the duplicate \`computeCounts\` call from \`import.commit\`. Both need targeted review. ## Test plan - [x] \`pnpm exec tsc --noEmit\` clean - [x] \`pnpm lint\` clean - [x] \`pnpm build\` clean - [x] \`pnpm test\` (4/4) pass - [ ] Smoke-test list endpoints for the 10 services touched (films, filters, cameras, lenses, isos, weathers, film-formats, shutter-speeds, shoots, frames) - verify search + pagination still work - [ ] Smoke-test changelog list/listSince/findOne endpoints - verify items still attached to the right entries - [ ] Smoke-test export endpoint - verify reference tables still scoped correctly (own + referenced globals) - [ ] Smoke-test frame-location create/update endpoints (upsert path) - [ ] Smoke-test frame create endpoint - verify unique-constraint 409 path still triggers correctly 🤖 Generated with [Claude Code](https://claude.com/claude-code)
- Add src/common/db-helpers.ts with ownerOrGlobal, ilikeContains,
  isUniqueViolation, awaitAllOrThrowFirst.
- Apply ownerOrGlobal across iso, weather, film-format, shutter-speed,
  film services (was 5x copies of `or(eq(createdBy), eq(isGlobal))`).
- Apply ilikeContains across film, filter, camera, lens, shoot services.
- Pull PG_UNIQUE_VIOLATION / isUniqueViolation out of frame.service into
  the shared helper.
- Replace Promise.allSettled boilerplate in frame.service and
  shoot.service with awaitAllOrThrowFirst.
- Fix O(N*M) compose in changelog.service: pre-group items by entryId.
- Collapse export.service 5-way ternary chain (52 lines -> 16) using a
  local ownedOrReferenced helper.
- Deduplicate frame-location upsert values; add limit(1) to
  assertOwnership query.
- Replace em-dashes with regular hyphens (project rule).
- Document deferred follow-ups in docs/simplify-followups.md.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Split validation from counting in import.service: commit() no longer
  re-runs preview()'s 9 reference-table scans before the transaction.
- Factor shoot.service shared columns and mapRow into module-private
  helpers; preserve findOne's filmIsColor extension.
- Standardize CRUD service signatures to (userId, id, dto?) across
  frame, frame-location, iso, filter, film, lens, camera, folder,
  shutter-speed, weather, film-format, shoot.
- Extract reference-validation pattern to src/common/reference-validator.ts
  (validateRefs<T>); frame and shoot now declare validator arrays.
- Add .env to .gitignore.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Services returned the bulk-update result without checking rowCount, so
PUT/DELETE on a missing or foreign id returned 200 with rowCount: 0
instead of 404. Add .returning() and throw NotFoundException when the
result is empty across camera, lens, filter, film, film-format, iso,
shutter-speed, weather, folder, shoot (incl. setPinned/setDone).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add IdParam decorator wrapping ParseUUIDPipe and apply it to every
@Param('id') across all controllers. Non-UUIDs now fail with 400 at
the boundary instead of leaking pg 22P02 errors from Drizzle.

Migrate seeded changelog ids from human-readable slugs (e.g.
seed-changelog-v5-0-0) to deterministic v7 UUIDs so admin update/delete
endpoints work with the new validation. Migration 0022 drops the legacy
rows; the cascade FK on changelog_item.entry_id removes orphans, and
the seed reinserts with the UUID constants.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
UpdateXDto previously extended CreateXDto, forcing PUT bodies to
include every required field. Switch to PartialType(CreateXDto) so
clients can ship only changed fields. Routes move from PUT to PATCH
to match the now-partial semantics; admin/changelog stays on PUT
since its update is a full replace (items array gets wiped and
reinserted).

Add pickDefined() helper that strips undefined keys before .set() so
omitted fields don't get NULL'd. Apply across every owner-scoped
service. Also add ParseUUIDPipe to the frame-location :frameId param.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add explicit ON DELETE clauses to FKs that previously relied on the
default NO ACTION. Frames cascade with their parent shoot (so the
manual cascade in shoot.delete is gone). Equipment-style references
(weather, lens, filter, shutter speed on frame; film, iso, film
format, camera, folder on shoot; format on camera) now SET NULL so
deleting a piece of gear unlinks it from history rather than blocking
the delete.

film.iso stays NOT NULL with default RESTRICT semantics; the FK
exception filter still surfaces 409 to clients in that case.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
decryptValue/deterministicDecryptValue used to swallow any decrypt
failure and return the raw value, so an ENCRYPTION_KEY rotation or
misconfiguration silently fed ciphertext to callers as gibberish
plaintext. Distinguish format-clearly-plaintext (return raw) from
auth-tag-mismatch (throw): legacy plaintext rows still pass through,
but ciphertext that fails to decrypt now surfaces the underlying
error instead of leaking corrupted data.

Update the encrypt-existing-data script to classify rows into
plaintext / encrypted / mismatch, refusing to re-encrypt mismatches
so an operator running the script with the wrong key cannot
permanently overwrite data they can't read.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
buildExport used to materialize every shoot, frame, and reference row
into one giant object and JSON.stringify it before handing off to
archiver, so heap usage scaled with the user's total data. Replace
with a paged streaming writer that fetches each table 500 rows at a
time and pushes JSON chunks straight into the zip entry, freeing each
page before the next loads.

Also switch reference-row collection from full-table SELECTs to
SELECT DISTINCT on the FK columns so we no longer scan iso/film/etc.
in their entirety per export.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Parameterized base service capturing the shape every owner-scoped
domain module implements: findAll with pagination + optional ILIKE
search, create with createdBy/updatedBy/isGlobal stamping, update and
delete with .returning() guarded NotFoundException. Modules with
custom joins or list shapes extend it and override only the methods
that diverge.

Migration of the nine existing modules onto this base lands in the
next commit.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
camera, lens, filter, film, film-format, iso, shutter-speed, weather
collapse onto the new generic. The five plain modules (lens, filter,
film-format, iso, weather) become 12-line classes; camera/film/
shutter-speed override findAll to keep their joins or custom orderBy.

Folder stays standalone since it has no pagination and no updatedBy.
Net change: ~250 lines removed, with NotFoundException correctness
and the pickDefined PATCH semantics inherited from the base.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- frame.findOne and shoot.findOne now throw NotFoundException instead
  of returning null, so GET /:id of a missing entity is 404 not 200.
- Merge SetPinDto and SetDoneDto (byte-identical) into a single
  BooleanValueDto.
- Centralize the v7 UUID id-column factory in db/id-column.ts and
  switch all schemas onto it; one place to swap to ULID later.
- Add common/error-message.ts with getMessage() and replace the
  scattered (err as Error).message casts.
- Centralize MAX_IMPORT_BYTES in import/import.constants.ts.
- ilikeContains escapes %, _, and \\ so user input can't broaden the
  match accidentally.
- Replace WeatherIcons enum with a const tuple + WeatherIcon type
  (cleaner kebab-case keys, simpler validator wiring with @IsIn).
- Changelog cursor is now (publishedAt, id) so entries published in
  the same instant page deterministically. Drop the redundant
  client-side item sort and the dead InternalServerErrorException
  branch.
- UpdateFolderDto becomes PartialType(CreateFolderDto), removing the
  duplicated hex-color @Transform.
- Drop unused getRequestId helper.
- nodemailer secure flag now derives from SMTP_PORT (true on 465).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Front lockstep version is v6.2.0 to match the new entry. API itself
bumps to 7.0.0 because the PATCH switch on entity update routes is a
breaking wire change for any external consumer (none today, but the
SemVer signal matters).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
eliott merged commit 0cd287adaa into develop 2026-05-07 16:04:27 +02:00
eliott deleted branch chore/simplify-cleanup 2026-05-07 16:04:27 +02:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
run-ci
Trigger the required CI run

No labels
run-ci
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
RollNote/rollnote-api!35
No description provided.