From 1f2f48226d479a18f94c44f5b66b551a5dda0d8e Mon Sep 17 00:00:00 2001 From: Pierre Marais Date: Fri, 14 Aug 2026 20:56:41 +0200 Subject: [PATCH 01/19] docs: spec for editable album creation date (#520) --- .../2026-08-14-album-creation-date-design.md | 475 ++++++++++++++++++ 1 file changed, 475 insertions(+) create mode 100644 docs/superpowers/specs/2026-08-14-album-creation-date-design.md diff --git a/docs/superpowers/specs/2026-08-14-album-creation-date-design.md b/docs/superpowers/specs/2026-08-14-album-creation-date-design.md new file mode 100644 index 0000000000000..195bef5effc9f --- /dev/null +++ b/docs/superpowers/specs/2026-08-14-album-creation-date-design.md @@ -0,0 +1,475 @@ +# Editable album creation date + +Design spec for [discussion #520](https://github.com/open-noodle/gallery/discussions/520). + +Date: 2026-08-14 + +## 1. Problem + +> Can **Change Album creation date** feature be implemented, as for example I am uploading many +> albums from previous years and only option to sort them by chronological time is to create albums +> in certain order, it would be nice to change creation date and time as if other shared album from +> another user was from lets say from 1996 but photos were digitized in current year it would appear +> on top and there is no way to change that. +> +> — jjaard, 2026-05-05 + +The requester wants albums to sort by _when the album's content is from_, not by when the album row +was inserted. + +Gallery already offers two sorts that read content dates — `MostRecentPhoto` and `OldestPhoto`, both +driven by `startDate` / `endDate` which `mapAlbum` derives from asset `localDateTime` +(`server/src/dtos/album.dto.ts:213-218`). Those do not help here: the photos were digitized this +year, so their asset dates _are_ current-year, and for an album shared **with** him he cannot +correct another user's asset dates. + +`album.createdAt` is the only per-album date, and today nothing can write it. + +## 2. Decision + +Make `album.createdAt` user-editable. + +`createdAt` is already plumbed end to end — it is on `AlbumTable` +(`server/src/schema/tables/album.table.ts:36`), in `AlbumResponseDto` +(`album.dto.ts:123`), in the `SyncAlbumV1` sync stream (`sync.repository.ts:214`), on mobile's +`RemoteAlbum` model, rendered by the albums table (`AlbumsTableRow.svelte:61`), and sorted on by +`AlbumSortBy.DateCreated` (web), `AlbumSortMode.created` (mobile) and the fork's space-album sort. +The single missing link is that it is absent from `UpdateAlbumSchema` (`album.dto.ts:228`). + +The change is therefore one optional DTO field, one service pass-through, a date field in each of +the two existing album-edit surfaces, and a relaxation of three ownership gates. + +### 2.1 Rejected alternatives + +**A separate nullable `album.date` column.** Semantically cleaner — `createdAt` stays an audit +field — but it costs a fork migration, an `AlbumTable` change, a `SyncAlbumV1` payload change, a +mobile Drift column plus migration, a new sort option in three sort implementations, and nine +locales. It adds divergence in `sync.repository.ts` / `sync.dto.ts`, files upstream churns heavily, +in exchange for semantics no user sees. Rejected on cost. + +**Do nothing; point at asset date editing.** Correcting the assets' `dateTimeOriginal` is arguably +the _right_ fix — it also puts the photos in the right place on the timeline — but it does not +address the shared-album case, and it is a large manual chore for the "many albums from previous +years" workflow the requester describes. Rejected as insufficient. + +**A private per-viewer override.** Fixes every case including pure viewers, but needs a new +per-user table, endpoints, sync, and a merge rule in every sort path. Rejected as out of proportion. + +### 2.2 Accepted consequences + +- `createdAt` stops being an audit field. The previous value is overwritten with no undo. +- `album.repository.ts:148,283,421` order by `album.createdAt desc`. Web and mobile re-sort + client-side so they are unaffected, but raw API and CLI consumers will see backdated albums move + to the bottom of unsorted listings. +- Year grouping is disabled whenever sort is `DateCreated` (`album-utils.ts:130-133`). Once the date + is meaningful users will plausibly want it enabled. Out of scope; see §10. + +## 3. Scope + +In scope: server DTO + service, generated SDKs, web `AlbumEditModal`, mobile `_EditAlbumDialog`, +and the ownership gates guarding both. + +Out of scope: a new sort option (`DateCreated` already exists on all three surfaces); bulk +multi-album date editing; per-viewer overrides; adding a `createdAt` display to the album detail +page (it shows the asset date range and keeps doing so); enabling year grouping under `DateCreated`. + +## 4. Behaviour specification + +Written as Given/When/Then. Every scenario below maps to a test in §6; the numbering is shared. + +### 4.1 Server — writing the field + +**S1 — an owner sets the date** +Given an album owned by the caller +When the caller sends `PATCH /albums/:id` with `{ "createdAt": "1996-06-15T14:30:00.000Z" }` +Then the response is 200, `body.createdAt` is `1996-06-15T14:30:00.000Z`, and `body.updatedAt` has +advanced. + +**S2 — an editor sets the date** +Given an album shared with the caller with role `editor` +When the caller sends the same request +Then the response is 200 and the date is applied. +(`Permission.AlbumUpdate` already grants owner ∪ editor — `server/src/utils/access.ts:208-216`.) + +**S3 — a viewer is refused** +Given an album shared with the caller with role `viewer` +When the caller sends the same request +Then the response is 400 with `Not found or no album.update access`. + +**S4 — a non-member is refused** +Given an album the caller has no relationship to +Then the response is 400 with the same message. + +**S5 — the field is optional** +Given an existing album with a known `createdAt` +When the caller sends `PATCH /albums/:id` with only `{ "albumName": "…" }` +Then the name changes and `createdAt` is **unchanged**. +(Kysely omits `undefined` properties from `set()`; the service already relies on this for every +other optional field.) + +**S6 — combined update** +When the caller sends `albumName` and `createdAt` in one request +Then both are applied. + +**S7 — millisecond precision survives the round trip** +When the caller sends `1996-06-15T14:30:00.123Z` +Then `GET /albums/:id` returns `1996-06-15T14:30:00.123Z`. +This matters: sub-second precision is what keeps backdated albums from tying in `DateCreated` sorts. + +**S8 — the sync stream emits the album** +Given a mobile client with a sync checkpoint taken before the edit +When the date is changed +Then the album appears in the next `SyncAlbumV1` batch carrying the new `createdAt`. +(The `@UpdatedAtTrigger('album_updatedAt')` on `AlbumTable` bumps `updatedAt` and `updateId`.) + +### 4.2 Server — input validation + +`createdAt` uses the existing `isoDatetimeToDate` codec (`server/src/validation.ts:139-152`), which +is `z.iso.datetime({ offset: true })` decoded to a `Date`. That fixes the accepted grammar: + +**S9** `"1996-06-15T14:30:00.000Z"` → accepted. +**S10** `"1996-06-15T14:30:00+02:00"` → accepted; stored as the equivalent UTC instant. +**S11** `"1996-06-15T14:30:00"` (no offset) → **400**. This is the trap for API users and must be +documented in the field description. +**S12** `"1996-06-15"` (date only) → 400. +**S13** `"not-a-date"` → 400. +**S14** `""` → 400. +**S15** `null` → 400 (the field is optional, not nullable). +**S16** `"12345-06-15T14:30:00Z"` (5-digit year) → 400; `z.iso.datetime` requires exactly 4. +**S17** `"0001-01-01T00:00:00.000Z"` → accepted; within `timestamptz` range. +**S18** a future date → **accepted**, deliberately. See §5. + +### 4.3 Web + +**W1 — the modal pre-fills** +Given an album with `createdAt` `1996-06-15T12:30:00.000Z` and a browser in `Europe/Berlin` +When `AlbumEditModal` opens +Then the date input shows `1996-06-15T14:30:00.000` (local time, matching how +`dateLocaleString(album.createdAt)` renders it in the albums table). + +**W2 — editing submits a zoned ISO string** +When the user changes the input to `1996-06-15T14:30:00.000` and submits +Then `updateAlbumInfo` is called with a `createdAt` carrying an offset +(Luxon `DateTime.fromISO(local).toISO()`), never a bare local string — S11 would otherwise 400. + +**W3 — no blur required** +When the user types a date and clicks Save without blurring the input first +Then the typed value is submitted. +(`DateInput` uses `bind:value`, so this should already hold; the fork has been bitten by the +equivalent bug on `@immich/ui`'s `DatePicker` — `PersonEditBirthDateModal.spec.ts` — so it gets +pinned rather than assumed.) + +**W4 — an unchanged date is not sent** +When the user edits only the name and submits +Then the DTO contains `albumName` and no `createdAt`. + +**W5 — an emptied or invalid date is not sent** +When the user clears the date input and submits +Then the DTO omits `createdAt` and the name/description edits still apply. + +**W6 — gating: owner** +Given an album owned by the caller +When the caller right-clicks it in the albums list +Then the context menu shows Edit, Share, Download and Delete. + +**W7 — gating: editor** +Given an album shared with the caller as `editor` +When the caller right-clicks it in the albums list +Then the context menu shows Edit and Download, and **not** Share or Delete. +This is the change: `showFullContextMenu` (`AlbumsList.svelte:173`) is one flag covering Edit, +Share and Delete, gated on `albumUsers[0].user.id === authManager.user.id`. It splits into +`canEdit` (owner ∪ editor) and `isOwner` (Share, Delete). + +**W8 — gating: viewer** +Given an album shared with the caller as `viewer` +When the caller right-clicks it in the albums list +Then the context menu shows Download only. + +**W9 — gating: command palette** +Given the album detail page for an album the caller edits but does not own +Then `cmd:album_rename` is available. +`AlbumContext` already carries `isEditor`, documented as "Owner or Editor" +(`command-context-manager.svelte.ts:20-21`), so this is `ctx.album.isOwner` → `ctx.album.isEditor` +at `command-items.ts:260`. + +**W10 — `DateCreated` sort honours the edited value** +Given three albums with `createdAt` 1996, 2010 and 2026 +When sorted by `AlbumSortBy.DateCreated` descending +Then the order is 2026, 2010, 1996. +Characterization test over untouched code (`album-utils.ts:243`), included because the feature's +entire value rests on it. + +### 4.4 Mobile + +**M1 — `canEditAlbum` for an owner** → true. +**M2 — for an `editor`** → true. +**M3 — for a `viewer`** → false. +**M4 — for a null `currentUserRole`** → **true, failing open.** +This mirrors the documented posture of `canAddAssetsToAlbum` in the same file +(`mobile/lib/utils/album_permissions.dart`): a null role means "not known", the server is the real +enforcer, and hiding an affordance we are merely unsure about is the worse failure. It also +defuses the #985 trap — `RemoteAlbum.currentUserRole` is null unless `getAll` was passed +`currentUserId`. `remote_album.provider.dart:56-57` does pass it, so the role is populated for +albums reached from the main list; fail-open covers every other entry point, including the fact +that `updateAlbum` replaces the state album with `toRemoteAlbum()` output, which carries no role. + +**M5 — the dialog shows the current date** +Given `_EditAlbumDialog` opened for an album created 1996-06-15 +Then a date row displays that date alongside the title and description fields. + +**M6 — picking a date saves it** +When the user picks a new date and taps Save +Then `updateAlbum` is called with `createdAt` set to the picked instant. + +**M7 — cancelling the picker changes nothing** +When the user opens the picker and dismisses it +Then the pending date is unchanged and Save sends the original value. + +**M8 — the API repository maps it** +When `updateAlbum(albumId, owner, createdAt: dt)` is called +Then `UpdateAlbumDto.createdAt` is `Optional.present(dt)`; when `createdAt` is null it is +`Optional.absent()`, matching every other field in `drift_album_api_repository.dart:71-99`. + +**M9 — the wire format carries an offset** +Given a `createdAt` picked in any timezone the picker offers +When `UpdateAlbumDto` is serialized +Then the JSON value ends in `Z`. +The Dart generator emits `value.toUtc().toIso8601String()` (verified in +`mobile/openapi/lib/model/shared_link_create_dto.dart`), so S11 cannot be tripped from mobile +regardless of the picker's timezone selection. Pinned because it is invisible generated code. + +**M10 — the local Drift row is updated** +Then the album's `createdAt` in `remoteAlbumEntity` matches the new value. +`remote_album.repository.dart:227-240` already writes `createdAt: Value(album.createdAt)`, and +`toRemoteAlbum` already maps it (`drift_album_api_repository.dart:143`), so this is a pin on an +existing path rather than new code. + +## 5. Future dates + +Mobile's shared `showDateTimePicker` hard-codes `lastDate: now` +(`mobile/lib/widgets/common/date_time_picker.dart`), so future dates are unreachable there. Web's +`DateInput` allows anything up to `9999-12-31T23:59`. + +**Decision: leave the asymmetry, add no server-side future check.** Reasons: it is exactly the +behaviour asset date editing already has (`AssetChangeDateModal` uses the same unbounded +`DateInput` against the same capped mobile picker), a server-side check would produce spurious 400s +under client/server clock skew, and a permissive API keeps scripted bulk backdating viable — which +is the requester's actual workflow. S18 pins acceptance so this stays a decision rather than an +accident. + +## 6. Implementation slices + +TDD throughout: each slice writes the failing test first, confirms it fails for the stated reason, +then makes it pass. Slices are ordered so each one is independently green. + +`pnpm install` is required first — a fresh worktree has no `node_modules`. + +### Slice 1 — server contract + +1. **Red.** Add the service-level halves of S1, S5 and S6 to `describe('update')` in + `server/src/services/album.service.spec.ts:525` — that `mocks.album.update` receives `createdAt` + when the DTO carries it, and receives no `createdAt` key when it does not. The HTTP-level + assertions of those same scenarios (status codes, response body, persistence) belong to Slice 2; + the unit test only pins the pass-through. + Note the existing assertion style at :573-577 passes an exact object literal; + `toHaveBeenCalledWith` uses `toEqual` semantics, which ignore `undefined`-valued keys, so the + existing tests keep passing once the service gains the field. +2. **Green.** `album.dto.ts:228` — add to `UpdateAlbumSchema`: + ```ts + createdAt: isoDatetimeToDate + .optional() + .describe('Album creation date. Must be an ISO 8601 string including a UTC offset.'), + ``` + `album.service.ts:231-240` — add `createdAt: dto.createdAt` to the object handed to + `albumRepository.update`. +3. No repository change: `update(id, album: Updateable, authUserId)` + (`album.repository.ts:621`) already accepts it because `createdAt` is `Generated`. +4. Regenerate both clients. **Do not use `make open-api` or `mise open-api`** — the make target is a + removed stub that exits 1, and mise's composite task hardcodes `//server:install`, + `//server:build`, `//server:sync-open-api`, where `//` resolves to the **main checkout**, so it + would silently generate clients from main's server source rather than this branch's. From the + worktree: + ```bash + cd server && pnpm build && node ./dist/bin/sync-open-api.js + cd .. && mise run open-api-typescript && mise run open-api-dart # needs JDK 21 + ``` + Regenerating a second time must be byte-identical. `mobile/openapi/**/*.dart` is marked + `-diff -merge` in `.gitattributes`, so git reports those files as `Bin N -> M bytes` with no + textual diff — verify the new field landed with `grep`, not `git diff`. Skipping the Dart half + passes locally and fails CI's **OpenAPI Clients** job. + +### Slice 2 — server validation and permissions (e2e) + +Add the HTTP halves of S1, S5 and S6 plus S2, S3, S4, S7 and S9–S18 to `describe('PATCH /albums/:id')` +in `e2e/src/specs/server/api/album.e2e-spec.ts:589`. The suite already has the fixtures: +`user1Albums[0]` is shared with `user2` as editor, `user1Albums[3]` as viewer, and the existing +"should be able to update as an editor" / "should not be able to update as a viewer" tests give the +exact shape and error string (`Not found or no album.update access`) to mirror. + +S5 in particular has to be proved here rather than in the unit test — "an omitted `createdAt` leaves +the stored value alone" is a claim about Kysely's `set()` dropping `undefined`, which only a real +database can settle. + +S8 goes in `server/test/medium/specs/sync/sync-album.spec.ts`, next to the existing album sync +coverage: assert the album appears in the next `SyncEntityType.AlbumV1` batch carrying the new +`createdAt`. Medium tests need Docker plus, in a fresh worktree, `@immich/sdk` and +`@immich/plugin-sdk` built first (`pnpm --filter @immich/sdk build`, then `@immich/plugin-sdk`, or +`mise run plugins`). + +### Slice 3 — web modal + +1. **Red.** New `web/src/lib/modals/AlbumEditModal.spec.ts` covering W1–W5, mocking `@immich/sdk`'s + `updateAlbumInfo` and asserting the exact DTO. Model it on + `PersonEditBirthDateModal.spec.ts`. Fix the timezone in the test (`process.env.TZ` / + vitest config) so W1 and W2 assert real instants rather than whatever the runner's zone is. + Note `web/vitest` does not clear mocks between tests in a file — assert on call arguments, not + call counts, or reset explicitly. +2. **Green.** `AlbumEditModal.svelte` — a third `Field` labelled `$t('date_created')` between Name + and Description, holding `DateInput` with `type="datetime-local"`. Keep local state as a Luxon + `DateTime` string in `yyyy-MM-dd'T'HH:mm:ss.SSS`, seeded from `album.createdAt`; on submit + include `createdAt: DateTime.fromISO(value).toISO()` only when the parsed value is valid **and** + differs from `album.createdAt`. +3. `DateInput` (`web/src/lib/elements/DateInput.svelte`) is the right element rather than + `@immich/ui`'s `DatePicker`: it is what `AssetChangeDateModal` uses for date **and** time, and + `step=".001"` preserves the milliseconds S7 protects. No timezone combobox — unlike an asset's + `dateTimeOriginal`, an album's `createdAt` is a plain instant, so the browser's local zone is the + correct and only interpretation. +4. `handleUpdateAlbum` (`album.service.ts:305`) passes `UpdateAlbumDto` through verbatim and needs + no change. + +### Slice 4 — web gating + +1. **Red.** W6–W8 against `AlbumsList.svelte`, W9 against `command-items.ts` (extend + `command-items.spec.ts:604`, which already asserts the modal opens with the raw DTO). +2. **Green.** In `AlbumsList.svelte`, split `showFullContextMenu` (:173) into `canEditSelectedAlbum` + (owner ∪ editor, computed the way `isAlbumEditor` is on the space-album page, + `spaces/[spaceId]/albums/[albumId]/+page.svelte:94-99`) and `isSelectedAlbumOwner` (the existing + check). Edit uses the former; Share and Delete keep the latter. In `command-items.ts:260`, switch + `cmd:album_rename` from `ctx.album.isOwner` to `ctx.album.isEditor`. +3. Leave `cmd:album_share` on `isOwner`. + +### Slice 5 — mobile permission predicate + +1. **Red.** M1–M4 in the existing `mobile/test/utils/album_permissions_test.dart`. +2. **Green.** Add `canEditAlbum(RemoteAlbum album, {required String? currentUserId})` to + `mobile/lib/utils/album_permissions.dart`, returning true when the caller is the owner or when + `currentUserRole != AlbumUserRole.viewer`. Document the fail-open reasoning in the same voice as + the existing `canAddAssetsToAlbum` doc comment, and cross-reference + `Permission.AlbumUpdate` in `server/src/utils/access.ts`. + +### Slice 6 — mobile plumbing + +1. **Red.** M8 in `mobile/test/repositories/drift_album_api_repository_test.dart` (including M9's + wire assertion), M6/M10 in `mobile/test/domain/services/remote_album_service_test.dart`. +2. **Green.** Thread `DateTime? createdAt` through + `drift_album_api_repository.dart:71` → `UpdateAlbumDto(createdAt: …)`, + `remote_album.service.dart:137`, and `remote_album.provider.dart:154`. Nothing else changes: + `toRemoteAlbum` and `RemoteAlbumRepository.update` already carry `createdAt`. + +### Slice 7 — mobile dialog + +1. **Red.** M5–M7 in `mobile/test/presentation/pages/drift_remote_album_page_test.dart`. Widget + tests here have a documented history of passing vacuously — prove each one red first by + inverting the expectation, and assert on the mock call rather than on rendered text where the + text could match an unrelated widget. +2. **Green.** Add a date row to `_EditAlbumDialog` + (`drift_remote_album.page.dart:242`) driven by `showDateTimePicker`, which returns a + `String?` ISO value carrying an offset; parse with `DateTime.parse` and pass to `updateAlbum`. + Gate `onEditTitle` (:226) and `onEditAlbum` (:459) on `canEditAlbum` instead of `isOwner`, leaving + `onDeleteAlbum`, `onAddUsers`, `onCreateSharedLink` and `onToggleAlbumOrder` owner-gated. + +### Slice 8 — sort regression guard + +W10 in a new `web/src/lib/utils/album-utils.spec.ts`. Small, and it is the assertion the whole +feature exists to satisfy. + +## 7. i18n + +**No new keys.** `date_created` already exists in `en` and all nine required locales +(verified), the mobile picker's own strings (`date_and_time`, `timezone`, `cancel`, +`action_common_update`) exist, and the error paths reuse `errors.unable_to_update_album_info` (web) +and `album_update_error` (mobile). + +If implementation turns up a genuinely new string, it lands in `de fr it nl pl es ru zh_Hans +zh_Hant` in the same commit, inserted in alphabetical position, followed by +`npx prettier --write i18n/*.json`. + +## 8. Verification + +`pnpm install` first — a fresh worktree has none. + +Several documented gates in `CLAUDE.md` do not do what they say. The commands below are the +corrected forms, verified against this worktree: + +```bash +# server — NOTE: `pnpm test -- --run ` silently runs the WHOLE suite (pnpm passes the +# literal `--` through and vitest drops the path filter). Omit the `--`. +cd server && pnpm test --run src/services/album.service.spec.ts +cd server && pnpm test:medium --run test/medium/specs/sync/sync-album.spec.ts # needs Docker +cd server && pnpm check && pnpm lint && pnpm format + +# web +cd web && pnpm test --run src/lib/modals/AlbumEditModal.spec.ts +cd web && pnpm test --run src/lib/utils/album-utils.spec.ts src/lib/managers/command-items.spec.ts +cd web && pnpm check:typescript && pnpm check:svelte && pnpm lint && pnpm format + +# e2e — needs a running stack; `make e2e-api-dev` does not exist +cd e2e && pnpm test src/specs/server/api/album.e2e-spec.ts + +# mobile +cd mobile && flutter test test/utils/album_permissions_test.dart \ + test/repositories/drift_album_api_repository_test.dart \ + test/domain/services/remote_album_service_test.dart \ + test/presentation/pages/drift_remote_album_page_test.dart +cd mobile && dart analyze --fatal-infos +cd mobile && dart format --set-exit-if-changed --output=none +``` + +Traps this list is built to avoid: + +- `pnpm test -- --run ` runs everything and reports unrelated pre-existing failures. + `pnpm exec vitest run ` from `server/` is the opposite failure — it loads no config and dies + with `describe is not defined` before running anything. Both forms appear in `CLAUDE.md`. +- **Check the reported file and test counts.** A vitest run of zero files is green. +- `vitest` does not typecheck. `pnpm check` (`tsc --noEmit`), `pnpm lint` and `pnpm format` are + three separate CI gates; eslint-green is not prettier-green. +- `dart format .` across `mobile/` reformats hundreds of files, because the local Flutter formats + differently from CI and CI's task covers `lib` only. Format only the files this change touches. +- `dart analyze` is not a substitute for `flutter test` — generated-code compile errors only surface + when a test actually compiles. + +Mobile prerequisites: use the Flutter version pinned in `mobile/mise.toml` (read it — this worktree +says 3.44.8, and the pin has moved before), then `flutter pub get` and generate localization/keys +once: `dart run easy_localization:generate -S ../i18n && dart run bin/generate_keys.dart`. If a +`mise install` symlinks a patch that self-reports the wrong version, invoke the binary directly from +`~/.local/share/mise/installs/aqua-flutter-flutter//flutter/bin/`. Export `PATH` _before_ +`cd`, not chained after it — a failed `cd` short-circuits the `&&` and you silently get the default +toolchain. + +Prettier must also run over this spec before committing; CI's Docs Build is strict about markdown +under `docs/`. + +Manual check, both platforms: backdate an album to 1996, sort the albums list by Date created +descending, confirm it lands last; confirm the mobile list agrees after a sync. + +## 9. Risks + +- **`createdAt` loses its audit meaning**, irreversibly per album. Accepted (§2.2). +- **Unsorted API listings reorder.** `album.repository.ts:148,283,421`. Web and mobile re-sort + client-side; CLI and third-party consumers may notice. +- **Rebase surface.** Upstream files touched: `album.dto.ts`, `album.service.ts`, + `AlbumEditModal.svelte`, `AlbumsList.svelte`, `drift_remote_album.page.dart`, + `drift_album_api_repository.dart`, `remote_album.service.dart`, `remote_album.provider.dart` — one + or two lines each. Fork-only files: `command-items.ts`, `album_permissions.dart`. If upstream ever + adds its own album date field, reconcile then. +- **Silent gate regression.** The mobile affordance depends on `currentUserRole`, which is null + unless `getAll` receives `currentUserId` (#985). Fail-open (M4) means a regression there degrades + to "affordance shown, server refuses" rather than "affordance silently vanishes". + +## 10. Follow-ups (not this change) + +- Enable year grouping under the `DateCreated` sort once the date is meaningful + (`album-utils.ts:130-133`). +- Bulk date editing across selected albums, for the "many albums from previous years" import. +- Reply to #520 explaining that a viewer of someone else's album still cannot reorder it, and what + would be needed (per-viewer override, §2.1). From 395491a6926d858d04c47dc0d1396182e5b6eb29 Mon Sep 17 00:00:00 2001 From: Pierre Marais Date: Fri, 14 Aug 2026 21:13:23 +0200 Subject: [PATCH 02/19] docs: correct and extend the album creation date spec after review --- .../2026-08-14-album-creation-date-design.md | 316 +++++++++++++----- 1 file changed, 239 insertions(+), 77 deletions(-) diff --git a/docs/superpowers/specs/2026-08-14-album-creation-date-design.md b/docs/superpowers/specs/2026-08-14-album-creation-date-design.md index 195bef5effc9f..85d445c0420c7 100644 --- a/docs/superpowers/specs/2026-08-14-album-creation-date-design.md +++ b/docs/superpowers/specs/2026-08-14-album-creation-date-design.md @@ -31,10 +31,11 @@ Make `album.createdAt` user-editable. `createdAt` is already plumbed end to end — it is on `AlbumTable` (`server/src/schema/tables/album.table.ts:36`), in `AlbumResponseDto` -(`album.dto.ts:123`), in the `SyncAlbumV1` sync stream (`sync.repository.ts:214`), on mobile's -`RemoteAlbum` model, rendered by the albums table (`AlbumsTableRow.svelte:61`), and sorted on by -`AlbumSortBy.DateCreated` (web), `AlbumSortMode.created` (mobile) and the fork's space-album sort. -The single missing link is that it is absent from `UpdateAlbumSchema` (`album.dto.ts:228`). +(`album.dto.ts:124`), in the `SyncAlbumV1` sync stream (`sync.repository.ts:214`), on mobile's +`RemoteAlbum` model, rendered by the albums table (`AlbumsTableRow.svelte:61`) and the fork's space +albums table (`space-albums-table.svelte:56`), and sorted on by `AlbumSortBy.DateCreated` (web), +`AlbumSortMode.created` (mobile) and the fork's space-album sort. +The single missing link is that it is absent from `UpdateAlbumSchema` (`album.dto.ts:57`). The change is therefore one optional DTO field, one service pass-through, a date field in each of the two existing album-edit surfaces, and a relaxation of three ownership gates. @@ -73,9 +74,43 @@ Out of scope: a new sort option (`DateCreated` already exists on all three surfa multi-album date editing; per-viewer overrides; adding a `createdAt` display to the album detail page (it shows the asset date range and keeps doing so); enabling year grouping under `DateCreated`. +### 3.1 Which surfaces reach the editor + +Web reaches `AlbumEditModal` from exactly two places, and neither is on an album page: + +| Surface | Edits name/description via | Reaches the date editor? | +| ---------------------------------------------------------------------------------- | ------------------------------------------------------------ | ------------------------ | +| `/albums` list, right-click → Edit album (`AlbumsList.svelte:205`) | `AlbumEditModal` | yes | +| Command palette `cmd:album_rename` (`command-items.ts:265`) | `AlbumEditModal` | yes | +| Album detail page | inline `AlbumTitle` / `AlbumDescription` | **no** | +| Space album detail page (`spaces/[spaceId]/albums/[albumId]/+page.svelte:431-443`) | inline `AlbumTitle` / `AlbumDescription`, gated on `isOwned` | **no** | +| Space albums list (`space-albums-table.svelte:56`) | not editable; renders `createdAt` only | **no** | + +**Decision: leave it at those two entry points for this change.** Adding an inline date affordance +to the album detail pages means inventing a display element next to `AlbumSummary`, which currently +shows the asset date range — a bigger UI question than this feature warrants, and one that would +have to be answered twice (regular and space album pages). + +This is an accepted asymmetry with mobile, where the date sits in the kebab menu's Edit dialog and +is therefore visible. Call it out in the PR description: on web, an album's date is changed from the +albums list, not from the album. If that proves too hidden, the follow-up is an inline affordance on +both detail pages (§10), not a second modal. + +Note the consequence for #520 specifically: an album shared **into a Space** is most naturally +reached through the space album page, which cannot open the modal. The requester's own workflow +therefore routes through `/albums`, where the album also appears. + ## 4. Behaviour specification -Written as Given/When/Then. Every scenario below maps to a test in §6; the numbering is shared. +Written as Given/When/Then. Every scenario below maps to a test in §6 and the numbering is shared. +Each scenario is owned by exactly one slice, with one deliberate exception: S1, S5 and S6 are +asserted twice — Slice 1 pins the service pass-through against a mock, Slice 2 pins the HTTP +behaviour against a real database. Both halves are named where they appear. + +Two groups deliberately use a compact input → outcome form instead: §4.2 (grammar validation) and +M1–M4/M11 (one affordance, varying only by the caller's role). Each is a single input with no +meaningful Given, and spelling out three lines apiece would obscure the boundary table rather than +clarify it. ### 4.1 Server — writing the field @@ -125,33 +160,76 @@ Then the album appears in the next `SyncAlbumV1` batch carrying the new `created ### 4.2 Server — input validation `createdAt` uses the existing `isoDatetimeToDate` codec (`server/src/validation.ts:139-152`), which -is `z.iso.datetime({ offset: true })` decoded to a `Date`. That fixes the accepted grammar: - -**S9** `"1996-06-15T14:30:00.000Z"` → accepted. -**S10** `"1996-06-15T14:30:00+02:00"` → accepted; stored as the equivalent UTC instant. -**S11** `"1996-06-15T14:30:00"` (no offset) → **400**. This is the trap for API users and must be -documented in the field description. -**S12** `"1996-06-15"` (date only) → 400. -**S13** `"not-a-date"` → 400. -**S14** `""` → 400. -**S15** `null` → 400 (the field is optional, not nullable). -**S16** `"12345-06-15T14:30:00Z"` (5-digit year) → 400; `z.iso.datetime` requires exactly 4. -**S17** `"0001-01-01T00:00:00.000Z"` → accepted; within `timestamptz` range. -**S18** a future date → **accepted**, deliberately. See §5. +is `z.iso.datetime({ offset: true })` decoded to a `Date`. + +Read `offset: true` correctly: it **permits** `±HH:MM` _in addition to_ `Z`. It does not require a +timezone — that is the default, and `local` (which would permit a bare local datetime) is left +false. Removing `offset: true` would make the grammar stricter, not looser. + +The accepted grammar is not a matter of interpretation: Zod emits the regex below into +`open-api/immich-openapi-specs.json`, where it also becomes the generated clients' validation. Every +scenario here was checked against it rather than reasoned about. + +``` +^(?:(?:\d\d[2468][048]|\d\d[13579][26]|\d\d0[48]|[02468][048]00|[13579][26]00)-02-29 + |\d{4}-(?:(?:0[13578]|1[02])-(?:0[1-9]|[12]\d|3[01]) + |(?:0[469]|11)-(?:0[1-9]|[12]\d|30) + |(?:02)-(?:0[1-9]|1\d|2[0-8]))) +T(?:(?:[01]\d|2[0-3]):[0-5]\d(?::[0-5]\d(?:\.\d+)?)?(?:Z|([+-](?:[01]\d|2[0-3]):[0-5]\d)))$ +``` + +| # | Input | Result | Why | +| --- | ----------------------------- | ----------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| S9 | `1996-06-15T14:30:00.000Z` | accepted | baseline | +| S10 | `1996-06-15T14:30:00+02:00` | accepted | stored as the equivalent UTC instant | +| S11 | `1996-06-15T14:30:00` | **400** | the `(?:Z\|offset)` group is required — the API trap | +| S12 | `1996-06-15` | 400 | no `T` segment | +| S13 | `not-a-date` | 400 | — | +| S14 | `""` | 400 | — | +| S15 | `null` | 400 | the field is optional, not nullable | +| S16 | `12345-06-15T14:30:00Z` | 400 | `\d{4}-` requires the dash at position 5 | +| S17 | `0001-01-01T00:00:00.000Z` | accepted | `\d{4}` matches `0001`; within `timestamptz` range | +| S18 | a future date | accepted | **deliberate** — see §5 | +| S19 | `1996-06-15T14:30Z` | accepted | seconds are optional in the pattern | +| S20 | `1996-06-31T00:00:00Z` | 400 | June is in the 30-day branch — real calendar validation | +| S21 | `1997-02-29T00:00:00Z` | 400 | 1997 is not a leap year | +| S22 | `1996-02-29T00:00:00Z` | accepted | 1996 **is** — the leap branch matches | +| S23 | `1996-06-15T24:00:00Z` | 400 | hours cap at `2[0-3]` | +| S24 | `1996-06-15T14:30:00.123456Z` | accepted, **truncated** | the pattern allows `\.\d+`, but `new Date()` keeps only milliseconds. Assert the stored value is `…123Z`, so the loss is pinned rather than discovered later | +| S25 | `1996-06-15t14:30:00z` | 400 | the pattern requires uppercase `T` / `Z`, though ISO 8601 permits lowercase | + +S20–S22 matter more than they look: this feature exists to backdate albums into the 1990s, and +February 29 is exactly the input that quietly breaks naive date handling. + +**S26 — an empty body is a no-op-ish update** +Given any album the caller may update +When the caller sends `PATCH /albums/:id` with `{}` +Then the response is 200, no field changes, and `updatedAt` still advances (the service always sets +`id`, so the `UPDATE` runs and the `album_updatedAt` trigger fires). Pre-existing behaviour, pinned +here because adding a field to the DTO is when someone would think to change it. ### 4.3 Web -**W1 — the modal pre-fills** +**W1 — the modal pre-fills in local time** Given an album with `createdAt` `1996-06-15T12:30:00.000Z` and a browser in `Europe/Berlin` When `AlbumEditModal` opens -Then the date input shows `1996-06-15T14:30:00.000` (local time, matching how -`dateLocaleString(album.createdAt)` renders it in the albums table). +Then the date input shows `1996-06-15T14:30:00.000`. + +Local time is the right interpretation, but note it is **not** what the albums table shows. That +column renders `dateLocaleString` — a local closure in `AlbumsTableRow.svelte:24`, duplicated in +`space-albums-table.svelte:31` — which formats with `dateFormats.album` +(`constants.ts:35-39`): `{ month, day, year }`, **date only**. See §4.5. **W2 — editing submits a zoned ISO string** When the user changes the input to `1996-06-15T14:30:00.000` and submits Then `updateAlbumInfo` is called with a `createdAt` carrying an offset (Luxon `DateTime.fromISO(local).toISO()`), never a bare local string — S11 would otherwise 400. +The offset must be the **historical** one for that instant, not today's: Luxon resolves +`Europe/Berlin` in June 1996 to `+02:00` via the IANA database. A hand-rolled +`new Date().getTimezoneOffset()` would stamp the _current_ offset onto a 1996 date and land the +album an hour off. Assert the full string, not just that an offset is present. + **W3 — no blur required** When the user types a date and clicks Save without blurring the input first Then the typed value is submitted. @@ -163,6 +241,12 @@ pinned rather than assumed.) When the user edits only the name and submits Then the DTO contains `albumName` and no `createdAt`. +"Unchanged" must be decided on **instants**, not strings: +`DateTime.fromISO(input).toMillis() !== DateTime.fromISO(album.createdAt).toMillis()`. +The input is local-zone and `album.createdAt` is normally `…Z`, so string comparison would report +every album as changed. Include a test where the user opens and closes the modal untouched, in a +non-UTC zone, and assert `createdAt` is absent — that is the case string comparison fails. + **W5 — an emptied or invalid date is not sent** When the user clears the date input and submits Then the DTO omits `createdAt` and the name/description edits still apply. @@ -199,19 +283,50 @@ Then the order is 2026, 2010, 1996. Characterization test over untouched code (`album-utils.ts:243`), included because the feature's entire value rests on it. +**W11 — the list re-sorts after an edit** +Given `/albums` sorted by Date created +When the user changes an album's date through the context menu +Then the list reflects the new position without a reload. +`handleUpdateAlbum` emits `eventManager.emit('AlbumUpdate', response)` (`album.service.ts:310`) and +`sortAlbums` runs on the derived album list, so this should follow — but it is the user-visible +payoff of the whole feature, so it gets asserted rather than assumed. + ### 4.4 Mobile -**M1 — `canEditAlbum` for an owner** → true. -**M2 — for an `editor`** → true. -**M3 — for a `viewer`** → false. -**M4 — for a null `currentUserRole`** → **true, failing open.** -This mirrors the documented posture of `canAddAssetsToAlbum` in the same file -(`mobile/lib/utils/album_permissions.dart`): a null role means "not known", the server is the real -enforcer, and hiding an affordance we are merely unsure about is the worse failure. It also -defuses the #985 trap — `RemoteAlbum.currentUserRole` is null unless `getAll` was passed -`currentUserId`. `remote_album.provider.dart:56-57` does pass it, so the role is populated for -albums reached from the main list; fail-open covers every other entry point, including the fact -that `updateAlbum` replaces the state album with `toRemoteAlbum()` output, which carries no role. +Mobile already answers "may this user edit?" **twice, differently**, on this one screen, and the +spec must not add a third answer: + +- `_RemoteAlbumPageState.build` (`drift_remote_album.page.dart:204`) computes + `isOwner = user.id == _album.ownerId` and gates `onEditTitle` on it. Synchronous, no role lookup. +- `_AlbumKebabMenu` (`:442-450`) computes `isOwner` the same way **and** resolves editor-ness + asynchronously — `FutureBuilder` over `remoteAlbumServiceProvider.getUserRole(album.id, user.id)`, + combined as `isOwner || canAddPhotos`, defaulting to **false** while the future is pending + (`snapshot.data ?? false`). + +So the kebab menu already has a working owner-or-editor signal that **fails closed**, three lines +above the callback this change re-gates. + +**Decision: reuse `getUserRole`, do not introduce a `currentUserRole` predicate.** `onEditAlbum` +moves inside the existing `FutureBuilder` and is gated on the same `isOwner || canAddPhotos` +expression that already gates `onAddPhotos`. Nothing new is invented, and the two adjacent +affordances cannot disagree. + +`RemoteAlbum.currentUserRole` is explicitly **not** used here. It is null unless `getAll` was passed +`currentUserId` (#985), and `updateAlbum` replaces the state album with `toRemoteAlbum()` output, +which carries no role at all — so an edit would silently revoke the affordance it just used. A +fail-open predicate over that field would also contradict the fail-closed `FutureBuilder` beside it. + +`onEditTitle` (`:226`) stays gated on `isOwner`: that widget has no `FutureBuilder`, and adding one +to the app bar to relax a title-tap is out of proportion. Editors reach the dialog through the kebab. + +**M1 — kebab Edit album for an owner** → shown (`isOwner` true, no lookup needed). +**M2 — for an `editor`** → shown once `getUserRole` resolves to `editor`. +**M3 — for a `viewer`** → hidden. +**M4 — while `getUserRole` is pending** → **hidden**, then shown if the role resolves to editor. +Fail-closed, matching `onAddPhotos`. A brief flicker into existence is the accepted cost of +consistency with the affordance beside it. +**M11 — with no current user** (`currentUserProvider` null) → hidden; `isOwner` is false and +`getUserRole` is called with `''`, which cannot match a role. **M5 — the dialog shows the current date** Given `_EditAlbumDialog` opened for an album created 1996-06-15 @@ -224,6 +339,8 @@ Then `updateAlbum` is called with `createdAt` set to the picked instant. **M7 — cancelling the picker changes nothing** When the user opens the picker and dismisses it Then the pending date is unchanged and Save sends the original value. +`showDateTimePicker` returns `null` on dismiss; `action.service.dart:209-211` is the established +handling to mirror. **M8 — the API repository maps it** When `updateAlbum(albumId, owner, createdAt: dt)` is called @@ -244,6 +361,21 @@ Then the album's `createdAt` in `remoteAlbumEntity` matches the new value. `toRemoteAlbum` already maps it (`drift_album_api_repository.dart:143`), so this is a pin on an existing path rather than new code. +### 4.5 Date, or date and time? + +`createdAt` is displayed in exactly two places, both date-only (`dateFormats.album`). A +`datetime-local` editor therefore lets a user set a time they can never read back. + +**Decision: keep date _and_ time.** The requester asked for "creation date and time"; more +importantly, minute-or-coarser precision makes ties likely precisely in the bulk-backdating workflow +this exists for (a dozen albums all stamped 1996-01-01T00:00), and `DateCreated` ties resolve +arbitrarily. Time is doing real work as a tiebreaker even when invisible. + +The alternative — a date-only editor writing local midnight — is simpler and matches every display +surface, at the cost of unbreakable ties. If we take it, S24's truncation scenario and W2's +historical-offset trap both disappear, and `@immich/ui`'s `DatePicker` replaces `DateInput`. Revisit +only if the datetime input tests badly. + ## 5. Future dates Mobile's shared `showDateTimePicker` hard-codes `lastDate: now` @@ -274,7 +406,7 @@ then makes it pass. Slices are ordered so each one is independently green. Note the existing assertion style at :573-577 passes an exact object literal; `toHaveBeenCalledWith` uses `toEqual` semantics, which ignore `undefined`-valued keys, so the existing tests keep passing once the service gains the field. -2. **Green.** `album.dto.ts:228` — add to `UpdateAlbumSchema`: +2. **Green.** `album.dto.ts:57` — add to `UpdateAlbumSchema`: ```ts createdAt: isoDatetimeToDate .optional() @@ -300,15 +432,23 @@ then makes it pass. Slices are ordered so each one is independently green. ### Slice 2 — server validation and permissions (e2e) -Add the HTTP halves of S1, S5 and S6 plus S2, S3, S4, S7 and S9–S18 to `describe('PATCH /albums/:id')` -in `e2e/src/specs/server/api/album.e2e-spec.ts:589`. The suite already has the fixtures: -`user1Albums[0]` is shared with `user2` as editor, `user1Albums[3]` as viewer, and the existing -"should be able to update as an editor" / "should not be able to update as a viewer" tests give the -exact shape and error string (`Not found or no album.update access`) to mirror. +1. **Red.** Add the HTTP halves of S1, S5, S6 plus S2, S3, S4, S7 and S9–S26 to + `describe('PATCH /albums/:id')` in `e2e/src/specs/server/api/album.e2e-spec.ts:589`. Written + before Slice 1 ships they fail on the field being stripped by Zod; confirm that is the failure + reason, not a fixture problem. + The suite already has the fixtures: `user1Albums[0]` is shared with `user2` as editor, + `user1Albums[3]` as viewer, and the existing "should be able to update as an editor" / "should + not be able to update as a viewer" tests give the exact shape and error string + (`Not found or no album.update access`) to mirror. +2. **Green.** Slice 1 already satisfies these; if any fail, the DTO or service is wrong, not the test. +3. Drive S9–S25 from a table rather than 17 hand-written cases — the scenarios are `(input, +expectedStatus)` pairs and read better as `it.each`. Assert the **stored** value for the accepted + ones, not just the status, or S24's truncation passes silently. S5 in particular has to be proved here rather than in the unit test — "an omitted `createdAt` leaves -the stored value alone" is a claim about Kysely's `set()` dropping `undefined`, which only a real -database can settle. +the stored value alone" is a claim about Kysely's `set()` dropping `undefined` keys. The existing +"should update an album" test at :590 is the empirical precedent (it sends only `albumName` and +`description` and expects every other field untouched), but only a real database settles it. S8 goes in `server/test/medium/specs/sync/sync-album.spec.ts`, next to the existing album sync coverage: assert the album appears in the next `SyncEntityType.AlbumV1` batch carrying the new @@ -328,7 +468,8 @@ coverage: assert the album appears in the next `SyncEntityType.AlbumV1` batch ca and Description, holding `DateInput` with `type="datetime-local"`. Keep local state as a Luxon `DateTime` string in `yyyy-MM-dd'T'HH:mm:ss.SSS`, seeded from `album.createdAt`; on submit include `createdAt: DateTime.fromISO(value).toISO()` only when the parsed value is valid **and** - differs from `album.createdAt`. + its `.toMillis()` differs from `DateTime.fromISO(album.createdAt).toMillis()` (W4 — never compare + the strings). 3. `DateInput` (`web/src/lib/elements/DateInput.svelte`) is the right element rather than `@immich/ui`'s `DatePicker`: it is what `AssetChangeDateModal` uses for date **and** time, and `step=".001"` preserves the milliseconds S7 protects. No timezone combobox — unlike an asset's @@ -346,42 +487,52 @@ coverage: assert the album appears in the next `SyncEntityType.AlbumV1` batch ca `spaces/[spaceId]/albums/[albumId]/+page.svelte:94-99`) and `isSelectedAlbumOwner` (the existing check). Edit uses the former; Share and Delete keep the latter. In `command-items.ts:260`, switch `cmd:album_rename` from `ctx.album.isOwner` to `ctx.album.isEditor`. + **Both derived flags keep the `allowEdit &&` conjunct.** It is the prop that makes the whole + context menu conditional (`AlbumsList.svelte:35,46`) and only `/albums` passes it + (`routes/(user)/albums/+page.svelte:50`); dropping it from the edit branch would surface Edit on + a list that opted out of editing entirely. 3. Leave `cmd:album_share` on `isOwner`. -### Slice 5 — mobile permission predicate - -1. **Red.** M1–M4 in the existing `mobile/test/utils/album_permissions_test.dart`. -2. **Green.** Add `canEditAlbum(RemoteAlbum album, {required String? currentUserId})` to - `mobile/lib/utils/album_permissions.dart`, returning true when the caller is the owner or when - `currentUserRole != AlbumUserRole.viewer`. Document the fail-open reasoning in the same voice as - the existing `canAddAssetsToAlbum` doc comment, and cross-reference - `Permission.AlbumUpdate` in `server/src/utils/access.ts`. - -### Slice 6 — mobile plumbing +### Slice 5 — mobile plumbing -1. **Red.** M8 in `mobile/test/repositories/drift_album_api_repository_test.dart` (including M9's - wire assertion), M6/M10 in `mobile/test/domain/services/remote_album_service_test.dart`. +1. **Red.** M8 in `mobile/test/repositories/drift_album_api_repository_test.dart`, including M9's + wire assertion — serialize the DTO and assert the JSON string ends in `Z`, rather than trusting + the generated code. M10 in `mobile/test/domain/services/remote_album_service_test.dart`. 2. **Green.** Thread `DateTime? createdAt` through `drift_album_api_repository.dart:71` → `UpdateAlbumDto(createdAt: …)`, - `remote_album.service.dart:137`, and `remote_album.provider.dart:154`. Nothing else changes: - `toRemoteAlbum` and `RemoteAlbumRepository.update` already carry `createdAt`. + `remote_album.service.dart:137`, and `remote_album.provider.dart:154`, following the + `Optional.present` / `Optional.absent` shape every sibling field already uses. Nothing else + changes: `toRemoteAlbum` (`drift_album_api_repository.dart:143`) and + `RemoteAlbumRepository.update` (`remote_album.repository.dart:227-240`) already carry `createdAt`. -### Slice 7 — mobile dialog +No `canEditAlbum` predicate is added to `mobile/lib/utils/album_permissions.dart`. See §4.4 — the +kebab menu already resolves owner-or-editor through `getUserRole`, and a second, differently-failing +answer beside it would be worse than no helper at all. -1. **Red.** M5–M7 in `mobile/test/presentation/pages/drift_remote_album_page_test.dart`. Widget - tests here have a documented history of passing vacuously — prove each one red first by - inverting the expectation, and assert on the mock call rather than on rendered text where the - text could match an unrelated widget. -2. **Green.** Add a date row to `_EditAlbumDialog` - (`drift_remote_album.page.dart:242`) driven by `showDateTimePicker`, which returns a - `String?` ISO value carrying an offset; parse with `DateTime.parse` and pass to `updateAlbum`. - Gate `onEditTitle` (:226) and `onEditAlbum` (:459) on `canEditAlbum` instead of `isOwner`, leaving - `onDeleteAlbum`, `onAddUsers`, `onCreateSharedLink` and `onToggleAlbumOrder` owner-gated. - -### Slice 8 — sort regression guard +### Slice 6 — mobile dialog and gating -W10 in a new `web/src/lib/utils/album-utils.spec.ts`. Small, and it is the assertion the whole -feature exists to satisfy. +1. **Red.** M1–M7 and M11 in `mobile/test/presentation/pages/drift_remote_album_page_test.dart`. + Widget tests here have a documented history of passing vacuously — prove each one red first by + inverting the expectation, and assert on the mock call rather than on rendered text where the + text could match an unrelated widget. M4 needs a pending `getUserRole` future, not a resolved + one, or it silently becomes a duplicate of M2. +2. **Green.** Add a date row to `_EditAlbumDialog` (`drift_remote_album.page.dart:242`) driven by + `showDateTimePicker`, which returns `String?` — an ISO value with a `+HH:MM` offset from + `formatAsOffset` (`duration_extensions.dart:3-4`), or `null` on dismiss. Parse with + `DateTime.parse` and pass to `updateAlbum`; `action.service.dart:202-219` is the existing + consumer to mirror, including its null-means-cancelled handling. +3. Move `onEditAlbum` (`:459`) inside the existing `FutureBuilder` and gate it on the same + `isOwner || canAddPhotos` expression that already gates `onAddPhotos` (`:457`). Leave + `onDeleteAlbum`, `onAddUsers`, `onCreateSharedLink`, `onToggleAlbumOrder` and `onLinkToSpace` + owner-gated, and leave `onEditTitle` (`:226`) on `isOwner` — §4.4 explains why the app bar does + not get a role lookup. + +### Slice 7 — sort regression guard + +1. **Red.** W10 in a new `web/src/lib/utils/album-utils.spec.ts`, and W11 wherever the `/albums` + list is exercised. W10 passes against today's code — that is the point of a characterization + test — so prove it meaningful by flipping the expected order and watching it fail. +2. It is small, and it is the assertion the whole feature exists to satisfy. ## 7. i18n @@ -417,8 +568,7 @@ cd web && pnpm check:typescript && pnpm check:svelte && pnpm lint && pnpm format cd e2e && pnpm test src/specs/server/api/album.e2e-spec.ts # mobile -cd mobile && flutter test test/utils/album_permissions_test.dart \ - test/repositories/drift_album_api_repository_test.dart \ +cd mobile && flutter test test/repositories/drift_album_api_repository_test.dart \ test/domain/services/remote_album_service_test.dart \ test/presentation/pages/drift_remote_album_page_test.dart cd mobile && dart analyze --fatal-infos @@ -460,16 +610,28 @@ descending, confirm it lands last; confirm the mobile list agrees after a sync. - **Rebase surface.** Upstream files touched: `album.dto.ts`, `album.service.ts`, `AlbumEditModal.svelte`, `AlbumsList.svelte`, `drift_remote_album.page.dart`, `drift_album_api_repository.dart`, `remote_album.service.dart`, `remote_album.provider.dart` — one - or two lines each. Fork-only files: `command-items.ts`, `album_permissions.dart`. If upstream ever - adds its own album date field, reconcile then. -- **Silent gate regression.** The mobile affordance depends on `currentUserRole`, which is null - unless `getAll` receives `currentUserId` (#985). Fail-open (M4) means a regression there degrades - to "affordance shown, server refuses" rather than "affordance silently vanishes". + or two lines each. Fork-only file: `command-items.ts`. If upstream ever adds its own album date + field, reconcile then. +- **The mobile affordance fails closed.** Gating `onEditAlbum` on the `getUserRole` future (M4) means + that if the lookup is slow or errors, an editor sees no Edit entry rather than one the server would + refuse. That is the deliberate cost of matching `onAddPhotos` beside it; the alternative was two + affordances on one screen disagreeing about the same permission. +- **Concurrent edits are last-write-wins.** Two clients editing the same album's date race with no + detection — same as every other field on this endpoint, which carries no `updatedAt` precondition. + Not introduced here, not addressed here. +- **Millisecond truncation is silent.** An API client sending microsecond precision gets it rounded + to milliseconds with a 200 (S24). Pinned by test so it is documented behaviour. ## 10. Follow-ups (not this change) - Enable year grouping under the `DateCreated` sort once the date is meaningful (`album-utils.ts:130-133`). -- Bulk date editing across selected albums, for the "many albums from previous years" import. +- **`createdAt` on `CreateAlbumSchema`** (`album.dto.ts:34`), so an importer can create a + correctly-dated album in one call instead of create-then-patch. This is the closest thing to what + the requester actually described ("uploading many albums from previous years") and is a smaller + change than bulk editing — but it widens the create contract, so it gets its own decision. +- Bulk date editing across selected albums. +- An inline date affordance on the album detail pages, if the albums-list-only entry point (§3.1) + proves too hidden. It would have to be answered for the space album page too. - Reply to #520 explaining that a viewer of someone else's album still cannot reorder it, and what would be needed (per-viewer override, §2.1). From f9a41b3284b22decec68abc42efa344b0fedb70e Mon Sep 17 00:00:00 2001 From: Pierre Marais Date: Fri, 14 Aug 2026 21:24:09 +0200 Subject: [PATCH 03/19] docs: implementation plan for editable album creation date --- .../plans/2026-08-14-album-creation-date.md | 1474 +++++++++++++++++ .../2026-08-14-album-creation-date-design.md | 20 +- 2 files changed, 1488 insertions(+), 6 deletions(-) create mode 100644 docs/superpowers/plans/2026-08-14-album-creation-date.md diff --git a/docs/superpowers/plans/2026-08-14-album-creation-date.md b/docs/superpowers/plans/2026-08-14-album-creation-date.md new file mode 100644 index 0000000000000..9ae9eb07916d5 --- /dev/null +++ b/docs/superpowers/plans/2026-08-14-album-creation-date.md @@ -0,0 +1,1474 @@ +# Editable Album Creation Date Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Let an album's owner or editor change its creation date, so albums of old photos sort chronologically instead of by upload order. + +**Architecture:** `album.createdAt` is already carried end to end — response DTO, sync stream, mobile Drift model, both sort implementations. The only missing link is that it is not writable. This adds one optional field to `UpdateAlbumSchema`, one pass-through in `AlbumService.update`, a date field in each of the two existing album-edit surfaces (web `AlbumEditModal`, mobile `_EditAlbumDialog`), and relaxes three ownership gates to owner-or-editor. No migration, no schema change, no sync change. + +**Tech Stack:** NestJS 11 + Zod DTOs + Kysely (server), Vitest (unit/e2e/medium), SvelteKit 5 + `@immich/ui` + Luxon (web), Flutter + Riverpod + mocktail (mobile), OpenAPI → `@immich/sdk` + Dart client. + +**Spec:** `docs/superpowers/specs/2026-08-14-album-creation-date-design.md` + +## Global Constraints + +- **Endpoint is `PATCH /albums/:id`** (not PUT). +- **Permission model:** owner ∪ album editor. `Permission.AlbumUpdate` already grants exactly this (`server/src/utils/access.ts:208-216`) — no server-side access change. +- **Wire format:** `createdAt` must be an ISO 8601 string **with a timezone designator** (`Z` or `±HH:MM`). A bare local datetime is rejected with 400. +- **Zero new i18n keys.** Use the existing `date_created` key, present in `en` + all nine required locales. If a genuinely new string appears, it lands in `de fr it nl pl es ru zh_Hans zh_Hant` in the same commit followed by `npx prettier --write i18n/*.json`. +- **Never run `make open-api` or `mise open-api`** — see Task 2. +- **`pnpm test -- --run ` silently runs the whole suite.** Always `pnpm test --run `, no `--`. +- **Check reported test counts.** A vitest run of zero files reports green. +- Run `pnpm install` once before starting; a fresh worktree has no `node_modules`. + +--- + +## File Structure + +| File | Responsibility | Task | +| --------------------------------------------------------------------------------------------- | ---------------------------------- | ---- | +| `server/src/dtos/album.dto.ts` | accept `createdAt` on update | 1 | +| `server/src/services/album.service.ts` | pass `createdAt` to the repository | 1 | +| `server/src/services/album.service.spec.ts` | pass-through unit tests | 1 | +| `open-api/immich-openapi-specs.json`, `packages/sdk/src/fetch-client.ts`, `mobile/openapi/**` | generated clients | 2 | +| `e2e/src/specs/server/api/album.e2e-spec.ts` | permissions + validation grammar | 3 | +| `server/test/medium/specs/sync/sync-album.spec.ts` | sync stream carries the new date | 4 | +| `web/src/lib/utils/album-utils.ts` | `isAlbumEditor` helper | 5 | +| `web/src/lib/utils/album-utils.spec.ts` | helper + sort characterization | 5 | +| `web/src/lib/modals/AlbumEditModal.svelte` | the date field | 6 | +| `web/src/lib/modals/AlbumEditModal.spec.ts` | modal behaviour | 6 | +| `web/src/lib/components/album-page/AlbumsList.svelte` | context-menu gating | 7 | +| `web/src/lib/managers/command-items.ts` + `.spec.ts` | command-palette gating | 7 | +| `mobile/lib/repositories/drift_album_api_repository.dart` | `createdAt` → `UpdateAlbumDto` | 8 | +| `mobile/lib/domain/services/remote_album.service.dart` | thread the parameter | 8 | +| `mobile/lib/providers/infrastructure/remote_album.provider.dart` | thread the parameter | 8 | +| `mobile/lib/presentation/pages/drift_remote_album.page.dart` | date row + kebab gating | 9 | + +--- + +### Task 1: Server accepts `createdAt` on album update + +**Files:** + +- Modify: `server/src/dtos/album.dto.ts:12` (import), `:57-65` (`UpdateAlbumSchema`) +- Modify: `server/src/services/album.service.ts:231-241` +- Test: `server/src/services/album.service.spec.ts:525-579` (`describe('update')`) + +**Interfaces:** + +- Consumes: nothing. +- Produces: `UpdateAlbumDto.createdAt?: Date` (decoded from an ISO string by the `isoDatetimeToDate` codec). Tasks 2, 3, 4 depend on this field existing. + +- [ ] **Step 1: Write the failing tests** + +Append these three tests inside `describe('update', ...)` in `server/src/services/album.service.spec.ts`, after the existing `'should allow the owner to update the album'` test: + +```ts +it('should allow the owner to update the album created date', async () => { + const album = AlbumFactory.create(); + const { user: owner } = album.albumUsers.find(({ role }) => role === AlbumUserRole.Owner)!; + const createdAt = new Date('1996-06-15T14:30:00.000Z'); + mocks.access.album.checkOwnerAccess.mockResolvedValue(new Set([album.id])); + mocks.album.getById.mockResolvedValue(getForAlbum(album)); + mocks.album.update.mockResolvedValue(getForAlbum(album)); + + await sut.update(AuthFactory.create(owner), album.id, { createdAt }); + + expect(mocks.album.update).toHaveBeenCalledWith(album.id, { id: album.id, createdAt }, owner.id); +}); + +it('should update the album name and created date together', async () => { + const album = AlbumFactory.create(); + const { user: owner } = album.albumUsers.find(({ role }) => role === AlbumUserRole.Owner)!; + const createdAt = new Date('1996-06-15T14:30:00.000Z'); + mocks.access.album.checkOwnerAccess.mockResolvedValue(new Set([album.id])); + mocks.album.getById.mockResolvedValue(getForAlbum(album)); + mocks.album.update.mockResolvedValue(getForAlbum(album)); + + await sut.update(AuthFactory.create(owner), album.id, { albumName: 'Summer 1996', createdAt }); + + expect(mocks.album.update).toHaveBeenCalledWith( + album.id, + { id: album.id, albumName: 'Summer 1996', createdAt }, + owner.id, + ); +}); + +it('should leave the created date undefined when the dto omits it', async () => { + const album = AlbumFactory.create(); + const { user: owner } = album.albumUsers.find(({ role }) => role === AlbumUserRole.Owner)!; + mocks.access.album.checkOwnerAccess.mockResolvedValue(new Set([album.id])); + mocks.album.getById.mockResolvedValue(getForAlbum(album)); + mocks.album.update.mockResolvedValue(getForAlbum(album)); + + await sut.update(AuthFactory.create(owner), album.id, { albumName: 'Renamed' }); + + const [, update] = mocks.album.update.mock.calls[0]; + expect(update.createdAt).toBeUndefined(); +}); +``` + +`toHaveBeenCalledWith` uses `toEqual` semantics, which ignore keys whose value is `undefined` — that is why the first two tests can assert an exact object even though the service passes six keys. + +- [ ] **Step 2: Run the tests to verify they fail** + +Run: `cd server && pnpm test --run src/services/album.service.spec.ts` + +Expected: the first two FAIL — the received object has no `createdAt` because Zod strips unknown keys and the service never forwards it. The third passes already (it asserts absence); that is fine, it is a regression guard for Step 3. + +- [ ] **Step 3: Add the field to the DTO** + +In `server/src/dtos/album.dto.ts`, widen the `src/validation` import on line 12: + +```ts +import { isoDatetimeToDate, stringToBool } from 'src/validation'; +``` + +Then add `createdAt` to `UpdateAlbumSchema` (line 57), after `description`: + +```ts +const UpdateAlbumSchema = z + .object({ + albumName: z.string().optional().describe('Album name'), + description: z.string().optional().describe('Album description'), + createdAt: isoDatetimeToDate + .optional() + .describe('Album creation date. Must include a timezone designator (Z or ±HH:MM).'), + albumThumbnailAssetId: z.uuidv4().optional().describe('Album thumbnail asset ID'), + isActivityEnabled: z.boolean().optional().describe('Enable activity feed'), + order: AssetOrderSchema.optional(), + }) + .meta({ id: 'UpdateAlbumDto' }); +``` + +- [ ] **Step 4: Forward it in the service** + +In `server/src/services/album.service.ts`, add one line to the object passed to `albumRepository.update` (line 233-240): + +```ts +const updatedAlbum = await this.albumRepository.update( + album.id, + { + id: album.id, + albumName: dto.albumName, + description: dto.description, + createdAt: dto.createdAt, + albumThumbnailAssetId: dto.albumThumbnailAssetId, + isActivityEnabled: dto.isActivityEnabled, + order: dto.order, + }, + auth.user.id, +); +``` + +No repository change: `update(id, album: Updateable, authUserId)` (`album.repository.ts:621`) already accepts `createdAt` because the column is `Generated`. + +- [ ] **Step 5: Run the tests to verify they pass** + +Run: `cd server && pnpm test --run src/services/album.service.spec.ts` +Expected: PASS, including the four pre-existing `update` tests. + +- [ ] **Step 6: Run the type and lint gates** + +Run: `cd server && pnpm check && pnpm lint && pnpm format` +Expected: all clean. `vitest` does not typecheck, and eslint-green is not prettier-green — these are three separate CI gates. + +- [ ] **Step 7: Commit** + +```bash +git add server/src/dtos/album.dto.ts server/src/services/album.service.ts server/src/services/album.service.spec.ts +git commit -m "feat(server): allow updating an album's creation date" +``` + +--- + +### Task 2: Regenerate the OpenAPI clients + +**Files:** + +- Modify: `open-api/immich-openapi-specs.json` +- Modify: `packages/sdk/src/fetch-client.ts` +- Modify: `mobile/openapi/lib/model/update_album_dto.dart` (+ any other regenerated files) + +**Interfaces:** + +- Consumes: `UpdateAlbumDto.createdAt` from Task 1. +- Produces: TypeScript `UpdateAlbumDto.createdAt?: string` for web (Tasks 6, 7); Dart `api.UpdateAlbumDto({Optional createdAt})` for mobile (Tasks 8, 9). + +- [ ] **Step 1: Regenerate** + +**Do not run `make open-api`** — it is a removed stub that prints a message and `exit 1`. **Do not run `mise open-api`** — that task hardcodes `//server:install`, `//server:build`, `//server:sync-open-api`, and `//` resolves to the **main checkout**, so it would generate clients from main's server source instead of this branch's. + +From the worktree root: + +```bash +cd server && pnpm build && node ./dist/bin/sync-open-api.js +cd .. && mise run open-api-typescript && mise run open-api-dart +``` + +`open-api-dart` needs Java (JDK 21 works). + +- [ ] **Step 2: Verify the field actually landed** + +`mobile/openapi/**/*.dart` is marked `-diff -merge` in `.gitattributes`, so git shows those files as `Bin N -> M bytes` with no textual diff. Verify with grep, not `git diff`: + +```bash +grep -n "createdAt" mobile/openapi/lib/model/update_album_dto.dart +grep -n "createdAt" open-api/immich-openapi-specs.json | head -3 +``` + +Expected: `update_album_dto.dart` declares `createdAt` and serializes it via `value.toUtc().toIso8601String()`. + +- [ ] **Step 3: Verify the generation is deterministic** + +Run Step 1 again. Expected: `git status` shows no further change. A second run that dirties the tree means the generator picked up something else — stop and investigate before committing. + +- [ ] **Step 4: Commit** + +```bash +git add open-api packages/sdk mobile/openapi +git commit -m "chore: regenerate api clients for album createdAt" +``` + +Skipping the Dart half passes locally and fails CI's **OpenAPI Clients** job. + +--- + +### Task 3: Server e2e — permissions and validation grammar + +**Files:** + +- Modify: `e2e/src/specs/server/api/album.e2e-spec.ts:589-634` (`describe('PATCH /albums/:id')`) + +**Interfaces:** + +- Consumes: `UpdateAlbumDto.createdAt` (Task 1), the regenerated `@immich/sdk` (Task 2). +- Produces: nothing. + +Every test below creates its **own** album rather than reusing the `user1Albums` / `user2Albums` fixtures. Those are built once in `beforeAll` and shared with the `GET /albums` and `DELETE /albums/:id/assets` blocks; the server orders album listings by `album.createdAt desc` (`album.repository.ts:148,283,421`), so backdating a shared fixture to 1996 would reorder other suites' expectations. + +- [ ] **Step 1: Write the permission tests** + +Add inside `describe('PATCH /albums/:id', ...)`: + +```ts +it('should set the album created date as the owner', async () => { + const album = await utils.createAlbum(user1.accessToken, { albumName: 'Backdated' }); + + const { status, body } = await request(app) + .patch(`/albums/${album.id}`) + .set('Authorization', `Bearer ${user1.accessToken}`) + .send({ createdAt: '1996-06-15T14:30:00.000Z' }); + + expect(status).toBe(200); + expect(body.createdAt).toBe('1996-06-15T14:30:00.000Z'); + expect(body.updatedAt).not.toBe(album.updatedAt); + + const after = await getAlbumInfo({ id: album.id }, { headers: asBearerAuth(user1.accessToken) }); + expect(after.createdAt).toBe('1996-06-15T14:30:00.000Z'); +}); + +it('should set the album created date as an editor', async () => { + const album = await utils.createAlbum(user1.accessToken, { + albumName: 'Editor may re-date', + albumUsers: [{ userId: user2.userId, role: AlbumUserRole.Editor }], + }); + + const { status, body } = await request(app) + .patch(`/albums/${album.id}`) + .set('Authorization', `Bearer ${user2.accessToken}`) + .send({ createdAt: '1996-06-15T14:30:00.000Z' }); + + expect(status).toBe(200); + expect(body.createdAt).toBe('1996-06-15T14:30:00.000Z'); +}); + +it('should not set the album created date as a viewer', async () => { + const album = await utils.createAlbum(user1.accessToken, { + albumName: 'Viewer may not re-date', + albumUsers: [{ userId: user2.userId, role: AlbumUserRole.Viewer }], + }); + + const { status, body } = await request(app) + .patch(`/albums/${album.id}`) + .set('Authorization', `Bearer ${user2.accessToken}`) + .send({ createdAt: '1996-06-15T14:30:00.000Z' }); + + expect(status).toBe(400); + expect(body).toEqual(errorDto.badRequest('Not found or no album.update access')); +}); + +it('should not set the album created date as a non-member', async () => { + const album = await utils.createAlbum(user2.accessToken, { albumName: 'Not yours' }); + + const { status, body } = await request(app) + .patch(`/albums/${album.id}`) + .set('Authorization', `Bearer ${user1.accessToken}`) + .send({ createdAt: '1996-06-15T14:30:00.000Z' }); + + expect(status).toBe(400); + expect(body).toEqual(errorDto.badRequest('Not found or no album.update access')); +}); + +it('should leave the created date alone when the request omits it', async () => { + const album = await utils.createAlbum(user1.accessToken, { albumName: 'Keep my date' }); + + const { status, body } = await request(app) + .patch(`/albums/${album.id}`) + .set('Authorization', `Bearer ${user1.accessToken}`) + .send({ albumName: 'Renamed' }); + + expect(status).toBe(200); + expect(body.albumName).toBe('Renamed'); + expect(body.createdAt).toBe(album.createdAt); +}); + +it('should accept an empty body without changing anything', async () => { + const album = await utils.createAlbum(user1.accessToken, { albumName: 'Untouched' }); + + const { status, body } = await request(app) + .patch(`/albums/${album.id}`) + .set('Authorization', `Bearer ${user1.accessToken}`) + .send({}); + + expect(status).toBe(200); + expect(body.albumName).toBe('Untouched'); + expect(body.createdAt).toBe(album.createdAt); +}); +``` + +- [ ] **Step 2: Write the grammar table** + +The accepted grammar is fixed by the regex Zod emits for `isoDatetimeToDate` into `open-api/immich-openapi-specs.json`. It enforces a **required** timezone designator and **real calendar validity** (leap years, month lengths). Add: + +```ts +it.each([ + ['1996-06-15T14:30:00.000Z', 200, 'UTC with milliseconds'], + ['1996-06-15T14:30:00+02:00', 200, 'a numeric offset'], + ['1996-06-15T14:30Z', 200, 'omitted seconds'], + ['1996-02-29T00:00:00.000Z', 200, 'a real leap day'], + ['0001-01-01T00:00:00.000Z', 200, 'the earliest four-digit year'], + ['1996-06-15T14:30:00', 400, 'no timezone designator'], + ['1996-06-15', 400, 'a date with no time'], + ['not-a-date', 400, 'a non-date string'], + ['', 400, 'an empty string'], + [null, 400, 'null'], + ['12345-06-15T14:30:00Z', 400, 'a five-digit year'], + ['1996-06-31T00:00:00.000Z', 400, 'the 31st of a 30-day month'], + ['1997-02-29T00:00:00.000Z', 400, 'a leap day in a non-leap year'], + ['1996-06-15T24:00:00.000Z', 400, 'hour 24'], + ['1996-06-15t14:30:00z', 400, 'lowercase t and z'], +])('should return %s → %i for createdAt with %s', async (createdAt, expected) => { + const album = await utils.createAlbum(user1.accessToken, { albumName: `Grammar ${String(createdAt)}` }); + + const { status } = await request(app) + .patch(`/albums/${album.id}`) + .set('Authorization', `Bearer ${user1.accessToken}`) + .send({ createdAt }); + + expect(status).toBe(expected); +}); +``` + +- [ ] **Step 3: Write the two deliberate-behaviour tests** + +These pin decisions from the spec (§5 and the millisecond note) so a later change has to be a choice rather than an accident: + +```ts +it('should accept a future created date', async () => { + const album = await utils.createAlbum(user1.accessToken, { albumName: 'From the future' }); + const future = new Date(Date.now() + 365 * 24 * 60 * 60 * 1000).toISOString(); + + const { status, body } = await request(app) + .patch(`/albums/${album.id}`) + .set('Authorization', `Bearer ${user1.accessToken}`) + .send({ createdAt: future }); + + expect(status).toBe(200); + expect(body.createdAt).toBe(future); +}); + +it('should truncate sub-millisecond precision', async () => { + const album = await utils.createAlbum(user1.accessToken, { albumName: 'Microseconds' }); + + const { status, body } = await request(app) + .patch(`/albums/${album.id}`) + .set('Authorization', `Bearer ${user1.accessToken}`) + .send({ createdAt: '1996-06-15T14:30:00.123456Z' }); + + expect(status).toBe(200); + expect(body.createdAt).toBe('1996-06-15T14:30:00.123Z'); +}); +``` + +- [ ] **Step 4: Run the suite** + +The e2e stack must be running. Run: `cd e2e && pnpm test src/specs/server/api/album.e2e-spec.ts` + +Expected: PASS. `make e2e-api-dev` does not exist. Check the reported test count matches what you added — a path that matches nothing reports green. + +- [ ] **Step 5: Commit** + +```bash +git add e2e/src/specs/server/api/album.e2e-spec.ts +git commit -m "test(e2e): cover album createdAt permissions and validation" +``` + +--- + +### Task 4: Server medium — the sync stream carries the edited date + +**Files:** + +- Modify: `server/test/medium/specs/sync/sync-album.spec.ts` (inside `describe(SyncRequestType.AlbumsV1, ...)`) + +**Interfaces:** + +- Consumes: `AlbumRepository.update(id, { createdAt }, authUserId)`. +- Produces: nothing. + +This proves mobile picks up an edit made anywhere. `AlbumTable` carries `@UpdatedAtTrigger('album_updatedAt')`, so writing `createdAt` bumps `updatedAt` and `updateId`, which is what puts the row in the next sync batch. + +- [ ] **Step 1: Write the failing test** + +```ts +it('should detect and sync a changed album created date', async () => { + const { auth, ctx } = await setup(); + const albumRepo = ctx.get(AlbumRepository); + const { album } = await ctx.newAlbum({ ownerId: auth.user.id }); + + const initial = await ctx.syncStream(auth, [SyncRequestType.AlbumsV1]); + await ctx.syncAckAll(auth, initial); + await ctx.assertSyncIsComplete(auth, [SyncRequestType.AlbumsV1]); + + const createdAt = new Date('1996-06-15T14:30:00.000Z'); + await albumRepo.update(album.id, { createdAt }, auth.user.id); + + const response = await ctx.syncStream(auth, [SyncRequestType.AlbumsV1]); + const entry = response.find((item) => item.type === SyncEntityType.AlbumV1); + + expect(entry).toBeDefined(); + expect(entry!.data).toEqual(expect.objectContaining({ id: album.id })); + // Compare instants, not representations: the medium harness may hand back a Date + // or the encoded ISO string depending on serialization. + expect(new Date((entry!.data as { createdAt: string | Date }).createdAt).toISOString()).toBe( + '1996-06-15T14:30:00.000Z', + ); + + await ctx.syncAckAll(auth, response); + await ctx.assertSyncIsComplete(auth, [SyncRequestType.AlbumsV1]); +}); +``` + +- [ ] **Step 2: Run it** + +Medium tests need Docker, and in a fresh worktree the SDKs must be built first: + +```bash +pnpm --filter @immich/sdk build +pnpm --filter @immich/plugin-sdk build +cd server && pnpm test:medium --run test/medium/specs/sync/sync-album.spec.ts +``` + +Expected: PASS. This test exercises only existing plumbing, so it should pass immediately — that is the point of it. To prove it is meaningful, temporarily change the expected string to `1997-…` and confirm it fails, then change it back. + +- [ ] **Step 3: Commit** + +```bash +git add server/test/medium/specs/sync/sync-album.spec.ts +git commit -m "test(server): pin album createdAt propagation through the sync stream" +``` + +--- + +### Task 5: Web — `isAlbumEditor` helper and the sort characterization + +**Files:** + +- Modify: `web/src/lib/utils/album-utils.ts:1` (import), append the helper near the sorting section +- Create: `web/src/lib/utils/album-utils.spec.ts` + +**Interfaces:** + +- Consumes: nothing. +- Produces: `isAlbumEditor(album: AlbumResponseDto, userId: string): boolean` — true when `userId` holds an `albumUsers` role of `Owner` or `Editor`. Task 7 consumes it. + +The helper exists so the gating logic is unit-testable. `AlbumsList.svelte` has no spec and rendering it would pull in `authManager`, the preferences stores and `modalManager` for very little return; testing the predicate plus a type-checked template edit is the honest trade. Note this duplicates the inline `isAlbumEditor` at `web/src/routes/(user)/spaces/[spaceId]/albums/[albumId=id]/[[photos=photos]]/[[assetId=id]]/+page.svelte:94-99` — leave that copy alone, converging it is out of scope. + +- [ ] **Step 1: Write the failing tests** + +Create `web/src/lib/utils/album-utils.spec.ts`: + +```ts +import { AlbumUserRole, type AlbumResponseDto } from '@immich/sdk'; +import { AlbumSortBy, SortOrder } from '$lib/stores/preferences.store'; +import { isAlbumEditor, sortAlbums } from '$lib/utils/album-utils'; + +const A = (o: Partial): AlbumResponseDto => + ({ + id: 'a', + albumName: 'A', + description: '', + createdAt: '2026-01-01T00:00:00.000Z', + updatedAt: '2026-01-01T00:00:00.000Z', + albumThumbnailAssetId: null, + shared: false, + hasSharedLink: false, + assetCount: 0, + isActivityEnabled: false, + albumUsers: [], + ...o, + }) as never; + +const withUsers = (roles: [userId: string, role: AlbumUserRole][]) => + A({ albumUsers: roles.map(([id, role]) => ({ user: { id }, role })) as never }); + +describe('isAlbumEditor', () => { + it('is true for the owner', () => { + expect(isAlbumEditor(withUsers([['u1', AlbumUserRole.Owner]]), 'u1')).toBe(true); + }); + + it('is true for an editor', () => { + expect( + isAlbumEditor( + withUsers([ + ['u1', AlbumUserRole.Owner], + ['u2', AlbumUserRole.Editor], + ]), + 'u2', + ), + ).toBe(true); + }); + + it('is false for a viewer', () => { + expect( + isAlbumEditor( + withUsers([ + ['u1', AlbumUserRole.Owner], + ['u2', AlbumUserRole.Viewer], + ]), + 'u2', + ), + ).toBe(false); + }); + + it('is false for someone with no role on the album', () => { + expect(isAlbumEditor(withUsers([['u1', AlbumUserRole.Owner]]), 'u3')).toBe(false); + }); +}); + +describe('sortAlbums by DateCreated', () => { + it('orders by the stored createdAt, newest first', () => { + const albums = [ + A({ id: 'old', albumName: '1996', createdAt: '1996-06-15T14:30:00.000Z' }), + A({ id: 'mid', albumName: '2010', createdAt: '2010-01-01T00:00:00.000Z' }), + A({ id: 'new', albumName: '2026', createdAt: '2026-01-01T00:00:00.000Z' }), + ]; + + const sorted = sortAlbums(albums, { sortBy: AlbumSortBy.DateCreated, orderBy: SortOrder.Desc }); + + expect(sorted.map(({ albumName }) => albumName)).toEqual(['2026', '2010', '1996']); + }); + + it('orders oldest first when ascending', () => { + const albums = [ + A({ id: 'new', albumName: '2026', createdAt: '2026-01-01T00:00:00.000Z' }), + A({ id: 'old', albumName: '1996', createdAt: '1996-06-15T14:30:00.000Z' }), + ]; + + const sorted = sortAlbums(albums, { sortBy: AlbumSortBy.DateCreated, orderBy: SortOrder.Asc }); + + expect(sorted.map(({ albumName }) => albumName)).toEqual(['1996', '2026']); + }); +}); +``` + +- [ ] **Step 2: Run to verify failure** + +Run: `cd web && pnpm test --run src/lib/utils/album-utils.spec.ts` +Expected: FAIL to import — `isAlbumEditor` is not exported. The `sortAlbums` tests describe existing behaviour and will pass once the import resolves. + +- [ ] **Step 3: Add the helper** + +In `web/src/lib/utils/album-utils.ts`, change line 1 from a type-only import to a value import: + +```ts +import { AlbumUserRole, type AlbumResponseDto } from '@immich/sdk'; +``` + +Then add, immediately above the `Album Sorting` banner comment: + +```ts +/** + * Whether `userId` may edit `album`'s metadata. + * + * Mirrors the server's `Permission.AlbumUpdate`, which grants owner ∪ shared-with-editor + * (`server/src/utils/access.ts`). Sharing and deletion stay owner-only, so callers must keep + * using their own ownership check for those. + */ +export const isAlbumEditor = (album: AlbumResponseDto, userId: string) => + album.albumUsers.some( + ({ user, role }) => user.id === userId && (role === AlbumUserRole.Owner || role === AlbumUserRole.Editor), + ); +``` + +- [ ] **Step 4: Run to verify the tests pass** + +Run: `cd web && pnpm test --run src/lib/utils/album-utils.spec.ts` +Expected: PASS, 6 tests. + +- [ ] **Step 5: Prove the sort tests can fail** + +Temporarily flip an expectation to `['1996', '2010', '2026']` and re-run. Expected: FAIL. Restore it. A characterization test over untouched code is worth nothing until you have seen it go red. + +- [ ] **Step 6: Commit** + +```bash +git add web/src/lib/utils/album-utils.ts web/src/lib/utils/album-utils.spec.ts +git commit -m "feat(web): add isAlbumEditor helper and pin the DateCreated sort" +``` + +--- + +### Task 6: Web — the date field in `AlbumEditModal` + +**Files:** + +- Modify: `web/src/lib/modals/AlbumEditModal.svelte` (whole file) +- Create: `web/src/lib/modals/AlbumEditModal.spec.ts` + +**Interfaces:** + +- Consumes: `UpdateAlbumDto.createdAt?: string` (Task 2), `handleUpdateAlbum(album, dto)` from `$lib/services/album.service`. +- Produces: nothing consumed by later tasks. + +Two traps to design around: + +1. **Comparison must be on instants.** The input is local-zone, `album.createdAt` is normally `…Z`. String comparison reports every album as changed and rewrites `createdAt` on every rename. +2. **The offset must be historical.** Luxon resolves `Europe/Berlin` in June 1996 to `+02:00` from the IANA database. `new Date().getTimezoneOffset()` would stamp today's offset onto a 1996 date. + +- [ ] **Step 1: Write the failing tests** + +Create `web/src/lib/modals/AlbumEditModal.spec.ts`. It follows `SpaceEditModal.spec.ts` — the same `vi.hoisted` service mock, `data-testid` queries (`@immich/ui`'s `Field`/`Label` wiring uses `aria-labelledby`, which happy-dom does not reliably associate), and the capitalised `Save` button label (which comes from `@immich/ui`'s own translation service, not svelte-i18n). + +`web/vite.config.ts` pins `TZ: 'UTC'` for unit tests, so the runner's zone cannot exercise the local↔UTC conversion. Force a zone through Luxon instead, which is what the component reads: + +```ts +import { type AlbumResponseDto } from '@immich/sdk'; +import '@testing-library/jest-dom'; +import { render, screen, waitFor } from '@testing-library/svelte'; +import userEvent from '@testing-library/user-event'; +import { Settings } from 'luxon'; +import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest'; +import AlbumEditModal from './AlbumEditModal.svelte'; + +const handleUpdateAlbumMock = vi.hoisted(() => vi.fn()); +vi.mock('$lib/services/album.service', () => ({ handleUpdateAlbum: handleUpdateAlbumMock })); + +const originalZone = Settings.defaultZone; + +const album = (o: Partial = {}): AlbumResponseDto => + ({ + id: 'a1', + albumName: 'Summer', + description: 'Trip', + createdAt: '1996-06-15T12:30:00.000Z', + updatedAt: '2026-01-01T00:00:00.000Z', + albumUsers: [], + ...o, + }) as never; + +const createdAtInput = () => screen.getByTestId('album-edit-created-at') as HTMLInputElement; +const saveButton = () => screen.getByRole('button', { name: 'Save' }); + +beforeEach(() => { + // 1996-06-15T12:30Z is 14:30 in Berlin summer time (+02:00). Pinning the zone here + // rather than via TZ makes the local <-> UTC conversion observable under the + // config's TZ: 'UTC'. + Settings.defaultZone = 'Europe/Berlin'; + handleUpdateAlbumMock.mockResolvedValue(true); +}); + +afterAll(() => { + Settings.defaultZone = originalZone; +}); + +describe('AlbumEditModal', () => { + it('pre-fills the created date in local time', () => { + render(AlbumEditModal, { props: { album: album(), onClose: vi.fn() } }); + + expect(createdAtInput().value).toBe('1996-06-15T14:30:00.000'); + }); + + it('submits the edited date as an ISO string with the historical offset', async () => { + const onClose = vi.fn(); + render(AlbumEditModal, { props: { album: album(), onClose } }); + + await userEvent.clear(createdAtInput()); + await userEvent.type(createdAtInput(), '1996-06-15T09:00:00.000'); + await userEvent.click(saveButton()); + + await waitFor(() => expect(handleUpdateAlbumMock).toHaveBeenCalled()); + const [, dto] = handleUpdateAlbumMock.mock.calls[0]; + expect(dto.createdAt).toBe('1996-06-15T09:00:00.000+02:00'); + expect(onClose).toHaveBeenCalled(); + }); + + it('omits the created date when it was not touched', async () => { + render(AlbumEditModal, { props: { album: album(), onClose: vi.fn() } }); + + await userEvent.click(saveButton()); + + await waitFor(() => expect(handleUpdateAlbumMock).toHaveBeenCalled()); + const [, dto] = handleUpdateAlbumMock.mock.calls[0]; + expect(dto).not.toHaveProperty('createdAt'); + expect(dto.albumName).toBe('Summer'); + }); + + it('omits the created date when the input is cleared, and still saves the name', async () => { + render(AlbumEditModal, { props: { album: album(), onClose: vi.fn() } }); + + await userEvent.clear(createdAtInput()); + await userEvent.click(saveButton()); + + await waitFor(() => expect(handleUpdateAlbumMock).toHaveBeenCalled()); + const [, dto] = handleUpdateAlbumMock.mock.calls[0]; + expect(dto).not.toHaveProperty('createdAt'); + expect(dto.albumName).toBe('Summer'); + }); +}); +``` + +- [ ] **Step 2: Run to verify failure** + +Run: `cd web && pnpm test --run src/lib/modals/AlbumEditModal.spec.ts` +Expected: FAIL — `getByTestId('album-edit-created-at')` finds nothing. + +- [ ] **Step 3: Implement the modal** + +Replace `web/src/lib/modals/AlbumEditModal.svelte` with: + +```svelte + + + +
+