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

[Publisher][Bug] Alternate Solution: Fix bug where adding team members to the admin panel clears out all other team members #1529

Merged
merged 1 commit into from
Oct 4, 2024

Conversation

mxosman
Copy link
Contributor

@mxosman mxosman commented Oct 4, 2024

Description of the change

Alternative solution to #1528 - this is my preferred solution. Explained more here:

https://www.loom.com/share/b657651b698b4430ac2ce3711b755b35

Fix bug where adding team members to the admin panel clears out all other team members due to recent optimizations of fetching team members separately.

This issue was raised by CSG members who noticed that adding a team member to an agency resulted in clearing out all other team members from the agency aside from the one recently added. Because we now fetch team members separately and ONLY when an agency card is clicked vs. before where they were populated for each agency immediately after the admin panel loads, we need to await the fetch of team members before updating them in the admin panel so all references to the team members will be the freshly fetched list.

Follow up TODO: Add unit tests to cover this area

Type of change

All pull requests must have at least one of the following labels applied (otherwise the PR will fail):

Label Description
Type: Bug non-breaking change that fixes an issue
Type: Feature non-breaking change that adds functionality
Type: Breaking Change fix or feature that would cause existing functionality to not work as expected
Type: Non-breaking refactor change addresses some tech debt item or prepares for a later change, but does not change functionality
Type: Configuration Change adjusts configuration to achieve some end related to functionality, development, performance, or security
Type: Dependency Upgrade upgrades a project dependency - these changes are not included in release notes

Related issues

Closes #XXXX

Checklists

Development

This box MUST be checked by the submitter prior to merging:

  • Double- and triple-checked that there is no Personally Identifiable Information (PII) being mistakenly added in this pull request

These boxes should be checked by the submitter prior to merging:

  • Tests have been written to cover the code changed/added as part of this pull request

Code review

These boxes should be checked by reviewers prior to merging:

  • This pull request has a descriptive title and information useful to a reviewer
  • This pull request has been moved out of a Draft state, has no "Work In Progress" label, and has assigned reviewers
  • Potential security implications or infrastructural changes have been considered, if relevant

@helperbot-recidiviz
Copy link
Collaborator

@mxosman successfully triggered a playtest deployment. Full deployment usually takes 5 minutes. Your playtest link is https://test---publisher-web-b47yvyxs3q-uc.a.run.app/

Copy link
Collaborator

@lilidworkin lilidworkin left a comment

Choose a reason for hiding this comment

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

Thank you @mxosman ! I don't have time right now to follow all the details -- will come back to that later -- but just confirming that you've tested both agency and user provisioning flows on staging? If so, this sounds good to me. Thanks so much for the quick fix and for thinking through it so thoughtfully! Would be great to do a staging + prod deploy fix today to get this out, and then to coordinate with @nichelle-hall on followups to restore data loss. Thank you both so much!

@mxosman
Copy link
Contributor Author

mxosman commented Oct 4, 2024

Thank you, Lili! I tested this thoroughly and both flows work as expected and I feel super comfortable with this solution vs. the other. Will coordinate on follow ups to the data loss.

@mxosman mxosman merged commit 60800a2 into main Oct 4, 2024
8 of 9 checks passed
@mxosman mxosman deleted the mahmoud/bug-fix-admin-panel-team-alternate branch October 4, 2024 15:21
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.

3 participants