TRUNK-5900: Migrate Hibernate Mapping to Annotation #5010
Conversation
|
@dkayiwa @ibacher @vasharma05 |
|
Am requesting 4 a review please @dkayiwa @ibacher @vasharma05 |
| private String locale; | ||
|
|
||
| @Column(name = "uuid", nullable = false, unique = true, length = 38) | ||
| private String uuid; |
There was a problem hiding this comment.
This is inherited from the base object.
There was a problem hiding this comment.
Thanks @ibacher, you're right — uuid is inherited from BaseOpenmrsObject. I'll remove the duplicate declaration
| @Column(name = "locale", nullable = false) | ||
| private String locale; |
There was a problem hiding this comment.
I don't understand why migrating to Hibernate annotations requires that we add properties. That sort of thing should be moved to a separate PR.
| public String getUuid() { | ||
| return uuid; | ||
| } | ||
|
|
||
| public void setUuid(String uuid) { | ||
| this.uuid = uuid; | ||
| } |
There was a problem hiding this comment.
Good point, thanks! I'll remove the duplicate uuid methods since they’re inherited already
| * Convenience method to mark this proposal as rejected. Be sure to call | ||
| * Context.getConceptService().saveConceptProposal(this) after calling this method | ||
| */ | ||
| public void rejectConceptProposal() { |
| public String toString() { | ||
| if (conceptProposalId == null) { | ||
| return ""; | ||
| } | ||
| return conceptProposalId.toString(); | ||
| } |
| <mapping resource="org/openmrs/api/db/hibernate/ConceptClass.hbm.xml" /> | ||
| <mapping resource="org/openmrs/api/db/hibernate/ConceptDatatype.hbm.xml" /> | ||
| <mapping resource="org/openmrs/api/db/hibernate/ConceptProposal.hbm.xml" /> | ||
| <mapping class="org.openmrs.ConceptProposal"/> |
There was a problem hiding this comment.
This isn't how we've handled any other of the migrations. Please look at similar tickets for how this is done.
| <concept_proposal concept_proposal_id="8" original_text="not married" state="UNMAPPED" encounter_id="3" obs_concept_id="21" creator="1" date_created="2009-06-03 13:55:12.0" uuid="87a68666-5067-11de-80cb-001e378eb67e"/> | ||
| <concept_proposal concept_proposal_id="9" original_text="not married" state="UNMAPPED" encounter_id="4" obs_concept_id="4" creator="1" date_created="2009-06-03 13:55:12.0" uuid="97a68666-5067-11de-80cb-001e378eb67e"/> | ||
| <concept_proposal concept_proposal_id="10" original_text="single" state="UNMAPPED" encounter_id="3" obs_concept_id="4" creator="1" date_created="2009-06-03 13:55:12.0" uuid="88a68666-5067-11de-80cb-001e378eb67e"/> | ||
| <concept_proposal concept_proposal_id="5" original_text="not married" state="UNMAPPED" encounter_id="3" obs_concept_id="4" creator="1" date_created="2009-06-03 13:55:12.0" uuid="67a68666-5067-11de-80cb-001e378eb67e" locale="en"/> |
There was a problem hiding this comment.
Changing to Hibernate Annotations should not require changes to test data. That's a sign this PR is doing more than it should. PRs should be focused on a minimal set of changes that belong together so that we can back them out with minimal disruption.
There was a problem hiding this comment.
Thanks for pointing that out! I’ll revert the test data changes to keep this PR focused strictly on the annotations migration
snowvirus
force-pushed
the
TRUNK-5900-conceptproposal-annotations
branch
from
5b9fdf3 to
eab865d
Compare
2 participants
Description of the PR
This PR refactors the ConceptProposal class to use JPA annotations, replacing the previous Hibernate XML mapping. The changes include:
Annotated ConceptProposal.java with JPA annotations like @entity, @table, @id, @manytoone, etc.
Removed ConceptProposal.hbm.xml
Updated hibernate.cfg.xml to remove reference to the XML mapping
--- Adjusted related test datasets and test cases:
--standardTestDataset.xml
--standardTest-1.9.7-dataSet.xml
--ConceptServiceTest-proposals.xml
Why.
Issue I worked on
see jira_issue
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 --amendI 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 --amendI ran
mvn clean packageright before creating this pull request andadded 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