feat(spaces): in-space album management + shared-space-album scope helper#752
Open
Deeds67 wants to merge 256 commits into
Open
feat(spaces): in-space album management + shared-space-album scope helper#752Deeds67 wants to merge 256 commits into
Deeds67 wants to merge 256 commits into
Conversation
Add addAlbum / removeAlbum / getLinkedAlbums / getSpacesLinkedToAlbum / hasAlbumLink / setAlbumShowInTimeline to SharedSpaceRepository, mirroring the existing library link CRUD block. Covered by a new medium test (shared-space-album.spec.ts) that exercises all six methods against a real Postgres testcontainer — 5/5 green.
Add a shared_space_album branch to AssetAccess.checkSpaceAccess and checkSpaceAccessForSpace, joining through album_asset so that space members can read assets contained in albums linked to their space. Adds the authoritative permission-matrix medium spec for Phase 1, covering Grid 1 (full AssetRead chain for all actor roles) and Grid 6 (multi-path read: direct-asset vs album-link, unlink behaviour across two spaces sharing the same album).
Add AlbumAccess.checkSpaceLinkedAlbumAccess which returns album ids the user can write because they are an Owner or Editor of a space that links the album. Wire it into Permission.AlbumAssetCreate and Permission.AlbumAssetDelete in access.ts (union after owner + shared-album paths). Add mock entry for the new method. Add Grid 2 (WRITE) cells and a write-wins cell to the matrix spec.
Implement SharedSpaceService.linkAlbum: requireRole(Editor+), requireAccess(AlbumUpdate), addAlbum (idempotent), queue SharedSpaceAlbumFaceSync only on new link with faceRecognitionEnabled. No isAdmin gate (diverges from linkLibrary). Add Grid 3 permission-matrix medium tests (5 cases, isolated space/albums to avoid world mutation) and 8 unit tests mirroring linkLibrary patterns.
- Add getAlbumAssetIdsWithoutOtherSpacePath to SharedSpaceRepository: returns asset IDs that have no other path into the space (not direct-added, not in another linked album, not via a linked library) - Add unlinkAlbum to SharedSpaceService: requires Editor role, fetches orphaned asset IDs, removes the album link, then removes stale shared_space_person_face rows and deletes orphaned space persons - Extend shared-space-album.repository.spec with two getAlbumAssetIdsWithoutOtherSpacePath tests (direct+another-album paths excluded; library path excluded) - Add Grid 4 auth matrix to shared-space-album-permissions.service.spec: owner+editor ALLOW, viewer+nonMember DENY - New shared-space-album.service.spec: face-retention test asserting album-only asset face is removed on unlink while directly-added asset face is retained; orphaned-person deletion test
- SharedSpaceService.updateAlbumLink: requires Editor role, delegates to sharedSpaceRepository.setAlbumShowInTimeline - SharedSpaceService.getLinkedAlbums: requires membership, maps getLinkedAlbums rows to SharedSpaceLinkedAlbumDto with assetCount via new sharedSpaceRepository.getAlbumAssetCount (count album_asset rows) - Grid 4b permission matrix tests (toggle: owner/editor ALLOW, viewer/nonMember DENY) with persistence assertion via getLinkedAlbums - getLinkedAlbums medium tests: correct DTO mapping, assetCount, empty case, non-member rejection - Unit tests: updateAlbumLink requiresEditor gate + setAlbumShowInTimeline call; getLinkedAlbums requiresMembership gate + row→DTO mapping
- Add AssetRepository.getByAlbumIdWithFaces (mirrors getByLibraryIdWithFaces, joins album_asset) - Add SharedSpaceService.handleSharedSpaceAlbumFaceSync handler (@onjob FacialRecognition queue): mirrors handleSharedSpaceLibraryFaceSync - batched album asset iteration, hasAlbumLink re-check per batch, processSpaceFaceMatch per asset, queueSpaceIdentityReconciliation on match, dedup queue on completion - Extend SharedSpaceRepository.isAssetInSpace with album path (shared_space_album → album_asset → asset union) so processSpaceFaceMatch recognises album-linked assets as in-space - Extend SharedSpaceRepository.getSpaceAssetAdder with album path for correct addedById attribution - Add getByAlbumIdWithFaces to asset repository mock - 4 medium handler tests (RED→GREEN) + 6 unit tests in service.spec.ts
Adds 7 medium tests to shared-space-album.service.spec.ts covering: - asset removal from album revokes space access - direct-add fallback preserves access after album removal - hard album delete cascades shared_space_album link + revokes access - soft-deleted asset excluded from space access and timeline - live-photo motion part reachable via album branch - locked (AssetVisibility.Locked) asset pinned behavior (visibility not filtered) - empty album link is a no-op (assetCount 0, timeline unchanged)
…y guard Add Grid 5 (AlbumDelete — album owner only, space link does not grant), Grid 7 medium layer (DB assertion that linking writes no album_user rows for space members), and a cross-layer set-equality guard that pins checkSpaceAccess output against an independently-derived expected set (seeded paths, not the predicate) to catch future predicate drift.
Regenerated OpenAPI spec (sync-open-api), TypeScript SDK (oazapfts), and
Dart client (openapi-generator) to reflect the 4 new shared-space album
endpoints (GET/PUT/PATCH/DELETE /shared-spaces/{id}/albums[/{albumId}]),
new DTOs (SharedSpaceAlbumLinkUpdateDto, SharedSpaceLinkedAlbumDto), new
permissions (sharedSpaceAlbum.create/update/delete), and new job name
(SharedSpaceAlbumFaceSync).
SQL docs deferred to CI (mise sql requires a live DB).
- SpaceLinkedAlbums component (space-linked-albums.svelte): loads linked albums via getSharedSpaceAlbums, renders list with per-album thumbnail, name, and asset count; canManage gate controls unlink button, timeline Switch toggle, and album picker (calls getAllAlbums filtered to owner/editor roles, excluding already-linked) - SpaceLinkedAlbumsModal: thin modal wrapper mirroring SpaceLinkedLibrariesModal - Wire into +page.svelte: import + handleLinkAlbums handler + "Linked Albums" MenuOption gated on isEditor (matches Link Libraries gate) - 12 i18n keys in en.json (prettier-sorted) - space-linked-albums.spec.ts: 14 tests covering render, canManage=true (unlink/toggle/picker/linkAlbum calls), and canManage=false (read-only)
Adds the `shared_space_album` leg (gated on `showInTimeline = true`) to the two `searchAssetBuilder` blocks in `database.ts` (spaceId path and timelineSpaceIds path) and to all three space-legs in `map.repository.ts` (`getAlbumMapMarkers` timelineSpaceIds scope, `getMapMarkers` albumScope, and `getMapMarkers` direct timelineSpaceIds union), mirroring the pattern already used by the timeline builders. Adds `newSharedSpaceAlbum` factory helper to `medium.factory.ts`. Tests (TDD, RED→GREEN): search service (+2 cases: showInTimeline=true found / showInTimeline=false excluded) and map repository (+2 cases: GPS asset in linked album shown / excluded when showInTimeline=false). All 19 search-service and 13 map-repository medium tests pass. Regenerates SQL docs for `map.repository.sql` and `search.repository.sql`.
…etSpacesLinkedToAlbum Phase-2
Wrap the thumbnail+name section of each linked-album row in an
<a href> using Route.viewAlbum({ id }) so all members can click through
to browse the album. The manage controls (timeline Switch + unlink
Button) are sibling elements outside the anchor so they remain
independently clickable without triggering navigation.
Add a test asserting the link element carries the correct /albums/<id>
href.
…d-flow, edge tests)
…lbums Add checkSpaceLinkedAlbumReadAccess (no role filter) to AlbumAccess and wire it into the AlbumRead and AlbumDownload auth branches in access.ts, so any space member (Owner/Editor/Viewer) can open a linked album entity and download from it without needing an album_user row.
Adds /spaces/:spaceId/albums: SpaceAlbumCard grid with link/unlink/toggle-timeline handlers relocated from space-linked-albums.svelte. Editors/Owners get the Link-album inline picker; Viewers see the grid read-only. Mirrors People sub-page shell (UserPageLayout, load pattern, isEditor derived).
Route /spaces/:spaceId/albums/:albumId=id opens a linked album inside the space chrome. The +page.ts verifies the album appears in getSharedSpaceAlbums before calling getAlbumInfo — an unlinked album owned by the user redirects to /spaces/:id/albums instead. The +page.svelte renders the album Timeline (enableRouting=false, options via buildAlbumTimelineOptions + grouping:month) with a canManage-gated "Add photos" button (data-testid=add-photos-button) for Task 7 to wire.
…T19) - Add 7 e2e cases to shared-space-album.e2e-spec.ts proving the checkSpaceLinkedAlbumReadAccess grant: · space viewer GET /albums/:id linked album → 200 · non-member GET /albums/:id → 400 · space member GET /albums/:id for non-linked album → 400 (boundary) · space viewer GET /timeline/buckets?albumId → 200 · space viewer GET /timeline/bucket?albumId → 200 (content path) · non-member GET /timeline/buckets?albumId → 400 · re-assert: editor PUT /albums/:id/assets still → 200 - Regenerate server/src/queries/access.repository.sql against the running immich_postgres (all fork tables present). Diff is scoped to 12 additions: new checkSpaceLinkedAlbumReadAccess block only. No lines dropped from any existing query. e2e deferred to CI (e2e compose stack not running locally).
…reate/link/viewer)
…rt link modal via dialog role
…slice 1) Pins the timeline album leg + showInTimeline gate (asset.repository, previously uncovered) and the current pre-fix A1 soft-delete-hole behavior at the four sites that omit album.deletedAt (isFaceInSpace, getSpaceAssetAdder, getAlbumAssetIdsWithoutOtherSpacePath, getAssetIdsWithoutOtherSpacePath). Slices 2-14 must keep these green; Slice 15 flips the pre-fix assertions. Spec: data/sa-abstraction-spec-t8/report.md
New fork-only module src/utils/shared-space-album-scope.ts encodes the linked-album access path (shared_space_album -> album(deletedAt) -> album_asset) and the A1 invariant exactly once, as composable Kysely EXISTS helpers: spaceAlbumAssetExists (the workhorse album arm, positive or negated), plus spaceDirectAssetExists / spaceLibraryAssetExists / spaceAssetPathBranches for the clean asset-outer sites. Parameterized by scope (spaceId / spaceIds / member / correlated ref), requireShowInTimeline, requireAlbumNotDeleted (A1), excludeAlbumId. DB-backed tests cover every path combination + edge case (soft-deleted album, showInTimeline off, multi-space dedup, member vs non-member, excludeAlbumId, non-asset correlation). No call sites migrated yet. Spec: data/sa-abstraction-spec-t8/report.md
…ice 3) spaceAlbumAssetExistsSql emits the linked-album EXISTS arm for sql``-authored queries (face-identity.repository.ts). A DB-backed equivalence test proves it returns the identical asset set as the Kysely spaceAlbumAssetExists over the same data + scope, with A1 on and off — the wire that keeps the two authoring styles from drifting. Spec: data/sa-abstraction-spec-t8/report.md
Scans all 12 scoping files and asserts every shared_space_library access-scope arm has a nearby shared_space_album arm or fork album-scope helper call — the backstop for the recurring 'library present, album forgotten' defect class, incl. future upstream-rebase additions. Benign refs (CRUD, column lists, library-only sync) are pattern-filtered; genuine album-free sites are named in ALLOWLIST with reasons. The guard already surfaced two PRE-EXISTING missing-album gaps the review's grep missed (findSpaceForAssetAndUser, getPersonalThumbnailForSpacePerson — both union/or direct+library, no album arm). Allowlisted + flagged for follow-up; NOT fixed here (unplanned behavior change). Spec: data/sa-abstraction-spec-t8/report.md
…e module (slice 5) Move the two scoping subqueries out of the upstream sync.repository.ts into the fork-owned scope module (reducing fork footprint in an upstream file) and re-export them from sync for its existing callers. No external importers; all 359 sync medium tests unchanged. Behavior identical. Spec: data/sa-abstraction-spec-t8/report.md
… (slice 6) Replace the two hand-cloned 3-path OR clusters in searchAssetBuilder (spaceId and timelineSpaceIds forms) with spaceAssetPathBranches. ~44 inline lines -> 2 helper spreads. Behavior identical (repositories suite 521 tests + timeline characterization green). Spec: data/sa-abstraction-spec-t8/report.md
…lice 7) Replace the four hand-cloned 3-path OR clusters in withTimeBucketAssetFilters and getTimeBucket (AR-1..AR-4) with spaceAssetPathBranches (requireShowInTimeline). Behavior identical (asset.repository 46 + timeline characterization 8 green). Spec: data/sa-abstraction-spec-t8/report.md
…elper (slice 8) Replace the 5 hand-cloned album/3-path clusters in getAccessibleTags and applySuggestionScope (SR-1..SR-5, no showInTimeline) with spaceAssetPathBranches. Behavior identical (search.repository 13 green). Spec: data/sa-abstraction-spec-t8/report.md
MR-1 (albumScope 3-arm push) via spaceAssetPathBranches; MR-2 keeps its AND(visibility=Timeline, ...) wrapper and routes the album leg through spaceAlbumAssetExists. Behavior identical (map.repository 13 green). Spec: data/sa-abstraction-spec-t8/report.md
…(slice 10) view-repository.ownedOrSpaceAccessible: full 3-path via spaceAssetPathBranches (member scope). memory.accessibleSearchBuilder: album arm via spaceAlbumAssetExists (direct/library kept inline — library carries a bespoke isOffline filter). tag.repository (tag_asset-walk correlation) and download.repository (UNION-of-rows) are left inline as structural outliers — both keep album.deletedAt and are covered by the co-occurrence guard. Behavior identical (view 26 + memory 20 green). Spec: data/sa-abstraction-spec-t8/report.md
… 11) PersonAccess.checkSharedSpaceAccess (AC-5) 3-path OR -> spaceAssetPathBranches (member scope). checkSpaceLinkedAlbumAccess/ReadAccess (album-id JOIN, the canonical album-access derivation) and checkSpaceAccess/checkSpaceAccessForSpace (UNION-of-rows) are left inline as distinct query shapes (not clones); both keep album.deletedAt. checkSpaceEditAccess album-arm gap remains allowlisted (pre-existing). Behavior identical (RBAC suites 85 green). Spec: data/sa-abstraction-spec-t8/report.md
…ent (slice 12) Replace the 6 byte-identical inline EXISTS album clones (getAccessiblePeople* statistics/counts/page, hydrateAccessiblePeople) with spaceAlbumAssetExistsSql. The 2 UNION-arm sites (face-match backfill targets) keep their bespoke inline SQL (shared_space_library/album adjacent, guard-covered). Behavior identical (face-identity 114 + face-matching 34 green; raw≡Kysely equivalence pinned). Spec: data/sa-abstraction-spec-t8/report.md
…elper (slice 13) Consolidate the 5 correlated-EXISTS album clones in the person/face paths (getSpaceRepresentativeFace(s), getIdentityEvidenceForSpacePerson, isSpacePersonRepresentativeFaceValid, getFirstValidRepresentativeFaceForPerson) via spaceAlbumAssetExists. getPersonsBySpaceId's arm uses an aliased-CTE eb (spf2/af2) not assignable to the helper's type, so it stays inline. The UNION-row (getAssetCount/getRecentAssets/getNewAssetCount/isAssetInSpace/isFaceInSpace/ getSpaceIdsForAsset), raw-stats and adder-attribution forms remain inline (structural outliers, fork-only, guard-covered). Behavior identical (148 green). Spec: data/sa-abstraction-spec-t8/report.md
…e 15) THE ONE INTENTIONAL BEHAVIOR CHANGE in this branch — reviewable/revertable on its own. Four sites resolved a linked-album access path WITHOUT the album.deletedAt IS NULL guard (A1), so an asset reachable only via a SOFT-DELETED album was still treated as in-space — leaking/retaining its faces and mis-attributing its adder: - getAlbumAssetIdsWithoutOtherSpacePath (branch 2) [review finding #5] - getAssetIdsWithoutOtherSpacePath (branch 2) [review finding #5] - isFaceInSpace (album leg) [newly found by this work] - getSpaceAssetAdder (album leg) [newly found by this work] Each album leg now joins album with deletedAt IS NULL, matching isAssetInSpace and the shared album-scope helper's default. RED-first: the 4 characterization assertions were flipped to the correct post-fix expectation, failed on the prior commit, and pass now (211 shared-space tests green). Spec: data/sa-abstraction-spec-t8/report.md
…ame) Prettier-format 4 space-album-scope files and switch the guard's server-root resolution from __dirname (banned by unicorn/prefer-module) / import.meta.dirname (invalid under CJS) to process.cwd() (matches face-identity-query-shape.spec.ts). No behavior change. Local: format --check clean, eslint --max-warnings 0 clean, tsc 0 errors, 4752 unit tests pass.
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
1 participant
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.
In-space album management (search / create / sort / group / list-covers)
Brings the full main albums-page toolbar into the shared-space Albums tab, and consolidates shared-space-album scoping behind a single shared helper.
Web — space Albums tab
AlbumCover,sortAlbums, and the album mapper; the toolbar/list/table live in fork-only components with a space-scoped view-settings store (isolated from the global/albumssettings). The/albumsCreate button moves to the right so both pages match.Server
SharedSpaceLinkedAlbumDtoto theAlbumResponseDtoshape (+showInTimeline/addedById/linkedAt) and removes the per-album N+1 (bulkgetMetadataForIds+ reusedmapAlbum).shared-space-album-scopehelper routed through ~9 repositories (access / asset / search / timeline / map / memory / view / person-face / face-identity), de-duplicating inline space-scoping SQL — proven equivalent by medium characterization tests.Testing
TDD throughout: server unit + real-DB medium (incl. the scope-equivalence characterization spec), full web component suite, and Playwright e2e journeys (search/sort/group/view/create/link/viewer gating). Regenerated SQL / OpenAPI / Dart clients.