feat(backend): per-file review status with DSL filter and bulk endpoint
Replaces the old "untagged" sentinel tag with a proper per-file workflow status: needs_review starts true on upload/import and is cleared by an explicit action (no auto-clear on tagging). Surfaced as a filter token (r=1 needs review, r=0 done) so it combines with tag/MIME conditions, and toggled via POST /files/bulk/review (single id or many, edit-ACL enforced, audit-logged as file_review). needs_review lives on data.files (column added to the original 003 migration, partial index in 006, action type seeded in 007). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -23,6 +23,7 @@ const (
|
||||
ftkTag // t=<uuid>
|
||||
ftkMimeExact // m=<int>
|
||||
ftkMimeLike // m~<pattern>
|
||||
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:
|
||||
|
||||
@@ -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")
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -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
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
@@ -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)
|
||||
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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).
|
||||
|
||||
@@ -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
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user