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

Feature: Modify User Status Via API #34557

Merged
merged 5 commits into from May 21, 2024

Conversation

szymonbalasz
Copy link
Contributor

@szymonbalasz szymonbalasz commented May 6, 2024

Product Description

This change allows activating and deactivating mobile workers via the CommCare HQ API. It enables toggling the active status of mobile worker accounts through API endpoints.

Technical Summary

The changes allow activating and deactivating mobile workers through the CommCare HQ API by adding new endpoints to the UserResource API resource:

  1. POST /api/v0.5/user/{user_id}/activate/: Activates the specified mobile worker
  2. POST /api/v0.5/user/{user_id}/deactivate/: Deactivates the specified mobile worker

The implementation leverages existing code patterns and extracts shared logic into reusable methods. Permissions checks ensure only users with appropriate privileges can perform these actions.
Two new tests were added in corehq/apps/api/tests/user_resources.py to verify the activate and deactivate functionality, following the existing test patterns.

Feature Flag

Not specific to any feature flag

Safety Assurance

Safety story

The changes were tested locally to verify the activate/deactivate functionality works as expected through the API. Existing permission checks were utilized to restrict access. The implementation follows established coding patterns in the codebase to limit risk.

Since this modifies the status of existing mobile worker data, caution should be exercised when activating/deactivating users. As far as I can tell, existing permission checks are in play.

Automated test coverage

The added tests in corehq/apps/api/tests/user_resources.py verify that:

  1. Mobile workers can be successfully activated through the API
  2. Mobile workers can be successfully deactivated through the API
  3. Deactivation is blocked for location-based mobile workers
  4. UserHistory entities are created for the modification action

These tests would catch regressions related to the activate/deactivate API functionality.

QA Plan

  1. Verify that a mobile worker can be activated through the API by an authorized user
  2. Verify that a mobile worker can be deactivated through the API by an authorized user
  3. Verify that deactivation is blocked for location-based mobile workers
  4. Verify that unauthorized users cannot access the activate/deactivate API endpoints

Rollback instructions

  • This PR can be reverted after deploy with no further considerations

Labels & Review

  • Risk label is set correctly
  • The set of people pinged as reviewers is appropriate for the level of risk of the change

Please let me know if you would like me to modify or expand the PR description in any way. I tried to capture the key aspects based on the information provided, but feel free to adapt it as needed.

@dimagimon dimagimon added the Risk: High Change affects files that have been flagged as high risk. label May 6, 2024
@szymonbalasz szymonbalasz marked this pull request as draft May 6, 2024 14:14
@szymonbalasz szymonbalasz marked this pull request as ready for review May 6, 2024 14:28
Copy link
Contributor

@snopoke snopoke left a comment

Choose a reason for hiding this comment

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

Generally looks good but a few changes required.

corehq/apps/api/resources/v0_5.py Outdated Show resolved Hide resolved
corehq/apps/api/resources/v0_5.py Outdated Show resolved Hide resolved
corehq/apps/api/tests/user_resources.py Show resolved Hide resolved
corehq/apps/api/resources/v0_5.py Show resolved Hide resolved
corehq/apps/api/resources/v0_5.py Show resolved Hide resolved
@szymonbalasz
Copy link
Contributor Author

Generally looks good but a few changes required.

Thanks for the help! Have made updates as requested

@szymonbalasz szymonbalasz requested a review from snopoke May 9, 2024 09:38
@szymonbalasz
Copy link
Contributor Author

szymonbalasz commented May 20, 2024

@snopoke Just a friendly reminder of this PR. I also have another PR #34634 - but can't assign a reviewer - can you also take a look, please?

@snopoke snopoke merged commit 9bb4dca into dimagi:master May 21, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Risk: High Change affects files that have been flagged as high risk.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants