chore: simplify codebase reuse, quality, and efficiency

eliott commented

2026-05-06 23:03:17 +02:00

Owner

Summary

Codebase-wide simplify pass: plug subscription leaks, remove redundant state, replace ad-hoc string-building with framework primitives. No behavior changes intended.

What changed

Leaks

Added `takeUntilDestroyed(this.destroyRef)` to every `.subscribe()` across:
- 9 select components (film-format, film, camera, weather, iso, lens, filter, folder, shutter-speed)
- 11 form drawers
- shoots.ts (4 calls), whats-new-modal.ts (2 calls), export-data-drawer.ts (1 call)
- import-data-drawer.ts (preview + commit calls)

Redundant state

Replaced 5 manual signals + 5 `valueChanges` subscriptions in `import-data-drawer` with `toSignal()` and a single preview-clearing `effect()`. Net -42 lines, same behavior.
Dropped redundant `WritableSignal` annotations in `login.ts` (let TS infer).

Reuse

Migrated 5 services (films, cameras, lenses, filters, shoots) from manual `encodeURIComponent` URL string-building to `HttpParams`.

Quality

Replaced em-dashes with regular hyphens (project rule).

Deferred

See `docs/simplify-followups.md` for items intentionally not addressed - notably:

Generic CRUD service base for the 7 reference-table services
Form-drawer base component / shared template wrapper
Route paths as constants
Lazy-init guard for the Leaflet map effects in location-picker / frame-map (needs UI verification)

Test plan

`pnpm exec tsc --noEmit` clean
`pnpm build` clean (warnings unchanged from develop)
Vitest tests are broken on `develop` already (pre-existing config issue: `describe is not defined`). Not addressed here. Verify once test setup is fixed.
Smoke-test each `*-select` component: open dropdown, verify list loads, verify "add new" still works
Smoke-test each `*-form-drawer`: create + edit flow, error states
Smoke-test shoots page: load, search, pin, folder grouping
Smoke-test import-data-drawer: stepper navigation, preview triggers, password validation, commit + connected-account-deletion path
Smoke-test export-data-drawer + whats-new-modal
Visual check on shoot-detail "Done - unmark to add frames" copy (em-dash replacement)

🤖 Generated with Claude Code

## Summary Codebase-wide simplify pass: plug subscription leaks, remove redundant state, replace ad-hoc string-building with framework primitives. No behavior changes intended. ## What changed ### Leaks - Added \`takeUntilDestroyed(this.destroyRef)\` to every \`.subscribe()\` across: - 9 select components (film-format, film, camera, weather, iso, lens, filter, folder, shutter-speed) - 11 form drawers - shoots.ts (4 calls), whats-new-modal.ts (2 calls), export-data-drawer.ts (1 call) - import-data-drawer.ts (preview + commit calls) ### Redundant state - Replaced 5 manual signals + 5 \`valueChanges\` subscriptions in \`import-data-drawer\` with \`toSignal()\` and a single preview-clearing \`effect()\`. Net -42 lines, same behavior. - Dropped redundant \`WritableSignal<boolean>\` annotations in \`login.ts\` (let TS infer). ### Reuse - Migrated 5 services (films, cameras, lenses, filters, shoots) from manual \`encodeURIComponent\` URL string-building to \`HttpParams\`. ### Quality - Replaced em-dashes with regular hyphens (project rule). ### Deferred See \`docs/simplify-followups.md\` for items intentionally not addressed - notably: - Generic CRUD service base for the 7 reference-table services - Form-drawer base component / shared template wrapper - Route paths as constants - Lazy-init guard for the Leaflet map effects in location-picker / frame-map (needs UI verification) ## Test plan - [x] \`pnpm exec tsc --noEmit\` clean - [x] \`pnpm build\` clean (warnings unchanged from develop) - [ ] Vitest tests are broken on \`develop\` already (pre-existing config issue: \`describe is not defined\`). Not addressed here. Verify once test setup is fixed. - [ ] Smoke-test each \`*-select\` component: open dropdown, verify list loads, verify \"add new\" still works - [ ] Smoke-test each \`*-form-drawer\`: create + edit flow, error states - [ ] Smoke-test shoots page: load, search, pin, folder grouping - [ ] Smoke-test import-data-drawer: stepper navigation, preview triggers, password validation, commit + connected-account-deletion path - [ ] Smoke-test export-data-drawer + whats-new-modal - [ ] Visual check on shoot-detail \"Done - unmark to add frames\" copy (em-dash replacement) 🤖 Generated with [Claude Code](https://claude.com/claude-code)

eliott added 1 commit

2026-05-06 23:03:17 +02:00

chore: simplify codebase reuse, quality, and efficiency 496d0b7c14

- Plug subscription leaks: add takeUntilDestroyed(this.destroyRef) to
  every .subscribe() across 9 select components, 11 form drawers,
  shoots, whats-new-modal, export-data-drawer, and import-data-drawer.
- Replace 5 manual signals + 5 valueChanges subscriptions in
  import-data-drawer with toSignal() and a single preview-clearing
  effect (-42 lines, same behavior).
- Migrate film/camera/lens/filter/shoot services from manual
  encodeURIComponent string-building to HttpParams.
- Drop redundant `WritableSignal<boolean>` annotations in login.ts.
- 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>

eliott added 1 commit

2026-05-07 10:51:58 +02:00

chore: address simplify-followups (route constants, error helper, confirm-delete, form-drawer wrapper, leaflet guard) c04ca10a01

- Centralize route paths in src/lib/constants/routes.ts (ROUTES.<name>);
  replace magic-string navigations across components, guards, interceptors.
- Add extractErrorMessage(err, fallback) in src/lib/utils/error-handling.ts;
  apply to ~24 inline err?.error?.message patterns.
- Add ConfirmDeleteService wrapping PrimeNG ConfirmationService with the
  standard rollnote messaging; migrate 12 call sites. passkeys-drawer
  keeps its scoped ConfirmationService provider.
- Wire passwordMatchValidator on import-data-drawer (gated to the
  create-new-account branch).
- Add mapInitialized short-circuit before Leaflet dynamic import in
  location-picker-dialog and frame-map; reset on destroy.
- Add <app-form-drawer> wrapper in src/lib/components/common/form-drawer/;
  migrate 9 reference-entity drawers (films, cameras, isos, weathers,
  lenses, filters, film-formats, shutter-speeds, folders).
- Move ConfirmationService from App component providers into
  app.config.ts so root-injected services (ConfirmDeleteService) can
  resolve it.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

eliott added 10 commits

2026-05-07 15:06:51 +02:00

refactor(front): typed auth wrappers replace bracket-access useSession 41c7328dd8

Add src/app/auth-session.ts with a typed SessionState (including the
custom isAdmin and twoFactorEnabled fields) and helper hooks:
injectSession, injectIsAuthenticated, injectUserId, injectIsAdmin,
injectTwoFactorEnabled.

Migrate all 10 callsites off the auth['useSession']() bracket-access
+ inline cast pattern: the four route guards, navbar, sidebar-nav,
whats-new-modal, changelog-page, account, and post-login-passkey-
prompt.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

chore(front): drop dead Films component 68d2d3c1bc

src/app/films/components/films/ duplicated FilmsSettingsTab and was
not referenced from app.routes.ts (only FilmsSettingsTab is wired).
Delete it so future fixes don't need to hop between two parallel
implementations.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

refactor(front): collapse entity HTTP services onto CrudBaseService eb9f1070d2

Add lib/services/crud-base.service.ts with the generic shape every
owner-scoped resource exposes (getAll, getPaginated, create, update,
delete). withCredentials, query-string building, and the PATCH verb
on update live in one place.

Migrate camera, film, film-format, filter, iso, lens, shutter-speed,
weather, and folder to extend the base. The nine services collapse
from ~40 lines each to ~12. Folder overrides getAll since it's not
paginated server-side.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

feat(front): add BaseFormDrawer abstract for entity drawers e48b675686

Captures the create/edit drawer shape: visible model, entity input,
saved output, saving signal, edit effect that patches the form, and
the onSubmit orchestration that picks update vs create and emits the
saved entity. Subclasses define the form schema and the four hooks
(patchFromEntity, resetForm, buildCreateDto, buildUpdateDto).

Drawer migration of the nine existing entities lands in the bulk
migration commit alongside the entity-select and settings-list-tab
work.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

feat(front): add BaseEntitySelect abstract for CVA boilerplate da801927d0

Shared base for the nine *-select ControlValueAccessor components.
Encapsulates the CVA contract (writeValue, registerOnChange,
registerOnTouched, setDisabledState), the items/value/loading/
disabled/drawerVisible signals, the initial getAll fetch, and the
onSaved handler that prepends a newly created entity and selects it.

Subclasses still own their templates (icon, label, optionLabel/value,
drawer wiring) but the ~50-line TS body collapses to ~10. Wiring of
existing selects onto this base lands in the bulk migration commit.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

feat(front): add BaseSettingsListTab abstract for entity tabs 7c3f401be6

Captures the shape of the entity settings tabs: createPaginatedList
plumbing, search wiring, drawerVisible + editingEntity signals, the
openCreateDrawer/openEditDrawer/onDeleteConfirm trio, and the delete
flow that goes through ConfirmDeleteService and reloads the list.

Subclasses provide the service and entityName; templates remain
per-entity since each features unique cards, drawers, and empty-state
copy. Wiring of existing tabs onto this base lands in the bulk
migration commit.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

refactor(front): collapse skeleton components onto BaseSkeletonRows 9a8e6d718f

Eleven *-card-skeleton / *-row-skeleton components had identical TS
classes (count input + rows computed) with only their templates
differing. Move the shared logic into lib/components/ui/skeletons/
base-skeleton-rows.ts and have each skeleton extend it with no body.

Per-entity templates and SCSS stay where they are.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

refactor(front): migrate entities onto CRUD generics 8e8c314f6a

Sweep the nine owner-scoped entities (camera, film, film-format,
filter, folder, iso, lens, shutter-speed, weather) onto the three
generics added earlier:

- form drawers extend BaseFormDrawer with patchFromEntity / resetForm
  / buildCreateDto / buildUpdateDto hooks; the entity input is now
  named `entity` uniformly,
- *-select components extend BaseEntitySelect (CVA boilerplate gone,
  ngOnInit fetch gone),
- settings tabs (incl. the Cameras page) extend BaseSettingsListTab;
  no more per-tab createPaginatedList wiring or delete-confirm
  plumbing.

Net diff: 53 files changed, ~870 lines removed. Templates rebound
[entity], items() / search() / editingEntity() to match the base
class API.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

chore(front): misc cleanups from review c964b35553

- Centralize SESSION_TIMEOUT_MS in auth-session.ts; both guards now
  import it instead of re-declaring the constant.
- Drop the never-emitted `delete` output on FolderGroupHeader.
- error.interceptor.parseErrorMessage now delegates to the existing
  extractErrorMessage util instead of reimplementing it.
- Add lib/utils/route-param.ts (signal-backed route param reader) so
  shoot-detail / frame-detail / new-frame can stop reading
  route.snapshot.params['id'] via getter on every access.
- AuthLayout: replace the typographer's apostrophe with an ASCII one
  and rotate through the QUOTES array on mount instead of always
  picking [0].

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

fix(front): defer settings-tab paginated-list init to ngOnInit c950a15c28

The previous lazy `state` getter was first invoked during template
render (when reading `items()`), triggering createPaginatedList ->
synchronous reload() -> signal writes. Angular blocks signal writes
during render and threw NG0600 the moment a settings tab was opened.

Move state setup to ngOnInit so the signal writes happen before the
first change-detection cycle. Subclass field initializers have run
by then, so `this.service` is set when createPaginatedList reads it.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

eliott added 1 commit

2026-05-07 15:08:52 +02:00

chore(front): drop docs/ 4a233085ef

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

eliott added 1 commit

2026-05-07 16:04:12 +02:00

chore(front): bump to 6.2.0 for the v6.2.0 changelog release 77a1faf018

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

eliott merged commit fd21a23409 into develop

2026-05-07 16:04:42 +02:00

eliott deleted branch chore/simplify-cleanup

2026-05-07 16:04:42 +02:00

eliott referenced this pull request from a commit

2026-05-07 16:04:44 +02:00

chore: simplify codebase reuse, quality, and efficiency (#42)

Rows
Columns

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

Summary

What changed

Leaks

Redundant state

Reuse

Quality

Deferred

Test plan