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

FM2-136 : Appointment resource initial implementation #136

Closed
wants to merge 1 commit into from
Closed

FM2-136 : Appointment resource initial implementation #136

wants to merge 1 commit into from

Conversation

jecihjoy
Copy link
Member

@jecihjoy jecihjoy commented Apr 6, 2020

Description of what I changed

Added an initial implementation for appointment resource

Issue I worked on

see https://issues.openmrs.org/browse/FM2-136

Checklist: I completed these to help reviewers :)

  • My pull request only contains ONE single commit.

    No? -> read here on how to squash multiple commits into one

  • 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

@codecov
Copy link

codecov bot commented Apr 6, 2020

Codecov Report

Merging #136 into master will increase coverage by 0.03%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #136      +/-   ##
============================================
+ Coverage     90.40%   90.43%   +0.03%     
- Complexity     1206     1212       +6     
============================================
  Files           128      130       +2     
  Lines          2834     2843       +9     
  Branches        353      354       +1     
============================================
+ Hits           2562     2571       +9     
  Misses          147      147              
  Partials        125      125              
Impacted Files Coverage Δ Complexity Δ
...ule/fhir2/api/impl/FhirAppointmentServiceImpl.java 100.00% <100.00%> (ø) 2.00 <2.00> (?)
...ir2/providers/AppointmentFhirResourceProvider.java 100.00% <100.00%> (ø) 5.00 <5.00> (?)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cdeef45...72ab3ec. Read the comment docs.

@mozzy11
Copy link
Member

mozzy11 commented Feb 4, 2021

well done @jecihjoy ,
will you implement other Operations like Update ,Search etc in a different PR ??

Copy link
Member

@sherrif10 sherrif10 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can however be merged to free this PR , And it will leverage code workflow and tracking, Am thinking since this is the initial implementation it would be more clear to track different changes that may come along, cc @mozzy11 we also need to track mappings for appointmentResources FYI cc @ibacher @mozzy11

@corneliouzbett
Copy link
Member

This can however be merged to free this PR , And it will leverage code workflow and tracking, Am thinking since this is the initial implementation it would be more clear to track different changes that may come along, cc @mozzy11 we also need to track mappings for appointmentResources FYI cc @ibacher @mozzy11

Hello @sherrif10
There was a decision made in one of the FHIR squad meetings, suggesting this kind of implementation be implemented in the dependant module i.e this code should be moved to bahmni appointments module. Oh! forgot there's already a PR for this here. Perhaps, we should be closing this PR.

@sherrif10
Copy link
Member

@corneliouzbett , Thanks for the updates will follow up with the pr you shared

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants