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

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;
 	}
 }