feat(web): Add stacking option to deduplication utilities #11114
Conversation
|
Thank you for working on this! I will take a look shortly |
web/src/lib/components/utilities-page/duplicates/duplicates-compare-control.svelte
Outdated
Show resolved
Hide resolved
…mpare-control.svelte Co-authored-by: Alex <[email protected]>
There was a problem hiding this comment.
Thanks for working on this! I'm sure a lot of people will love this feature.
How do you feel about the "select keep all" / "select trash all" buttons? I think they feel weird now since keeping and trashing aren't the only actions. IMO it'd be best to just fold them into one button that says "select all" if not all assets are selected, and "unselect all" if all of them are.
web/src/routes/(user)/utilities/duplicates/[[photos=photos]]/[[assetId=id]]/+page.svelte
Outdated
Show resolved
Hide resolved
web/src/routes/(user)/utilities/duplicates/[[photos=photos]]/[[assetId=id]]/+page.svelte
Outdated
Show resolved
Hide resolved
web/src/routes/(user)/utilities/duplicates/[[photos=photos]]/[[assetId=id]]/+page.svelte
Outdated
Show resolved
Hide resolved
web/src/routes/(user)/utilities/duplicates/[[photos=photos]]/[[assetId=id]]/+page.svelte
Outdated
Show resolved
Hide resolved
|
I created some new endpoints in the server to handle the one request stackAll operation. I hope I captured all the functionality of the old updateAll method. I am not certain if the two DTO's are necessary but it seems to work for now. I did not look into the app development yet, but the stack functionality will also need to change there. |
|
Sorry for the back-and-forth on this PR. We are refactoring the stack mechanism, so I would avoid changing the code on the server for stacking. Would it be possible to use the current stack endpoint and mechanism for this purpose? |
|
It would be possible, but we would need to live with multiple requests for stacking all duplicates. |
|
That would be unfortunate. If we did it one-by-one, the user has 50k duplicate groups, and handling each group takes 20ms, it would take over 16 minutes to get through all of them. The user has to wait on that page that long and not assume that clicking the button was enough, or that something went wrong and they need to refresh. I'd almost rather not have the Stack All button in that case. You could do requests in parallel to mitigate this, but it would still be very slow regardless. |
|
Yeah, no stack all button is fine |
|
I updated it to the best of my knowledge, it is a bit sad that the stack all button is not possible yet, but I understand. |
web/src/lib/utils/asset-utils.ts
Outdated
| @@ -324,13 +324,17 @@ export const getSelectedAssets = (assets: Set<AssetResponseDto>, user: UserRespo | |||
| return ids; | |||
| }; | |||
|
|
|||
| export const stackAssets = async (assets: AssetResponseDto[]) => { | |||
| export const stackAssets = async (assets: AssetResponseDto[], parent?: AssetResponseDto, showGoToButton?: boolean) => { | |||
There was a problem hiding this comment.
Why do we need to change the logic of this method?
web/src/lib/utils/asset-utils.ts
Outdated
| @@ -361,17 +365,19 @@ export const stackAssets = async (assets: AssetResponseDto[]) => { | |||
| parent.stack ??= []; | |||
| parent.stack = parent.stack.concat(children, grandChildren); | |||
| parent.stackCount = parent.stack.length + 1; | |||
|
|
|||
| notificationController.show({ | |||
| const notificationObject = { | |||
There was a problem hiding this comment.
What are we trying to achieve here? The button property is also not a public property AFAIK
| @@ -141,6 +162,36 @@ | |||
| $t('confirm'), | |||
| ); | |||
| }; | |||
|
|
|||
There was a problem hiding this comment.
I don't think we need to keep this commented code here
| @@ -157,6 +208,16 @@ | |||
| {$t('keep_all')} | |||
| </div> | |||
| </LinkButton> | |||
| <!-- | |||
There was a problem hiding this comment.
I don't think we need to keep this commented code here
Since stacked photos exist in immich and de-duplication only allows for deletion, I figured it would be nice to have the option to stack the pictures instead of deleting them.
I created a basic version of this functionality in this pull request.
It (partially) addresses #2479
Interface:
Let me know if I need to modify something, i did not fix the internationalization error in the tests, otherwise everything passed I think.