Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TRUNK-6202 Replace Hibernate Criteria API with JPA for HibernateConceptDAO #4466

Merged
merged 1 commit into from
Dec 9, 2023

Conversation

k4pran
Copy link
Contributor

@k4pran k4pran commented Nov 28, 2023

Description of what I changed

This task is to migrate the deprecated hibernate criteria to jpa criteria for the HibernateConceptDAO class.

Additionally, I added some additional tests where I noticed gaps, I validated these tests with both the original hibernate code and the migrated JPA code.

There are a few changes I want to highlight where it wasn't a as direct translation from hibernate to JPA:

  • I noticed some of the case matching was not consistent, for example here the isDatabaseStringComparisonCaseSensitive seemed the wrong way around.

  • In JPA you can't order by a non-selected field in combination with distinct. In these cases I filtered by distinct outside of the query. An example of this can be seen here

Issue I worked on

see https://issues.openmrs.org/browse/TRUNK-6202

Checklist: I completed these to help reviewers :)

  • My IDE is configured to follow the code style of this project.

    No? Unsure? -> configure your IDE, format the code and add the changes with git add . && git commit --amend

  • I have added tests to cover my changes. (If you refactored
    existing code that was well tested you do not have to add tests)

    No? -> write tests and add them to this commit git add . && git commit --amend

  • I ran mvn clean package right before creating this pull request and
    added all formatting changes to my commit.

    No? -> execute above command

  • All new and existing tests passed.

    No? -> figure out why and add the fix to your commit. It is your responsibility to make sure your code works.

  • My pull request is based on the latest changes of the master branch.

    No? Unsure? -> execute command git pull --rebase upstream master

@k4pran k4pran force-pushed the TRUNK-6202-HibernateConceptDAO branch from 7958547 to 6b6069b Compare November 28, 2023 10:30
@k4pran k4pran force-pushed the TRUNK-6202-HibernateConceptDAO branch from 6b6069b to 0224a9b Compare November 28, 2023 11:37
@k4pran k4pran marked this pull request as ready for review December 6, 2023 16:51
@dkayiwa
Copy link
Member

dkayiwa commented Dec 7, 2023

@k4pran did you find it uncomfortable creating the smaller pull requests? 😊

@k4pran
Copy link
Contributor Author

k4pran commented Dec 8, 2023

@k4pran did you find it uncomfortable creating the smaller pull requests? 😊

Haha 😂. The plan was to do one DAO at a time to break them up but I guess this was a big DAO, I can try to break some of the larger DAOs into smaller PRs in the future but not sure if there is any natural way to divide them up except a few methods at a time?

@dkayiwa
Copy link
Member

dkayiwa commented Dec 9, 2023

I can try to break some of the larger DAOs into smaller PRs in the future but not sure if there is any natural way to divide them up except a few methods at a time?

No need to break larger DAOs. Let us go with what you have already done of one DAO at a time. 😊

@dkayiwa dkayiwa merged commit 461dc61 into openmrs:master Dec 9, 2023
6 checks passed
Wandji69 pushed a commit to Wandji69/openmrs-core that referenced this pull request Mar 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants