EA-184 - Move reporting module to an aware_of dependency #229
Conversation
…g for dispositions to use obs groups
…ve all reporting dependencies into a new submodule
| <artifactId>emrapi-api-reporting</artifactId> | ||
| <packaging>jar</packaging> | ||
| <name>EMR API Reporting API</name> | ||
|
|
There was a problem hiding this comment.
Here, I create a new maven sub-module, which will result in a jar that can be conditionally loaded. All of the reporting-related dependencies are able to be moved in here.
| @Handler(supports = AwaitingAdmissionVisitQuery.class) | ||
| @OpenmrsProfile(modules = { "reporting:*" }) | ||
| public class AwaitingAdmissionVisitQueryEvaluator implements VisitQueryEvaluator { | ||
|
|
There was a problem hiding this comment.
The logic for retrieving visits awaiting admission was moved into the AdtService so that it can be used without relying on the reporting module. Then, the reporting module evaluator below was updated to call the implementation in the AdtService. None of the unit tests were changed, and all continue to pass.
| @@ -12,56 +12,13 @@ | |||
| <name>EMR API Module API</name> | |||
| <description>API project for EMRAPI</description> | |||
|
|
|||
| <dependencies> | |||
There was a problem hiding this comment.
This is residual cleanup from prior refactor, where these dependencies can live in the top level pom and used by all submodules.
| * @param visitIds - if non-null, only returns matches for visits with the given ids | ||
| * @return List<Visit></Visit> of the matching visits | ||
| */ | ||
| List<Visit> getVisitsAwaitingAdmission(Location location, Collection<Integer> patientIds, Collection<Integer> visitIds); |
There was a problem hiding this comment.
This is the new AdtService method that now contains the logic that was previously implemented in the reporting module
|
|
||
| import java.util.List; | ||
| import java.util.Map; | ||
|
|
There was a problem hiding this comment.
I was able to get rid of the EmrVisitDAO, which only really existed to execute hql queries, and replaced it with this new EmrApiDAO that provides more general purpose HQL execution methods. The logic that was in the EmrVisitDAO methods has been moved to the service layer.
| @@ -82,13 +75,13 @@ public enum SortOrder { | |||
| @Autowired | |||
| protected DispositionService dispositionService; | |||
|
|
|||
| @Autowired | |||
| protected VisitQueryService visitQueryService; | |||
|
|
|||
There was a problem hiding this comment.
I was able to remove usage of reporting module constructs here, and use the new Adt service method instead, so this no longer relies on the reporting module code.
| @@ -6,5 +6,5 @@ where | |||
| p.voided = 'false' | |||
| and p.patientId in(select distinct personId from Obs o | |||
| where | |||
There was a problem hiding this comment.
There were some errors / inconsistencies in some of these hql queries, where on one side of a condition it was looking for an id, and the other side an object. I changed them to consistently refer to objects rather than deconstruct to ids.
| @@ -0,0 +1,36 @@ | |||
| select | |||
There was a problem hiding this comment.
This is actually the main thing that was needed to move off of the reporting module dependency. I moved what was previously being done in a reporting module evaluator using the HqlQueryBuilder, and built a similar query here using HQL directly and the new EmrApiDAO methods that evaluate it.
| catch (Exception e) { | ||
| throw new RuntimeException(e); | ||
| } | ||
| } |
There was a problem hiding this comment.
This is to eliminate the use of a reporting module utility method
| @@ -13,18 +13,20 @@ | |||
| <description>OMOD project for EMRAPI</description> | |||
|
|
|||
| <dependencies> | |||
|
|
|||
| <dependency> | |||
There was a problem hiding this comment.
This is partially clean up of a prior commit that removed the condition-list submodule, and partially part of this new commit that adds the reporting api submodule jar to the omod.
| import static java.util.Collections.EMPTY_LIST; | ||
| import static java.util.Collections.reverseOrder; | ||
| import static java.util.Collections.sort; | ||
| import static java.util.Collections.*; |
There was a problem hiding this comment.
There was a problem hiding this comment.
Thanks @dkayiwa I'll fix. Annoying that intellij defaults to wildcards for static imports with > 3 usages.
| @@ -31,4 +33,13 @@ public ObsBuilder buildDiagnosis(Patient patient, String dateYmd, Diagnosis.Orde | |||
| } | |||
| return builder; | |||
| } | |||
|
|
|||
| private Date parseYmd(String ymd) { | |||
There was a problem hiding this comment.
Are we intentionally duplicating this?
There was a problem hiding this comment.
It might be easier to swap to using common-langs DateUtils#parseDate(), e.g., DateUtils.parseDate(dateYmd, "yyyy-MM-dd") should work. 😁
There was a problem hiding this comment.
Actually that's a different version of DateUtils that we have available and it doesn't really seem to buy anything, so I'm going to leave it as is.
|
This basically LGTM, but I'd fix the |
|
Belated LGTM @mseaton .... one thought looking at this, we are actually not super far from removing the need for reporting for Awaiting Admission altogether if we wanted to, right? (not suggesting we do so now). |
No description provided.