Update to Platform 2.7.3 and Support Java 21 by wikumChamith · Pull Request #13 · openmrs/openmrs-module-calculation

wikumChamith · 2025-03-26T14:02:28Z

https://openmrs.atlassian.net/browse/CALC-58

dkayiwa · 2025-03-26T18:26:28Z

omod/pom.xml

+		<dependency>
+			<groupId>org.openmrs.module</groupId>
+			<artifactId>legacyui-omod</artifactId>
+			<version>1.22.0</version>


Shouldn't we add scope?

dkayiwa · 2025-03-26T18:27:02Z

api/pom.xml

-			<artifactId>groovy-all</artifactId>
+			<groupId>org.openmrs.module</groupId>
+			<artifactId>reportingcompatibility-api</artifactId>
+			<version>2.0.6</version>


Shouldn't we add scope?

I added the scope

Looking at this again, do you mean the calculation module depends on the reportingcompatibility module?

PatientSetService was removed in OpenMRS Platform 2.0.0, so I used the reporting compatibility module to replace it.

Replaced:

Context.getPatientSetService().getObservations(new Cohort(cohort), whichConcept);

with

Context.getService(ReportingCompatibilityService.class).getObservations(new Cohort(cohort), whichConcept);

https://openmrs.atlassian.net/browse/TRUNK-4874
https://talk.openmrs.org/t/we-should-remove-patientsetservice-in-openmrs-core-in-platform-2-0/6173

The calculation module is supposed to be very light weight and hence should not depend on the reporting compatibility module. The reportingcompatibilty module is being deprecated.

@dkayiwa I removed the reporting compatibility module from this

dkayiwa · 2025-03-27T08:00:05Z

We are also going to bump the major version on the master branch and create a new branch for older versions of the platform.

wikumChamith · 2025-03-27T08:29:19Z

We are also going to bump the major version on the master branch and create a new branch for older versions of the platform.

should we create a separate PR for that?

dkayiwa · 2025-03-27T08:54:45Z

We are also going to bump the major version on the master branch and create a new branch for older versions of the platform.

should we create a separate PR for that?

We can do the version bump as part of this pull request. What do you think about it?

wikumChamith · 2025-03-27T09:39:50Z

We are also going to bump the major version on the master branch and create a new branch for older versions of the platform.

should we create a separate PR for that?

We can do the version bump as part of this pull request. What do you think about it?

I agree. What is the name I should give for the legacy branch? For now, I created a branch called master-2.x. We could change the name if you don't like it.

https://github.com/openmrs/openmrs-module-calculation/tree/master-2.x

dkayiwa · 2025-03-27T10:23:29Z

Can you follow the same branch naming convention as we have done before?

wikumChamith · 2025-03-27T10:29:59Z

I updated the name to 1.x

dkayiwa · 2025-03-29T22:59:04Z

api/src/test/java/org/openmrs/calculation/MostRecentObsCalculation.java

-}
+
+	private Map<Integer, List<Obs>> getObservations(Cohort cohort, Concept concept) {
+		if (cohort == null || concept == null || cohort.getMemberIds().isEmpty()) {


Isn't getMemberIds deprecated?

@dkayiwa I updated this.

dkayiwa · 2025-03-30T21:28:08Z

pom.xml

-			</dependency>
-
-			<dependency>
-				<groupId>org.codehaus.groovy</groupId>


Is there a reason why you changed it from org.codehaus.groovy?

Yes, org.codehaus.groovy has been relocated to org.apache.groovy. https://mvnrepository.com/artifact/org.codehaus.groovy/groovy-all

Which one are we using in platform 2.7.3?

org.codehaus.groovy:2.4.21. Should I update it there or remove it from here?

https://github.com/openmrs/openmrs-core/blob/02babbcd3d63f717d3f1a118e790b5b957dfa11d/api/pom.xml#L244-L248

You can update the one in core

Can you also update the version?

dkayiwa · 2025-03-30T21:33:49Z

pom.xml

+				<groupId>org.apache.groovy</groupId>
 				<artifactId>groovy-all</artifactId>
 				<version>${groovyVersion}</version>
-				<type>jar</type>


The same goes for changing jar to pom

https://mvnrepository.com/artifact/org.codehaus.groovy/groovy-all/3.0.24

pom.xml

dkayiwa · 2025-03-31T13:43:04Z

api/src/test/java/org/openmrs/calculation/MostRecentObsCalculation.java

+			return Collections.emptyMap();
+		}
+
+		return cohort.getMemberships().stream()


I find this to be a very inefficient replacement when compared to the original implementation: https://github.com/openmrs/openmrs-core/blob/1.12.x/api/src/main/java/org/openmrs/api/db/hibernate/HibernatePatientSetDAO.java#L1092-L1141

should we update this to do a Hibernate query?

Whatever you find better than the current code.

@dkayiwa, does performance matter much in this case since it's only used in tests?

That is a very good point and you are indeed correct. 😊

dkayiwa · 2025-06-09T12:53:15Z

@wikumChamith can you add the rest of the Java versions?

wikumChamith requested a review from dkayiwa March 26, 2025 14:02

wikumChamith force-pushed the CALC-58 branch 2 times, most recently from e3137f8 to d7d5494 Compare March 26, 2025 14:04

dkayiwa reviewed Mar 26, 2025

View reviewed changes

wikumChamith force-pushed the CALC-58 branch from d7d5494 to ef15ddf Compare March 27, 2025 05:36

wikumChamith requested a review from dkayiwa March 27, 2025 05:37

wikumChamith force-pushed the CALC-58 branch from ef15ddf to 9b0c30a Compare March 27, 2025 08:25

wikumChamith force-pushed the CALC-58 branch from 9b0c30a to bda9d52 Compare March 27, 2025 09:43

wikumChamith force-pushed the CALC-58 branch from bda9d52 to 4167d04 Compare March 28, 2025 12:47

dkayiwa reviewed Mar 29, 2025

View reviewed changes

dkayiwa reviewed Mar 30, 2025

View reviewed changes

wikumChamith force-pushed the CALC-58 branch 2 times, most recently from 28b876b to bf8b10d Compare March 31, 2025 13:18

dkayiwa reviewed Mar 31, 2025

View reviewed changes

pom.xml Show resolved Hide resolved

Update to platform 2.7.3

b88224d

wikumChamith force-pushed the CALC-58 branch from bf8b10d to b88224d Compare March 31, 2025 13:25

dkayiwa reviewed Mar 31, 2025

View reviewed changes

dkayiwa merged commit 58a8541 into openmrs:master Apr 1, 2025
1 check passed

Conversation

wikumChamith commented Mar 26, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dkayiwa commented Mar 27, 2025

Uh oh!

wikumChamith commented Mar 27, 2025

Uh oh!

dkayiwa commented Mar 27, 2025

Uh oh!

wikumChamith commented Mar 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dkayiwa commented Mar 27, 2025

Uh oh!

wikumChamith commented Mar 27, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dkayiwa commented Jun 9, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

wikumChamith commented Mar 27, 2025 •

edited

Loading