feat: Add filter by role and username#2659
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
1dd3846 to
eaa2b5c
Compare
dffa816 to
dc962e1
Compare
Signed-off-by: Kostiantyn Miakshyn <molodchick@gmail.com>
dc962e1 to
7ba1339
Compare
| params: { | ||
| search, | ||
| role, | ||
| ...(limit ? { limit } : {}), |
There was a problem hiding this comment.
The members controller doesn't appear accept these filter params. Is that still coming in a separate back-end PR?
| 'circle.id': { | ||
| handler() { | ||
| this.fetchCircleMembers() | ||
| }, | ||
|
|
||
| immediate: true, | ||
| }, |
There was a problem hiding this comment.
We already dispatch getCircleMembers on teamId change in TeamPage.loadCircle. This means we hit the endpoint twice on every team page open, so perhaps we can remove this one?
| return this.$store.getters.getCircle(this.selectedCircle) | ||
| }, | ||
|
|
||
| members() { |
There was a problem hiding this comment.
Is this used anywhere?
| if (!this.circle || !this.circle.id) { | ||
| return | ||
| } | ||
| if (!this.circle.canManageMembers) { |
There was a problem hiding this comment.
If I understand correctly: the filter controls render for every member, but here we return early when !canManageMembers, so a regular member gets a search box and role dropdown that silently do nothing.
Is the guard intentional? Viewing the member list doesn't require manage rights, so should filtering it? If there's a reason to keep it, would it make sense to hide the filter controls for people who can't use them?
|
|
||
| try { | ||
| await this.$store.dispatch('getCircleMembers', payload) | ||
| console.log('debug: getCircleMembers', this.list) |
There was a problem hiding this comment.
Please change console logs to use this.logger
| :options="roles" | ||
| :placeholder="t('circles', 'Role')" | ||
| :multiple="false" | ||
| style="min-width: 160px;" /> |
There was a problem hiding this comment.
Please move this inline style to <style module> below where the other styles live
| <NcSelect | ||
| v-model="searchRole" | ||
| :options="roles" | ||
| :placeholder="t('circles', 'Role')" |
There was a problem hiding this comment.
Please add an aria-label or label so screen readers can announce the role filter
| * @param {string} circleId the circle id | ||
| */ | ||
| resetCircleMembers(state, circleId) { | ||
| state.circles[circleId].members = [] |
There was a problem hiding this comment.
MemberList is Record<string, Member> (see src/teams/team-page/models/circle.ts#L14)
| state.circles[circleId].members = [] | |
| state.circles[circleId].members = {} |
2 participants
Summary
This is a follow-up for #2561 inspired by nextcloud/contacts#5330:
Checklist
3. to review, feature component)stable32)AI (if applicable)