feat: let album owners and editors change an album's creation date (#520) - #990
Open
Deeds67 wants to merge 19 commits into
Open
feat: let album owners and editors change an album's creation date (#520)#990Deeds67 wants to merge 19 commits into
Deeds67 wants to merge 19 commits into
Conversation
…n local time A second edit in the same session silently reverted a just-saved album date: showEditTitleAndDescription (renamed showEditAlbum) rebuilt _album via copyWith(name:, description:) only, so _album.createdAt stayed stale and the next unrelated Save resent the old value. _EditAlbumData now carries createdAt end to end. Separately, the created-at picker rendered the picked instant in UTC instead of local time: DateTime.parse of the picker's offset-bearing string is always UTC-flagged. Apply .toLocal(), matching the sibling pattern in action.service.dart. Adds four widget tests covering both regressions plus the previously untested dismiss-the-picker and no-current-user paths.
The context it builds (isOwner: false, isEditor: false) is a non-owner AND non-editor member, not just a non-owner — the test name predates the editor-permissive gating this command now has. Assertion unchanged.
…able The grammar table's it.each rows only asserted status codes, leaving the +02:00 offset row — the primary web wire format — unverified beyond a 200. Widen the tuple to carry an expected stored value (undefined for the rejected rows) and assert body.createdAt on the 5 accepted rows, notably that 1996-06-15T14:30:00+02:00 stores as 1996-06-15T12:30:00.000Z. Also fix the title format string: %i was bound to the createdAt string (printing "NaN"); reorder so it binds to the numeric status. Also adds the missing S6 HTTP half (albumName + createdAt combined in one request) and S26's missing updatedAt-advances clause on the empty-body test. NOT run locally — Docker was unavailable in this environment. tsc, eslint and prettier are clean on this file; the suite itself needs verification against a live stack (CI on the PR).
Year 1 does not survive the Postgres round trip — 0001-01-01T00:00:00.000Z comes back as 2001-01-01T00:00:00.000Z. That row exists to pin the grammar boundary (the schema's four-digit year accepts 0001), not storage fidelity, so it now asserts status only. Asserting the returned value would enshrine the truncation as intended behaviour.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the request in discussion #520: albums of scanned photos sort by upload order rather than by when the photos are actually from, and nothing could change that.
What this does
Makes
album.createdAtwritable by the album's owner or an editor, and adds a date field to both existing album-edit surfaces.createdAtwas already carried end to end — response DTO, mobile sync stream, mobile Drift model, and all three sort implementations (web, mobile, and the fork's space-album sort). The only missing link was that it was absent fromUpdateAlbumSchema, so nothing could write it. This adds one optional field, one service pass-through, and the two UI affordances. No migration, no schema change, no sync-contract change.createdAtonPATCH /albums/:id, validated by the existingisoDatetimeToDatecodec (ISO 8601, timezone designator required).AlbumEditModal, plus the album-edit gates relaxed from owner-only to owner-or-editor, matchingPermission.AlbumUpdate.Design decisions worth knowing
createdAtis overloaded rather than given a new column. A separatealbum.datecolumn would be semantically cleaner, but it costs a fork migration, aSyncAlbumV1payload change, a mobile Drift column plus migration, and a new sort option in three places — divergence in files upstream churns heavily, in exchange for semantics no user sees. The consequences are accepted deliberately:createdAtstops being an audit field and the previous value is overwritten with no undo, and becausealbum.repository.tsorders listings by it, raw API/CLI consumers will see backdated albums move. Web and mobile re-sort client-side and are unaffected.On web, the date is changed from the albums list, not from the album.
AlbumEditModalis reachable from the/albumsright-click menu and the command palette; both album detail pages edit title and description inline and never open it. Mobile puts the date in the kebab's Edit dialog, where it is visible. Adding an inline affordance to the detail pages means inventing a display element next toAlbumSummary— a bigger UI question than this change warrants, and one that would have to be answered twice. Noted as a follow-up if it proves too hidden.Future dates are accepted by the API. No server-side check: it matches how asset date editing already behaves, a check would produce spurious 400s under clock skew, and a permissive API keeps scripted bulk backdating viable — which is the requester's actual workflow. Mobile's shared picker caps at now; web's input does not. That asymmetry already exists for asset dates.
Sharing stays owner-gated in the UI even though the server allows editors.
Permission.AlbumShareis byte-identical toAlbumUpdate(owner ∪ editor); onlyAlbumDeleteis owner-only. The albums-list menu has always gated Share on ownership, and widening it would be an unrequested behaviour change riding along with a date picker. The UI is deliberately the stricter of the two, which is the safe direction.One new intra-web asymmetry: on the album detail page an editor can now rename via the command palette, while the inline title remains owner-only. Before this change both were owner-only.
Testing
createdAtreaches the repository; omitted when absent.createdAtre-emits the album on the sync stream with the new value.1996-06-31and1997-02-29rejected,1996-02-29accepted), a required timezone designator, and pinned stored values for the accepted rows.createdAtwhen unchanged or cleared;isAlbumEditoracross all four roles; theDateCreatedsort.Full suites green: server 5768 tests, web 5701 tests,
dart analyze --fatal-infosclean across mobile, all type/lint/format gates clean.Reviewer notes
Two defects were found by a whole-branch review after the per-surface reviews had passed, both in the mobile dialog and both fixed in
dbd2397e87e: a second edit in the same session wrote the stale pre-edit date back to the server, and the picked date rendered in UTC rather than local time. Both are now covered by regression tests proven to fail against the un-fixed code.Not verified locally: the e2e changes in
74b3012e21cwere written after the local Docker daemon became unavailable and could not be executed here. They compile, lint and format cleanly, and the stored-value expectations were checked by hand againstDate,pgandpostgres-datesemantics — but CI is their first real run. Watch that job.Manual checks not automated (the spec records why): that the albums list re-sorts live after an edit, and the context-menu entries for owner/editor/viewer.
AlbumsList.sveltehas no spec and standing a page harness up for it costs more than it proves.Follow-ups, deliberately not in this PR
isAlbumEditoris now the fifth copy of the owner-∪-editor predicate. The other four live in fork-only files, so converging them would reduce divergence.DateCreatedsort; now that the date is meaningful, users will plausibly want it.createdAton album creation, so an importer can create a correctly-dated album in one call instead of create-then-patch — closest to the "uploading many albums from previous years" workflow described in the discussion.