openmrs-module-webservices.rest icon indicating copy to clipboard operation
openmrs-module-webservices.rest copied to clipboard

(fix) RESTWS-908 Diagnosis resource should return formNamespace and formPath on core 2.5+

Open suubi-joshua opened this issue 1 year ago • 10 comments

RESTWS-908 Diagnosis resource should return formNamespace and formPath on core 2.5+ RESTWS-908 I created a new submodule 2.5 and created a diagnosisResource class that extends the DiagnosisResource2_2 class in the 2.2 module. I added the formNamespace properties such that they can be returned.

Issue I worked on

see https://openmrs.atlassian.net/browse/RESTWS-908

Checklist: I completed these to help reviewers :)

  • [x] 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

suubi-joshua avatar May 03 '23 23:05 suubi-joshua

Thanks @ibacher let me work on these changes.

suubi-joshua avatar May 05 '23 09:05 suubi-joshua

Thanks for this! A few comments, but I don't think anything major is unexpected here. It might be good if we can expose a combined "formNamespaceAndPath" property (I believe there's a getter and setter for this already), as that is most likely to be useful.

@ibacher yes there is a getter and setter for the formNamespaceAndPath. I have added the property.

suubi-joshua avatar May 07 '23 09:05 suubi-joshua

Can you also fix the link to the issue?

dkayiwa avatar May 08 '23 10:05 dkayiwa

Can you also fix the link to the issue?

THANKS @dkayiwa I think am not failing to understand this advise? I beg your pardon please

suubi-joshua avatar May 08 '23 11:05 suubi-joshua

Can you also fix the link to the issue?

THANKS @dkayiwa I think am not failing to understand this advise? I beg your pardon please

Are you able to open the jira ticket link that you included at the top of this pull request?

dkayiwa avatar May 08 '23 13:05 dkayiwa

Can you also fix the link to the issue?

THANKS @dkayiwa I think am not failing to understand this advise? I beg your pardon please

Are you able to open the jira ticket link that you included at the top of this pull request? @dkayiwa Ohhh let me fix that sorry about that.

suubi-joshua avatar May 08 '23 13:05 suubi-joshua

My bad this is way overdue @dkayiwa @ibacher . Made some changes as requested.

suubi-joshua avatar Jul 09 '24 11:07 suubi-joshua

@suubi-joshua did you see the build failures?

dkayiwa avatar Jul 09 '24 12:07 dkayiwa

@suubi-joshua did you see the build failures?

Seeing them now. let me fix that. The Parent version was pointing at 2.40 I have changed it to point at 2.45

suubi-joshua avatar Jul 09 '24 12:07 suubi-joshua

The build should be successful now @dkayiwa . It was successful locally.

suubi-joshua avatar Jul 10 '24 07:07 suubi-joshua