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

Clean up state management for user management page #9535

Merged

Conversation

rtibbles
Copy link
Member

@rtibbles rtibbles commented Jun 24, 2022

Summary

  • Removes loading state when moving between pages of the user management page
  • Set default state to ensure reactivity from Object.assign.

References

Fixes #9528

Reviewer guidance

Does the user management page in Facility still function as expected - for filtering, pagination? Does it show the correct number of users once filtering has happened or been removed?


Testing checklist

  • Contributor has fully tested the PR manually
  • If there are any front-end changes, before/after screenshots are included
  • Critical user journeys are covered by Gherkin stories
  • Critical and brittle code paths are covered by unit tests

PR process

  • PR has the correct target branch and milestone
  • PR has 'needs review' or 'work-in-progress' label
  • If PR is ready for review, a reviewer has been added. (Don't use 'Assignees')
  • If this is an important user-facing change, PR or related issue has a 'changelog' label
  • If this includes an internal dependency change, a link to the diff is provided

Reviewer checklist

  • Automated test coverage is satisfactory
  • PR is fully functional
  • PR has been tested for accessibility regressions
  • External dependency files were updated if necessary (yarn and pip)
  • Documentation is updated
  • Contributor is in AUTHORS.md

Set default state to ensure reactivity from `Object.assign`.
@rtibbles rtibbles added the TODO: needs review Waiting for review label Jun 24, 2022
@rtibbles rtibbles added this to the Planned Patch 4: Coach usability improvements milestone Jun 24, 2022
@rtibbles rtibbles requested a review from marcellamaki June 24, 2022 21:39
Copy link
Member

@marcellamaki marcellamaki left a comment

Choose a reason for hiding this comment

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

Welp, I approved this with just a code review, but it actually looks like there's an error when I run it...so, I take that back. The filtering and searching itself seems fine, just the pagination numbers and buttons are the issue seems like

Although, I can't quite figure out how this is exactly related to these changes

Screen Shot 2022-06-28 at 10 49 44 AM

@marcellamaki
Copy link
Member

I checked out 0.15.x because I briefly panicked that I had broken this during our patch release speeding along, but it seems to be working there

@marcellamaki marcellamaki self-requested a review June 29, 2022 22:03
Copy link
Member

@marcellamaki marcellamaki left a comment

Choose a reason for hiding this comment

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

Can not replicate again -- seems to be due to branch switch or some such. Approved!

@marcellamaki marcellamaki merged commit 357becc into learningequality:release-v0.15.x Jun 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
TODO: needs review Waiting for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants