diff --git a/internal/api/resolver_mutation_performer.go b/internal/api/resolver_mutation_performer.go index ab9abf6cf..fd18ecb95 100644 --- a/internal/api/resolver_mutation_performer.go +++ b/internal/api/resolver_mutation_performer.go @@ -43,7 +43,7 @@ func (r *mutationResolver) PerformerCreate(ctx context.Context, input models.Per newPerformer.Name = strings.TrimSpace(input.Name) newPerformer.Disambiguation = translator.string(input.Disambiguation) - newPerformer.Aliases = models.NewRelatedStrings(stringslice.TrimSpace(input.AliasList)) + newPerformer.Aliases = models.NewRelatedStrings(stringslice.UniqueExcludeFold(stringslice.TrimSpace(input.AliasList), newPerformer.Name)) newPerformer.Gender = input.Gender newPerformer.Ethnicity = translator.string(input.Ethnicity) newPerformer.Country = translator.string(input.Country) @@ -348,6 +348,27 @@ func (r *mutationResolver) PerformerUpdate(ctx context.Context, input models.Per } } + if updatedPerformer.Aliases != nil { + p, err := qb.Find(ctx, performerID) + if err != nil { + return err + } + if p != nil { + if err := p.LoadAliases(ctx, qb); err != nil { + return err + } + + effectiveAliases := updatedPerformer.Aliases.Apply(p.Aliases.List()) + name := p.Name + if updatedPerformer.Name.Set { + name = updatedPerformer.Name.Value + } + + sanitized := stringslice.UniqueExcludeFold(effectiveAliases, name) + updatedPerformer.Aliases.Values = sanitized + updatedPerformer.Aliases.Mode = models.RelationshipUpdateModeSet + } + } if err := performer.ValidateUpdate(ctx, performerID, *updatedPerformer, qb); err != nil { return err } diff --git a/internal/api/resolver_mutation_studio.go b/internal/api/resolver_mutation_studio.go index da3aa1983..fdd700490 100644 --- a/internal/api/resolver_mutation_studio.go +++ b/internal/api/resolver_mutation_studio.go @@ -38,7 +38,7 @@ func (r *mutationResolver) StudioCreate(ctx context.Context, input models.Studio newStudio.Favorite = translator.bool(input.Favorite) newStudio.Details = translator.string(input.Details) newStudio.IgnoreAutoTag = translator.bool(input.IgnoreAutoTag) - newStudio.Aliases = models.NewRelatedStrings(stringslice.TrimSpace(input.Aliases)) + newStudio.Aliases = models.NewRelatedStrings(stringslice.UniqueExcludeFold(stringslice.TrimSpace(input.Aliases), newStudio.Name)) newStudio.StashIDs = models.NewRelatedStashIDs(models.StashIDInputs(input.StashIds).ToStashIDs()) var err error @@ -167,6 +167,28 @@ func (r *mutationResolver) StudioUpdate(ctx context.Context, input models.Studio if err := r.withTxn(ctx, func(ctx context.Context) error { qb := r.repository.Studio + if updatedStudio.Aliases != nil { + s, err := qb.Find(ctx, studioID) + if err != nil { + return err + } + if s != nil { + if err := s.LoadAliases(ctx, qb); err != nil { + return err + } + + effectiveAliases := updatedStudio.Aliases.Apply(s.Aliases.List()) + name := s.Name + if updatedStudio.Name.Set { + name = updatedStudio.Name.Value + } + + sanitized := stringslice.UniqueExcludeFold(effectiveAliases, name) + updatedStudio.Aliases.Values = sanitized + updatedStudio.Aliases.Mode = models.RelationshipUpdateModeSet + } + } + if err := studio.ValidateModify(ctx, updatedStudio, qb); err != nil { return err } diff --git a/internal/api/resolver_mutation_tag.go b/internal/api/resolver_mutation_tag.go index f8d4943be..8fb295d40 100644 --- a/internal/api/resolver_mutation_tag.go +++ b/internal/api/resolver_mutation_tag.go @@ -35,7 +35,7 @@ func (r *mutationResolver) TagCreate(ctx context.Context, input TagCreateInput) newTag.Name = strings.TrimSpace(input.Name) newTag.SortName = translator.string(input.SortName) - newTag.Aliases = models.NewRelatedStrings(stringslice.TrimSpace(input.Aliases)) + newTag.Aliases = models.NewRelatedStrings(stringslice.UniqueExcludeFold(stringslice.TrimSpace(input.Aliases), newTag.Name)) newTag.Favorite = translator.bool(input.Favorite) newTag.Description = translator.string(input.Description) newTag.IgnoreAutoTag = translator.bool(input.IgnoreAutoTag) @@ -151,6 +151,28 @@ func (r *mutationResolver) TagUpdate(ctx context.Context, input TagUpdateInput) if err := r.withTxn(ctx, func(ctx context.Context) error { qb := r.repository.Tag + if updatedTag.Aliases != nil { + t, err := qb.Find(ctx, tagID) + if err != nil { + return err + } + if t != nil { + if err := t.LoadAliases(ctx, qb); err != nil { + return err + } + + newAliases := updatedTag.Aliases.Apply(t.Aliases.List()) + name := t.Name + if updatedTag.Name.Set { + name = updatedTag.Name.Value + } + + sanitized := stringslice.UniqueExcludeFold(newAliases, name) + updatedTag.Aliases.Values = sanitized + updatedTag.Aliases.Mode = models.RelationshipUpdateModeSet + } + } + if err := tag.ValidateUpdate(ctx, tagID, updatedTag, qb); err != nil { return err } diff --git a/pkg/performer/validate.go b/pkg/performer/validate.go index 68f7a8ef5..3baaa182b 100644 --- a/pkg/performer/validate.go +++ b/pkg/performer/validate.go @@ -225,6 +225,11 @@ func ValidateUpdateAliases(existing models.Performer, name models.OptionalString newName = name.Value } + // If aliases is nil, we're only changing the name - check existing aliases against new name + if aliases == nil { + return ValidateAliases(newName, existing.Aliases) + } + newAliases := aliases.Apply(existing.Aliases.List()) return ValidateAliases(newName, models.NewRelatedStrings(newAliases)) diff --git a/pkg/performer/validate_test.go b/pkg/performer/validate_test.go index 33f4b1cec..afd9c01c5 100644 --- a/pkg/performer/validate_test.go +++ b/pkg/performer/validate_test.go @@ -213,12 +213,12 @@ func TestValidateUpdateAliases(t *testing.T) { want error }{ {"both unset", osUnset, nil, nil}, - {"invalid name set", os2, nil, &DuplicateAliasError{name2}}, + {"name conflicts with alias", os2, nil, &DuplicateAliasError{name2}}, {"valid name set", os3, nil, nil}, {"valid aliases empty", os1, []string{}, nil}, - {"invalid aliases set", osUnset, []string{name1U}, &DuplicateAliasError{name1U}}, + {"alias matches name", osUnset, []string{name1U}, &DuplicateAliasError{name1U}}, {"valid aliases set", osUnset, []string{name3, name2}, nil}, - {"invalid both set", os4, []string{name4}, &DuplicateAliasError{name4}}, + {"alias matches new name", os4, []string{name4}, &DuplicateAliasError{name4}}, {"valid both set", os2, []string{name1}, nil}, } diff --git a/pkg/sliceutil/stringslice/string_collections.go b/pkg/sliceutil/stringslice/string_collections.go index f5251de5f..eff3409e2 100644 --- a/pkg/sliceutil/stringslice/string_collections.go +++ b/pkg/sliceutil/stringslice/string_collections.go @@ -45,6 +45,23 @@ func UniqueFold(s []string) []string { return ret } +// UniqueExcludeFold returns a deduplicated slice of strings with the excluded string removed. +// The comparison is case-insensitive. +func UniqueExcludeFold(values []string, exclude string) []string { + seen := make(map[string]struct{}, len(values)) + seen[strings.ToLower(exclude)] = struct{}{} + ret := make([]string, 0, len(values)) + for _, v := range values { + vLower := strings.ToLower(v) + if _, exists := seen[vLower]; exists { + continue + } + seen[vLower] = struct{}{} + ret = append(ret, v) + } + return ret +} + // TrimSpace trims whitespace from each string in a slice. func TrimSpace(s []string) []string { for i, v := range s { diff --git a/pkg/studio/validate.go b/pkg/studio/validate.go index 4e2f51c84..1654a2e78 100644 --- a/pkg/studio/validate.go +++ b/pkg/studio/validate.go @@ -135,6 +135,7 @@ func ValidateModify(ctx context.Context, s models.StudioPartial, qb ValidateModi } effectiveAliases := s.Aliases.Apply(existing.Aliases.List()) + if err := ValidateAliases(ctx, s.ID, effectiveAliases, qb); err != nil { return err } diff --git a/pkg/studio/validate_test.go b/pkg/studio/validate_test.go index 6562dc5ca..b196ba3c3 100644 --- a/pkg/studio/validate_test.go +++ b/pkg/studio/validate_test.go @@ -102,3 +102,72 @@ func TestValidateUpdateName(t *testing.T) { }) } } + +func TestValidateUpdateAliases(t *testing.T) { + db := mocks.NewDatabase() + + const ( + name1 = "name 1" + name2 = "name 2" + alias1 = "alias 1" + newAlias = "new alias" + ) + + existing1 := models.Studio{ + ID: 1, + Name: name1, + } + existing2 := models.Studio{ + ID: 2, + Name: name2, + } + + pp := 1 + findFilter := &models.FindFilterType{ + PerPage: &pp, + } + + aliasFilter := func(n string) *models.StudioFilterType { + return &models.StudioFilterType{ + Aliases: &models.StringCriterionInput{ + Value: n, + Modifier: models.CriterionModifierEquals, + }, + } + } + + // name1 matches existing1 name - ok + db.Studio.On("Query", testCtx, nameFilter(alias1), findFilter).Return(nil, 0, nil) + db.Studio.On("Query", testCtx, aliasFilter(alias1), findFilter).Return(nil, 0, nil) + + // name2 matches existing2 name - error + db.Studio.On("Query", testCtx, nameFilter(name2), findFilter).Return([]*models.Studio{&existing2}, 1, nil) + + // alias matches existing alias - error + db.Studio.On("Query", testCtx, nameFilter(newAlias), findFilter).Return(nil, 0, nil) + db.Studio.On("Query", testCtx, aliasFilter(newAlias), findFilter).Return([]*models.Studio{&existing2}, 1, nil) + + // valid alias + db.Studio.On("Query", testCtx, nameFilter("valid"), findFilter).Return(nil, 0, nil) + db.Studio.On("Query", testCtx, aliasFilter("valid"), findFilter).Return(nil, 0, nil) + + tests := []struct { + tName string + studio models.Studio + aliases []string + want error + }{ + {"valid alias", existing1, []string{alias1}, nil}, + {"alias duplicates other name", existing1, []string{name2}, &NameExistsError{name2}}, + {"alias duplicates other alias", existing1, []string{newAlias}, &NameUsedByAliasError{newAlias, existing2.Name}}, + {"valid new alias", existing1, []string{"valid"}, nil}, + {"empty alias", existing1, []string{""}, ErrEmptyAlias}, + } + + for _, tt := range tests { + t.Run(tt.tName, func(t *testing.T) { + got := ValidateAliases(testCtx, tt.studio.ID, tt.aliases, db.Studio) + assert.Equal(t, tt.want, got) + }) + } +} diff --git a/pkg/tag/validate.go b/pkg/tag/validate.go index 966cec945..abc260b5e 100644 --- a/pkg/tag/validate.go +++ b/pkg/tag/validate.go @@ -69,7 +69,9 @@ func ValidateUpdate(ctx context.Context, id int, partial models.TagPartial, qb m return err } - if err := EnsureAliasesUnique(ctx, id, partial.Aliases.Apply(existing.Aliases.List()), qb); err != nil { + newAliases := partial.Aliases.Apply(existing.Aliases.List()) + + if err := EnsureAliasesUnique(ctx, id, newAliases, qb); err != nil { return err } } diff --git a/pkg/tag/validate_test.go b/pkg/tag/validate_test.go new file mode 100644 index 000000000..539086a6d --- /dev/null +++ b/pkg/tag/validate_test.go @@ -0,0 +1,86 @@ +package tag + +import ( + "testing" + + "github.com/stashapp/stash/pkg/models" + "github.com/stashapp/stash/pkg/models/mocks" + "github.com/stretchr/testify/assert" +) + +func nameFilter(n string) *models.TagFilterType { + return &models.TagFilterType{ + Name: &models.StringCriterionInput{ + Value: n, + Modifier: models.CriterionModifierEquals, + }, + } +} + +func aliasFilter(n string) *models.TagFilterType { + return &models.TagFilterType{ + Aliases: &models.StringCriterionInput{ + Value: n, + Modifier: models.CriterionModifierEquals, + }, + } +} + +func TestEnsureAliasesUnique(t *testing.T) { + db := mocks.NewDatabase() + + const ( + name1 = "name 1" + name2 = "name 2" + alias1 = "alias 1" + newAlias = "new alias" + ) + + existing2 := models.Tag{ + ID: 2, + Name: name2, + } + + pp := 1 + findFilter := &models.FindFilterType{ + PerPage: &pp, + } + + // name1 matches existing1 name - ok + // EnsureAliasesUnique calls EnsureTagNameUnique. + // EnsureTagNameUnique calls ByName then ByAlias. + + // Case 1: valid alias + // ByName "alias 1" -> nil + // ByAlias "alias 1" -> nil + db.Tag.On("Query", testCtx, nameFilter(alias1), findFilter).Return(nil, 0, nil) + db.Tag.On("Query", testCtx, aliasFilter(alias1), findFilter).Return(nil, 0, nil) + + // Case 2: alias duplicates existing2 name + // ByName "name 2" -> existing2 + db.Tag.On("Query", testCtx, nameFilter(name2), findFilter).Return([]*models.Tag{&existing2}, 1, nil) + + // Case 3: alias duplicates existing2 alias + // ByName "new alias" -> nil + // ByAlias "new alias" -> existing2 + db.Tag.On("Query", testCtx, nameFilter(newAlias), findFilter).Return(nil, 0, nil) + db.Tag.On("Query", testCtx, aliasFilter(newAlias), findFilter).Return([]*models.Tag{&existing2}, 1, nil) + + tests := []struct { + tName string + id int + aliases []string + want error + }{ + {"valid alias", 1, []string{alias1}, nil}, + {"alias duplicates other name", 1, []string{name2}, &NameExistsError{name2}}, + {"alias duplicates other alias", 1, []string{newAlias}, &NameUsedByAliasError{newAlias, existing2.Name}}, + } + + for _, tt := range tests { + t.Run(tt.tName, func(t *testing.T) { + got := EnsureAliasesUnique(testCtx, tt.id, tt.aliases, db.Tag) + assert.Equal(t, tt.want, got) + }) + } +} diff --git a/ui/v2.5/src/components/Performers/PerformerDetails/PerformerEditPanel.tsx b/ui/v2.5/src/components/Performers/PerformerDetails/PerformerEditPanel.tsx index 7bb8d399a..e447c7484 100644 --- a/ui/v2.5/src/components/Performers/PerformerDetails/PerformerEditPanel.tsx +++ b/ui/v2.5/src/components/Performers/PerformerDetails/PerformerEditPanel.tsx @@ -44,7 +44,7 @@ import { yupInputNumber, yupInputEnum, yupDateString, - yupUniqueAliases, + yupRequiredStringArray, yupUniqueStringList, } from "src/utils/yup"; import { useTagsEdit } from "src/hooks/tagsEdit"; @@ -110,7 +110,7 @@ export const PerformerEditPanel: React.FC = ({ const schema = yup.object({ name: yup.string().required(), disambiguation: yup.string().ensure(), - alias_list: yupUniqueAliases(intl, "name"), + alias_list: yupRequiredStringArray(intl).defined(), gender: yupInputEnum(GQL.GenderEnum).nullable().defined(), birthdate: yupDateString(intl), death_date: yupDateString(intl), @@ -509,15 +509,15 @@ export const PerformerEditPanel: React.FC = ({ ))} {queryableScrapers ? queryableScrapers.map((s) => ( - onScraperSelected(s)} - > - {s.name} - - )) + onScraperSelected(s)} + > + {s.name} + + )) : ""} = ({ urls: yup.array(yup.string().required()).defined(), details: yup.string().ensure(), parent_id: yup.string().required().nullable(), - aliases: yupUniqueAliases(intl, "name"), + aliases: yupRequiredStringArray(intl).defined(), tag_ids: yup.array(yup.string().required()).defined(), ignore_auto_tag: yup.boolean().defined(), stash_ids: yup.mixed().defined(), @@ -103,10 +103,10 @@ export const StudioEditPanel: React.FC = ({ setParentStudio( studio.parent_studio ? { - id: studio.parent_studio.id, - name: studio.parent_studio.name, - aliases: [], - } + id: studio.parent_studio.id, + name: studio.parent_studio.name, + aliases: [], + } : null ); }, [studio.parent_studio]); diff --git a/ui/v2.5/src/components/Tags/TagDetails/TagEditPanel.tsx b/ui/v2.5/src/components/Tags/TagDetails/TagEditPanel.tsx index 077300788..22c99b80e 100644 --- a/ui/v2.5/src/components/Tags/TagDetails/TagEditPanel.tsx +++ b/ui/v2.5/src/components/Tags/TagDetails/TagEditPanel.tsx @@ -15,7 +15,7 @@ import { useToast } from "src/hooks/Toast"; import { useConfigurationContext } from "src/hooks/Config"; import { handleUnsavedChanges } from "src/utils/navigation"; import { formikUtils } from "src/utils/form"; -import { yupFormikValidate, yupUniqueAliases } from "src/utils/yup"; +import { yupFormikValidate, yupRequiredStringArray } from "src/utils/yup"; import { addUpdateStashID, getStashIDs } from "src/utils/stashIds"; import { Tag, TagSelect } from "../TagSelect"; import { Icon } from "src/components/Shared/Icon"; @@ -56,7 +56,7 @@ export const TagEditPanel: React.FC = ({ const schema = yup.object({ name: yup.string().required(), sort_name: yup.string().ensure(), - aliases: yupUniqueAliases(intl, "name"), + aliases: yupRequiredStringArray(intl).defined(), description: yup.string().ensure(), parent_ids: yup.array(yup.string().required()).defined(), child_ids: yup.array(yup.string().required()).defined(), diff --git a/ui/v2.5/src/utils/yup.ts b/ui/v2.5/src/utils/yup.ts index 5ae8123df..a9c4f69e1 100644 --- a/ui/v2.5/src/utils/yup.ts +++ b/ui/v2.5/src/utils/yup.ts @@ -92,45 +92,6 @@ export function yupUniqueStringList(intl: IntlShape) { }); } -export function yupUniqueAliases(intl: IntlShape, nameField: string) { - return yupRequiredStringArray(intl) - .defined() - .test({ - name: "unique", - test(value) { - const aliases = [this.parent[nameField].toLowerCase()]; - const dupes: number[] = []; - for (let i = 0; i < value.length; i++) { - const s = value[i].toLowerCase(); - if (aliases.includes(s)) { - dupes.push(i); - } else { - aliases.push(s); - } - } - if (dupes.length === 0) return true; - - const msg = yup.ValidationError.formatError( - intl.formatMessage({ id: "validation.unique" }), - { - label: this.schema.spec.label, - path: this.path, - } - ); - const errors = dupes.map( - (i) => - new yup.ValidationError( - msg, - value[i], - `${this.path}["${i}"]`, - "unique" - ) - ); - return new yup.ValidationError(errors, value, this.path, "unique"); - }, - }); -} - export function yupDateString(intl: IntlShape) { return yup .string()