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

Adds Nurse user type and promote existing staffs to nurse #1589

Closed
wants to merge 3 commits into from

Conversation

aeswibon
Copy link
Member

@aeswibon aeswibon commented Sep 9, 2023

Proposed Changes

  • Renamed "Staff" and "StaffReadOnly" to "Nurse" and "NurseReadOnly"
  • Added new roles: "Staff" and "StaffReadOnly"

Associated Issue

Merge Checklist

  • Tests added/fixed
  • Update docs in /docs
  • Linting Complete
  • Any other necessary step

Only PR's with test cases included and passing lint and test pipelines will be reviewed

@coronasafe/care-backend-maintainers @coronasafe/care-backend-admins

care/users/models.py Outdated Show resolved Hide resolved
Copy link
Member

@rithviknishad rithviknishad left a comment

Choose a reason for hiding this comment

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

  • You should rename the existing Staff type to Nurse type (10, "Nurse") and then create a new role for Staff. Effective order should be: 7: StaffReadOnly; 8: Staff; 9: NurseReadOnly; 10: Nurse

  • The permissions for Nurse and Staff should be the same for now. However, in a later PR we can downgrade the permissions of Staff to have access only to Admin / Facility data and not Patient Data.

@aeswibon aeswibon force-pushed the issue#6240 branch 2 times, most recently from 574db3f to 8438a22 Compare September 26, 2023 14:46
@codecov
Copy link

codecov bot commented Sep 26, 2023

Codecov Report

Attention: 54 lines in your changes are missing coverage. Please review.

Comparison is base (337d37d) 61.24% compared to head (3c6de8e) 61.33%.

Files Patch % Lines
care/facility/models/mixins/permissions/patient.py 11.76% 11 Missing and 4 partials ⚠️
...are/facility/models/mixins/permissions/facility.py 57.89% 7 Missing and 1 partial ⚠️
care/facility/models/daily_round.py 14.28% 4 Missing and 2 partials ⚠️
...are/facility/api/viewsets/patient_investigation.py 44.44% 5 Missing ⚠️
care/facility/api/serializers/patient_sample.py 20.00% 4 Missing ⚠️
care/facility/api/viewsets/patient_sample.py 0.00% 4 Missing ⚠️
care/facility/api/viewsets/file_upload.py 50.00% 3 Missing ⚠️
care/facility/models/mixins/permissions/base.py 25.00% 2 Missing and 1 partial ⚠️
care/facility/api/viewsets/notification.py 33.33% 2 Missing ⚠️
care/facility/api/viewsets/shifting.py 33.33% 2 Missing ⚠️
... and 2 more
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1589      +/-   ##
==========================================
+ Coverage   61.24%   61.33%   +0.08%     
==========================================
  Files         208      208              
  Lines       11480    11496      +16     
  Branches     1650     1650              
==========================================
+ Hits         7031     7051      +20     
+ Misses       4177     4169       -8     
- Partials      272      276       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@nihal467
Copy link
Member

nihal467 commented Oct 18, 2023

tested locally looks good to me

@rithviknishad rithviknishad changed the title fix: adds new user type "Nurse" Adds Nurse user type and promote existing staffs to nurse Oct 19, 2023
@rithviknishad
Copy link
Member

@cp-coder the tests are failing

@nihal467
Copy link
Member

@cp-coder clear the merge conflict

Copy link
Member

@rithviknishad rithviknishad left a comment

Choose a reason for hiding this comment

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

For everywhere that authzn checks are done at serializer validate: Why are Authz checks done in the serializer validate? Shouldn't this be done by DRY Permissions?

care/facility/api/serializers/file_upload.py Outdated Show resolved Hide resolved
care/facility/api/serializers/patient_external_test.py Outdated Show resolved Hide resolved
care/facility/api/serializers/patient_external_test.py Outdated Show resolved Hide resolved
@nihal467
Copy link
Member

@aeswibon

Copy link
Member

@rithviknishad rithviknishad left a comment

Choose a reason for hiding this comment

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

tests and lint checks are failing. also one of the earlier review is still not resolved

@aeswibon aeswibon force-pushed the issue#6240 branch 2 times, most recently from aa1eade to 404b811 Compare December 15, 2023 03:14
@aeswibon
Copy link
Member Author

The PR is good for testing

@sainak
Copy link
Member

sainak commented Jan 10, 2024

closing in favor of #1819

@sainak sainak closed this Jan 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants