diff --git a/backend/internal/db/postgres/tag_repo.go b/backend/internal/db/postgres/tag_repo.go index 458263d..42dd990 100644 --- a/backend/internal/db/postgres/tag_repo.go +++ b/backend/internal/db/postgres/tag_repo.go @@ -574,19 +574,46 @@ JOIN data.tags t ON t.id = ins.then_tag_id` return &result, nil } -func (r *TagRuleRepo) SetActive(ctx context.Context, whenTagID, thenTagID uuid.UUID, active bool) error { - const query = ` +func (r *TagRuleRepo) SetActive(ctx context.Context, whenTagID, thenTagID uuid.UUID, active, applyToExisting bool) error { + const updateQuery = ` UPDATE data.tag_rules SET is_active = $3 WHERE when_tag_id = $1 AND then_tag_id = $2` q := connOrTx(ctx, r.pool) - ct, err := q.Exec(ctx, query, whenTagID, thenTagID, active) + ct, err := q.Exec(ctx, updateQuery, whenTagID, thenTagID, active) if err != nil { return fmt.Errorf("TagRuleRepo.SetActive: %w", err) } if ct.RowsAffected() == 0 { return domain.ErrNotFound } + + if !active || !applyToExisting { + return nil + } + + // Retroactively apply the full transitive expansion of thenTagID to all + // files that already carry whenTagID. The recursive CTE walks active rules + // starting from thenTagID (mirrors the Go expandTagSet BFS). + const retroQuery = ` +WITH RECURSIVE expansion(tag_id) AS ( + SELECT $2::uuid + UNION + SELECT r.then_tag_id + FROM data.tag_rules r + JOIN expansion e ON r.when_tag_id = e.tag_id + WHERE r.is_active = true +) +INSERT INTO data.file_tag (file_id, tag_id) +SELECT ft.file_id, e.tag_id +FROM data.file_tag ft +CROSS JOIN expansion e +WHERE ft.tag_id = $1 +ON CONFLICT DO NOTHING` + + if _, err := q.Exec(ctx, retroQuery, whenTagID, thenTagID); err != nil { + return fmt.Errorf("TagRuleRepo.SetActive retroactive apply: %w", err) + } return nil } diff --git a/backend/internal/handler/tag_handler.go b/backend/internal/handler/tag_handler.go index 59bc1f7..c79f922 100644 --- a/backend/internal/handler/tag_handler.go +++ b/backend/internal/handler/tag_handler.go @@ -371,14 +371,20 @@ func (h *TagHandler) PatchRule(c *gin.Context) { } var body struct { - IsActive *bool `json:"is_active"` + IsActive *bool `json:"is_active"` + ApplyToExisting *bool `json:"apply_to_existing"` } if err := c.ShouldBindJSON(&body); err != nil || body.IsActive == nil { respondError(c, domain.ErrValidation) return } - rule, err := h.tagSvc.SetRuleActive(c.Request.Context(), whenTagID, thenTagID, *body.IsActive) + applyToExisting := false + if body.ApplyToExisting != nil { + applyToExisting = *body.ApplyToExisting + } + + rule, err := h.tagSvc.SetRuleActive(c.Request.Context(), whenTagID, thenTagID, *body.IsActive, applyToExisting) if err != nil { respondError(c, err) return diff --git a/backend/internal/integration/server_test.go b/backend/internal/integration/server_test.go index 35bf036..ebba3ba 100644 --- a/backend/internal/integration/server_test.go +++ b/backend/internal/integration/server_test.go @@ -552,6 +552,90 @@ func TestPoolReorder(t *testing.T) { assert.Equal(t, id1, items2[1].(map[string]any)["id"]) } +// TestTagRuleActivateApplyToExisting verifies that activating a rule with +// apply_to_existing=true retroactively tags existing files, including +// transitive rules (A→B active+apply, B→C already active → file gets A,B,C). +func TestTagRuleActivateApplyToExisting(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test in short mode") + } + + h := setupSuite(t) + tok := h.login("admin", "admin") + + // Create three tags: A, B, C. + mkTag := func(name string) string { + resp := h.doJSON("POST", "/tags", map[string]any{"name": name}, tok) + require.Equal(t, http.StatusCreated, resp.StatusCode, resp.String()) + var obj map[string]any + resp.decode(t, &obj) + return obj["id"].(string) + } + tagA := mkTag("animal") + tagB := mkTag("living-thing") + tagC := mkTag("organism") + + // Rule A→B: created inactive so it does NOT fire on assign. + resp := h.doJSON("POST", "/tags/"+tagA+"/rules", map[string]any{ + "then_tag_id": tagB, + "is_active": false, + }, tok) + require.Equal(t, http.StatusCreated, resp.StatusCode, resp.String()) + + // Rule B→C: active, so it fires transitively when B is applied. + resp = h.doJSON("POST", "/tags/"+tagB+"/rules", map[string]any{ + "then_tag_id": tagC, + "is_active": true, + }, tok) + require.Equal(t, http.StatusCreated, resp.StatusCode, resp.String()) + + // Upload a file and assign only tag A. A→B is inactive so only A is set. + file := h.uploadJPEG(tok, "cat.jpg") + fileID := file["id"].(string) + resp = h.doJSON("PUT", "/files/"+fileID+"/tags", map[string]any{ + "tag_ids": []string{tagA}, + }, tok) + require.Equal(t, http.StatusOK, resp.StatusCode, resp.String()) + + tagNames := func() []string { + r := h.doJSON("GET", "/files/"+fileID+"/tags", nil, tok) + require.Equal(t, http.StatusOK, r.StatusCode) + var items []any + r.decode(t, &items) + names := make([]string, 0, len(items)) + for _, it := range items { + names = append(names, it.(map[string]any)["name"].(string)) + } + return names + } + + // Before activation: file should only have tag A. + assert.ElementsMatch(t, []string{"animal"}, tagNames()) + + // Activate A→B WITHOUT apply_to_existing — existing file must not change. + resp = h.doJSON("PATCH", "/tags/"+tagA+"/rules/"+tagB, map[string]any{ + "is_active": true, + "apply_to_existing": false, + }, tok) + require.Equal(t, http.StatusOK, resp.StatusCode, resp.String()) + assert.ElementsMatch(t, []string{"animal"}, tagNames(), "file should be unchanged when apply_to_existing=false") + + // Deactivate again so we can test the positive case cleanly. + resp = h.doJSON("PATCH", "/tags/"+tagA+"/rules/"+tagB, map[string]any{ + "is_active": false, + }, tok) + require.Equal(t, http.StatusOK, resp.StatusCode, resp.String()) + + // Activate A→B WITH apply_to_existing=true. + // Expectation: file gets B directly, and C transitively via the active B→C rule. + resp = h.doJSON("PATCH", "/tags/"+tagA+"/rules/"+tagB, map[string]any{ + "is_active": true, + "apply_to_existing": true, + }, tok) + require.Equal(t, http.StatusOK, resp.StatusCode, resp.String()) + assert.ElementsMatch(t, []string{"animal", "living-thing", "organism"}, tagNames()) +} + // TestTagAutoRule verifies that adding a tag automatically applies then_tags. func TestTagAutoRule(t *testing.T) { if testing.Short() { diff --git a/backend/internal/port/repository.go b/backend/internal/port/repository.go index 4925bd0..e1ca9ef 100644 --- a/backend/internal/port/repository.go +++ b/backend/internal/port/repository.go @@ -83,8 +83,10 @@ type TagRuleRepo interface { // ListByTag returns all rules where WhenTagID == tagID. ListByTag(ctx context.Context, tagID uuid.UUID) ([]domain.TagRule, error) Create(ctx context.Context, r domain.TagRule) (*domain.TagRule, error) - // SetActive toggles a rule's is_active flag. - SetActive(ctx context.Context, whenTagID, thenTagID uuid.UUID, active bool) error + // SetActive toggles a rule's is_active flag. When active and applyToExisting + // are both true, the full transitive expansion of thenTagID is retroactively + // applied to all files that already carry whenTagID. + SetActive(ctx context.Context, whenTagID, thenTagID uuid.UUID, active, applyToExisting bool) error Delete(ctx context.Context, whenTagID, thenTagID uuid.UUID) error } diff --git a/backend/internal/service/tag_service.go b/backend/internal/service/tag_service.go index c0959e4..9de7e9a 100644 --- a/backend/internal/service/tag_service.go +++ b/backend/internal/service/tag_service.go @@ -192,8 +192,10 @@ func (s *TagService) CreateRule(ctx context.Context, whenTagID, thenTagID uuid.U } // SetRuleActive toggles a rule's is_active flag and returns the updated rule. -func (s *TagService) SetRuleActive(ctx context.Context, whenTagID, thenTagID uuid.UUID, active bool) (*domain.TagRule, error) { - if err := s.rules.SetActive(ctx, whenTagID, thenTagID, active); err != nil { +// When active and applyToExisting are both true, the full transitive expansion +// of thenTagID is retroactively applied to files already carrying whenTagID. +func (s *TagService) SetRuleActive(ctx context.Context, whenTagID, thenTagID uuid.UUID, active, applyToExisting bool) (*domain.TagRule, error) { + if err := s.rules.SetActive(ctx, whenTagID, thenTagID, active, applyToExisting); err != nil { return nil, err } rules, err := s.rules.ListByTag(ctx, whenTagID) diff --git a/openapi.yaml b/openapi.yaml index 4d7cc3e..9fce40b 100644 --- a/openapi.yaml +++ b/openapi.yaml @@ -846,6 +846,10 @@ paths: properties: is_active: type: boolean + apply_to_existing: + type: boolean + default: false + description: When activating, apply rule retroactively to files already tagged responses: '200': description: Rule updated