Skip to content

Conversation

@MareStare
Copy link
Contributor

@MareStare MareStare commented May 15, 2025

There are several changes packed in this single PR

Zero changes edge case

Fixed this edge case when the user doesn't select any tags to revert at all and clicks on the "Revert selected" button:
image

(this bug should active be in prod too).

Eliminate redundant preloads

I removed the redundant :tag and :tag_change (reverse) preloads for the initial TagChange select. The inside Images.batch_update doesn't need the full TagChange.Tag struct, it can simply work with tag_ids lists.

More info in the flash

Also added the number of actually updated tags to the result of the revert:
image

TODO

I'll try to add some auto tests for this code

@MareStare MareStare changed the title Post-refactoring and fixing of the new batch tags update flow Post-refactoring and improving of the new batch tags update flow May 15, 2025
@Meow
Copy link
Member

Meow commented May 16, 2025

You should've asked me why there was mass_revert_tags... I intend to implement per-tag reverts as a mod tool in the future, so that mods can select individual tags to be reverted as opposed to whole batches. Hence why this whole structure and tag_changes relation preloads on TagChanges.Tag.

@MareStare MareStare force-pushed the fix/tag-changes-edge-cases branch 4 times, most recently from 13a8114 to 3d1e53e Compare May 17, 2025 18:51
@Meow
Copy link
Member

Meow commented May 19, 2025

Split refactoring / logic fixes, and unit tests, into two separate PRs, this PR is becoming way too large. Maybe even more PRs to be honest, your initial fixes were small and easily mergeable, but this has gotten really, really huge now.

@MareStare MareStare force-pushed the fix/tag-changes-edge-cases branch 2 times, most recently from 24d2985 to 73eaa68 Compare May 20, 2025 23:06
@MareStare MareStare force-pushed the fix/tag-changes-edge-cases branch from 73eaa68 to bc24332 Compare May 20, 2025 23:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants