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

Remove the User.isActive flag #14871

Merged
merged 2 commits into from
Jun 5, 2024
Merged

Remove the User.isActive flag #14871

merged 2 commits into from
Jun 5, 2024

Conversation

benmartin-coforma
Copy link
Collaborator

@benmartin-coforma benmartin-coforma commented May 22, 2024

Description

This PR removes a half-implemented feature; the ability to deactivate users within the app.

Background: SEDS user data is split across internal (dynamodb) and external (IDM) systems. In non-dev environments, users must exist in IDM before they can log in to SEDS. Once they exist in SEDS, details like their state list and role can be added or changed in dynamodb; from that point forward we consider dynamo to be authoritative. The isActive flag was intended to be such a detail, but even though we added code to manage this flag, we never changed any part of the app to read the flag. Rather than completing that feature, we will henceforth rely on IDM for user deletion/deactivation.

This allows us to remove a good deal of code:

  1. The UI elements that controlled the flag
  2. The UI elements that displayed the flag
  3. The UI functions which called the API to set the flag
  4. The API endpoint to set the flag
  5. Lots of references to the flag

Additionally, this PR contains a migration to purge the isActive flag from all existing users. A list of all users which had isActive set to false has been collected and is in the Business Owners' hands; they will do whatever is appropriate for those users in IDM and in SEDS. Once this migration has run in all environments, there will be no lingering traces of isActive anywhere in the app.

Related ticket(s)

CMDCT-3230


How to test

Cloudfront url: https://d3k0z4kfh0z5fc.cloudfront.net/

  1. Log in as an admin
  2. Go to the user list
    • The isActive flag should not be in the table
  3. Export the user list to excel
    • The isActive flag should not be in the spreadsheet
  4. Click in the user list to edit a user
    • The isActive flag should not be on the page
  5. Make a change to the user and save it
    • The save should be successful

(After merging into master)

  1. Confirm that the master-auth-user table has isActive flags on the users
  2. Run the migration
  3. Confirm that the isActive flags are now gone

Notes

n/a


Pre-review checklist

  • [ ] I have added thorough tests, if necessary
  • [ ] I have updated relevant documentation, if necessary
  • I have performed a self-review of my code
  • [ ] I have manually tested this PR in the deployed cloud environment
    • (except for the migration, which I tested locally and will test against dev)

Pre-merge checklist

Review

  • Design: This work has been reviewed and approved by design, if necessary
  • Product: This work has been reviewed and approved by product owner, if necessary

Security

If either of the following are true, notify the team's ISSO (Information System Security Officer).

  • [ ] These changes are significant enough to require an update to the SIA.
  • [ ] These changes are significant enough to require a penetration test.

Copy link

codeclimate bot commented Jun 4, 2024

Code Climate has analyzed commit 0d1372d and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (90% is the threshold).

This pull request will bring the total coverage in the repository to 72.3% (1.0% change).

View more on Code Climate.

@benmartin-coforma benmartin-coforma marked this pull request as ready for review June 4, 2024 13:43
@benmartin-coforma benmartin-coforma changed the title Remove the isActive flag, mostly probably maybe too much Remove the User.isActive flag Jun 4, 2024
Copy link
Contributor

@gmrabian gmrabian left a comment

Choose a reason for hiding this comment

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

validated admin UI shows no signs of this attribute

@benmartin-coforma benmartin-coforma merged commit e81741f into master Jun 5, 2024
18 checks passed
@benmartin-coforma benmartin-coforma deleted the remove-isactive branch June 5, 2024 17:26
@benmartin-coforma
Copy link
Collaborator Author

This migration has run in all three environments (master, val, production). The isActive flag is no more.

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