destinationSet.removeAllExcept doesn't remove destinations if no match is found #133
+230
−4
Conversation
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
rogin
force-pushed
the
fix-destinationset
branch
3 times, most recently
from
7caadea to
d484454
Compare
Member
|
I was literally working on this one a couple nights ago 😄 I love the tests. |
tonygermano
force-pushed
the
fix-destinationset
branch
from
d484454 to
e640b02
Compare
This adds several tests for the DestinationSet.removeAllExcept(Object) method, and confirms a bug exists. Signed-off-by: Richard Ogin <rogin@users.noreply.github.com> Issue: nextgenhealthcare/connect#5875
The DestinationSet.removeAllExcept(Object) method takes a destination metadata id or destination name. If it's passed a reference to a destination which does not exist, then it should remove all entries from the set. This was working correctly if a number was passed to the method which did not refer to a destination which remained in the set, but not for a name. This change ensures that when a destination name does not appear in the destination set, that all destinations are removed. Additonal tests were also added to DestinationSetTest for greater coverage. Signed-off-by: Tony Germano <tony@germano.name> Issue: nextgenhealthcare/connect#5875 Reported-by: Mike Hokanson <mhokanson@gmail.com>
tonygermano
force-pushed
the
fix-destinationset
branch
from
e640b02 to
e686d61
Compare
tonygermano
approved these changes
Jul 4, 2025
Member
tonygermano
left a comment
tonygermano
left a comment
There was a problem hiding this comment.
I made this two commits. The first adds your test class. The second has the fix and adds additional tests. It looks good to me at this point.
gibson9583
approved these changes
Jul 4, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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 bug where
destinationSet.removeAllExcept(Object)fails to remove destinations when the given id/name is not found.I verified the test method
test_removeAllExceptObject_withSourceMap_removeAllForMetadataIdWhichDoesNotExistfailed before the code fix then succeeded after the code fix.Related
Original ticket with code change recommendation - nextgenhealthcare/connect#5875
Summary table in #121