Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TRUNK-4095 #404

Merged
merged 3 commits into from Oct 31, 2013
Merged

TRUNK-4095 #404

merged 3 commits into from Oct 31, 2013

Conversation

k-joseph
Copy link
Member

https://tickets.openmrs.org/browse/TRUNK-4095 Fix Validators to compare by uuid when checking for duplicates

@@ -177,8 +177,8 @@ else if (nameInLocale.isLocalePreferred() && preferredNameForLocaleFound) {
continue;

//skip same
if (conceptToValidate.getConceptId() != null
&& conceptToValidate.getConceptId().equals(concept.getConceptId()))
if (conceptToValidate.getUuid() != null
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only thing you needed to change in the if clause is the equals check to use uuids and not id because newly created items have no id before they are saved but do have a uuid .
You will still need the conceptToValidate.getConceptId() != null because the reason it is there is to ensure that when you are saving an existing item, the code doesn't end up thinking it is a duplicate because getConceptMapTypeName will always return it if you pass in an existing item's name

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In theory the code is checking that if it we found a duplicate, make sure it is not the item itself we are validating that was found and this only arises for existing items that are getting edited

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks please @wluyima for your review and response, however i think i need to hear farther from you to respond to these two comments, i think it is better either on the IRC or here; by making further clear what you are meaning in your wonderful explanation above,
thanks

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i mean you still need the if(conceptToValidate.getId() != null) and not if (conceptToValidate.getUuid() != null)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, i think i better understand you now after the above comment, how can i implement comparison on uuids instead of ids. in this case besides what i had originally proposed as my ticket interpretation.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your commit is fine except undoing the change NOT to replace conceptToValidate.getConceptId() != null with conceptToValidate.getUuid() != null on line 180 above in red but the equals should still use getUuid()

@dkayiwa
Copy link
Member

dkayiwa commented Oct 28, 2013

Are these all the validators that you could find?

@k-joseph
Copy link
Member Author

yes, @dkayiwa those are the ones i was able to find, save if you have others for me to consider!!!

@dkayiwa
Copy link
Member

dkayiwa commented Oct 30, 2013

I can see the following:
ConceptReferenceTermValidator
PersonAttributeTypeValidator

…eTermValidator.java and PersonAttributeTypeValidator.java
dkayiwa added a commit that referenced this pull request Oct 31, 2013
@dkayiwa dkayiwa merged commit 76f2aed into openmrs:master Oct 31, 2013
RandilaP pushed a commit to RandilaP/openmrs-core that referenced this pull request Jul 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants