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
TRUNK-4095 #404
Conversation
@@ -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 |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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()
…onceptToValidate.getUuid() != null
Are these all the validators that you could find? |
yes, @dkayiwa those are the ones i was able to find, save if you have others for me to consider!!! |
I can see the following: |
…eTermValidator.java and PersonAttributeTypeValidator.java
https://tickets.openmrs.org/browse/TRUNK-4095 Fix Validators to compare by uuid when checking for duplicates