-
Notifications
You must be signed in to change notification settings - Fork 14.3k
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
feat: filter by me on CRUD list view #11683
Conversation
Codecov Report
@@ Coverage Diff @@
## master #11683 +/- ##
===========================================
- Coverage 67.30% 55.05% -12.25%
===========================================
Files 905 411 -494
Lines 43872 14535 -29337
Branches 4202 3729 -473
===========================================
- Hits 29527 8002 -21525
+ Misses 14241 6533 -7708
+ Partials 104 0 -104
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
({ text: label, value }: { text: string; value: any }) => ({ | ||
label, | ||
value, | ||
}), | ||
); | ||
|
||
return options.concat(data); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe some of these results are paginated so each request will end up with multiple me
entries.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, nvm. Looks like these results are merged in with the existing ones and get properly deduped. 👍
) => async (filterValue = '', pageIndex?: number, pageSize?: number) => { | ||
const resourceEndpoint = `/api/v1/${resource}/${method}/${relation}`; | ||
|
||
const options = | ||
userId && pageIndex === 0 ? [{ label: 'me', value: userId }] : []; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
small nit, but should me
be capitalized?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM after rebase
d481a51
to
24e6866
Compare
SUMMARY
Implement filter with
Me
option for list views:BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TEST PLAN
ADDITIONAL INFORMATION