From 3b745fa73728a4aad3d74c5c1a08a014171d0c23 Mon Sep 17 00:00:00 2001 From: Iu Date: Sun, 11 Oct 2026 20:54:54 +0300 Subject: [PATCH] feat(photos): fix trash permanent delete and add empty trash MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Permanently deleting a photo whose album was deleted failed because authorization resolved through the album. Fall back to uploader ownership when the photo is albumless. Add a bulk DELETE /trash that removes every trashed photo the caller may delete (uncapped listing) and returns the deleted count, with an «Очистить корзину» action on the trash screen. --- backend/internal/app/app.go | 1 + backend/internal/e2e/sqlite_test.go | 18 +- backend/internal/e2e/trash_test.go | 245 ++++++++++++++++++ backend/internal/photos/handlers.go | 9 + backend/internal/photos/repository.go | 11 + backend/internal/photos/service.go | 36 ++- backend/internal/photos/sqlite_store.go | 28 ++ backend/internal/photos/store.go | 30 +++ frontend/src/lib/api.ts | 1 + frontend/src/routes/(app)/trash/+page.svelte | 40 +++ .../.openspec.yaml | 2 + .../design.md | 94 +++++++ .../proposal.md | 45 ++++ .../specs/photos/spec.md | 48 ++++ .../specs/ui-workspace/spec.md | 20 ++ .../trash-permanent-delete-and-empty/tasks.md | 28 ++ 16 files changed, 643 insertions(+), 13 deletions(-) create mode 100644 backend/internal/e2e/trash_test.go create mode 100644 openspec/changes/trash-permanent-delete-and-empty/.openspec.yaml create mode 100644 openspec/changes/trash-permanent-delete-and-empty/design.md create mode 100644 openspec/changes/trash-permanent-delete-and-empty/proposal.md create mode 100644 openspec/changes/trash-permanent-delete-and-empty/specs/photos/spec.md create mode 100644 openspec/changes/trash-permanent-delete-and-empty/specs/ui-workspace/spec.md create mode 100644 openspec/changes/trash-permanent-delete-and-empty/tasks.md diff --git a/backend/internal/app/app.go b/backend/internal/app/app.go index cdd4504..3ce4885 100644 --- a/backend/internal/app/app.go +++ b/backend/internal/app/app.go @@ -166,6 +166,7 @@ func (s *Server) Router() *gin.Engine { authed.POST("/photo-batch/copy", photosHandler.CopyBatch) authed.GET("/trash", photosHandler.Trash) authed.GET("/trash/count", photosHandler.TrashCount) + authed.DELETE("/trash", photosHandler.EmptyTrash) authed.POST("/photos/:id/likes", socialHandler.AddLike) authed.DELETE("/photos/:id/likes", socialHandler.RemoveLike) diff --git a/backend/internal/e2e/sqlite_test.go b/backend/internal/e2e/sqlite_test.go index 12b8e0a..95bf5ff 100644 --- a/backend/internal/e2e/sqlite_test.go +++ b/backend/internal/e2e/sqlite_test.go @@ -39,11 +39,14 @@ func makePNG(t *testing.T) []byte { } type stack struct { - db *sql.DB - photos *photos.Service - albums *albums.Service - auth *auth.Service - hub *sse.Hub + db *sql.DB + photos *photos.Service + albums *albums.Service + auth *auth.Service + hub *sse.Hub + tm *auth.TokenManager + repos store.Repos + storage storage.Backend } // openStack opens a migrated SQLite database at path and builds the services. @@ -70,10 +73,11 @@ func openStack(t *testing.T, path, blobRoot string) (*stack, func()) { if err != nil { t.Fatalf("fs: %v", err) } - authSvc := auth.NewService(repos.Auth, auth.NewTokenManager("secret", 15*time.Minute, time.Hour)) + tm := auth.NewTokenManager("secret", 15*time.Minute, time.Hour) + authSvc := auth.NewService(repos.Auth, tm) albumsSvc := albums.NewService(repos.Albums, repos.Members, hub, blob) photosSvc := photos.NewService(repos.Photos, albumsSvc, blob, hub) - return &stack{db: database, photos: photosSvc, albums: albumsSvc, auth: authSvc, hub: hub}, func() { database.Close() } + return &stack{db: database, photos: photosSvc, albums: albumsSvc, auth: authSvc, hub: hub, tm: tm, repos: repos, storage: blob}, func() { database.Close() } } func TestUploadSetsDefaultCover(t *testing.T) { diff --git a/backend/internal/e2e/trash_test.go b/backend/internal/e2e/trash_test.go new file mode 100644 index 0000000..2c8d8bf --- /dev/null +++ b/backend/internal/e2e/trash_test.go @@ -0,0 +1,245 @@ +package e2e + +import ( + "context" + "database/sql" + "encoding/json" + "net/http" + "net/http/httptest" + "path/filepath" + "testing" + + "github.com/gin-gonic/gin" + + "galo/internal/albums" + "galo/internal/app" + "galo/internal/auth" + "galo/internal/config" + "galo/internal/photos" +) + +func TestPermanentDeleteAlbumlessPhoto(t *testing.T) { + ctx := context.Background() + st, closeDB := openStack(t, filepath.Join(t.TempDir(), "galo.db"), t.TempDir()) + defer closeDB() + + user, _, err := st.auth.Register(ctx, "albumless@test.local", "password123", "A") + if err != nil { + t.Fatalf("register: %v", err) + } + album, err := st.albums.Create(ctx, user.ID, albums.CreateInput{Title: "Doomed"}) + if err != nil { + t.Fatalf("create album: %v", err) + } + p, err := st.photos.Upload(ctx, photos.UploadInput{AlbumID: album.ID, UserID: user.ID, Filename: "a.png", Data: makePNG(t)}) + if err != nil { + t.Fatalf("upload: %v", err) + } + + var objectID string + if err := st.db.QueryRowContext(ctx, + `SELECT object_id FROM photo_objects WHERE photo_id = $1 AND role = 'original'`, p.ID).Scan(&objectID); err != nil { + t.Fatalf("original object: %v", err) + } + + // Deleting the album moves its photo to the trash with no album binding. + if err := st.albums.Delete(ctx, album.ID, user.ID); err != nil { + t.Fatalf("delete album: %v", err) + } + var albumID sql.NullString + if err := st.db.QueryRowContext(ctx, `SELECT album_id FROM photos WHERE id = $1`, p.ID).Scan(&albumID); err != nil { + t.Fatalf("read album_id: %v", err) + } + if albumID.Valid { + t.Fatalf("photo still bound to album %q after album deletion", albumID.String) + } + + if err := st.photos.PermanentDelete(ctx, p.ID, user.ID); err != nil { + t.Fatalf("permanent delete albumless photo: %v", err) + } + var n int + if err := st.db.QueryRowContext(ctx, `SELECT count(*) FROM photos WHERE id = $1`, p.ID).Scan(&n); err != nil { + t.Fatalf("count photos: %v", err) + } + if n != 0 { + t.Fatalf("photo row survived permanent delete") + } + var state string + if err := st.db.QueryRowContext(ctx, `SELECT state FROM objects WHERE id = $1`, objectID).Scan(&state); err != nil { + t.Fatalf("object row missing after delete: %v", err) + } + if state != photos.ObjectDeleting { + t.Fatalf("object state = %q, want %q", state, photos.ObjectDeleting) + } +} + +func TestEmptyTrashSkipsForeignPhotos(t *testing.T) { + ctx := context.Background() + st, closeDB := openStack(t, filepath.Join(t.TempDir(), "galo.db"), t.TempDir()) + defer closeDB() + + owner, _, err := st.auth.Register(ctx, "owner@test.local", "password123", "O") + if err != nil { + t.Fatalf("register owner: %v", err) + } + member, _, err := st.auth.Register(ctx, "member@test.local", "password123", "M") + if err != nil { + t.Fatalf("register member: %v", err) + } + album, err := st.albums.Create(ctx, owner.ID, albums.CreateInput{Title: "Shared"}) + if err != nil { + t.Fatalf("create album: %v", err) + } + if _, err := st.db.ExecContext(ctx, + `INSERT INTO album_members (album_id, user_id, role) VALUES ($1, $2, 'member')`, album.ID, member.ID); err != nil { + t.Fatalf("add member: %v", err) + } + + ownerPhoto, err := st.photos.Upload(ctx, photos.UploadInput{AlbumID: album.ID, UserID: owner.ID, Filename: "o.png", Data: makePNG(t)}) + if err != nil { + t.Fatalf("upload owner photo: %v", err) + } + memberPhoto, err := st.photos.Upload(ctx, photos.UploadInput{AlbumID: album.ID, UserID: member.ID, Filename: "m.png", Data: makePNG(t)}) + if err != nil { + t.Fatalf("upload member photo: %v", err) + } + if err := st.photos.Delete(ctx, ownerPhoto.ID, owner.ID); err != nil { + t.Fatalf("trash owner photo: %v", err) + } + if err := st.photos.Delete(ctx, memberPhoto.ID, member.ID); err != nil { + t.Fatalf("trash member photo: %v", err) + } + + if n, _ := st.photos.TrashCount(ctx, member.ID); n != 2 { + t.Fatalf("member trash count = %d, want 2", n) + } + + deleted, err := st.photos.EmptyTrash(ctx, member.ID) + if err != nil { + t.Fatalf("empty trash: %v", err) + } + if deleted != 1 { + t.Fatalf("deleted = %d, want 1 (only the member's own photo)", deleted) + } + + var n int + if err := st.db.QueryRowContext(ctx, `SELECT count(*) FROM photos WHERE id = $1`, memberPhoto.ID).Scan(&n); err != nil { + t.Fatalf("count member photo: %v", err) + } + if n != 0 { + t.Fatalf("member's own photo survived empty trash") + } + if err := st.db.QueryRowContext(ctx, `SELECT count(*) FROM photos WHERE id = $1`, ownerPhoto.ID).Scan(&n); err != nil { + t.Fatalf("count owner photo: %v", err) + } + if n != 1 { + t.Fatalf("foreign photo was removed by emptier") + } + if c, _ := st.photos.TrashCount(ctx, member.ID); c != 1 { + t.Fatalf("trash count after empty = %d, want 1 (the foreign photo)", c) + } + + // Nothing the caller may delete remains, so a second pass deletes nothing. + if deleted, err := st.photos.EmptyTrash(ctx, member.ID); err != nil || deleted != 0 { + t.Fatalf("second empty = %d, err=%v; want 0, nil", deleted, err) + } +} + +func TestListTrashIdentitiesNotCapped(t *testing.T) { + ctx := context.Background() + st, closeDB := openStack(t, filepath.Join(t.TempDir(), "galo.db"), t.TempDir()) + defer closeDB() + + user, _, err := st.auth.Register(ctx, "many@test.local", "password123", "M") + if err != nil { + t.Fatalf("register: %v", err) + } + + tx, err := st.db.BeginTx(ctx, nil) + if err != nil { + t.Fatalf("begin: %v", err) + } + for i := 0; i < 205; i++ { + if _, err := tx.ExecContext(ctx, + `INSERT INTO photos (id, uploader_id, deleted_at) VALUES ($1, $2, $3)`, + auth.NewID(), user.ID, "2026-01-01T00:00:00Z"); err != nil { + t.Fatalf("insert trashed photo %d: %v", i, err) + } + } + if err := tx.Commit(); err != nil { + t.Fatalf("commit: %v", err) + } + + items, err := st.repos.Photos.ListTrashIdentities(ctx, user.ID) + if err != nil { + t.Fatalf("list trash identities: %v", err) + } + if len(items) != 205 { + t.Fatalf("got %d identities, want 205 (display cap leaked into purge)", len(items)) + } +} + +func TestEmptyTrashReturnsZeroWhenEmpty(t *testing.T) { + ctx := context.Background() + st, closeDB := openStack(t, filepath.Join(t.TempDir(), "galo.db"), t.TempDir()) + defer closeDB() + + user, _, err := st.auth.Register(ctx, "empty@test.local", "password123", "E") + if err != nil { + t.Fatalf("register: %v", err) + } + deleted, err := st.photos.EmptyTrash(ctx, user.ID) + if err != nil { + t.Fatalf("empty trash: %v", err) + } + if deleted != 0 { + t.Fatalf("deleted = %d, want 0", deleted) + } +} + +func TestEmptyTrashHTTP(t *testing.T) { + ctx := context.Background() + st, closeDB := openStack(t, filepath.Join(t.TempDir(), "galo.db"), t.TempDir()) + defer closeDB() + + user, pair, err := st.auth.Register(ctx, "http@test.local", "password123", "H") + if err != nil { + t.Fatalf("register: %v", err) + } + album, err := st.albums.Create(ctx, user.ID, albums.CreateInput{Title: "HTTP"}) + if err != nil { + t.Fatalf("create album: %v", err) + } + p, err := st.photos.Upload(ctx, photos.UploadInput{AlbumID: album.ID, UserID: user.ID, Filename: "a.png", Data: makePNG(t)}) + if err != nil { + t.Fatalf("upload: %v", err) + } + if err := st.photos.Delete(ctx, p.ID, user.ID); err != nil { + t.Fatalf("trash photo: %v", err) + } + + gin.SetMode(gin.TestMode) + srv := app.New(&config.Config{}, st.repos, st.db.PingContext, st.tm, st.storage, st.hub) + router := srv.Router() + + req := httptest.NewRequest(http.MethodDelete, "/api/trash", nil) + req.Header.Set("Authorization", "Bearer "+pair.AccessToken) + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d body=%s", rec.Code, rec.Body.String()) + } + var body struct { + Deleted int `json:"deleted"` + } + if err := json.Unmarshal(rec.Body.Bytes(), &body); err != nil { + t.Fatalf("decode: %v", err) + } + if body.Deleted != 1 { + t.Fatalf("deleted = %d, want 1", body.Deleted) + } + if n, _ := st.photos.TrashCount(ctx, user.ID); n != 0 { + t.Fatalf("trash count after empty = %d, want 0", n) + } +} diff --git a/backend/internal/photos/handlers.go b/backend/internal/photos/handlers.go index 7bb6bd6..500ac6c 100644 --- a/backend/internal/photos/handlers.go +++ b/backend/internal/photos/handlers.go @@ -166,6 +166,15 @@ func (h *Handler) TrashCount(c *gin.Context) { c.JSON(http.StatusOK, gin.H{"count": n}) } +func (h *Handler) EmptyTrash(c *gin.Context) { + n, err := h.svc.EmptyTrash(c.Request.Context(), auth.UserID(c)) + if err != nil { + httpx.RespondError(c, err) + return + } + c.JSON(http.StatusOK, gin.H{"deleted": n}) +} + func (h *Handler) Tags(c *gin.Context) { tags, err := h.svc.Tags(c.Request.Context(), auth.UserID(c), c.Query("q")) if err != nil { diff --git a/backend/internal/photos/repository.go b/backend/internal/photos/repository.go index 8a0a3b8..ae5599d 100644 --- a/backend/internal/photos/repository.go +++ b/backend/internal/photos/repository.go @@ -7,6 +7,14 @@ import ( "galo/internal/models" ) +// TrashIdentity is the minimal fact set needed to authorize a trashed photo +// during a bulk purge without loading the full photo. +type TrashIdentity struct { + ID string + AlbumID string + UploaderID string +} + // Repository is the persistence contract for the photos domain, including the // object ledger, variant queue and garbage collection. type Repository interface { @@ -19,6 +27,9 @@ type Repository interface { Clone(ctx context.Context, src *models.Photo, albumID, uploaderID, id string) error HardDelete(ctx context.Context, id string) error ListTrash(ctx context.Context, userID string) ([]models.Photo, error) + // ListTrashIdentities returns every trashed photo accessible to the user + // (no display cap) so a bulk purge can authorize and remove each one. + ListTrashIdentities(ctx context.Context, userID string) ([]TrashIdentity, error) TrashCount(ctx context.Context, userID string) (int, error) RestoreToAlbum(ctx context.Context, id, albumID string) error AlbumExists(ctx context.Context, albumID string) (bool, error) diff --git a/backend/internal/photos/service.go b/backend/internal/photos/service.go index 06e0858..f6839b1 100644 --- a/backend/internal/photos/service.go +++ b/backend/internal/photos/service.go @@ -597,12 +597,19 @@ func (s *Service) PermanentDelete(ctx context.Context, photoID, userID string) e } return err } - _, role, err := s.albums.Access(ctx, p.AlbumID, userID) - if err != nil { - return err - } - if !canDeletePhoto(role, p.UploaderID == userID) { - return httpx.Forbidden("forbidden", "you cannot delete this photo") + if p.AlbumID == "" { + // A photo whose album was deleted only has its uploader to vouch for it. + if p.UploaderID != userID { + return httpx.Forbidden("forbidden", "you cannot delete this photo") + } + } else { + _, role, err := s.albums.Access(ctx, p.AlbumID, userID) + if err != nil { + return err + } + if !canDeletePhoto(role, p.UploaderID == userID) { + return httpx.Forbidden("forbidden", "you cannot delete this photo") + } } objectIDs, err := s.store.PhotoObjectIDs(ctx, photoID) if err != nil { @@ -624,6 +631,23 @@ func (s *Service) PermanentDelete(ctx context.Context, photoID, userID string) e return nil } +// EmptyTrash permanently deletes every trashed photo the user may remove, +// skipping the rest, and returns how many were deleted. +func (s *Service) EmptyTrash(ctx context.Context, userID string) (int, error) { + items, err := s.store.ListTrashIdentities(ctx, userID) + if err != nil { + return 0, err + } + deleted := 0 + for _, it := range items { + if err := s.PermanentDelete(ctx, it.ID, userID); err != nil { + continue + } + deleted++ + } + return deleted, nil +} + // MoveBatch moves photos to another album (metadata only; stored objects untouched). func (s *Service) MoveBatch(ctx context.Context, ids []string, userID, targetAlbumID string) (int, error) { _, targetRole, err := s.albums.Access(ctx, targetAlbumID, userID) diff --git a/backend/internal/photos/sqlite_store.go b/backend/internal/photos/sqlite_store.go index 8b5180d..c3b549b 100644 --- a/backend/internal/photos/sqlite_store.go +++ b/backend/internal/photos/sqlite_store.go @@ -322,6 +322,34 @@ func (s *SQLiteStore) TrashCount(ctx context.Context, userID string) (int, error return n, err } +func (s *SQLiteStore) ListTrashIdentities(ctx context.Context, userID string) ([]TrashIdentity, error) { + rows, err := s.db.QueryContext(ctx, + `SELECT p.id, COALESCE(p.album_id, ''), p.uploader_id + FROM photos p + LEFT JOIN albums a ON a.id = p.album_id + WHERE p.deleted_at IS NOT NULL + AND ( + (a.id IS NOT NULL AND (a.owner_id = $1 OR EXISTS( + SELECT 1 FROM album_members m WHERE m.album_id = a.id AND m.user_id = $1))) + OR (p.album_id IS NULL AND p.uploader_id = $1) + ) + ORDER BY p.deleted_at DESC`, + userID) + if err != nil { + return nil, err + } + defer rows.Close() + out := []TrashIdentity{} + for rows.Next() { + var it TrashIdentity + if err := rows.Scan(&it.ID, &it.AlbumID, &it.UploaderID); err != nil { + return nil, err + } + out = append(out, it) + } + return out, rows.Err() +} + func (s *SQLiteStore) RestoreToAlbum(ctx context.Context, id, albumID string) error { res, err := s.db.ExecContext(ctx, `UPDATE photos SET album_id = $2, deleted_at = NULL WHERE id = $1`, id, albumID) diff --git a/backend/internal/photos/store.go b/backend/internal/photos/store.go index 95d49f4..e6ab2b2 100644 --- a/backend/internal/photos/store.go +++ b/backend/internal/photos/store.go @@ -341,6 +341,36 @@ func (s *Store) TrashCount(ctx context.Context, userID string) (int, error) { return n, err } +// ListTrashIdentities returns every trashed photo accessible to the user +// (no display cap) so a bulk purge can authorize and remove each one. +func (s *Store) ListTrashIdentities(ctx context.Context, userID string) ([]TrashIdentity, error) { + rows, err := s.pool.Query(ctx, + `SELECT p.id, COALESCE(p.album_id, ''), p.uploader_id + FROM photos p + LEFT JOIN albums a ON a.id = p.album_id + WHERE p.deleted_at IS NOT NULL + AND ( + (a.id IS NOT NULL AND (a.owner_id = $1 OR EXISTS( + SELECT 1 FROM album_members m WHERE m.album_id = a.id AND m.user_id = $1))) + OR (p.album_id IS NULL AND p.uploader_id = $1) + ) + ORDER BY p.deleted_at DESC`, + userID) + if err != nil { + return nil, err + } + defer rows.Close() + out := []TrashIdentity{} + for rows.Next() { + var it TrashIdentity + if err := rows.Scan(&it.ID, &it.AlbumID, &it.UploaderID); err != nil { + return nil, err + } + out = append(out, it) + } + return out, rows.Err() +} + // RestoreToAlbum reassigns a soft-deleted photo to an album and clears the trash flag. func (s *Store) RestoreToAlbum(ctx context.Context, id, albumID string) error { tag, err := s.pool.Exec(ctx, diff --git a/frontend/src/lib/api.ts b/frontend/src/lib/api.ts index 5323c6a..6c98cf4 100644 --- a/frontend/src/lib/api.ts +++ b/frontend/src/lib/api.ts @@ -121,6 +121,7 @@ export const photosApi = { }), trash: () => api<{ photos: Photo[] }>('/trash'), trashCount: () => api<{ count: number }>('/trash/count'), + emptyTrash: () => api<{ deleted: number }>('/trash', { method: 'DELETE' }), restore: (id: string, albumId?: string) => api<{ photo: Photo }>(`/photos/${id}/restore`, { method: 'POST', diff --git a/frontend/src/routes/(app)/trash/+page.svelte b/frontend/src/routes/(app)/trash/+page.svelte index 760c87a..a2cadcb 100644 --- a/frontend/src/routes/(app)/trash/+page.svelte +++ b/frontend/src/routes/(app)/trash/+page.svelte @@ -4,6 +4,7 @@ import { portal } from '$lib/actions/portal'; import { photosApi, ApiError } from '$lib/api'; import { albums } from '$lib/stores/albums.svelte'; + import { trash } from '$lib/stores/trash.svelte'; import type { Photo } from '$lib/types'; let photos = $state([]); @@ -12,6 +13,9 @@ let pending = $state(null); let busy = $state(false); + let confirmingEmpty = $state(false); + let emptyBusy = $state(false); + let restoring = $state(null); let targetId = $state(''); let createNew = $state(false); @@ -92,12 +96,37 @@ busy = false; } } + + async function emptyAll() { + if (emptyBusy) return; + emptyBusy = true; + try { + await photosApi.emptyTrash(); + await load(); + await trash.load(); + confirmingEmpty = false; + } catch (e) { + error = (e as Error).message; + } finally { + emptyBusy = false; + } + }

Корзина

+ {#if photos.length > 0} + + {/if}
{#if error}

{error}

{/if} @@ -166,6 +195,17 @@ /> {/if} +{#if confirmingEmpty} + (confirmingEmpty = false)} + /> +{/if} + {#if restoring}
}`, backed by +`photos.Service.EmptyTrash(ctx, userID)`. It iterates the caller's accessible trashed +photos and calls the existing `PermanentDelete` for each, counting successes and +skipping photos the caller may not delete. Reusing `PermanentDelete` keeps authorization +and object GC identical to single-photo deletion. + +- Alternative considered: client enumerates trash and calls `POST /photo-batch/...` + with every id. Rejected - the display list is capped at 200 and this is racy; the + server already knows the full accessible set. +- Alternative considered: a single `DELETE FROM photos ... WHERE `. Rejected + for now - album-role authorization lives in Go (`roles.CanDelete*`), and per-photo object + GC is required anyway. + +### Bypass the 200-item display cap for purging +`EmptyTrash` must see more than the display page. Add a store method that returns all +accessible trashed photo identities (id, album_id, uploader_id) with no display cap, and +authorize each in the service. This avoids looping on a capped list (which would also +risk an infinite loop when some items are not deletable). + +- Alternative considered: paginate `ListTrash` until a pass removes nothing. Rejected - + undeletable items would reappear and a zero-progress pass is needed to stop; the + uncapped identity listing is simpler and bounded by the user's own trash size. + +### Frontend: explicit action plus confirmation +Add an «Очистить корзину» control on the trash screen header, enabled when the list is +non-empty. It opens the existing `ConfirmDialog`; on confirm it calls `DELETE /trash`, +then reloads the list and refreshes `trash` store count. Show a busy state while running. + +## Risks / Trade-offs + +- [Large trash makes the per-photo loop slow] -> Each `PermanentDelete` is a small fixed + number of queries; bulk deletes are O(n) round trips. Acceptable for now; a set-based + purge can replace the loop later without changing the spec behavior. Keep the frontend + busy state to make latency legible. +- [Uncapped listing memory] -> The query returns only the caller's trashed photo + identities; bounded in practice by trash size. Revisit with chunking if it grows. +- [Partial failure mid-purge] -> `EmptyTrash` skips failures and returns the count + deleted; the remaining items stay in the trash and can be retried. +- [Albumless deletes and realtime] -> `PermanentDelete` only publishes when `AlbumID != ""`; + albumless removals publish nothing, which is correct since no album is watching. + +## Migration Plan + +No schema change. Deploy backend and frontend together; the new endpoint is additive and +the authorizaton fix is behavior-only. Rollback is reverting the deploy. + +## Open Questions + +None blocking. diff --git a/openspec/changes/trash-permanent-delete-and-empty/proposal.md b/openspec/changes/trash-permanent-delete-and-empty/proposal.md new file mode 100644 index 0000000..fade67f --- /dev/null +++ b/openspec/changes/trash-permanent-delete-and-empty/proposal.md @@ -0,0 +1,45 @@ +# Proposal + +## Why + +Permanently deleting a photo from the trash fails when the photo has no album +(`album_id IS NULL`) — the normal result of deleting an album, whose photos move +to the trash unbound (`photos/spec.md`, «Удаление альбома»). `PermanentDelete` +authorizes through the album, so these photos can never be removed, and the UI +surfaces `album_not_found`. In the local database 4010 of 4011 trashed photos are +in this state. There is also no way to clear the trash in bulk. + +## What Changes + +- Fix `PermanentDelete` authorization so a photo without an album can be + permanently deleted by a user who may delete it (its uploader), instead of + failing on a missing album. +- Add a bulk permanent-delete operation that empties the trash for the caller, + skipping photos the caller is not allowed to remove, and reporting how many + were deleted. +- Add an «Очистить корзину» action on the trash screen with a confirmation + dialog, refreshing the list and the rail counter afterwards. + +## Capabilities + +### New Capabilities + + + +### Modified Capabilities + +- `photos`: «Окончательное удаление» also covers photos with no album + (deleted album); add an «Очистка корзины» requirement for bulk permanent + removal. +- `ui-workspace`: trash screen gains an «Очистить корзину» control with + confirmation and post-action refresh. + +## Impact + +- Backend: `backend/internal/photos/service.go` (`PermanentDelete`), the photos + repository/store (new bulk purge method), `handlers.go`, and a route in + `backend/internal/app/app.go`. +- Frontend: `frontend/src/lib/api.ts`, `frontend/src/lib/stores/trash.svelte.ts`, + `frontend/src/routes/(app)/trash/+page.svelte`. +- Tests: e2e/contract coverage for permanently deleting an albumless photo and + for the bulk empty-trash path. diff --git a/openspec/changes/trash-permanent-delete-and-empty/specs/photos/spec.md b/openspec/changes/trash-permanent-delete-and-empty/specs/photos/spec.md new file mode 100644 index 0000000..323901a --- /dev/null +++ b/openspec/changes/trash-permanent-delete-and-empty/specs/photos/spec.md @@ -0,0 +1,48 @@ +# Spec Delta + +## MODIFIED Requirements + +### Requirement: Корзина фото +Удаление фото SHALL помещать его в корзину (мягкое удаление). Восстановление SHALL возвращать +фото в его альбом. Окончательное удаление SHALL удалять запись, а объекты хранилища SHALL +удаляться только когда на них не остаётся ссылок ни в одном альбоме. Окончательное удаление +SHALL быть доступно и для фото без альбома (например, после удаления альбома). + +#### Scenario: Удаление в корзину +- **WHEN** пользователь удаляет фото +- **THEN** фото пропадает из альбома и появляется в корзине + +#### Scenario: Восстановление +- **WHEN** пользователь восстанавливает фото из корзины +- **THEN** фото возвращается в свой альбом + +#### Scenario: Окончательное удаление без ссылок +- **WHEN** пользователь удаляет фото из корзины навсегда, и на те же объекты нет ссылок в других альбомах +- **THEN** запись и объекты хранилища удаляются + +#### Scenario: Окончательное удаление при наличии ссылок +- **WHEN** на объект хранилища ссылается копия в другом альбоме +- **THEN** запись удаляется, а объект хранилища сохраняется + +#### Scenario: Окончательное удаление фото без альбома +- **WHEN** пользователь удаляет навсегда фото из корзины, у которого нет альбома +- **THEN** запись удаляется, а операция не завершается ошибкой отсутствующего альбома + +## ADDED Requirements + +### Requirement: Очистка корзины +Пользователь SHALL иметь возможность окончательно удалить все элементы корзины одним действием. +Система SHALL удалять только те элементы, которые пользователь вправе удалить навсегда, и SHALL +пропускать остальные. Операция SHALL возвращать число удалённых элементов. + +#### Scenario: Очистка корзины +- **WHEN** пользователь очищает корзину +- **THEN** все элементы, которые он вправе удалить навсегда, удаляются + +#### Scenario: Чужие элементы +- **WHEN** в корзине есть фото, которые пользователь не вправе удалить навсегда +- **THEN** они остаются в корзине, а остальные удаляются + +#### Scenario: Пустая корзина +- **WHEN** пользователь очищает пустую корзину +- **THEN** операция завершается успешно, удалив ноль элементов diff --git a/openspec/changes/trash-permanent-delete-and-empty/specs/ui-workspace/spec.md b/openspec/changes/trash-permanent-delete-and-empty/specs/ui-workspace/spec.md new file mode 100644 index 0000000..3f220b6 --- /dev/null +++ b/openspec/changes/trash-permanent-delete-and-empty/specs/ui-workspace/spec.md @@ -0,0 +1,20 @@ +# Spec Delta + +## ADDED Requirements + +### Requirement: Очистка корзины в интерфейсе +Экран корзины SHALL содержать действие «Очистить корзину», доступное, когда в корзине есть +элементы. Действие SHALL открывать подтверждение и после подтверждения очищать корзину, а +список и счётчик корзины в рейле SHALL отражать результат. + +#### Scenario: Наличие действия +- **WHEN** пользователь открывает корзину с элементами +- **THEN** на экране показано действие «Очистить корзину» + +#### Scenario: Подтверждение очистки +- **WHEN** пользователь выбирает «Очистить корзину» +- **THEN** открывается диалог подтверждения, и только после подтверждения корзина очищается + +#### Scenario: Обновление после очистки +- **WHEN** корзина очищена +- **THEN** на экране показывается пустая корзина, а счётчик корзины в рейле обновляется diff --git a/openspec/changes/trash-permanent-delete-and-empty/tasks.md b/openspec/changes/trash-permanent-delete-and-empty/tasks.md new file mode 100644 index 0000000..7425021 --- /dev/null +++ b/openspec/changes/trash-permanent-delete-and-empty/tasks.md @@ -0,0 +1,28 @@ +# Tasks + +## 1. Fix permanent delete for albumless photos + +- [x] 1.1 In `photos.Service.PermanentDelete`, when `p.AlbumID == ""`, authorize by `p.UploaderID == userID` instead of calling `albums.Access`; keep the album-role check when an album is present. Verify with a Go test that permanently deletes an albumless trashed photo (add to `backend/internal/e2e/sqlite_test.go`) and passes. +- [x] 1.2 Confirm object garbage collection still runs for albumless deletes (`PhotoObjectIDs` + `MarkObjectDeleting`); verify the test asserts the photo row is gone and an unbound object is marked deleting. + +## 2. Add empty-trash operation + +- [x] 2.1 Add a photos repository/store method that lists all accessible trashed photo identities (`id`, `album_id`, `uploader_id`) without the 200-item display cap; verify with a test that seeds more than 200 trashed photos and asserts all are returned. +- [x] 2.2 Add `Service.EmptyTrash(ctx, userID)` that authorizes each listed photo with the same rules as `PermanentDelete` (album role when bound, uploader when albumless), skips forbidden ones, and returns the deleted count; verify with a test where a shared album holds another user's trashed photo — the foreign photo remains, the caller's are removed, and the count matches. +- [x] 2.3 Add `Handler.EmptyTrash` and the `DELETE /trash` route returning `{ "deleted": n }`; verify with an HTTP-level test asserting the status code and response body. +- [x] 2.4 Add end-to-end coverage: emptying an empty trash returns `0`, and after emptying a non-empty trash `TrashCount` is `0`; verify with `make test`. + +## 3. Trash screen action + +- [x] 3.1 Add `photosApi.emptyTrash()` calling `DELETE /trash` in `frontend/src/lib/api.ts`; verify with `npm run check`. +- [x] 3.2 Add an «Очистить корзину» control to `frontend/src/routes/(app)/trash/+page.svelte`, enabled only when the list is non-empty, opening the existing `ConfirmDialog`; on confirm call `emptyTrash`, then reload the list and call `trash.load()`; verify by clearing a non-empty trash and observing the empty state and a busy state during the call. +- [x] 3.3 Verify the rail counter reflects the result: after clearing, the trash count in `AlbumRail` becomes `0` (observable in the running app). + +## 4. Integration verification + +- [x] 4.1 Run `make test` (backend) and `npm run check` plus `npm run build` (frontend); verify all pass with no regressions. + +## Workflow follow-up + +- Run `/opsx-apply` to implement this change. +- Archive the change after review and verification (sync delta specs into main specs).