Skip to content

Commit

Permalink
HTML-841: Errors with Form Namespace and Path (#301)
Browse files Browse the repository at this point in the history
  • Loading branch information
@mogoodrich
mogoodrich authored Apr 3, 2024
1 parent 6c6b902 commit 12d3f5a
Show file tree
Hide file tree
Showing 5 changed files with 241 additions and 119 deletions.
81 changes: 8 additions & 73 deletions api/src/main/java/org/openmrs/module/htmlformentry/FormSubmissionActions.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,7 @@ public Obs createObs(Concept concept, Object value, Date datetime, String access
* @param comment comment for the obs
*/
public void modifyObs(Obs existingObs, Concept concept, Object newValue, Date newDatetime, String accessionNumber,
String comment) {
String comment, String controlFormPath) {
if (newValue == null || "".equals(newValue)) {
// we want to delete the existing obs
if (log.isDebugEnabled())
Expand All @@ -397,6 +397,7 @@ public void modifyObs(Obs existingObs, Concept concept, Object newValue, Date ne
return;
}
Obs newObs = HtmlFormEntryUtil.createObs(concept, newValue, newDatetime, accessionNumber);
// random note... it is possible that the existing obs has changed even though the value as string is the same (probably not?)
String oldString = existingObs.getValueAsString(Context.getLocale());
String newString = newObs.getValueAsString(Context.getLocale());
if (log.isDebugEnabled() && concept != null) {
Expand All @@ -418,7 +419,7 @@ public void modifyObs(Obs existingObs, Concept concept, Object newValue, Date ne
// TODO: really the voided obs should link to the new one, but this is a pain to implement due to the dreaded error: org.hibernate.NonUniqueObjectException: a different object with the same identifier value was already associated with the session
obsToVoid.add(existingObs);

newObs = createObs(concept, newValue, newDatetime, accessionNumber, comment);
newObs = createObs(concept, newValue, newDatetime, accessionNumber, comment, controlFormPath);
newObs.setPreviousVersion(existingObs);
} else {
if (existingObs != null && StringUtils.isNotBlank(comment))
Expand All @@ -430,79 +431,13 @@ public void modifyObs(Obs existingObs, Concept concept, Object newValue, Date ne
}
}

/**
* Legacy modifyObs methods without the comment argument
*/
public void modifyObs(Obs existingObs, Concept concept, Object newValue, Date newDatetime, String accessionNumber) {
modifyObs(existingObs, concept, newValue, newDatetime, accessionNumber, null);
public void modifyObs(Obs existingObs, Concept concept, Object newValue, Date newDatetime, String accessionNumber,
String comment) {
modifyObs(existingObs, concept, newValue, newDatetime, accessionNumber, comment, null);
}

/**
* Modifies an existing Obs.
* <p/>
* This method works by adding the current Obs to a list of Obs to void, and then adding the new Obs
* to a list of Obs to create. Note that this method does not commit the changes to the
* database--the changes are applied elsewhere in the framework.
*
* @param existingObs the Obs to modify
* @param concept concept associated with the Obs
* @param newValue the new value of the Obs
* @param newDatetime the new date information for the Obs
* @param accessionNumber new accession number for the Obs
* @param comment comment for the obs
*/
public void modifyObs(Obs existingObs, Concept concept, Object newValue, Date newDatetime, String accessionNumber,
String comment, String controlFormPath) {
if (newValue == null || "".equals(newValue)) {
// we want to delete the existing obs
if (log.isDebugEnabled()) {
log.debug("VOID: " + printObsHelper(existingObs));
}
obsToVoid.add(existingObs);
return;
}
if (concept == null) {
// we want to delete the existing obs
if (log.isDebugEnabled())
log.debug("VOID: " + printObsHelper(existingObs));
obsToVoid.add(existingObs);
return;
}
Obs newObs = HtmlFormEntryUtil.createObs(concept, newValue, newDatetime, accessionNumber);
if (controlFormPath != null) {
newObs.setFormField(FORM_NAMESPACE, controlFormPath);
}
String oldString = existingObs.getValueAsString(Context.getLocale());
String newString = newObs.getValueAsString(Context.getLocale());
if (log.isDebugEnabled() && concept != null) {
log.debug("For concept " + concept.getName(Context.getLocale()) + ": " + oldString + " -> " + newString);
}
boolean valueChanged = !newString.equals(oldString);
// TODO: handle dates that may equal encounter date
boolean dateChanged = dateChangedHelper(existingObs.getObsDatetime(), newObs.getObsDatetime());
boolean accessionNumberChanged = accessionNumberChangedHelper(existingObs.getAccessionNumber(),
newObs.getAccessionNumber());
boolean conceptsHaveChanged = false;
if (!existingObs.getConcept().getConceptId().equals(concept.getConceptId())) {
conceptsHaveChanged = true;
}
if (valueChanged || dateChanged || accessionNumberChanged || conceptsHaveChanged) {
if (log.isDebugEnabled()) {
log.debug("CHANGED: " + printObsHelper(existingObs));
}
// TODO: really the voided obs should link to the new one, but this is a pain to implement due to the dreaded error: org.hibernate.NonUniqueObjectException: a different object with the same identifier value was already associated with the session
obsToVoid.add(existingObs);

newObs = createObs(concept, newValue, newDatetime, accessionNumber, comment);
newObs.setPreviousVersion(existingObs);
} else {
if (existingObs != null && StringUtils.isNotBlank(comment))
existingObs.setComment(comment);

if (log.isDebugEnabled()) {
log.debug("SAME: " + printObsHelper(existingObs));
}
}
public void modifyObs(Obs existingObs, Concept concept, Object newValue, Date newDatetime, String accessionNumber) {
modifyObs(existingObs, concept, newValue, newDatetime, accessionNumber, null, null);
}

/**
Expand Down
71 changes: 35 additions & 36 deletions api/src/main/java/org/openmrs/module/htmlformentry/ObsGroupComponent.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ public class ObsGroupComponent {

private Drug answerDrug;

private String controlId = null;

private Boolean partOfSet = false;

private Boolean lastInSet = false;
Expand All @@ -40,21 +42,25 @@ public class ObsGroupComponent {
public ObsGroupComponent() {
}

public ObsGroupComponent(Concept question, Concept answer) {
public ObsGroupComponent(Concept question, Concept answer, String controlId) {
this.question = question;
this.answer = answer;
this.controlId = controlId;
}

public ObsGroupComponent(Concept question, Concept answer, Drug answerDrug) {
public ObsGroupComponent(Concept question, Concept answer, Drug answerDrug, String controlId) {
this.question = question;
this.answer = answer;
this.answerDrug = answerDrug;
this.controlId = controlId;
}

public ObsGroupComponent(Concept question, Concept answer, Drug answerDrug, Boolean partOfSet, Boolean lastInSet) {
public ObsGroupComponent(Concept question, Concept answer, Drug answerDrug, String controlId, Boolean partOfSet,
Boolean lastInSet) {
this.question = question;
this.answer = answer;
this.answerDrug = answerDrug;
this.controlId = controlId;
this.partOfSet = partOfSet;
this.lastInSet = lastInSet;
}
Expand Down Expand Up @@ -86,32 +92,7 @@ public Drug getAnswerDrug() {
public void setAnswerDrug(Drug answerDrug) {
this.answerDrug = answerDrug;
}

@Deprecated
public static boolean supports(List<ObsGroupComponent> questionsAndAnswers, Obs parentObs, Set<Obs> group) {
for (Obs obs : group) {
boolean match = false;
for (ObsGroupComponent test : questionsAndAnswers) {
boolean questionMatches = test.getQuestion().getConceptId().equals(obs.getConcept().getConceptId());
boolean answerMatches = test.getAnswer() == null || (obs.getValueCoded() != null
&& test.getAnswer().getConceptId().equals(obs.getValueCoded().getConceptId()));

if (questionMatches && !answerMatches) {
match = false;
break;
}

if (questionMatches && answerMatches) {
match = true;
}
}
if (!match) {
return false;
}
}
return true;
}


public static int supportingRank(List<ObsGroupComponent> obsGroupComponents, Set<Obs> obsSet) {
int rank = 0;

Expand All @@ -121,6 +102,13 @@ public static int supportingRank(List<ObsGroupComponent> obsGroupComponents, Set

// iterate though all form obs elements for obs group we are testing for a match against
for (ObsGroupComponent obsGroupComponent : obsGroupComponents) {

// if any one of the obs group components have a control id, and it matches the obs control id, just return 1000 to force a match
if (obsGroupComponent.getControlId() != null
&& obsGroupComponent.getControlId().equals(HtmlFormEntryUtil.getControlId(obs))) {
return 1000;
}

Concept groupComponentQuestion = obsGroupComponent.getQuestion();
if (groupComponentQuestion == null) {
// The correct error should be thrown with useful contextual information from ObsSubmissionElement:174
Expand Down Expand Up @@ -180,7 +168,7 @@ public static List<ObsGroupComponent> findQuestionsAndAnswersForGroup(String par
Pair<Concept, Concept> hiddenObs, Node node) {
List<ObsGroupComponent> ret = new ArrayList<ObsGroupComponent>();
if (hiddenObs != null) { // consider the hidden obs when making a match
ret.add(new ObsGroupComponent(hiddenObs.getKey(), hiddenObs.getValue(), null, false, false));
ret.add(new ObsGroupComponent(hiddenObs.getKey(), hiddenObs.getValue(), null, null, false, false));
}
findQuestionsAndAnswersForGroupHelper(parentGroupingConceptId, node, ret);
return ret;
Expand All @@ -195,6 +183,7 @@ private static void findQuestionsAndAnswersForGroupHelper(String parentGroupingC
Concept answer = null;
Drug answerDrug = null;
List<Concept> answersList = null;
String controlId = null;
NamedNodeMap attrs = node.getAttributes();
try {
String questionsStr = attrs.getNamedItem("conceptIds").getNodeValue();
Expand Down Expand Up @@ -250,6 +239,12 @@ private static void findQuestionsAndAnswersForGroupHelper(String parentGroupingC
}
}
}
try {
controlId = attrs.getNamedItem("controlId").getNodeValue();
}
catch (Exception ex) {
// this is fine
}

//determine whether or not the obs group parent of this obs is the obsGroup obs that we're looking at.
boolean thisObsInThisGroup = false;
Expand Down Expand Up @@ -278,25 +273,25 @@ private static void findQuestionsAndAnswersForGroupHelper(String parentGroupingC
while (i.hasNext()) {
Concept c = i.next();
// add all the answers as separate obs group components, flagging them all as part of a set, and flagging the last one as the last one in the set
obsGroupComponents.add(new ObsGroupComponent(question, c, null, true, !i.hasNext()));
obsGroupComponents.add(new ObsGroupComponent(question, c, null, controlId, true, !i.hasNext()));
}
} else if (questions != null && questions.size() > 0) {
Iterator<Concept> i = questions.iterator();
while (i.hasNext()) {
Concept c = i.next();
// add all the questions as separate obs group components, flagging them all as part of a set, and flagging the last one as the last one in the set
obsGroupComponents.add(new ObsGroupComponent(c, answer, null, true, !i.hasNext()));
obsGroupComponents.add(new ObsGroupComponent(c, answer, null, controlId, true, !i.hasNext()));
}
} else {
addToObsGroupComponentList(obsGroupComponents, question, answer, answerDrug);
addToObsGroupComponentList(obsGroupComponents, question, answer, answerDrug, controlId);
}
}
} else if ("obsgroup".equals(node.getNodeName())) {
try {
NamedNodeMap attrs = node.getAttributes();
attrs.getNamedItem("groupingConceptId").getNodeValue();
obsGroupComponents.add(new ObsGroupComponent(
HtmlFormEntryUtil.getConcept(attrs.getNamedItem("groupingConceptId").getNodeValue()), null));
HtmlFormEntryUtil.getConcept(attrs.getNamedItem("groupingConceptId").getNodeValue()), null, null));
}
catch (Exception ex) {
throw new RuntimeException("Unable to get groupingConcept out of obsgroup tag.");
Expand All @@ -310,7 +305,7 @@ private static void findQuestionsAndAnswersForGroupHelper(String parentGroupingC

// see: https://issues.openmrs.org/browse/HTML-806
private static void addToObsGroupComponentList(List<ObsGroupComponent> list, Concept question, Concept answer,
Drug answerDrug) {
Drug answerDrug, String controlId) {
boolean isSet = false;
// if there are any existing components with the same question, make sure they are flagged as part of set, but *not* the last one in the set
for (ObsGroupComponent component : list) {
Expand All @@ -321,7 +316,7 @@ private static void addToObsGroupComponentList(List<ObsGroupComponent> list, Con
}
}
// if existing components with the same question were found, flag this new component as part of a set, *and* the last element in the set
list.add(new ObsGroupComponent(question, answer, answerDrug, isSet, isSet));
list.add(new ObsGroupComponent(question, answer, answerDrug, controlId, isSet, isSet));
}

/**
Expand Down Expand Up @@ -378,4 +373,8 @@ public Boolean isLastInSet() {
public void setLastInSet(Boolean lastInSet) {
this.lastInSet = lastInSet;
}

public String getControlId() {
return controlId;
}
}
Loading

0 comments on commit 12d3f5a

Please sign in to comment.