fix(backend): apply auto-tag rule to existing files on creation
CreateRule accepted apply_to_existing but ignored it, so enabling the checkbox while creating a rule never retroactively tagged files already carrying the when-tag — only activating an existing rule did. Extract the retroactive expansion into TagRuleRepo.ApplyToExisting (reused by SetActive) and call it from CreateRule when the rule is active, inside one transaction so a file is never left half-tagged. Mirrors SetRuleActive semantics, including following only active downstream rules. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -604,10 +604,14 @@ WHERE when_tag_id = $1 AND then_tag_id = $2`
|
|||||||
if !active || !applyToExisting {
|
if !active || !applyToExisting {
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
return r.ApplyToExisting(ctx, whenTagID, thenTagID)
|
||||||
|
}
|
||||||
|
|
||||||
// Retroactively apply the full transitive expansion of thenTagID to all
|
// ApplyToExisting retroactively applies the full transitive expansion of
|
||||||
// files that already carry whenTagID. The recursive CTE walks active rules
|
// thenTagID to all files that already carry whenTagID. The recursive CTE walks
|
||||||
// starting from thenTagID (mirrors the Go expandTagSet BFS).
|
// active rules starting from thenTagID (mirrors the Go expandTagSet BFS), so
|
||||||
|
// inactive downstream rules are not followed. Idempotent via ON CONFLICT.
|
||||||
|
func (r *TagRuleRepo) ApplyToExisting(ctx context.Context, whenTagID, thenTagID uuid.UUID) error {
|
||||||
const retroQuery = `
|
const retroQuery = `
|
||||||
WITH RECURSIVE expansion(tag_id) AS (
|
WITH RECURSIVE expansion(tag_id) AS (
|
||||||
SELECT $2::uuid
|
SELECT $2::uuid
|
||||||
@@ -624,8 +628,9 @@ CROSS JOIN expansion e
|
|||||||
WHERE ft.tag_id = $1
|
WHERE ft.tag_id = $1
|
||||||
ON CONFLICT DO NOTHING`
|
ON CONFLICT DO NOTHING`
|
||||||
|
|
||||||
|
q := connOrTx(ctx, r.pool)
|
||||||
if _, err := q.Exec(ctx, retroQuery, whenTagID, thenTagID); err != nil {
|
if _, err := q.Exec(ctx, retroQuery, whenTagID, thenTagID); err != nil {
|
||||||
return fmt.Errorf("TagRuleRepo.SetActive retroactive apply: %w", err)
|
return fmt.Errorf("TagRuleRepo.ApplyToExisting: %w", err)
|
||||||
}
|
}
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -674,6 +674,94 @@ func TestTagRuleActivateApplyToExisting(t *testing.T) {
|
|||||||
assert.ElementsMatch(t, []string{"animal", "living-thing", "organism"}, tagNames())
|
assert.ElementsMatch(t, []string{"animal", "living-thing", "organism"}, tagNames())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestTagRuleCreateApplyToExisting verifies that *creating* a rule with
|
||||||
|
// apply_to_existing=true retroactively tags existing files — the same contract
|
||||||
|
// as activating one (TestTagRuleActivateApplyToExisting), exercised through the
|
||||||
|
// POST path. Also checks that apply_to_existing=false and an inactive rule both
|
||||||
|
// leave existing files untouched.
|
||||||
|
func TestTagRuleCreateApplyToExisting(t *testing.T) {
|
||||||
|
if testing.Short() {
|
||||||
|
t.Skip("skipping integration test in short mode")
|
||||||
|
}
|
||||||
|
|
||||||
|
h := setupSuite(t)
|
||||||
|
tok := h.login("admin", "admin")
|
||||||
|
|
||||||
|
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 B→C: active, so it fires transitively once B lands on the file.
|
||||||
|
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())
|
||||||
|
|
||||||
|
// A file carrying only tag A — no A→B rule exists yet.
|
||||||
|
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
|
||||||
|
}
|
||||||
|
assert.ElementsMatch(t, []string{"animal"}, tagNames())
|
||||||
|
|
||||||
|
// Creating an INACTIVE rule, even with apply_to_existing=true, must not tag.
|
||||||
|
resp = h.doJSON("POST", "/tags/"+tagA+"/rules", map[string]any{
|
||||||
|
"then_tag_id": tagB,
|
||||||
|
"is_active": false,
|
||||||
|
"apply_to_existing": true,
|
||||||
|
}, tok)
|
||||||
|
require.Equal(t, http.StatusCreated, resp.StatusCode, resp.String())
|
||||||
|
assert.ElementsMatch(t, []string{"animal"}, tagNames(), "inactive rule must not tag existing files")
|
||||||
|
|
||||||
|
// Drop it so we can recreate as active.
|
||||||
|
resp = h.doJSON("DELETE", "/tags/"+tagA+"/rules/"+tagB, nil, tok)
|
||||||
|
require.Equal(t, http.StatusNoContent, resp.StatusCode, resp.String())
|
||||||
|
|
||||||
|
// Creating an ACTIVE rule with apply_to_existing=false leaves the file alone.
|
||||||
|
resp = h.doJSON("POST", "/tags/"+tagA+"/rules", map[string]any{
|
||||||
|
"then_tag_id": tagB,
|
||||||
|
"is_active": true,
|
||||||
|
"apply_to_existing": false,
|
||||||
|
}, tok)
|
||||||
|
require.Equal(t, http.StatusCreated, resp.StatusCode, resp.String())
|
||||||
|
assert.ElementsMatch(t, []string{"animal"}, tagNames(), "apply_to_existing=false must not touch existing files")
|
||||||
|
|
||||||
|
resp = h.doJSON("DELETE", "/tags/"+tagA+"/rules/"+tagB, nil, tok)
|
||||||
|
require.Equal(t, http.StatusNoContent, resp.StatusCode, resp.String())
|
||||||
|
|
||||||
|
// Creating A→B active WITH apply_to_existing=true: the file gets B directly
|
||||||
|
// and C transitively via the already-active B→C rule.
|
||||||
|
resp = h.doJSON("POST", "/tags/"+tagA+"/rules", map[string]any{
|
||||||
|
"then_tag_id": tagB,
|
||||||
|
"is_active": true,
|
||||||
|
"apply_to_existing": true,
|
||||||
|
}, tok)
|
||||||
|
require.Equal(t, http.StatusCreated, resp.StatusCode, resp.String())
|
||||||
|
assert.ElementsMatch(t, []string{"animal", "living-thing", "organism"}, tagNames())
|
||||||
|
}
|
||||||
|
|
||||||
// TestTagAutoRule verifies that adding a tag automatically applies then_tags.
|
// TestTagAutoRule verifies that adding a tag automatically applies then_tags.
|
||||||
func TestTagAutoRule(t *testing.T) {
|
func TestTagAutoRule(t *testing.T) {
|
||||||
if testing.Short() {
|
if testing.Short() {
|
||||||
|
|||||||
@@ -101,6 +101,10 @@ type TagRuleRepo interface {
|
|||||||
// are both true, the full transitive expansion of thenTagID is retroactively
|
// are both true, the full transitive expansion of thenTagID is retroactively
|
||||||
// applied to all files that already carry whenTagID.
|
// applied to all files that already carry whenTagID.
|
||||||
SetActive(ctx context.Context, whenTagID, thenTagID uuid.UUID, active, applyToExisting bool) error
|
SetActive(ctx context.Context, whenTagID, thenTagID uuid.UUID, active, applyToExisting bool) error
|
||||||
|
// ApplyToExisting retroactively applies the full transitive expansion of
|
||||||
|
// thenTagID (following active rules) to every file that already carries
|
||||||
|
// whenTagID. Used when a rule is created or activated with apply_to_existing.
|
||||||
|
ApplyToExisting(ctx context.Context, whenTagID, thenTagID uuid.UUID) error
|
||||||
Delete(ctx context.Context, whenTagID, thenTagID uuid.UUID) error
|
Delete(ctx context.Context, whenTagID, thenTagID uuid.UUID) error
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -192,16 +192,32 @@ func (s *TagService) ListRules(ctx context.Context, tagID uuid.UUID) ([]domain.T
|
|||||||
return s.rules.ListByTag(ctx, tagID)
|
return s.rules.ListByTag(ctx, tagID)
|
||||||
}
|
}
|
||||||
|
|
||||||
// CreateRule adds a tag rule. If applyToExisting is true, the then_tag is
|
// CreateRule adds a tag rule. When the rule is active and applyToExisting is
|
||||||
// retroactively applied to all files that already carry the when_tag.
|
// true, the full transitive expansion of thenTagID is retroactively applied to
|
||||||
// Retroactive application requires a FileRepo; it is deferred until wired
|
// every file already carrying whenTagID — same semantics as activating an
|
||||||
// in a future iteration (see port.FileRepo.ListByTag).
|
// existing rule via SetRuleActive. The insert and retroactive apply run in one
|
||||||
func (s *TagService) CreateRule(ctx context.Context, whenTagID, thenTagID uuid.UUID, isActive, _ bool) (*domain.TagRule, error) {
|
// transaction so a file is never left half-tagged.
|
||||||
return s.rules.Create(ctx, domain.TagRule{
|
func (s *TagService) CreateRule(ctx context.Context, whenTagID, thenTagID uuid.UUID, isActive, applyToExisting bool) (*domain.TagRule, error) {
|
||||||
WhenTagID: whenTagID,
|
var created *domain.TagRule
|
||||||
ThenTagID: thenTagID,
|
err := s.tx.WithTx(ctx, func(ctx context.Context) error {
|
||||||
IsActive: isActive,
|
rule, err := s.rules.Create(ctx, domain.TagRule{
|
||||||
|
WhenTagID: whenTagID,
|
||||||
|
ThenTagID: thenTagID,
|
||||||
|
IsActive: isActive,
|
||||||
|
})
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
created = rule
|
||||||
|
if isActive && applyToExisting {
|
||||||
|
return s.rules.ApplyToExisting(ctx, whenTagID, thenTagID)
|
||||||
|
}
|
||||||
|
return nil
|
||||||
})
|
})
|
||||||
|
if err != nil {
|
||||||
|
return nil, err
|
||||||
|
}
|
||||||
|
return created, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// SetRuleActive toggles a rule's is_active flag and returns the updated rule.
|
// SetRuleActive toggles a rule's is_active flag and returns the updated rule.
|
||||||
|
|||||||
Reference in New Issue
Block a user