Update the module to Platform 3.x#221
Merged
dkayiwa merged 25 commits intoopenmrs:masterfrom Feb 24, 2026
Merged
Conversation
Member
Author
wikumChamith
force-pushed
the
3.x-new
branch
2 times, most recently
from
947106b to
93606dd
Compare
dkayiwa
reviewed
Oct 3, 2025
| </property> | ||
| </bean> | ||
|
|
||
| <bean parent="obsServiceTarget" > |
Member
There was a problem hiding this comment.
Do you mind explaining why we removed this?
Member
Author
There was a problem hiding this comment.
Having this causes an error.
Caused by: org.springframework.beans.factory.NoSuchBeanDefinitionException: No bean named 'obsServiceTarget' available
dkayiwa
reviewed
Oct 3, 2025
| <!-- Add here beans related to the web context --> | ||
|
|
||
| <bean id="legacyUiUrlMapping" class="org.springframework.web.servlet.handler.SimpleUrlHandlerMapping"> | ||
| <property name="patternParser"><null/></property> |
Member
There was a problem hiding this comment.
Why add it if the value is null?
Member
Author
There was a problem hiding this comment.
Caused by: org.springframework.web.util.pattern.PatternParseException: No more pattern data allowed after {*...} or ** pattern element
This is a hack I used to avoid PatternParseException.
Member
So what's happening is that there are two "org.springframework.web.servlet.mvc.SimpleControllerHandlerAdapter" beans registered in the context. One we explicitly register here and one that is implicitly registered here by the use of the The bean we explicitly create gets named I'm not entirely sure what happened, but this commit is part of Spring 6 and it will just remove the implicit alias of So a couple of things suggest themselves:
Personally I think 3 is the cleanest thing, but may require more tweaks, e.g., to ensure that the new |
Member
Author
@ibacher I tried both 1 and 3. However, I am still seeing the error. Here are the PRs for them. |
Member
|
Well, I stupidly assumed that the XML stuff wouldn't interfere with the |
Member
|
Anyways, @wikumChamith I added a commit here and to openmrs/openmrs-core#5374 that fixes that issue in a way I'm happy with. There are still some test failures but they aren't caused by this issue. Btw, we should take every opportunity available to dump XML Spring configurations if possible. There were some issues with the services wrapped in |
Member
Author
|
PR to fix test errors on ChangePasswordFormControllerTest: openmrs/openmrs-core#5394 |
Member
|
I put a comment on that pull request. |
Member
Author
Member
|
@wikumChamith did you eventually figure this out? |
Member
|
@wikumChamith in that failing test, try changing |
Member
Author
|
@dkayiwa, @ibacher the module is compiling now, but I’m seeing an error when I run the module: To resolve this, I removed the taglib dependencies from core and used org.glassfish.web:jakarta.servlet.jsp.jstl in this module. However, I’m still seeing the same error. We’re currently using the JstlCoreTLV validator, but since it now uses Jakarta EE, removing it didn’t fix the issue either. Is there anywhere else that might still be using a javax-era taglib that I may have missed? |
Member
|
@wikumChamith did you figure this out? |
Member
Author
Nope, still trying to sort it out. Any suggestions?? |
dkayiwa
reviewed
Oct 28, 2025
| Concept concept = cs.getConcept(5497); | ||
| //sanity check, the current preferred Name should be different from what will get set in the form | ||
| Assert.assertNotSame("CD3+CD4+ABS CNT", concept.getPreferredName(britishEn).getName()); | ||
Member
There was a problem hiding this comment.
Is there a reason why you did not use a different pull request for these formatting changes?
dkayiwa
reviewed
Oct 28, 2025
| ProgramFormController controller = (ProgramFormController) applicationContext.getBean("programForm"); | ||
| controller.handleRequest(request, new MockHttpServletResponse()); | ||
|
|
||
| Context.clearSession(); |
Member
There was a problem hiding this comment.
What failure is the above addressing?
Member
|
@wikumChamith because |
Member
Author
@dkayiwa, that also gives a similar error: I also tried deleting all the |
Member
|
Even after removing the validator? |
Member
Author
Yes |
Member
|
Can you open a pull request for the changes you did in core for this? |
Member
Author
|
Member
|
How are you running it? I seem to be getting a different error: |
Member
Author
I tried running it with both the SDK and |
… for Jakarta EE compatibility.
…d centralize fallback logic.
wikumChamith
force-pushed
the
3.x-new
branch
from
fa602f6 to
d2d1005
Compare
Member
Author
|
@dkayiwa @ibacher, there’s an issue. It looks like Spring 7 has removed
What do you recommend? |
Member
|
My preference is to go with the Spring recommendation because i have really not seen people using the dynamic theme switching. |
Member
|
@wikumChamith how is this going? |
Member
Author
|
@dkayiwa I fixed the build errors. |
dkayiwa
reviewed
Feb 18, 2026
api/pom.xml
Outdated
| <groupId>org.openmrs.module</groupId> | ||
| <artifactId>legacyui</artifactId> | ||
| <version>2.1.0-SNAPSHOT</version> | ||
| <version>2.0.0-SNAPSHOT</version> |
Member
There was a problem hiding this comment.
Shouldn't this be 3.0.0-SNAPSHOT because it will not run on any previous version of the platform?
Member
Author
There was a problem hiding this comment.
Bumped the version to 3.0.0-SNAPSHOT
dkayiwa
reviewed
Feb 18, 2026
|
|
||
| ConceptFormController conceptFormController = (ConceptFormController) applicationContext.getBean("conceptForm"); | ||
|
|
||
| MockHttpServletRequest mockRequest = new MockHttpServletRequest(); |
Member
There was a problem hiding this comment.
What is all this formatting about?
dkayiwa
reviewed
Feb 18, 2026
|
|
||
| } | ||
| final String EXPECTED_PREFERRED_NAME = "no such concept"; | ||
| final String EXPECTED_SHORT_NAME = "nonesuch"; |
Member
There was a problem hiding this comment.
Lots of formatting going on here. Can we use a separate pull request for it?
Member
|
@wikumChamith did you also test and confirm that the module can now start in core 3.0.0-SNAPSHOT? |
Member
Author
|
@dkayiwa I’m getting this error with core 3.0.0-SNAPSHOT even without this module:
I think this is caused by the Spring 7 upgrade. It looks like Spring 7 has removed |
Member
Author
Member
|
@wikumChamith do you wanna fix it? :) |
Member
Author
Sure. I'll take a look |
Member
|
@wikumChamith Ok, I finally made it back this far. You're right that |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.