Feature Request: Bulk Add by StashID and Name#6310
Feature Request: Bulk Add by StashID and Name#6310WithoutPants merged 12 commits intostashapp:developfrom
Conversation
internal/manager/manager_tasks.go
Outdated
| } else if len(input.StashIDs) > 0 { | ||
| // The user is batch adding performers by stash ID | ||
| for i := range input.StashIDs { | ||
| stashID := input.StashIDs[i] | ||
| if len(stashID) > 0 { | ||
| tasks = append(tasks, StashBoxBatchTagTask{ | ||
| stashID: &stashID, | ||
| refresh: true, // use refresh mode to query by ID | ||
| box: box, | ||
| excludedFields: input.ExcludeFields, | ||
| taskType: Performer, | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
This looks like it won't handle if you have both stash ids and names in the one operation, due to the if/else structure. It will handle the stash ids only and not the names.
There was a problem hiding this comment.
I forgot to push my latest commit that fixes this.
feederbox826
left a comment
feederbox826
left a comment
There was a problem hiding this comment.
LGTM, still running into "studio alias must not be an empty string" bug
|
Yea, will need to rebase when WP makes his changes to see that bug fixed |
|
That fix was merged. |
| @@ -44,24 +45,32 @@ func (t *StashBoxBatchTagTask) Start(ctx context.Context) { | |||
| } | |||
There was a problem hiding this comment.
I think this task needs to be split up and refactored. I couldn't work out why you'd set refresh to true in manager_tasks.go until I looked through the code here. In the existing code refresh is used to determine whether to use the existing performer's applicable stash id or its name to search for the stash-box performer.
Your use of that parameter further muddies it's purpose because now it seems to be being used to indicate if it should search via a passed stash id. It's just a bit of a mess to follow.
Having a single task with two separate execution paths and inputs seems needlessly complicated as well.
My suggestion would be to split this into two private tasks:
type stashBoxBatchPerformerTagTask struct {
// stash-box instance to search
box *models.StashBox
// if set, searches by performer name
name *string
// if set, searches by stash-id
stashID *string
// if set, updates performer with matched stash-box performer. If unset, creates new performer
performer *models.Performer
}
type stashBoxBatchStudioTagTask struct {
// stash-box instance to search
box *models.StashBox
// if set, searches by performer name
name *string
// if set, searches by stash-id
stashID *string
// if set, updates performer with matched stash-box performer. If unset, creates new performer
studio *models.Studio
}
I think this gives a clearer indication of the expected behaviour of each task, and simplifies the execution path a bit.
WithoutPants
left a comment
WithoutPants
left a comment
There was a problem hiding this comment.
Refactoring requested. See related comment.
ui/v2.5/src/locales/en-GB.json
Outdated
| @@ -1289,6 +1289,7 @@ | |||
| "number_of_performers_will_be_processed": "{performer_count} performers will be processed", | |||
| "performer_already_tagged": "Performer already tagged", | |||
| "performer_names_separated_by_comma": "Performer names separated by comma", | |||
There was a problem hiding this comment.
This entry is now unused and should be removed.
ui/v2.5/src/locales/en-GB.json
Outdated
| @@ -1498,6 +1499,7 @@ | |||
| "status_tagging_studios": "Status: Tagging studios", | |||
| "studio_already_tagged": "Studio already tagged", | |||
| "studio_names_separated_by_comma": "Studio names separated by comma", | |||
There was a problem hiding this comment.
This entry is now unused and should be removed.
WithoutPants
changed the title
This comment was marked as outdated.
This comment was marked as outdated.
|
Lots of refactoring in this one. My eyes started to glaze over near the end but I was still able to test functionality. If documentation is still not to your liking then please feel free to update it yourself since that will probably be quicker lol |
WithoutPants
left a comment
WithoutPants
left a comment
There was a problem hiding this comment.
Minor things. I'll try to get some testing on it today.
5 participants
This PR allows users to uses StashID and/or name when bulk adding studios or performers.
I was able to get this to work on performers with no errors
Studios had errors that was related to the empty string aliases bug and I don't think it is due to this PR.
This PR was used to fix this issue with normal tagging and I believe it needs to be moved over to the batch create options too.
Testing:
1 performer by StashdID added (V4 and V7)
2 performers comma split by Stash ID added (V4 and V7)
3 performers comma split, 2 by stash ID and 1 by name (V4 and V7)