feat(spaces): let space editors edit space members' assets (#734) by Deeds67 · Pull Request #992 · open-noodle/gallery

Deeds67 · 2026-08-15T07:48:23Z

Closes the request in discussion #734.

What this does

A Shared Space Owner/Editor can now edit assets owned by other members of that space — rotate / crop / trim, the image editor, description, date, location, rating, tags, and the re-processing jobs (refresh faces / metadata / thumbnails, transcode). Delete, archive / visibility, and stacking stay owner-only.

The report's framing was off in a way that changed the design. #734 describes it as editor-vs-admin, and lists 8 actions an "editor" is missing. Those 8 are exactly the isOwner gates in AssetViewerNavBar.svelte — instance-admin status is never consulted there. The reporter's "admin" screenshot is simply the owner of that photo. So the real subject is a space editor acting on another member's asset, and everything below follows from that.

Of the 8 actions listed, 4 are granted (rotate left, rotate 180°, refresh faces, refresh metadata) and 4 are deliberately withheld (add to stack, archive, move to locked folder, view in timeline). The grant reaches well beyond that list, though — the whole detail panel, the image editor, video trim, thumbnail regeneration and transcode.

The rule

You may edit an asset if you own it, or if you are Owner/Editor of a space that shows it and its owner is a member of that space.

One sentence, total over all three ways an asset reaches a space (directly added, linked library, linked album). The client never learns which path an asset took — it learns only the answer, from the server.

Anchoring on the owner's membership is what makes this safe: authority comes from the owner having opted into the space, not from a role someone else handed out. A photo belonging to an album member who never joined the space stays untouchable.

⚠️ Three behaviour changes nobody asked for

1. A space editor can no longer edit an asset owned by a non-member. Direct-add accepts Permission.AssetShare = owner ∪ partner, so a member could add their partner's photo to a space — after which every space Owner/Editor gained write access over it, although the partner never joined and partner-sharing conveys read access only. Requiring owner-is-member closes that. Nothing legitimate is lost: the adder cannot edit their partner's photos anywhere else in the product either.

This surfaced as a pre-existing medium test going red. Its fixture never added the asset owner to the space, so it was asserting the old behaviour. The fixture was corrected (767e821f965) — no assertion was weakened, and the two-directional pins live in access-space-edit.repository.spec.ts (S-1/S-2/S-3 grant on all three paths; S-5 denies the non-member case).

2. Stack creation is now owner-only. StackService.create gated on AssetUpdate, which this change widens. Since StackUpdate/StackDelete are stack-owner-only, an editor could otherwise build a stack over your photos that you could never take apart. This closes a hole that was already live for direct-pool assets.

3. POST /faces reaches AssetUpdate — accepted, not fixed. The design originally claimed face writes don't go through AssetUpdate; that was wrong. PersonService.createFace gates the asset side on it, so a space editor can create a face box on a member's asset via the API (the web UI never offers it — TagPeople and the people row stay owner-gated). This was already true for direct- and library-path assets; this PR extends it to the album path like everything else. Note the asymmetry: DELETE /faces/:id resolves to the asset owner, so an editor cannot remove a box it created. Recorded in the spec as accepted-and-known. Say the word and I'll add an owner guard instead — I left it alone because guarding would narrow a pre-existing capability nobody asked to change.

How the client knows

The server answers per asset (canEdit on the single-asset response) and per selection (POST /assets/editable). The batch endpoint's body is a bare checkAccess call — deliberately the same call the write makes, so the answer cannot drift from enforcement.

canEdit is optional and never set by mapAsset: that helper has no auth context and feeds list endpoints, where resolving it would be an N+1 access check per asset and a required field would emit a wrong false for owners. Absent means "not resolved", and the client falls back to a space-membership derivation.

Attribution

An editor changing your photo now writes a row to the space's activity feed, so you can see what changed and who changed it. Owner self-edits log nothing, which keeps it low-volume. Bulk edits group by space — one row per space, since a selection can span several.

Known gap: tag add/remove are not attributed. The helper is private to AssetService, and wiring TagService needed either a protected method on the upstream-hot BaseService or a duplicate. Non-security; worth a follow-up.

Testing

access-space-edit.repository.spec.ts — 18 medium tests against real Postgres. The rule is SQL, so unit mocks prove nothing about a three-arm UNION with correlated EXISTS. Every deny case was mutation-proved non-vacuous: flip the single property under test and it turns to grant.
End-to-end journey — an editor opens a member's photo, rotates it, corrects the date, sees the activity row, and is refused on archive and delete. The negative half is what proves the line held.
Server unit 5785 · server medium 2552/2552 · web 5726 · Playwright ui green with 0 hangs.

Server medium needs --maxWorkers=4 locally; default parallelism exhausts Postgres connections and produces different failure sets per run. Same root cause as some intermittent controller-spec failures — resource contention, not code.

Other things worth knowing

POST /assets/editable has no payload cap. Deliberate: it receives a user's multi-selection, so a cap would turn a large select-all into a 400 instead of an answer. Bounded in practice by the 10MB JSON limit (~250k ids) and @ChunkedSet on the DB side.
A latent trap for future work: AssetViewer.onAssetUpdate replaces the viewer's asset wholesale, and canEdit is set only in AssetService.get. Every emitter today re-fetches or is owner-gated, but a future one passing a raw updateAsset response would silently strip canEdit and the widened actions would vanish mid-session.
Mobile is out of scope — a follow-up spec. Its generated OpenAPI client changed only as a side effect of regeneration. Editors will see actions on web that their phone doesn't offer until that lands.

Design and plan are committed under docs/superpowers/specs/ and docs/superpowers/plans/.

Table-driven coverage for PUT /assets/:id/edits, POST /assets/jobs (refresh-metadata), PUT /assets/:id/metadata, and PUT /tags/assets against a space member's asset, across owner/editor/viewer/non-member. Tags asserts the response body (count) rather than status, since bulkTagAssets uses a non-throwing checkAccess and always returns 200. Also pins the tightened partner-share regression (#992 audit item 4): a space editor can no longer edit an asset a fellow member direct-added on the strength of AssetShare's partner arm when the asset owner isn't themselves a space member.

…eal browser Every existing layer for #734/#992 (medium tests for the SQL rule, unit tests per server call site, web component tests with a canEdit prop, API-level e2e for permit/deny) stops short of proving a real user, in a real browser, against a real server, actually sees the widened buttons. Each of those layers would still pass even if the server stopped returning canEdit, the client stopped threading it, or a gate read the wrong flag. Adds two Playwright web-project specs against the real stack: - spaces-editor-asset-viewer-affordances: opens a space member's (Bob's) photo from inside the space as the space Owner, Editor, Viewer, and Bob himself. Confirms Rotate/Refresh-faces/Refresh-metadata are visible for Owner/Editor and absent for Viewer, while Delete/Archive/Add-to-stack/ View-in-timeline stay absent for everyone but Bob (the real owner). - spaces-editor-selection-toolbar-affordances: selects Bob's asset in the space timeline's multi-select toolbar and waits on the real, debounced POST /assets/editable round trip before asserting — the async resolution path is the least covered by anything else. Confirms the metadata-edit actions (rotate/change date/description/location) follow the resolved editable set while Archive/Set-visibility stay owner-only, including a mixed-selection case. Both suites anchor on a control that should be visible before asserting any control is absent, so a still-loading page can't pass an absence check for the wrong reason. Menu item text is matched against the real i18n keys. Verified each suite can fail: flipped AssetViewerNavBar's Refresh-jobs gate from isEditable back to isOwner (Owner/Editor tests went red on the missing Refresh actions) and flipped DeleteAction's gate from isOwner to isEditable (Owner/Editor tests went red on Delete now being present), then reverted both.

Table-driven coverage for PUT /assets/:id/edits, POST /assets/jobs (refresh-metadata), PUT /assets/:id/metadata, and PUT /tags/assets against a space member's asset, across owner/editor/viewer/non-member. Tags asserts the response body (count) rather than status, since bulkTagAssets uses a non-throwing checkAccess and always returns 200. Also pins the tightened partner-share regression (#992 audit item 4): a space editor can no longer edit an asset a fellow member direct-added on the strength of AssetShare's partner arm when the asset owner isn't themselves a space member.

…eal browser Every existing layer for #734/#992 (medium tests for the SQL rule, unit tests per server call site, web component tests with a canEdit prop, API-level e2e for permit/deny) stops short of proving a real user, in a real browser, against a real server, actually sees the widened buttons. Each of those layers would still pass even if the server stopped returning canEdit, the client stopped threading it, or a gate read the wrong flag. Adds two Playwright web-project specs against the real stack: - spaces-editor-asset-viewer-affordances: opens a space member's (Bob's) photo from inside the space as the space Owner, Editor, Viewer, and Bob himself. Confirms Rotate/Refresh-faces/Refresh-metadata are visible for Owner/Editor and absent for Viewer, while Delete/Archive/Add-to-stack/ View-in-timeline stay absent for everyone but Bob (the real owner). - spaces-editor-selection-toolbar-affordances: selects Bob's asset in the space timeline's multi-select toolbar and waits on the real, debounced POST /assets/editable round trip before asserting — the async resolution path is the least covered by anything else. Confirms the metadata-edit actions (rotate/change date/description/location) follow the resolved editable set while Archive/Set-visibility stay owner-only, including a mixed-selection case. Both suites anchor on a control that should be visible before asserting any control is absent, so a still-loading page can't pass an absence check for the wrong reason. Menu item text is matched against the real i18n keys. Verified each suite can fail: flipped AssetViewerNavBar's Refresh-jobs gate from isEditable back to isOwner (Owner/Editor tests went red on the missing Refresh actions) and flipped DeleteAction's gate from isOwner to isEditable (Owner/Editor tests went red on Delete now being present), then reverted both.

Follow-up to #992, which widened AssetUpdate but deliberately left the People row owner-gated because every person/face permission resolves to pure ownership. Approach A: new editor-gated endpoints on the fork-only shared-spaces controller, writing only the space's own taxonomy via the transaction confirmSpacePersonFaceSuggestion already runs. access.ts and the upstream person/face controllers are untouched. Records the constraint that shapes the design: face_identity_face is keyed on assetFaceId alone, so an attach rewrites a face's identity globally and propagates to other spaces sharing it. Accepted and pinned rather than partitioned. 31 BDD scenarios and an 8-slice TDD plan.

…icate Reachability plus the owner-hidden-person exclusion, in one predicate so the read (spec 6.1) and the write (6.3) cannot disagree. No owner-is-member clause: unlike #992's checkSpaceEditAccess this writes nothing of the owner's, so an editor may name a non-member's face (F-6). Covers F-1, F-2, F-3, F-6, F-7, F-9, F-10, F-11.

POST /shared-spaces/:id/assets/:assetId/faces (spec §6.5). Gated on requireRole(Editor) -> isAssetInSpace -> requireSpacePersonInSpace. The created asset_face gets personId = NULL, sourceType = Manual, createdBy = auth.user.id, then is attached to the space person via the existing linkFaceToSpacePerson transaction helper. Extracts the edit-aware coordinate transform out of PersonService.createFace into a shared convertFaceBoxToOriginalImageSpace helper (src/utils/asset.util.ts) so the owner path and the new space path can never drift on the geometry that converts a box drawn on an edited preview into original-image coordinates (#992 makes a rotated preview a likely path, not an exotic one -- F-16). The owner path is refactored to call the same helper; its existing unit tests pass unchanged, which is the refactor's own proof. PersonRepository.createAssetFace now takes a trailing db/trx param (defaulting to this.db) so it can be called from inside the space service's transaction, per the Slice 2 deadlock lesson: any repository method invoked inside a databaseRepository.transaction must take and use the trx, never this.db. Covers F-16 (rotated-preview coordinates, verified against independently computed expected values) and F-17 (400 when edits exist but exif dimensions are unavailable, matching the owner path's message).

Two new SharedSpaceActivityType members, PersonFaceAssign and PersonFaceDetach, written from attachFaceToSpacePerson/detachFaceFromSpacePerson inside the same transaction as the change (logActivity's trailing db param), so a row can never describe a link that then rolled back. Owner-self rule (F-26): nothing is logged when the acting user owns the asset, matching #992's asset_edit rule. Ownership comes from the asset, not from space role -- getFaceOwnerLink now also carries the asset's ownerId so both methods can check it without a second round-trip. Covers F-24, F-25, F-26.

Space-flavoured siblings of PersonSidePanel and FaceEditor, wired to the shared-space face endpoints (spec section 6.1-6.5) instead of the owner-only person/face endpoints: - SpacePersonSidePanel.svelte -- lists an asset's faces via getSpaceAssetFaces, and attaches/detaches/creates via attachSpacePersonFace / detachSpacePersonFace / createSpacePerson. Every action is immediate (PUT/DELETE), unlike the owner panel's batched "Done" -- the space endpoints are already transactional per action. Shows an error state with no affordances when the faces request fails (F-31), never guessing one. - SpaceFaceEditor.svelte -- draws a box via createSpaceAssetFace. SpaceAssetFaceCreateDto.spacePersonId is required (unlike the owner path, a box cannot be drawn unassigned), so "create a new person" opens CreateSpaceFaceModal (create person, then draw attached) rather than a bare create call. Extracted rather than copied, per the plan: - lib/utils/face-box-drag.ts -- the drag/canvas coordinate transform (content <-> natural image space) and selector-placement math, pulled out of FaceEditor.svelte and shared by SpaceFaceEditor.svelte. Two copies of this transform would drift and silently misplace boxes, especially with #992 rotated/edited assets in the mix. - PersonPickerGrid.svelte -- the people-search result grid, pulled out of AssignFaceSidePanel.svelte (owner) and reused by SpacePersonSidePanel. DetailPanel.svelte fix found while gating this task: canEditSpacePeople was missing the effectiveSpaceId check, so a non-owner with a server-authoritative canEdit=true but NO space context (e.g. a partner/album share) would incorrectly get space-editor affordances. Caught by the existing W-18 regression test in detail-panel.spec.ts. Fixed to `!!effectiveSpaceId && canEdit && !isOwner`. i18n: adds unassign_face (the detach icon's aria-label) in all nine maintained locales plus en, matching each file's existing tag/delete-face register. Covers F-27, F-28, F-29, F-30, F-31.

The first draft said the affordances render only under /spaces/:id/... That is not what ships: AssetService.get auto-resolves a space for any single-asset read without an explicit spaceId, so the affordance follows the ASSET -- it appears wherever a photo is reachable through a space the viewer edits, the main timeline included. Accepted rather than route-scoped, so the People row agrees with the rest of the detail panel, which already behaves this way for #992's canEdit. Every server gate is unchanged; spaceId is still an explicit path parameter on every endpoint, so no write can land in a space the caller did not name. F-28's negative case becomes 'in no space she edits' rather than 'not on a space URL' -- the real boundary. Recorded in 11 with the note that this was found by a Playwright failure, not by reading code: the component tests set the props directly and never exercised how they are derived.