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.

Sign up for GitHub

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

Jump to bottom

Paginated, searchable people page with other enhancements #1345

Merged
merged 11 commits into from
May 28, 2020
Merged

Paginated, searchable people page with other enhancements #1345

merged 11 commits into from
May 28, 2020

Conversation

lperson
Copy link
Collaborator

@lperson lperson commented Dec 30, 2019 •
edited
Loading

Fixes #1326

This PR brings people page enhancements over from the WFP fork.

Description

  • Filter people by
    • Campaign
    • Role (texter, admin, etc.)
    • Email, first name, last name
  • Sort people by
    • First name
    • Last name
    • Oldest
    • Newest
  • Paginated results
  • Number of people per page choices are configurable
  • Default sort is configurable

This is bigger than 300 lines because of tests and also because it's a big chunk of work.

Screen Shot 2020-01-03 at 8 51 07 PM

Checklist:

  • I have manually tested my changes on desktop and mobile
  • The test suite passes locally with my changes
  • If my change is a UI change, I have attached a screenshot to the description section of this pull request
  • [No] My change is 300 lines of code or less, or has a documented reason in the description why it’s longer
  • I have made any necessary changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • [N/A] My PR is labeled [WIP] if it is in progress

@lperson lperson added the S-work in progress (WIP) Status: PR label for work that is not yet ready to be reviewed label Dec 30, 2019
@lperson lperson force-pushed the lp_1326_paginated_people_page branch from 3cfae22 to 33b28a8 Compare December 30, 2019 04:44
@ibrand
Copy link
Collaborator

ibrand commented Jan 2, 2020

@lperson is this one out of WIP state now Larry? Should I assign a reviewer?

@lperson
Copy link
Collaborator Author

lperson commented Jan 3, 2020

@lperson is this one out of WIP state now Larry? Should I assign a reviewer?

No. I'm still writing tests. 😷 🤧 🎉

@ibrand ibrand added the S-needs tests Status: PR label marking PRs that don't include sufficient tests label Jan 3, 2020
@lperson lperson added S-waiting on review Status: Awaiting review from the assignee but also interested parties and removed S-needs tests Status: PR label marking PRs that don't include sufficient tests S-work in progress (WIP) Status: PR label for work that is not yet ready to be reviewed labels Jan 4, 2020
@lperson lperson changed the title Enhanced people page Paginated, searchable people page with other enhancements Jan 4, 2020
@ibrand ibrand requested a review from bdatkins January 4, 2020 21:41
@ibrand ibrand temporarily deployed to spoke-review-lp-1326-pa-zsgokr January 6, 2020 04:03 Inactive
@bdatkins
Copy link
Collaborator

bdatkins commented Jan 7, 2020

Deployment is throwing an error ReferenceError: INITIAL_ROW_SIZE is not defined on https://spoke-review-lp-1326-pa-zsgokr.herokuapp.com/admin/1/campaigns

bdatkins
bdatkins reviewed Jan 7, 2020
View reviewed changes
__test__/backend.test.js
});

it("updates the user if it is called with a userId and userData", async () => {
const userData = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Amusing test user :)

bdatkins
bdatkins reviewed Jan 7, 2020
View reviewed changes
__test__/server/api/people.test.js

testAdminUsers = [];
testAdminUsers.push(
await createUser({
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would there be any value in moving reused test data to a common place to reduce duplication?

bdatkins
bdatkins reviewed Jan 7, 2020
View reviewed changes
src/server/api/user.js
"%"
).toLocaleLowerCase();

if (filterBy === "FIRST_NAME") {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggestion to introduce constants for the filter strings

@ibrand ibrand temporarily deployed to spoke-review-lp-1326-pa-zsgokr January 7, 2020 18:26 Inactive
@lperson
Copy link
Collaborator Author

lperson commented Jan 7, 2020

@bdatkins wrote:

Deployment is throwing an error ReferenceError: INITIAL_ROW_SIZE is not defined on https://spoke-review-lp-1326-pa-zsgokr.herokuapp.com/admin/1/campaigns

Bad merge. I got rid of the variable in this branch.

bdatkins
bdatkins approved these changes Jan 7, 2020
View reviewed changes
Copy link
Collaborator

@bdatkins bdatkins left a comment

Choose a reason for hiding this comment

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

Nice work, @lperson ! Looks good.

@bdatkins
Copy link
Collaborator

bdatkins commented Jan 7, 2020

I verified the functionality in the Heroku environment listed above.

@bdatkins bdatkins removed the S-waiting on review Status: Awaiting review from the assignee but also interested parties label Jan 7, 2020
@lperson lperson added the S-work in progress (WIP) Status: PR label for work that is not yet ready to be reviewed label Jan 9, 2020
@lperson lperson removed the S-work in progress (WIP) Status: PR label for work that is not yet ready to be reviewed label Apr 23, 2020
@schuyler1d schuyler1d added the S-ready for stage-main (qa) Status (ADMINS ONLY): PR label for those ready to be added for stage: Approved, tests, etc label May 6, 2020
@schuyler1d schuyler1d mentioned this pull request May 6, 2020
Merged
@ibrand ibrand merged commit 1233594 into StateVoicesNational:main May 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bdatkins bdatkins bdatkins approved these changes

Assignees
No one assigned
Labels
S-ready for stage-main (qa) Status (ADMINS ONLY): PR label for those ready to be added for stage: Approved, tests, etc
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Paginated, searchable people page with other enhancements
4 participants
@lperson @ibrand @bdatkins @schuyler1d