diff --git a/pkg/api/resolver_mutation_tag.go b/pkg/api/resolver_mutation_tag.go index f65cf1fe5..23fefaa88 100644 --- a/pkg/api/resolver_mutation_tag.go +++ b/pkg/api/resolver_mutation_tag.go @@ -6,6 +6,7 @@ import ( "strconv" "time" + "github.com/stashapp/stash/pkg/logger" "github.com/stashapp/stash/pkg/models" "github.com/stashapp/stash/pkg/plugin" "github.com/stashapp/stash/pkg/tag" @@ -43,7 +44,24 @@ func (r *mutationResolver) TagCreate(ctx context.Context, input models.TagCreate } } - // Start the transaction and save the t + var parentIDs []int + var childIDs []int + + if len(input.ParentIds) > 0 { + parentIDs, err = utils.StringSliceToIntSlice(input.ParentIds) + if err != nil { + return nil, err + } + } + + if len(input.ChildIds) > 0 { + childIDs, err = utils.StringSliceToIntSlice(input.ChildIds) + if err != nil { + return nil, err + } + } + + // Start the transaction and save the tag var t *models.Tag if err := r.withTxn(ctx, func(repo models.Repository) error { qb := repo.Tag() @@ -75,24 +93,22 @@ func (r *mutationResolver) TagCreate(ctx context.Context, input models.TagCreate } } - if input.ParentIds != nil && len(input.ParentIds) > 0 { - ids, err := utils.StringSliceToIntSlice(input.ParentIds) - if err != nil { - return err - } - - if err := qb.UpdateParentTags(t.ID, ids); err != nil { + if len(parentIDs) > 0 { + if err := qb.UpdateParentTags(t.ID, parentIDs); err != nil { return err } } - if input.ChildIds != nil && len(input.ChildIds) > 0 { - ids, err := utils.StringSliceToIntSlice(input.ChildIds) - if err != nil { + if len(childIDs) > 0 { + if err := qb.UpdateChildTags(t.ID, childIDs); err != nil { return err } + } - if err := qb.UpdateChildTags(t.ID, ids); err != nil { + // FIXME: This should be called before any changes are made, but + // requires a rewrite of ValidateHierarchy. + if len(parentIDs) > 0 || len(childIDs) > 0 { + if err := tag.ValidateHierarchy(t, parentIDs, childIDs, qb); err != nil { return err } } @@ -128,6 +144,23 @@ func (r *mutationResolver) TagUpdate(ctx context.Context, input models.TagUpdate } } + var parentIDs []int + var childIDs []int + + if translator.hasField("parent_ids") { + parentIDs, err = utils.StringSliceToIntSlice(input.ParentIds) + if err != nil { + return nil, err + } + } + + if translator.hasField("child_ids") { + childIDs, err = utils.StringSliceToIntSlice(input.ChildIds) + if err != nil { + return nil, err + } + } + // Start the transaction and save the tag var t *models.Tag if err := r.withTxn(ctx, func(repo models.Repository) error { @@ -183,29 +216,6 @@ func (r *mutationResolver) TagUpdate(ctx context.Context, input models.TagUpdate } } - var parentIDs []int - var childIDs []int - - if translator.hasField("parent_ids") { - parentIDs, err = utils.StringSliceToIntSlice(input.ParentIds) - if err != nil { - return err - } - } - - if translator.hasField("child_ids") { - childIDs, err = utils.StringSliceToIntSlice(input.ChildIds) - if err != nil { - return err - } - } - - if parentIDs != nil || childIDs != nil { - if err := tag.EnsureUniqueHierarchy(tagID, parentIDs, childIDs, qb); err != nil { - return err - } - } - if parentIDs != nil { if err := qb.UpdateParentTags(tagID, parentIDs); err != nil { return err @@ -218,6 +228,15 @@ func (r *mutationResolver) TagUpdate(ctx context.Context, input models.TagUpdate } } + // FIXME: This should be called before any changes are made, but + // requires a rewrite of ValidateHierarchy. + if parentIDs != nil || childIDs != nil { + if err := tag.ValidateHierarchy(t, parentIDs, childIDs, qb); err != nil { + logger.Errorf("Error saving tag: %s", err) + return err + } + } + return nil }); err != nil { return nil, err @@ -317,6 +336,12 @@ func (r *mutationResolver) TagsMerge(ctx context.Context, input models.TagsMerge return err } + err = tag.ValidateHierarchy(t, parents, children, qb) + if err != nil { + logger.Errorf("Error merging tag: %s", err) + return err + } + return nil }); err != nil { return nil, err diff --git a/pkg/models/mocks/TagReaderWriter.go b/pkg/models/mocks/TagReaderWriter.go index f6b257ed2..1d5e14e34 100644 --- a/pkg/models/mocks/TagReaderWriter.go +++ b/pkg/models/mocks/TagReaderWriter.go @@ -131,15 +131,15 @@ func (_m *TagReaderWriter) Find(id int) (*models.Tag, error) { } // FindAllAncestors provides a mock function with given fields: tagID, excludeIDs -func (_m *TagReaderWriter) FindAllAncestors(tagID int, excludeIDs []int) ([]*models.Tag, error) { +func (_m *TagReaderWriter) FindAllAncestors(tagID int, excludeIDs []int) ([]*models.TagPath, error) { ret := _m.Called(tagID, excludeIDs) - var r0 []*models.Tag - if rf, ok := ret.Get(0).(func(int, []int) []*models.Tag); ok { + var r0 []*models.TagPath + if rf, ok := ret.Get(0).(func(int, []int) []*models.TagPath); ok { r0 = rf(tagID, excludeIDs) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).([]*models.Tag) + r0 = ret.Get(0).([]*models.TagPath) } } @@ -154,15 +154,15 @@ func (_m *TagReaderWriter) FindAllAncestors(tagID int, excludeIDs []int) ([]*mod } // FindAllDescendants provides a mock function with given fields: tagID, excludeIDs -func (_m *TagReaderWriter) FindAllDescendants(tagID int, excludeIDs []int) ([]*models.Tag, error) { +func (_m *TagReaderWriter) FindAllDescendants(tagID int, excludeIDs []int) ([]*models.TagPath, error) { ret := _m.Called(tagID, excludeIDs) - var r0 []*models.Tag - if rf, ok := ret.Get(0).(func(int, []int) []*models.Tag); ok { + var r0 []*models.TagPath + if rf, ok := ret.Get(0).(func(int, []int) []*models.TagPath); ok { r0 = rf(tagID, excludeIDs) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).([]*models.Tag) + r0 = ret.Get(0).([]*models.TagPath) } } diff --git a/pkg/models/model_tag.go b/pkg/models/model_tag.go index c56a49120..90fc67bd9 100644 --- a/pkg/models/model_tag.go +++ b/pkg/models/model_tag.go @@ -16,6 +16,11 @@ type TagPartial struct { UpdatedAt *SQLiteTimestamp `db:"updated_at" json:"updated_at"` } +type TagPath struct { + Tag + Path string `db:"path" json:"path"` +} + func NewTag(name string) *Tag { currentTime := time.Now() return &Tag{ @@ -35,6 +40,16 @@ func (t *Tags) New() interface{} { return &Tag{} } +type TagPaths []*TagPath + +func (t *TagPaths) Append(o interface{}) { + *t = append(*t, o.(*TagPath)) +} + +func (t *TagPaths) New() interface{} { + return &TagPath{} +} + // Original Tag image from: https://fontawesome.com/icons/tag?style=solid // Modified to change color and rotate // Licensed under CC Attribution 4.0: https://fontawesome.com/license diff --git a/pkg/models/tag.go b/pkg/models/tag.go index bcb7096f0..747e7a08e 100644 --- a/pkg/models/tag.go +++ b/pkg/models/tag.go @@ -20,8 +20,8 @@ type TagReader interface { Query(tagFilter *TagFilterType, findFilter *FindFilterType) ([]*Tag, int, error) GetImage(tagID int) ([]byte, error) GetAliases(tagID int) ([]string, error) - FindAllAncestors(tagID int, excludeIDs []int) ([]*Tag, error) - FindAllDescendants(tagID int, excludeIDs []int) ([]*Tag, error) + FindAllAncestors(tagID int, excludeIDs []int) ([]*TagPath, error) + FindAllDescendants(tagID int, excludeIDs []int) ([]*TagPath, error) } type TagWriter interface { diff --git a/pkg/sqlite/tag.go b/pkg/sqlite/tag.go index cb58e9b0a..87474ebcc 100644 --- a/pkg/sqlite/tag.go +++ b/pkg/sqlite/tag.go @@ -732,26 +732,21 @@ func (qb *tagQueryBuilder) UpdateChildTags(tagID int, childIDs []int) error { return nil } -func (qb *tagQueryBuilder) FindAllAncestors(tagID int, excludeIDs []int) ([]*models.Tag, error) { +// FindAllAncestors returns a slice of TagPath objects, representing all +// ancestors of the tag with the provided id. +func (qb *tagQueryBuilder) FindAllAncestors(tagID int, excludeIDs []int) ([]*models.TagPath, error) { inBinding := getInBinding(len(excludeIDs) + 1) query := `WITH RECURSIVE parents AS ( - SELECT t.id AS parent_id, t.id AS child_id FROM tags t WHERE t.id = ? + SELECT t.id AS parent_id, t.id AS child_id, t.name as path FROM tags t WHERE t.id = ? UNION - SELECT tr.parent_id, tr.child_id FROM tags_relations tr INNER JOIN parents p ON p.parent_id = tr.child_id WHERE tr.parent_id NOT IN` + inBinding + ` -), -children AS ( - SELECT tr.parent_id, tr.child_id FROM tags_relations tr INNER JOIN parents p ON p.parent_id = tr.parent_id WHERE tr.child_id NOT IN` + inBinding + ` - UNION - SELECT tr.parent_id, tr.child_id FROM tags_relations tr INNER JOIN children c ON c.child_id = tr.parent_id WHERE tr.child_id NOT IN` + inBinding + ` + SELECT tr.parent_id, tr.child_id, t.name || '->' || p.path as path FROM tags_relations tr INNER JOIN parents p ON p.parent_id = tr.child_id JOIN tags t ON t.id = tr.parent_id WHERE tr.parent_id NOT IN` + inBinding + ` ) -SELECT t.* FROM tags t INNER JOIN parents p ON t.id = p.parent_id -UNION -SELECT t.* FROM tags t INNER JOIN children c ON t.id = c.child_id +SELECT t.*, p.path FROM tags t INNER JOIN parents p ON t.id = p.parent_id ` - var ret models.Tags + var ret models.TagPaths excludeArgs := []interface{}{tagID} for _, excludeID := range excludeIDs { excludeArgs = append(excludeArgs, excludeID) @@ -765,26 +760,21 @@ SELECT t.* FROM tags t INNER JOIN children c ON t.id = c.child_id return ret, nil } -func (qb *tagQueryBuilder) FindAllDescendants(tagID int, excludeIDs []int) ([]*models.Tag, error) { +// FindAllDescendants returns a slice of TagPath objects, representing all +// descendants of the tag with the provided id. +func (qb *tagQueryBuilder) FindAllDescendants(tagID int, excludeIDs []int) ([]*models.TagPath, error) { inBinding := getInBinding(len(excludeIDs) + 1) query := `WITH RECURSIVE children AS ( - SELECT t.id AS parent_id, t.id AS child_id FROM tags t WHERE t.id = ? + SELECT t.id AS parent_id, t.id AS child_id, t.name as path FROM tags t WHERE t.id = ? UNION - SELECT tr.parent_id, tr.child_id FROM tags_relations tr INNER JOIN children c ON c.child_id = tr.parent_id WHERE tr.child_id NOT IN` + inBinding + ` -), -parents AS ( - SELECT tr.parent_id, tr.child_id FROM tags_relations tr INNER JOIN children c ON c.child_id = tr.child_id WHERE tr.parent_id NOT IN` + inBinding + ` - UNION - SELECT tr.parent_id, tr.child_id FROM tags_relations tr INNER JOIN parents p ON p.parent_id = tr.child_id WHERE tr.parent_id NOT IN` + inBinding + ` + SELECT tr.parent_id, tr.child_id, c.path || '->' || t.name as path FROM tags_relations tr INNER JOIN children c ON c.child_id = tr.parent_id JOIN tags t ON t.id = tr.child_id WHERE tr.child_id NOT IN` + inBinding + ` ) -SELECT t.* FROM tags t INNER JOIN children c ON t.id = c.child_id -UNION -SELECT t.* FROM tags t INNER JOIN parents p ON t.id = p.parent_id +SELECT t.*, c.path FROM tags t INNER JOIN children c ON t.id = c.child_id ` - var ret models.Tags + var ret models.TagPaths excludeArgs := []interface{}{tagID} for _, excludeID := range excludeIDs { excludeArgs = append(excludeArgs, excludeID) diff --git a/pkg/tag/update.go b/pkg/tag/update.go index 420e0846d..dfee55154 100644 --- a/pkg/tag/update.go +++ b/pkg/tag/update.go @@ -24,17 +24,15 @@ func (e *NameUsedByAliasError) Error() string { } type InvalidTagHierarchyError struct { - Direction string - InvalidTag string - ApplyingTag string + Direction string + CurrentRelation string + InvalidTag string + ApplyingTag string + TagPath string } func (e *InvalidTagHierarchyError) Error() string { - if e.InvalidTag == e.ApplyingTag { - return fmt.Sprintf("Cannot apply tag \"%s\" as it already is a %s", e.InvalidTag, e.Direction) - } else { - return fmt.Sprintf("Cannot apply tag \"%s\" as it is linked to \"%s\" which already is a %s", e.ApplyingTag, e.InvalidTag, e.Direction) - } + return fmt.Sprintf("cannot apply tag \"%s\" as a %s of \"%s\" as it is already %s (%s)", e.InvalidTag, e.Direction, e.ApplyingTag, e.CurrentRelation, e.TagPath) } // EnsureTagNameUnique returns an error if the tag name provided @@ -78,45 +76,55 @@ func EnsureAliasesUnique(id int, aliases []string, qb models.TagReader) error { return nil } -func EnsureUniqueHierarchy(id int, parentIDs, childIDs []int, qb models.TagReader) error { - allAncestors := make(map[int]*models.Tag) - allDescendants := make(map[int]*models.Tag) - excludeIDs := []int{id} +func ValidateHierarchy(tag *models.Tag, parentIDs, childIDs []int, qb models.TagReader) error { + id := tag.ID + allAncestors := make(map[int]*models.TagPath) + allDescendants := make(map[int]*models.TagPath) - validateParent := func(testID, applyingID int) error { - if parentTag, exists := allAncestors[testID]; exists { - applyingTag, err := qb.Find(applyingID) + parentsAncestors, err := qb.FindAllAncestors(id, nil) + if err != nil { + return err + } - if err != nil { - return nil - } + for _, ancestorTag := range parentsAncestors { + allAncestors[ancestorTag.ID] = ancestorTag + } + childsDescendants, err := qb.FindAllDescendants(id, nil) + if err != nil { + return err + } + + for _, descendentTag := range childsDescendants { + allDescendants[descendentTag.ID] = descendentTag + } + + validateParent := func(testID int) error { + if parentTag, exists := allDescendants[testID]; exists { return &InvalidTagHierarchyError{ - Direction: "parent", - InvalidTag: parentTag.Name, - ApplyingTag: applyingTag.Name, + Direction: "parent", + CurrentRelation: "a descendant", + InvalidTag: parentTag.Name, + ApplyingTag: tag.Name, + TagPath: parentTag.Path, } } return nil } - validateChild := func(testID, applyingID int) error { - if childTag, exists := allDescendants[testID]; exists { - applyingTag, err := qb.Find(applyingID) - - if err != nil { - return nil - } - + validateChild := func(testID int) error { + if childTag, exists := allAncestors[testID]; exists { return &InvalidTagHierarchyError{ - Direction: "child", - InvalidTag: childTag.Name, - ApplyingTag: applyingTag.Name, + Direction: "child", + CurrentRelation: "an ancestor", + InvalidTag: childTag.Name, + ApplyingTag: tag.Name, + TagPath: childTag.Path, } } - return validateParent(testID, applyingID) + return nil } if parentIDs == nil { @@ -142,33 +150,15 @@ func EnsureUniqueHierarchy(id int, parentIDs, childIDs []int, qb models.TagReade } for _, parentID := range parentIDs { - parentsAncestors, err := qb.FindAllAncestors(parentID, excludeIDs) - if err != nil { + if err := validateParent(parentID); err != nil { return err } - - for _, ancestorTag := range parentsAncestors { - if err := validateParent(ancestorTag.ID, parentID); err != nil { - return err - } - - allAncestors[ancestorTag.ID] = ancestorTag - } } for _, childID := range childIDs { - childsDescendants, err := qb.FindAllDescendants(childID, excludeIDs) - if err != nil { + if err := validateChild(childID); err != nil { return err } - - for _, descendentTag := range childsDescendants { - if err := validateChild(descendentTag.ID, childID); err != nil { - return err - } - - allDescendants[descendentTag.ID] = descendentTag - } } return nil @@ -217,10 +207,5 @@ func MergeHierarchy(destination int, sources []int, qb models.TagReader) ([]int, mergedChildren = addTo(mergedChildren, children) } - err := EnsureUniqueHierarchy(destination, mergedParents, mergedChildren, qb) - if err != nil { - return nil, nil, err - } - return mergedParents, mergedChildren, nil } diff --git a/pkg/tag/update_test.go b/pkg/tag/update_test.go index d3a7f226d..f7338da23 100644 --- a/pkg/tag/update_test.go +++ b/pkg/tag/update_test.go @@ -29,39 +29,50 @@ var testUniqueHierarchyTags = map[int]*models.Tag{ }, } +var testUniqueHierarchyTagPaths = map[int]*models.TagPath{ + 1: { + Tag: *testUniqueHierarchyTags[1], + }, + 2: { + Tag: *testUniqueHierarchyTags[2], + }, + 3: { + Tag: *testUniqueHierarchyTags[3], + }, + 4: { + Tag: *testUniqueHierarchyTags[4], + }, +} + type testUniqueHierarchyCase struct { id int parents []*models.Tag children []*models.Tag - onFindAllAncestors map[int][]*models.Tag - onFindAllDescendants map[int][]*models.Tag + onFindAllAncestors []*models.TagPath + onFindAllDescendants []*models.TagPath expectedError string } var testUniqueHierarchyCases = []testUniqueHierarchyCase{ { - id: 1, - parents: []*models.Tag{}, - children: []*models.Tag{}, - onFindAllAncestors: map[int][]*models.Tag{ - 1: {}, - }, - onFindAllDescendants: map[int][]*models.Tag{ - 1: {}, - }, - expectedError: "", + id: 1, + parents: []*models.Tag{}, + children: []*models.Tag{}, + onFindAllAncestors: []*models.TagPath{}, + onFindAllDescendants: []*models.TagPath{}, + expectedError: "", }, { id: 1, parents: []*models.Tag{testUniqueHierarchyTags[2]}, children: []*models.Tag{testUniqueHierarchyTags[3]}, - onFindAllAncestors: map[int][]*models.Tag{ - 2: {testUniqueHierarchyTags[2]}, + onFindAllAncestors: []*models.TagPath{ + testUniqueHierarchyTagPaths[2], }, - onFindAllDescendants: map[int][]*models.Tag{ - 3: {testUniqueHierarchyTags[3]}, + onFindAllDescendants: []*models.TagPath{ + testUniqueHierarchyTagPaths[3], }, expectedError: "", }, @@ -69,11 +80,11 @@ var testUniqueHierarchyCases = []testUniqueHierarchyCase{ id: 2, parents: []*models.Tag{testUniqueHierarchyTags[3]}, children: make([]*models.Tag, 0), - onFindAllAncestors: map[int][]*models.Tag{ - 3: {testUniqueHierarchyTags[3]}, + onFindAllAncestors: []*models.TagPath{ + testUniqueHierarchyTagPaths[3], }, - onFindAllDescendants: map[int][]*models.Tag{ - 2: {testUniqueHierarchyTags[2]}, + onFindAllDescendants: []*models.TagPath{ + testUniqueHierarchyTagPaths[2], }, expectedError: "", }, @@ -84,24 +95,23 @@ var testUniqueHierarchyCases = []testUniqueHierarchyCase{ testUniqueHierarchyTags[4], }, children: []*models.Tag{}, - onFindAllAncestors: map[int][]*models.Tag{ - 3: {testUniqueHierarchyTags[3], testUniqueHierarchyTags[4]}, - 4: {testUniqueHierarchyTags[4]}, + onFindAllAncestors: []*models.TagPath{ + testUniqueHierarchyTagPaths[3], testUniqueHierarchyTagPaths[4], }, - onFindAllDescendants: map[int][]*models.Tag{ - 2: {testUniqueHierarchyTags[2]}, + onFindAllDescendants: []*models.TagPath{ + testUniqueHierarchyTagPaths[2], }, - expectedError: "Cannot apply tag \"four\" as it already is a parent", + expectedError: "", }, { id: 2, parents: []*models.Tag{}, children: []*models.Tag{testUniqueHierarchyTags[3]}, - onFindAllAncestors: map[int][]*models.Tag{ - 2: {testUniqueHierarchyTags[2]}, + onFindAllAncestors: []*models.TagPath{ + testUniqueHierarchyTagPaths[2], }, - onFindAllDescendants: map[int][]*models.Tag{ - 3: {testUniqueHierarchyTags[3]}, + onFindAllDescendants: []*models.TagPath{ + testUniqueHierarchyTagPaths[3], }, expectedError: "", }, @@ -112,50 +122,49 @@ var testUniqueHierarchyCases = []testUniqueHierarchyCase{ testUniqueHierarchyTags[3], testUniqueHierarchyTags[4], }, - onFindAllAncestors: map[int][]*models.Tag{ - 2: {testUniqueHierarchyTags[2]}, + onFindAllAncestors: []*models.TagPath{ + testUniqueHierarchyTagPaths[2], }, - onFindAllDescendants: map[int][]*models.Tag{ - 3: {testUniqueHierarchyTags[3], testUniqueHierarchyTags[4]}, - 4: {testUniqueHierarchyTags[4]}, + onFindAllDescendants: []*models.TagPath{ + testUniqueHierarchyTagPaths[3], testUniqueHierarchyTagPaths[4], }, - expectedError: "Cannot apply tag \"four\" as it already is a child", + expectedError: "", }, { id: 1, parents: []*models.Tag{testUniqueHierarchyTags[2]}, children: []*models.Tag{testUniqueHierarchyTags[3]}, - onFindAllAncestors: map[int][]*models.Tag{ - 2: {testUniqueHierarchyTags[2], testUniqueHierarchyTags[3]}, + onFindAllAncestors: []*models.TagPath{ + testUniqueHierarchyTagPaths[2], testUniqueHierarchyTagPaths[3], }, - onFindAllDescendants: map[int][]*models.Tag{ - 3: {testUniqueHierarchyTags[3]}, + onFindAllDescendants: []*models.TagPath{ + testUniqueHierarchyTagPaths[3], }, - expectedError: "Cannot apply tag \"three\" as it already is a parent", + expectedError: "cannot apply tag \"three\" as a child of \"one\" as it is already an ancestor ()", }, { id: 1, parents: []*models.Tag{testUniqueHierarchyTags[2]}, children: []*models.Tag{testUniqueHierarchyTags[3]}, - onFindAllAncestors: map[int][]*models.Tag{ - 2: {testUniqueHierarchyTags[2]}, + onFindAllAncestors: []*models.TagPath{ + testUniqueHierarchyTagPaths[2], }, - onFindAllDescendants: map[int][]*models.Tag{ - 3: {testUniqueHierarchyTags[3], testUniqueHierarchyTags[2]}, + onFindAllDescendants: []*models.TagPath{ + testUniqueHierarchyTagPaths[3], testUniqueHierarchyTagPaths[2], }, - expectedError: "Cannot apply tag \"three\" as it is linked to \"two\" which already is a parent", + expectedError: "cannot apply tag \"two\" as a parent of \"one\" as it is already a descendant ()", }, { id: 1, parents: []*models.Tag{testUniqueHierarchyTags[3]}, children: []*models.Tag{testUniqueHierarchyTags[3]}, - onFindAllAncestors: map[int][]*models.Tag{ - 3: {testUniqueHierarchyTags[3]}, + onFindAllAncestors: []*models.TagPath{ + testUniqueHierarchyTagPaths[3], }, - onFindAllDescendants: map[int][]*models.Tag{ - 3: {testUniqueHierarchyTags[3]}, + onFindAllDescendants: []*models.TagPath{ + testUniqueHierarchyTagPaths[3], }, - expectedError: "Cannot apply tag \"three\" as it already is a parent", + expectedError: "cannot apply tag \"three\" as a parent of \"one\" as it is already a descendant ()", }, { id: 1, @@ -165,54 +174,55 @@ var testUniqueHierarchyCases = []testUniqueHierarchyCase{ children: []*models.Tag{ testUniqueHierarchyTags[3], }, - onFindAllAncestors: map[int][]*models.Tag{ - 2: {testUniqueHierarchyTags[2]}, + onFindAllAncestors: []*models.TagPath{ + testUniqueHierarchyTagPaths[2], }, - onFindAllDescendants: map[int][]*models.Tag{ - 3: {testUniqueHierarchyTags[3], testUniqueHierarchyTags[2]}, + onFindAllDescendants: []*models.TagPath{ + testUniqueHierarchyTagPaths[3], testUniqueHierarchyTagPaths[2], }, - expectedError: "Cannot apply tag \"three\" as it is linked to \"two\" which already is a parent", + expectedError: "cannot apply tag \"two\" as a parent of \"one\" as it is already a descendant ()", }, { id: 1, parents: []*models.Tag{testUniqueHierarchyTags[2]}, children: []*models.Tag{testUniqueHierarchyTags[2]}, - onFindAllAncestors: map[int][]*models.Tag{ - 2: {testUniqueHierarchyTags[2]}, + onFindAllAncestors: []*models.TagPath{ + testUniqueHierarchyTagPaths[2], }, - onFindAllDescendants: map[int][]*models.Tag{ - 2: {testUniqueHierarchyTags[2]}, + onFindAllDescendants: []*models.TagPath{ + testUniqueHierarchyTagPaths[2], }, - expectedError: "Cannot apply tag \"two\" as it already is a parent", + expectedError: "cannot apply tag \"two\" as a parent of \"one\" as it is already a descendant ()", }, { id: 2, parents: []*models.Tag{testUniqueHierarchyTags[1]}, children: []*models.Tag{testUniqueHierarchyTags[3]}, - onFindAllAncestors: map[int][]*models.Tag{ - 1: {testUniqueHierarchyTags[1]}, + onFindAllAncestors: []*models.TagPath{ + testUniqueHierarchyTagPaths[1], }, - onFindAllDescendants: map[int][]*models.Tag{ - 3: {testUniqueHierarchyTags[3], testUniqueHierarchyTags[1]}, + onFindAllDescendants: []*models.TagPath{ + testUniqueHierarchyTagPaths[3], testUniqueHierarchyTagPaths[1], }, - expectedError: "Cannot apply tag \"three\" as it is linked to \"one\" which already is a parent", + expectedError: "cannot apply tag \"one\" as a parent of \"two\" as it is already a descendant ()", }, } -func TestEnsureUniqueHierarchy(t *testing.T) { +func TestEnsureHierarchy(t *testing.T) { for _, tc := range testUniqueHierarchyCases { - testEnsureUniqueHierarchy(t, tc, false, false) - testEnsureUniqueHierarchy(t, tc, true, false) - testEnsureUniqueHierarchy(t, tc, false, true) - testEnsureUniqueHierarchy(t, tc, true, true) + testEnsureHierarchy(t, tc, false, false) + testEnsureHierarchy(t, tc, true, false) + testEnsureHierarchy(t, tc, false, true) + testEnsureHierarchy(t, tc, true, true) } } -func testEnsureUniqueHierarchy(t *testing.T, tc testUniqueHierarchyCase, queryParents, queryChildren bool) { +func testEnsureHierarchy(t *testing.T, tc testUniqueHierarchyCase, queryParents, queryChildren bool) { mockTagReader := &mocks.TagReaderWriter{} var parentIDs, childIDs []int find := make(map[int]*models.Tag) + find[tc.id] = testUniqueHierarchyTags[tc.id] if tc.parents != nil { parentIDs = make([]int, 0) for _, parent := range tc.parents { @@ -243,50 +253,25 @@ func testEnsureUniqueHierarchy(t *testing.T, tc testUniqueHierarchyCase, queryPa mockTagReader.On("FindByParentTagID", tc.id).Return(tc.children, nil).Once() } - mockTagReader.On("Find", mock.AnythingOfType("int")).Return(func(tagID int) *models.Tag { - for id, tag := range find { - if id == tagID { - return tag - } - } - return nil - }, func(tagID int) error { - return nil - }).Maybe() - - mockTagReader.On("FindAllAncestors", mock.AnythingOfType("int"), []int{tc.id}).Return(func(tagID int, excludeIDs []int) []*models.Tag { - for id, tags := range tc.onFindAllAncestors { - if id == tagID { - return tags - } - } - return nil + mockTagReader.On("FindAllAncestors", mock.AnythingOfType("int"), []int(nil)).Return(func(tagID int, excludeIDs []int) []*models.TagPath { + return tc.onFindAllAncestors }, func(tagID int, excludeIDs []int) error { - for id := range tc.onFindAllAncestors { - if id == tagID { - return nil - } + if tc.onFindAllAncestors != nil { + return nil } return fmt.Errorf("undefined ancestors for: %d", tagID) }).Maybe() - mockTagReader.On("FindAllDescendants", mock.AnythingOfType("int"), []int{tc.id}).Return(func(tagID int, excludeIDs []int) []*models.Tag { - for id, tags := range tc.onFindAllDescendants { - if id == tagID { - return tags - } - } - return nil + mockTagReader.On("FindAllDescendants", mock.AnythingOfType("int"), []int(nil)).Return(func(tagID int, excludeIDs []int) []*models.TagPath { + return tc.onFindAllDescendants }, func(tagID int, excludeIDs []int) error { - for id := range tc.onFindAllDescendants { - if id == tagID { - return nil - } + if tc.onFindAllDescendants != nil { + return nil } return fmt.Errorf("undefined descendants for: %d", tagID) }).Maybe() - res := EnsureUniqueHierarchy(tc.id, parentIDs, childIDs, mockTagReader) + res := ValidateHierarchy(testUniqueHierarchyTags[tc.id], parentIDs, childIDs, mockTagReader) assert := assert.New(t) diff --git a/ui/v2.5/src/components/Changelog/versions/v0110.md b/ui/v2.5/src/components/Changelog/versions/v0110.md index 7e160b76f..19a14652e 100644 --- a/ui/v2.5/src/components/Changelog/versions/v0110.md +++ b/ui/v2.5/src/components/Changelog/versions/v0110.md @@ -22,6 +22,8 @@ * Optimised scanning process. ([#1816](https://github.com/stashapp/stash/pull/1816)) ### 🐛 Bug fixes +* Fix tag hierarchy not being validated during tag creation. ([#1926](https://github.com/stashapp/stash/pull/1926)) +* Fix tag hierarchy validation incorrectly failing for some hierarchies. ([#1926](https://github.com/stashapp/stash/pull/1926)) * Fix exclusion pattern fields losing focus on keypress. ([#1952](https://github.com/stashapp/stash/pull/1952)) * Include stash ids in import/export. ([#1916](https://github.com/stashapp/stash/pull/1916)) * Fix tiny menu items in scrape menu when a stash-box instance has no name. ([#1889](https://github.com/stashapp/stash/pull/1889)) diff --git a/ui/v2.5/src/index.scss b/ui/v2.5/src/index.scss index 33d81626c..fae797154 100755 --- a/ui/v2.5/src/index.scss +++ b/ui/v2.5/src/index.scss @@ -421,6 +421,10 @@ div.dropdown-menu { text-shadow: none; } } + + .toast-body { + white-space: pre-wrap; + } } .image-input {