diff --git a/backend/internal/db/postgres/file_repo.go b/backend/internal/db/postgres/file_repo.go index 0dc0b44..2c7ec63 100644 --- a/backend/internal/db/postgres/file_repo.go +++ b/backend/internal/db/postgres/file_repo.go @@ -36,6 +36,7 @@ type fileRow struct { CreatorName string `db:"creator_name"` IsPublic bool `db:"is_public"` IsDeleted bool `db:"is_deleted"` + NeedsReview bool `db:"needs_review"` } // fileTagRow is used for both single-file and batch tag loading. @@ -81,6 +82,7 @@ func toFile(r fileRow) domain.File { CreatorName: r.CreatorName, IsPublic: r.IsPublic, IsDeleted: r.IsDeleted, + NeedsReview: r.NeedsReview, CreatedAt: domain.UUIDCreatedAt(r.ID), } } @@ -293,7 +295,7 @@ const fileSelectCTE = ` mt.name AS mime_type, mt.extension AS mime_extension, r.content_datetime, r.notes, r.metadata, r.exif, r.phash, r.creator_id, u.name AS creator_name, - r.is_public, r.is_deleted + r.is_public, r.is_deleted, r.needs_review FROM r JOIN core.mime_types mt ON mt.id = r.mime_id JOIN core.users u ON u.id = r.creator_id` @@ -316,7 +318,8 @@ func (r *FileRepo) Create(ctx context.Context, f *domain.File) (*domain.File, er $4, $5, $6, $7, $8, $9, $10 ) RETURNING id, original_name, mime_id, content_datetime, notes, - metadata, exif, phash, creator_id, is_public, is_deleted + metadata, exif, phash, creator_id, is_public, is_deleted, + needs_review )` + fileSelectCTE q := connOrTx(ctx, r.pool) @@ -346,7 +349,7 @@ func (r *FileRepo) GetByID(ctx context.Context, id uuid.UUID) (*domain.File, err mt.name AS mime_type, mt.extension AS mime_extension, f.content_datetime, f.notes, f.metadata, f.exif, f.phash, f.creator_id, u.name AS creator_name, - f.is_public, f.is_deleted + f.is_public, f.is_deleted, f.needs_review FROM data.files f JOIN core.mime_types mt ON mt.id = f.mime_id JOIN core.users u ON u.id = f.creator_id @@ -389,7 +392,8 @@ func (r *FileRepo) Update(ctx context.Context, id uuid.UUID, f *domain.File) (*d is_public = $6 WHERE id = $1 RETURNING id, original_name, mime_id, content_datetime, notes, - metadata, exif, phash, creator_id, is_public, is_deleted + metadata, exif, phash, creator_id, is_public, is_deleted, + needs_review )` + fileSelectCTE q := connOrTx(ctx, r.pool) @@ -416,6 +420,20 @@ func (r *FileRepo) Update(ctx context.Context, id uuid.UUID, f *domain.File) (*d return &updated, nil } +// SetNeedsReview sets the review status on the given files in one statement. +// Trashed files are left untouched. No-op for an empty id list. +func (r *FileRepo) SetNeedsReview(ctx context.Context, ids []uuid.UUID, value bool) error { + if len(ids) == 0 { + return nil + } + const sqlStr = `UPDATE data.files SET needs_review = $2 WHERE id = ANY($1) AND is_deleted = false` + q := connOrTx(ctx, r.pool) + if _, err := q.Exec(ctx, sqlStr, ids, value); err != nil { + return fmt.Errorf("FileRepo.SetNeedsReview: %w", err) + } + return nil +} + // --------------------------------------------------------------------------- // SoftDelete / Restore / DeletePermanent // --------------------------------------------------------------------------- @@ -444,7 +462,8 @@ func (r *FileRepo) Restore(ctx context.Context, id uuid.UUID) (*domain.File, err SET is_deleted = false WHERE id = $1 AND is_deleted = true RETURNING id, original_name, mime_id, content_datetime, notes, - metadata, exif, phash, creator_id, is_public, is_deleted + metadata, exif, phash, creator_id, is_public, is_deleted, + needs_review )` + fileSelectCTE q := connOrTx(ctx, r.pool) @@ -638,7 +657,7 @@ func (r *FileRepo) List(ctx context.Context, params domain.FileListParams) (*dom mt.name AS mime_type, mt.extension AS mime_extension, f.content_datetime, f.notes, f.metadata, f.exif, f.phash, f.creator_id, u.name AS creator_name, - f.is_public, f.is_deleted + f.is_public, f.is_deleted, f.needs_review FROM data.files f JOIN core.mime_types mt ON mt.id = f.mime_id JOIN core.users u ON u.id = f.creator_id diff --git a/backend/internal/db/postgres/filter_parser.go b/backend/internal/db/postgres/filter_parser.go index 90110c9..badbf7c 100644 --- a/backend/internal/db/postgres/filter_parser.go +++ b/backend/internal/db/postgres/filter_parser.go @@ -23,6 +23,7 @@ const ( ftkTag // t= ftkMimeExact // m= ftkMimeLike // m~ + ftkReview // r=<0|1> ) type filterToken struct { @@ -31,6 +32,7 @@ type filterToken struct { untagged bool // ftkTag with zero UUID → "file has no tags" mimeID int16 // ftkMimeExact pattern string // ftkMimeLike + review bool // ftkReview → needs_review value } // --------------------------------------------------------------------------- @@ -80,6 +82,8 @@ func (l *leafNode) toSQL(n int, args []any) (string, int, []any) { case ftkMimeLike: // mt alias comes from the JOIN in the main file query (always present). return fmt.Sprintf("mt.name LIKE $%d", n), n + 1, append(args, l.tok.pattern) + case ftkReview: + return fmt.Sprintf("f.needs_review = $%d", n), n + 1, append(args, l.tok.review) } panic("filterNode.toSQL: unknown leaf kind") } @@ -130,6 +134,15 @@ func lexFilter(dsl string) ([]filterToken, error) { case strings.HasPrefix(p, "m~"): // The pattern value is passed as a query parameter, so no SQL injection risk. tokens = append(tokens, filterToken{kind: ftkMimeLike, pattern: p[2:]}) + case strings.HasPrefix(p, "r="): + switch p[2:] { + case "1": + tokens = append(tokens, filterToken{kind: ftkReview, review: true}) + case "0": + tokens = append(tokens, filterToken{kind: ftkReview, review: false}) + default: + return nil, fmt.Errorf("filter: invalid review flag %q (want r=0 or r=1)", p[2:]) + } default: return nil, fmt.Errorf("filter: unknown token %q", p) } @@ -241,7 +254,7 @@ func (p *filterParser) parseAtom() (filterNode, error) { return expr, nil } switch t.kind { - case ftkTag, ftkMimeExact, ftkMimeLike: + case ftkTag, ftkMimeExact, ftkMimeLike, ftkReview: p.next() return &leafNode{t}, nil default: diff --git a/backend/internal/db/postgres/filter_parser_test.go b/backend/internal/db/postgres/filter_parser_test.go index bfa7fd7..b42c7b5 100644 --- a/backend/internal/db/postgres/filter_parser_test.go +++ b/backend/internal/db/postgres/filter_parser_test.go @@ -6,6 +6,50 @@ import ( "github.com/google/uuid" ) +func TestParseFilterReview(t *testing.T) { + t.Run("r=1 needs review", func(t *testing.T) { + sql, n, args, err := ParseFilter("{r=1}", 1) + if err != nil { + t.Fatalf("ParseFilter: %v", err) + } + if sql != "f.needs_review = $1" { + t.Fatalf("sql = %q", sql) + } + if n != 2 || len(args) != 1 || args[0] != true { + t.Fatalf("n=%d args=%v", n, args) + } + }) + + t.Run("r=0 reviewed", func(t *testing.T) { + sql, _, args, err := ParseFilter("{r=0}", 1) + if err != nil { + t.Fatalf("ParseFilter: %v", err) + } + if sql != "f.needs_review = $1" || len(args) != 1 || args[0] != false { + t.Fatalf("sql=%q args=%v", sql, args) + } + }) + + t.Run("combined with mime", func(t *testing.T) { + sql, n, args, err := ParseFilter("{r=1,&,m~image/%}", 1) + if err != nil { + t.Fatalf("ParseFilter: %v", err) + } + if sql != "(f.needs_review = $1 AND mt.name LIKE $2)" { + t.Fatalf("sql = %q", sql) + } + if n != 3 || len(args) != 2 || args[0] != true || args[1] != "image/%" { + t.Fatalf("n=%d args=%v", n, args) + } + }) + + t.Run("invalid flag rejected", func(t *testing.T) { + if _, _, _, err := ParseFilter("{r=2}", 1); err == nil { + t.Fatal("expected error for r=2") + } + }) +} + func TestFilterTagUses(t *testing.T) { a := uuid.MustParse("11111111-1111-1111-1111-111111111111") b := uuid.MustParse("22222222-2222-2222-2222-222222222222") diff --git a/backend/internal/domain/file.go b/backend/internal/domain/file.go index d5569e4..9cb289e 100644 --- a/backend/internal/domain/file.go +++ b/backend/internal/domain/file.go @@ -29,6 +29,7 @@ type File struct { CreatorName string // denormalized from core.users IsPublic bool IsDeleted bool + NeedsReview bool // tagging not yet marked done; cleared by an explicit review action CreatedAt time.Time // extracted from UUID v7 via UUIDCreatedAt Tags []Tag // loaded with the file } diff --git a/backend/internal/handler/file_handler.go b/backend/internal/handler/file_handler.go index 44ef8fb..681557a 100644 --- a/backend/internal/handler/file_handler.go +++ b/backend/internal/handler/file_handler.go @@ -84,6 +84,7 @@ type fileJSON struct { CreatorName string `json:"creator_name"` IsPublic bool `json:"is_public"` IsDeleted bool `json:"is_deleted"` + NeedsReview bool `json:"needs_review"` CreatedAt string `json:"created_at"` Tags []tagJSON `json:"tags"` } @@ -131,6 +132,7 @@ func toFileJSON(f domain.File) fileJSON { CreatorName: f.CreatorName, IsPublic: f.IsPublic, IsDeleted: f.IsDeleted, + NeedsReview: f.NeedsReview, CreatedAt: f.CreatedAt.Format(time.RFC3339), Tags: tags, } @@ -661,6 +663,33 @@ func (h *FileHandler) BulkDelete(c *gin.Context) { c.Status(http.StatusNoContent) } +// BulkReview sets the review status on one or more files. A single-file toggle +// is just a one-element file_ids array. Files the caller cannot edit are +// silently skipped (handled in the service). +func (h *FileHandler) BulkReview(c *gin.Context) { + var body struct { + FileIDs []string `json:"file_ids" binding:"required"` + NeedsReview *bool `json:"needs_review" binding:"required"` + } + if err := c.ShouldBindJSON(&body); err != nil || body.NeedsReview == nil { + respondError(c, domain.ErrValidation) + return + } + + fileIDs, err := parseUUIDs(body.FileIDs) + if err != nil { + respondError(c, domain.ErrValidation) + return + } + + if err := h.fileSvc.SetNeedsReview(c.Request.Context(), fileIDs, *body.NeedsReview); err != nil { + respondError(c, err) + return + } + + c.Status(http.StatusNoContent) +} + // --------------------------------------------------------------------------- // POST /files/bulk/common-tags // --------------------------------------------------------------------------- diff --git a/backend/internal/handler/router.go b/backend/internal/handler/router.go index 55ff518..85b5a5b 100644 --- a/backend/internal/handler/router.go +++ b/backend/internal/handler/router.go @@ -83,6 +83,7 @@ func NewRouter( // Bulk + import routes registered before /:id to prevent param collision. files.POST("/bulk/tags", fileHandler.BulkSetTags) files.POST("/bulk/delete", fileHandler.BulkDelete) + files.POST("/bulk/review", fileHandler.BulkReview) files.POST("/bulk/common-tags", fileHandler.CommonTags) files.POST("/import", fileHandler.Import) diff --git a/backend/internal/integration/server_test.go b/backend/internal/integration/server_test.go index a332f58..549b676 100644 --- a/backend/internal/integration/server_test.go +++ b/backend/internal/integration/server_test.go @@ -21,6 +21,7 @@ import ( "net" "net/http" "net/http/httptest" + "net/url" "os" "path/filepath" "strings" @@ -1339,6 +1340,58 @@ func TestImportOrdersByMtime(t *testing.T) { assert.Less(t, idx["b_middle.jpg"], idx["a_newest.jpg"], "middle should be processed before newest") } +// TestFileReviewStatus verifies the per-file "needs review" flag: new uploads +// start as needs_review=true, POST /files/bulk/review clears it, and the DSL +// filter r=0/r=1 selects reviewed/unreviewed files (also combined with others). +func TestFileReviewStatus(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test in short mode") + } + h := setupSuite(t) + tok := h.login("admin", "admin") + + file := h.uploadJPEG(tok, "review-me.jpg") + fileID := file["id"].(string) + assert.Equal(t, true, file["needs_review"], "new upload should need review") + + getReview := func() bool { + r := h.doJSON("GET", "/files/"+fileID, nil, tok) + require.Equal(t, http.StatusOK, r.StatusCode, r.String()) + var obj map[string]any + r.decode(t, &obj) + return obj["needs_review"].(bool) + } + listIDs := func(dsl string) []string { + r := h.doJSON("GET", "/files?filter="+url.QueryEscape(dsl), nil, tok) + require.Equal(t, http.StatusOK, r.StatusCode, r.String()) + var page map[string]any + r.decode(t, &page) + ids := []string{} + for _, it := range page["items"].([]any) { + ids = append(ids, it.(map[string]any)["id"].(string)) + } + return ids + } + + // Before marking done: appears under r=1 (needs review), not under r=0. + assert.True(t, getReview()) + assert.Contains(t, listIDs("{r=1}"), fileID) + assert.NotContains(t, listIDs("{r=0}"), fileID) + + // Mark as reviewed (done) via the bulk endpoint (single-element list). + resp := h.doJSON("POST", "/files/bulk/review", map[string]any{ + "file_ids": []string{fileID}, + "needs_review": false, + }, tok) + require.Equal(t, http.StatusNoContent, resp.StatusCode, resp.String()) + + // Now reviewed: appears under r=0, not r=1; combines with a MIME predicate. + assert.False(t, getReview(), "file should no longer need review") + assert.Contains(t, listIDs("{r=0}"), fileID) + assert.NotContains(t, listIDs("{r=1}"), fileID) + assert.Contains(t, listIDs("{r=0,&,m~image/%}"), fileID) +} + // TestContentRangeRequests verifies the original-content endpoint answers a // byte-range request with 206 Partial Content (so the browser can seek within // audio/video) rather than streaming the whole body. diff --git a/backend/internal/port/repository.go b/backend/internal/port/repository.go index 5bd7e1b..da60e57 100644 --- a/backend/internal/port/repository.go +++ b/backend/internal/port/repository.go @@ -48,6 +48,8 @@ type FileRepo interface { Create(ctx context.Context, f *domain.File) (*domain.File, error) // Update applies partial metadata changes and returns the updated record. Update(ctx context.Context, id uuid.UUID, f *domain.File) (*domain.File, error) + // SetNeedsReview sets the review status on the given (non-trashed) files. + SetNeedsReview(ctx context.Context, ids []uuid.UUID, value bool) error // SoftDelete moves a file to trash (sets is_deleted = true). SoftDelete(ctx context.Context, id uuid.UUID) error // Restore moves a file out of trash (sets is_deleted = false). diff --git a/backend/internal/service/file_service.go b/backend/internal/service/file_service.go index 29a09cb..59de2bc 100644 --- a/backend/internal/service/file_service.go +++ b/backend/internal/service/file_service.go @@ -558,6 +558,47 @@ func (s *FileService) BulkDelete(ctx context.Context, fileIDs []uuid.UUID) error return nil } +// SetNeedsReview sets the review status ("needs tagging" vs marked done) on the +// given files. Each file is checked against edit ACL; files the caller cannot +// edit, or that do not exist, are skipped (same forgiving semantics as +// BulkDelete). Authorized files are updated in a single statement and each is +// audit-logged. Works for one file (single-element slice) or many. +func (s *FileService) SetNeedsReview(ctx context.Context, ids []uuid.UUID, value bool) error { + userID, isAdmin, _ := domain.UserFromContext(ctx) + + authorized := make([]uuid.UUID, 0, len(ids)) + for _, id := range ids { + f, err := s.files.GetByID(ctx, id) + if err != nil { + if err == domain.ErrNotFound { + continue + } + return err + } + ok, err := s.acl.CanEdit(ctx, userID, isAdmin, f.CreatorID, fileObjectTypeID, id) + if err != nil { + return err + } + if ok { + authorized = append(authorized, id) + } + } + + if len(authorized) == 0 { + return nil + } + if err := s.files.SetNeedsReview(ctx, authorized, value); err != nil { + return err + } + + objType := fileObjectType + details := map[string]any{"needs_review": value} + for i := range authorized { + _ = s.audit.Log(ctx, "file_review", &objType, &authorized[i], details) + } + return nil +} + // --------------------------------------------------------------------------- // Import // --------------------------------------------------------------------------- diff --git a/backend/migrations/003_data_tables.sql b/backend/migrations/003_data_tables.sql index dd4fc1f..0fc9f1b 100644 --- a/backend/migrations/003_data_tables.sql +++ b/backend/migrations/003_data_tables.sql @@ -55,7 +55,8 @@ CREATE TABLE data.files ( creator_id smallint NOT NULL REFERENCES core.users(id) ON UPDATE CASCADE ON DELETE RESTRICT, is_public boolean NOT NULL DEFAULT false, - is_deleted boolean NOT NULL DEFAULT false -- soft delete (trash) + is_deleted boolean NOT NULL DEFAULT false, -- soft delete (trash) + needs_review boolean NOT NULL DEFAULT true -- tagging not yet marked done; cleared explicitly ); CREATE TABLE data.file_tag ( diff --git a/backend/migrations/006_indexes.sql b/backend/migrations/006_indexes.sql index cd9c81e..8f09758 100644 --- a/backend/migrations/006_indexes.sql +++ b/backend/migrations/006_indexes.sql @@ -20,6 +20,7 @@ CREATE INDEX idx__files__creator_id ON data.files USING hash (creator_id) CREATE INDEX idx__files__content_datetime ON data.files USING btree (content_datetime DESC NULLS LAST); CREATE INDEX idx__files__is_deleted ON data.files USING btree (is_deleted) WHERE is_deleted = true; CREATE INDEX idx__files__phash ON data.files USING btree (phash) WHERE phash IS NOT NULL; +CREATE INDEX idx__files__needs_review ON data.files USING btree (id) WHERE needs_review = true; -- data.file_tag CREATE INDEX idx__file_tag__tag_id ON data.file_tag USING hash (tag_id); @@ -74,6 +75,7 @@ DROP INDEX IF EXISTS data.idx__file_pool__pool_id; DROP INDEX IF EXISTS data.idx__pools__creator_id; DROP INDEX IF EXISTS data.idx__file_tag__file_id; DROP INDEX IF EXISTS data.idx__file_tag__tag_id; +DROP INDEX IF EXISTS data.idx__files__needs_review; DROP INDEX IF EXISTS data.idx__files__phash; DROP INDEX IF EXISTS data.idx__files__is_deleted; DROP INDEX IF EXISTS data.idx__files__content_datetime; diff --git a/backend/migrations/007_seed_data.sql b/backend/migrations/007_seed_data.sql index 8b3373f..2de972d 100644 --- a/backend/migrations/007_seed_data.sql +++ b/backend/migrations/007_seed_data.sql @@ -20,7 +20,7 @@ INSERT INTO activity.action_types (name) VALUES ('user_login'), ('user_logout'), -- Files ('file_create'), ('file_edit'), ('file_delete'), ('file_restore'), - ('file_permanent_delete'), ('file_replace'), + ('file_permanent_delete'), ('file_replace'), ('file_review'), -- Tags ('tag_create'), ('tag_edit'), ('tag_delete'), -- Categories