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

User Zustandification™️ #11527

Merged
merged 6 commits into from
Dec 7, 2023
Merged

User Zustandification™️ #11527

merged 6 commits into from
Dec 7, 2023

Conversation

karla-vm
Copy link
Collaborator

@karla-vm karla-vm commented Nov 27, 2023

Description

Converting the UserContext state management of MCR from Context API to Zustand (following the patterns implemented in MFP).

Related ticket(s)

CMDCT-2794


How to test

Running this locally, you should be able to log in, log out, and manage Banners (as an Admin) as usual.

Optionally: install the Redux Devtools Chrome extension or the Firefox Redux Devtools extension

  • Open devtools
  • Log in
  • You should see the user object on the extension ✨
Screenshot 2023-11-27 at 2 31 09 PM

Important updates


Author checklist

  • I have performed a self-review of my code
  • I have added thorough tests, if necessary
  • I have updated relevant documentation, if necessary

convert to a different template: test → val | val → prod

Copy link
Contributor

@benmartin-coforma benmartin-coforma left a comment

Choose a reason for hiding this comment

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

This is a ton of work! Well done.

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.

Requesting changes because: Two E2E cypress tests are failing in GitHub Actions and the same two fail for me locally.

Other notes:
I noticed we use this statement a lot when pulling in the user data to files:
useStore().user ?? {};

Is there a way we could make that logic part of the store / do we still need to optionally return the empty object? Just curious since it's used so often

Copy link

codeclimate bot commented Dec 7, 2023

Code Climate has analyzed commit f4c0cd8 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 95.8% (0.0% change).

View more on Code Climate.

@karla-vm karla-vm merged commit 9cb5713 into main Dec 7, 2023
14 checks passed
@karla-vm karla-vm deleted the zustand-moar branch December 7, 2023 21:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants