Change RoleResource from extending the MetadataDelegatingCrudResource class #575
Conversation
| * Subclass of {@link DelegatingCrudResource} with helper methods specific to | ||
| * {@link Role} | ||
| * | ||
| * @param <T> |
There was a problem hiding this comment.
What is the value of creating this new class?
There was a problem hiding this comment.
it didnt sit down well with me to for the RoleResource1_8 class extending directly from DelegatingCrudResource
There was a problem hiding this comment.
But still, it does not mean that you should create a new class.
There was a problem hiding this comment.
are you implying i should use DataDelegatingCrudResource instead? because thats the other option i had in mind.
or better yet, any suggestions of how i should go about it?
There was a problem hiding this comment.
i realized DataDelegatingCrudResource doesn't work for this case and none of the other BaseDelegatingCrudResource implementation classes did so i went with DelegatingCrudResource
|
@mherman22 will this run well within a platform that has these changes? openmrs/openmrs-core#4293 |
@dkayiwa the above quoted pullrequest needs to first be merged in core first. So I guess your reviews over there are highly welcome before we have it merged and then I can comfortably finish this one as well. Especially in regards to retirement of a role |
|
Have you tried to run a local platform with those changes and then deployed the rest webservices module with the changes in this pull request? |
i have deployed the two snapshots i got onto platform 2.6.0 and everything looks to be working in regards to requests Get role, Create role, Delete role and Update role. I think this is because some of the changes are still dependant on retire which is a metadata attribute |
|
|
@mherman22 with the changes in this pull request openmrs/openmrs-core#4293 it will no longer be platform 2.6.0 |
So how should i go about testing the two pull requests? FYI, these are distro.properties for the platform 2.6.0 local instance |
|
This is what i personally do. After making the changes in the platform, i simply go to the webapp folder and run |
@dkayiwa so after running this, i should upload the webservices module having these changes? i.e |
True dat! |
@dkayiwa i have added a comment at https://talk.openmrs.org/t/switching-from-xml-mappings-to-annotations/39277/8?u=mherman22 |
|
@dkayiwa please look through again and hopefully have it merged |
mherman22
force-pushed
the
RESTWS-910
branch
2 times, most recently
from
e8fa81e to
e4e2ec0
Compare
|
@dkayiwa pinging you on this! |
|
closing this because of the discussion on openmrs/openmrs-core#4293 that has led me to believe this is no longer needed |
Description of what I changed
I changed the Role resource from extending the metadataDelegatingCrudResource class and instead created a class called RoleDelegatingCrudResource that extends the DelegatingCrudResource class. So i ensured the Role resource class extends
RoleDelegatingCrudResourceIssue I worked on
see https://issues.openmrs.org/browse/RESTWS-910
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