Repository navigation
Allow removing tags from multiple bookmarks - #3919
Open
nabeeltahirdeveloper wants to merge 1 commit into
Open
nabeeltahirdeveloper wants to merge 1 commit into
nabeeltahirdeveloper wants to merge 1 commit into
Conversation
When several bookmarks were selected, TagBookmarkPresenter loaded no checked tags at all and saved with deleteNonTagged = false, which only ever adds tags - so a tag could be added to multiple ayat but never removed from them (quran#2208). Load the tags that all selected bookmarks share (their intersection) as the checked set, remember them as the common tags, and on save remove any common tag that was unchecked from every bookmark via removeBookmarkFromTag before adding the checked tags as before. Tags that only some of the selected bookmarks have are in neither set, so saving never removes them. Single-bookmark behaviour is unchanged. Adds TagBookmarkPresenterTest cases for the common-tag checked state, removing a common tag from all bookmarks, keeping partial tags, and adding a tag to multiple bookmarks.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2208
Tags could be added to multiple selected ayat at once, but never removed: when more than one bookmark was selected,
TagBookmarkPresenterloaded no checked tags at all (its tag-ids observable returned an empty list for multiple ids), and saving calledupdateBookmarkTagswithdeleteNonTagged = false, which only ever adds tags.This change makes multi-selection behave like a bulk version of the single-bookmark dialog:
removeBookmarkFromTag, and the checked tags are then added to all of them via the existing additiveupdateBookmarkTagscall.deleteNonTagged = true).Tests: four new cases in
TagBookmarkPresenterTestcovering the common-tag checked state, removing a common tag from all bookmarks, keeping tags that only one bookmark has when a common tag is removed, and adding a tag to multiple bookmarks without removing existing ones.Testing note: I was unable to run the Gradle test suite in my environment (the Gradle daemon cannot start there — it fails with "Could not dispatch/receive a message from the daemon" before any compilation happens, on unmodified code as well). I verified the change by compiling the touched production and test files standalone with kotlinc (only the expected unresolved-reference errors from the missing classpath, identical in kind to the unmodified files), and by running an assertion harness that ports the presenter's new load/toggle/save logic together with the fake DAO's semantics: all new unit-test assertions pass against the new logic (17/17 checks including the pre-existing single-bookmark scenarios), and the removal assertions fail against the old logic. Happy to adjust if CI or a maintainer run shows anything I couldn't.