TRUNK-5888: Fix business logic errors in saveCondition() method (open…

…mrs#3632)

Co-authored-by: Jude Niroshan <[email protected]>

api/src/main/java/org/openmrs/Condition.java

@@ -9,9 +9,6 @@
  */
 package org.openmrs;
 
-import java.util.Date;
-import java.util.Objects;
-
 import javax.persistence.AssociationOverride;
 import javax.persistence.AssociationOverrides;
 import javax.persistence.AttributeOverride;
@@ -29,6 +26,8 @@
 import javax.persistence.Table;
 import javax.persistence.Transient;
 
+import java.util.Date;
+
 /**
  * The condition class records detailed information about a condition, problem, diagnosis, or other
  * situation or issue. This records information about a disease/illness identified from diagnosis or
@@ -352,5 +351,5 @@ public Encounter getEncounter() {
  public void setEncounter(Encounter encounter) {
  this.encounter = encounter;
  }
- 
+
 }

api/src/main/java/org/openmrs/api/ConditionService.java

@@ -24,25 +24,6 @@
  */
 public interface ConditionService extends OpenmrsService {
 
- /**
- * Saves a condition
- *
- * @param condition - the condition to be saved
- * @throws APIException 
- */
- @Authorized({ PrivilegeConstants.EDIT_CONDITIONS })
- Condition saveCondition(Condition condition) throws APIException;
-
- /**
- * Voids a condition
- *
- * @param condition - the condition to be voided
- * @param voidReason - the reason for voiding the condition
- * @throws APIException 
- */
- @Authorized({ PrivilegeConstants.EDIT_CONDITIONS })
- Condition voidCondition(Condition condition, String voidReason) throws APIException;
-
  /**
  * Gets a condition based on the uuid
  *
@@ -87,15 +68,34 @@ public interface ConditionService extends OpenmrsService {
  /**
  * Gets a condition by id
  *
- * @param conditionId - the id of the Condition to retrieve
+ * @param conditionId the id of the Condition to retrieve
  * @return the Condition with the given id, or null if none exists
  * @throws APIException
  */
  @Authorized({ PrivilegeConstants.GET_CONDITIONS })
  Condition getCondition(Integer conditionId) throws APIException;
 
  /**
- * Revive a condition (pull a Lazarus)
+ * Saves a condition
+ *
+ * @param condition - the condition to be saved
+ * @throws APIException
+ */
+ @Authorized({ PrivilegeConstants.EDIT_CONDITIONS })
+ Condition saveCondition(Condition condition) throws APIException;
+
+ /**
+ * Voids a condition
+ *
+ * @param condition the condition to be voided
+ * @param voidReason the reason for voiding the condition
+ * @throws APIException if an error occurs while voiding the condition
+ */
+ @Authorized({ PrivilegeConstants.EDIT_CONDITIONS })
+ Condition voidCondition(Condition condition, String voidReason) throws APIException;
+
+ /**
+ * Revive a previously voided condition
  *
  * @param condition Condition to unvoid
  * @throws APIException
@@ -116,5 +116,4 @@ public interface ConditionService extends OpenmrsService {
  */
  @Authorized(PrivilegeConstants.DELETE_CONDITIONS)
  void purgeCondition(Condition condition) throws APIException;
-
 }

api/src/main/java/org/openmrs/api/db/ConditionDAO.java

@@ -24,12 +24,12 @@
 public interface ConditionDAO {
 
  /**
- * Saves the condition.
+ * Gets a condition by id
  *
- * @param condition the condition to save.
- * @return the saved condition.
+ * @param conditionId the id of the condition to return
+ * @return the condition associated with the id
  */
- Condition saveCondition(Condition condition);
+ Condition getCondition(Integer conditionId);
 
  /**
  * Gets the condition attached to the specified UUID.
@@ -38,15 +38,7 @@ public interface ConditionDAO {
  * @return the condition associated with the UUID.
  */
  Condition getConditionByUuid(String uuid);
-
- /**
- * Gets all conditions related to the specified patient.
- *
- * @param patient the patient whose condition history is being queried.
- * @return all active and non active conditions related to the specified patient.
- */
- List<Condition> getConditionHistory(Patient patient);
-
+
  /**
  * Gets all active conditions related to the specified patient.
  *
@@ -56,22 +48,22 @@ public interface ConditionDAO {
  List<Condition> getActiveConditions(Patient patient);
 
  /**
- * @see ConditionService#getAllConditions(Patient)
+ * @see org.openmrs.api.ConditionService#getAllConditions(Patient)
  */
  List<Condition> getAllConditions(Patient patient);
 
  /**
- * @see ConditionService#getConditionsByEncounter(Encounter)
+ * @see org.openmrs.api.ConditionService#getConditionsByEncounter(Encounter)
  */
  List<Condition> getConditionsByEncounter(Encounter encounter) throws APIException;
 
  /**
- * Gets a condition by id
+ * Saves the condition.
  *
- * @param conditionId the id of the condition to return
- * @return the condition associated with the id
+ * @param condition the condition to save.
+ * @return the saved condition.
  */
- Condition getCondition(Integer conditionId);
+ Condition saveCondition(Condition condition);
 
  /**
  * Removes a condition from the database

api/src/main/java/org/openmrs/api/db/hibernate/HibernateConditionDAO.java

@@ -11,7 +11,7 @@
 
 import java.util.List;
 
-import org.hibernate.Query;
+import org.hibernate.query.Query;
 import org.hibernate.SessionFactory;
 import org.openmrs.Condition;
 import org.openmrs.Encounter;
@@ -41,18 +41,6 @@ public void setSessionFactory(SessionFactory sessionFactory) {
  this.sessionFactory = sessionFactory;
  }
 
- /**
- * Saves the condition.
- *
- * @param condition the condition to save.
- * @return the saved condition.
- */
- @Override
- public Condition saveCondition(Condition condition) {
- sessionFactory.getCurrentSession().saveOrUpdate(condition);
- return condition;
- }
-
  /**
  * Gets the condition with the specified id.
  *
@@ -61,7 +49,7 @@ public Condition saveCondition(Condition condition) {
  */
  @Override
  public Condition getCondition(Integer conditionId) {
- return (Condition) sessionFactory.getCurrentSession().get(Condition.class, conditionId);
+ return sessionFactory.getCurrentSession().get(Condition.class, conditionId);
  }
 
  /**
@@ -72,22 +60,19 @@ public Condition getCondition(Integer conditionId) {
  */
  @Override
  public Condition getConditionByUuid(String uuid) {
- return (Condition) sessionFactory.getCurrentSession().createQuery("from Condition c where c.uuid = :uuid")
- .setString("uuid", uuid).uniqueResult();
+ return sessionFactory.getCurrentSession().createQuery("from Condition c where c.uuid = :uuid", Condition.class)
+ .setParameter("uuid", uuid).uniqueResult();
  }
- 
+
  /**
- * Gets all conditions related to the specified patient.
- *
- * @param patient the patient whose condition history is being queried.
- * @return all active and non active conditions related to the specified patient.
+ * @see org.openmrs.api.ConditionService#getConditionsByEncounter(Encounter)
  */
  @Override
- public List<Condition> getConditionHistory(Patient patient) {
- Query query = sessionFactory.getCurrentSession().createQuery(
-  "from Condition con where con.patient.patientId = :patientId and con.voided = false " +
-  "order by con.onsetDate desc");
- query.setInteger("patientId", patient.getId());
+ public List<Condition> getConditionsByEncounter(Encounter encounter) throws APIException {
+ Query<Condition> query = sessionFactory.getCurrentSession().createQuery(
+ "from Condition c where c.encounter.encounterId = :encounterId and c.voided = false order "
+ + "by c.dateCreated desc", Condition.class);
+ query.setParameter("encounterId", encounter.getId());
  return query.list();
  }
 
@@ -99,19 +84,23 @@ public List<Condition> getConditionHistory(Patient patient) {
  */
  @Override
  public List<Condition> getActiveConditions(Patient patient) {
- Query query = sessionFactory.getCurrentSession().createQuery(
- "from Condition c where c.patient.patientId = :patientId and c.voided = false and c.endDate is null order "
- + "by c.onsetDate desc");
- query.setInteger("patientId", patient.getId());
+ Query<Condition> query = sessionFactory.getCurrentSession().createQuery(
+ "from Condition c where c.patient.patientId = :patientId and c.clinicalStatus = 'ACTIVE' and c.voided = false order "
+ + "by c.dateCreated desc", Condition.class);
+ query.setParameter("patientId", patient.getId());
  return query.list();
  }
 
  /**
- * @see ConditionService#getAllConditions(Patient)
+ * @see org.openmrs.api.ConditionService#getAllConditions(Patient)
  */
  @Override
  public List<Condition> getAllConditions(Patient patient) {
- return this.getConditionHistory(patient);
+ Query<Condition> query = sessionFactory.getCurrentSession().createQuery(
+ "from Condition con where con.patient.patientId = :patientId " +
+ "order by con.dateCreated desc", Condition.class);
+ query.setParameter("patientId", patient.getId());
+ return query.list();
  }
 
  /**
@@ -125,14 +114,14 @@ public void deleteCondition(Condition condition) throws DAOException {
  }
 
  /**
- * @see ConditionService#getConditionsByEncounter(Encounter)
+ * Saves the condition.
+ *
+ * @param condition the condition to save.
+ * @return the saved condition.
  */
  @Override
- public List<Condition> getConditionsByEncounter(Encounter encounter) throws APIException {
- Query query = sessionFactory.getCurrentSession().createQuery(
- "from Condition c where c.encounter.encounterId = :encounterId and c.voided = false order "
- + "by c.onsetDate desc");
- query.setInteger("encounterId", encounter.getId());
- return query.list();
+ public Condition saveCondition(Condition condition) {
+ sessionFactory.getCurrentSession().saveOrUpdate(condition);
+ return condition;
  }
 }