Skip to content
This repository has been archived by the owner on Feb 1, 2024. It is now read-only.

Change facility history API endpoint switch to a flag activated via group #881

Merged
merged 7 commits into from Oct 25, 2019

Conversation

kellyi
Copy link
Contributor

@kellyi kellyi commented Oct 17, 2019

Overview

Change the facility history API endpoint feature flagging mechanism from a waffle-switch to a waffle-flag which restricts access based on whether the user is a member of a can_get_facility_history Group.

Update tests and views to replace switch with flag, adding tests to check that the endpoint works as expected for various levels of authorization.

Un-hide facility history API endpoint from Swagger docs, as it will now be public but permission-ed.

Add a data migration to delete the previous facility_history switch.

Update django-admin to display Group objects.

Connects #875

Notes

Due to an override we have made to the django-admin User object interactions, there is as yet no mechanism for assigning users to groups outside of the command line. I don't know what the preferred way to handle this is, but two options are:

  • update the custom User interface we wrote for the django-admin to allow adding users to groups
  • add a section to the React dashboard that allows adding users to groups

Testing Instructions

  • get branch, then ./scripts/resetdb
  • ./scripts/test and verify that they pass
  • sign in as c1@example then use the history endpoint and verify that you get a 200 response
  • sign in as c10@example then use the history endpoint and verify that you get a 403 response

Checklist

  • fixup! commits have been squashed
  • CI passes after rebase
  • CHANGELOG.md updated with summary of features or fixes, following Keep a Changelog guidelines

Copy link
Contributor

@jwalgran jwalgran left a comment

Choose a reason for hiding this comment

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

I verified that this works properly by first confirming that c2@example.com could not use the history endpoint, then added them to the group and confirmed that the history endpoint began working.

>>> u = User.objects.get(email='c2@example.com')
>>> g = Group.objects.get(name='can_get_facility_history')
>>> g.user_set.add(u)

I pushed up an additional commit that removes from scripts/resetdb the attempt to set the now deleted history flag.

@rajadain rajadain force-pushed the ki/change-history-switch-to-flag branch from f3b303f to 246bf5d Compare October 24, 2019 17:50
@rajadain
Copy link
Contributor

Rebased on develop to fix conflicts. Doing a final review, will merge after.

@jwalgran
Copy link
Contributor

Please hold off on merging until after the 2.16.0 release is complete to prevent a staging deploy.

@rajadain rajadain force-pushed the ki/change-history-switch-to-flag branch from 246bf5d to 38508d0 Compare October 24, 2019 18:21
Kelly Innes and others added 7 commits October 24, 2019 14:52
Add a data migration to add `can_get_facility_history` flag and group.
Update facility history endpoint tests to use the
`can_get_facility_history` feature flag.

Update facility history endpoint code to replace switch with flag.

Replace endpoint decorator with custom flag_is_active check to
circumvent a quirk with using waffle & DRF documented here:

jazzband/django-waffle#221
Add tests to check the following history endpoint auth cases:

- superuser receives 200
- not-signed-in-user receives a 401
- not-permitted-user receives a 403
- permitted user receives 200
This switch was replaced with a flag that is tied to a user group.
@jwalgran jwalgran force-pushed the ki/change-history-switch-to-flag branch from 38508d0 to 391f3fd Compare October 24, 2019 21:54
@jwalgran jwalgran merged commit 430a524 into develop Oct 25, 2019
@kellyi
Copy link
Contributor Author

kellyi commented Oct 25, 2019

🏄

@rajadain rajadain deleted the ki/change-history-switch-to-flag branch October 25, 2019 18:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants