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

Regression: Search on Member List #26273

Merged
merged 7 commits into from
Jul 15, 2022
Merged

Regression: Search on Member List #26273

merged 7 commits into from
Jul 15, 2022

Conversation

tassoevan
Copy link
Contributor

Proposed changes (including videos or screenshots)

Issue(s)

Steps to test or reproduce

Further comments

@tassoevan tassoevan added this to the 5.0.0 milestone Jul 14, 2022
@tassoevan tassoevan marked this pull request as ready for review July 15, 2022 01:37
@tassoevan tassoevan requested a review from a team as a code owner July 15, 2022 01:37
sampaiodiego
sampaiodiego previously approved these changes Jul 15, 2022
View reviewed changes
Copy link
Member

@sampaiodiego sampaiodiego left a comment

Choose a reason for hiding this comment

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

looks like there are other places calling findPaginatedByActiveUsersExcept without providing the searchFields param.. I can change them later, or if you can update this PR to fix all places it would be awesome.

@tassoevan
Copy link
Contributor Author

I really wanted to add a test ensure this changes on dm.members, but geez... It's so slow to write tests without mocking the models.

sampaiodiego
sampaiodiego previously approved these changes Jul 15, 2022
View reviewed changes
@ggazzo ggazzo added stat: ready to merge PR tested and approved waiting for merge and removed stat: needs QA labels Jul 15, 2022
@sampaiodiego sampaiodiego merged commit 7ec011d into develop Jul 15, 2022
@sampaiodiego sampaiodiego deleted the regression/members-list branch July 15, 2022 14:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sampaiodiego sampaiodiego sampaiodiego approved these changes

@LucianoPierdona LucianoPierdona LucianoPierdona approved these changes

@matheusbsilva137 matheusbsilva137 Awaiting requested review from matheusbsilva137

Assignees
No one assigned
Labels
stat: QA skipped stat: ready to merge PR tested and approved waiting for merge
Projects
None yet
Milestone
5.0.0
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@tassoevan @sampaiodiego @LucianoPierdona @ggazzo