TRUNK-6171: Address inconsistency issues with ConditionService #4290
Conversation
api/src/main/java/org/openmrs/api/db/hibernate/HibernateConditionDAO.java
Show resolved
Hide resolved
|
|
||
| // If there is an existing condition, create a new condition from it and reset the existing instance state | ||
| Condition newCondition = Condition.newInstance(condition); | ||
| Context.refreshEntity(condition); |
There was a problem hiding this comment.
what does this do again? loads it in from the DB/cache again? (ie prior to any changes?)
There was a problem hiding this comment.
RTFM :)
/**
* Used to re-read the state of the given instance from the underlying database.
* @since 2.0
* @param obj The object to refresh from the database in the session
*/
| boolean conditionHasChanged = !newCondition.matches(condition); | ||
| boolean existingVoided = BooleanUtils.isTrue(condition.getVoided()); | ||
| boolean newVoided = BooleanUtils.isTrue(newCondition.getVoided()); | ||
| boolean unVoidOriginal = existingVoided && !newVoided; |
There was a problem hiding this comment.
we unvoid the original even if the new one differs from the old? wouldn't we just create a new condition in this case that we both want to unvoid and modify?
There was a problem hiding this comment.
It's a good point. I had originally re-written that way, and it was failing, which is why I preserved the existing behavior. I'll have a look.
There was a problem hiding this comment.
OK, per @mogoodrich comment and suggestion, I've removed the special "unvoid" handling, which I agree is unnecessary and wrong. Unvoiding shouldn't eliminate the audit trail of the previously voided data, it should just be treated like an edit, linking back to voided row. That is now working.
| * Tests of methods within the Condition class | ||
| * @see Condition | ||
| */ | ||
| public class ConditionTest { |
There was a problem hiding this comment.
Unit test added here for the Condition matches method @mogoodrich
|
@ibacher and @mogoodrich - I think I've addressed all of your comments. Let me know how things are looking... |
|
SonarCloud Quality Gate failed. |
Description of what I changed
This makes the following changes to conditions:
Issue I worked on
see https://issues.openmrs.org/browse/TRUNK-6171