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

Task/WP-32: onboarding page filter incomplete users #890

Closed

Conversation

asimregmi
Copy link
Contributor

@asimregmi asimregmi commented Oct 25, 2023

Overview

The onboarding Admin page has checkbox to filter users whose onboarding steps aren't complete. This makes it easier to view users who have Allocations and System Access steps not completed.

Related

Changes

  • Added Checkbox component
  • Added new handler toggleShowIncomplete based on user interaction with checkbox
  • Replaced users with filteredUsers

Testing

  1. Create a new file users.jsx and add dummy users (https://gist.github.com/asimregmi/891566eab2ecf34b71d26f6d226be10c)
  2. Add the following code to OnboardingAdmin.jsx
    Import : import dummyusers from './users'
    In line 243, remove { users }
    In line 246: const users = useSelector((state) => dummyusers)

Another testing method: Add new entry to the props from the React dev tools, but might be inefficient

UI

image
image

Notes

Dummy Users file for testing

users.jsx

@codecov
Copy link

codecov bot commented Oct 25, 2023

Codecov Report

Merging #890 (c78fefc) into main (f3e7f18) will increase coverage by 0.02%.
Report is 1 commits behind head on main.
The diff coverage is 81.81%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #890      +/-   ##
==========================================
+ Coverage   63.36%   63.39%   +0.02%     
==========================================
  Files         429      430       +1     
  Lines       12271    12300      +29     
  Branches     2532     2543      +11     
==========================================
+ Hits         7776     7797      +21     
- Misses       4288     4294       +6     
- Partials      207      209       +2     
Flag Coverage Δ
javascript 69.74% <81.81%> (+0.01%) ⬆️
unittests 56.99% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
...ient/src/components/Onboarding/OnboardingAdmin.jsx 73.03% <81.81%> (+0.19%) ⬆️

... and 3 files with indirect coverage changes

@asimregmi asimregmi marked this pull request as ready for review October 25, 2023 18:38
@asimregmi asimregmi changed the title Task/wp 32 onboarding page filter incomplete users Task/WP-32: onboarding page filter incomplete users Oct 25, 2023
@asimregmi asimregmi requested review from sophia-massie and removed request for wesleyboar October 25, 2023 18:42
Copy link
Contributor

@sophia-massie sophia-massie left a comment

Choose a reason for hiding this comment

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

It looks like the git history is showing that this branch was off your datafiles PR (WP-66) instead of main. You may need to create a fresh branch off of main and add only the onboarding changes that apply to the WP-32 task for a clean commit history.

Accidentally branching off another branch is easy to accidentally do. It can be tricky so ping me if you have questions. Make sure you get a fresh main branch

You could also pull main into this branch since we squash commits anyway and save you some work.

Screenshot 2023-10-25 at 4 36 35 PM

@asimregmi
Copy link
Contributor Author

I see the issue. Thank you @sophia-massie . Been trying to rebase for an hour. I will fix it. Thanks :)

Copy link
Collaborator

@chandra-tacc chandra-tacc left a comment

Choose a reason for hiding this comment

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

Looks good and tested with dummy data and my account.
I'll wait for your rebase or new PR and then can approve.

@@ -233,11 +234,24 @@ OnboardingAdminList.propTypes = {
const OnboardingAdmin = () => {
const dispatch = useDispatch();
const [eventLogModalParams, setEventLogModalParams] = useState(null);
const [showIncompleteOnly, setShowIncompleteOnly] = useState(false); // Add state to set the incomplete
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: comment does not add any more info in this, I think. Might as well remove it. useState helps us know that this is a set for showing incomplete only.

Comment on lines +247 to +253
const filteredUsers = users.filter((user) => {
if (showIncompleteOnly) {
return !user.setupComplete;
} else {
return true;
}
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: could make it a bit more concise like this:```suggestion
users.filter((user) => (showIncompleteOnly ? !user.setupComplete : true));

@asimregmi
Copy link
Contributor Author

Closing this branch. Creating a fresh branch off main and will try again.

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