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

[full-ci] Admin settings add group members panel #8826

Merged
merged 16 commits into from
Apr 19, 2023
Merged
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Enhancement: Admin settings groups members panel

We've added the members panel to the groups page in admin settings.

https://github.com/owncloud/web/pull/8826
https://github.com/owncloud/web/issues/8596
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
<template>
<div class="oc-ml-s">
<oc-text-input
v-model="filterTerm"
class="oc-text-truncate oc-mr-s oc-mt-m"
:label="$gettext('Filter members')"
/>
<div ref="membersListRef" data-testid="space-members">
<div v-if="!filteredGroupMembers.length">
<h3 class="oc-text-bold oc-text-medium" v-text="$gettext('No members found')" />
</div>
<div v-if="filteredGroupMembers.length" class="oc-mb-m" data-testid="group-members">
<h3 class="oc-text-bold oc-text-medium" v-text="$gettext('Members')" />
<members-role-section :group-members="filteredGroupMembers" />
</div>
</div>
</div>
</template>
<script lang="ts">
import { computed, defineComponent, inject, ref, watch, unref } from 'vue'
import MembersRoleSection from '../../Groups/SideBar/MembersRoleSection.vue'
import Fuse from 'fuse.js'
import Mark from 'mark.js'
import { Group } from 'web-client/src/generated'

export default defineComponent({
name: 'GroupsMembersPanel',
components: { MembersRoleSection },
setup() {
const group = inject<Group>('group')
lookacat marked this conversation as resolved.
Show resolved Hide resolved
const filterTerm = ref('')
const markInstance = ref(null)
const membersListRef = ref(null)

const filterMembers = (collection, term) => {
if (!(term || '').trim()) {
return collection
}

const searchEngine = new Fuse(collection, {
includeScore: false,
useExtendedSearch: true,
threshold: 0.1,
keys: ['displayName']
})

return searchEngine.search(term).map((r) => r.item)
}

const members = computed(() => {
if (group) {
return unref(group)
.members.sort((a, b) => a.displayName.localeCompare(b.displayName))
.map((u) => ({ ...u, kind: 'user' }))
}
return []
})

const filteredGroupMembers = computed(() => {
return filterMembers(unref(members), unref(filterTerm))
})

watch(filterTerm, () => {
if (unref(membersListRef)) {
markInstance.value = new Mark(unref(membersListRef))
unref(markInstance).unmark()
unref(markInstance).mark(unref(filterTerm), {
element: 'span',
className: 'highlight-mark'
})
}
})

return {
filterTerm,
membersListRef,
members,
filteredGroupMembers
}
}
})
</script>
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<template>
<ul class="oc-list">
<li
v-for="(member, index) in groupMembers"
:key="index"
class="oc-flex oc-flex-middle oc-mb-s"
data-testid="space-members-list"
lookacat marked this conversation as resolved.
Show resolved Hide resolved
>
<oc-avatar :user-name="member.displayName" :width="36" class="oc-mr-s" />
{{ member.displayName }}
</li>
</ul>
</template>
<script lang="ts">
import { defineComponent, PropType } from 'vue'
import { User } from 'web-client/src/helpers'

export default defineComponent({
name: 'MembersRoleSection',
props: {
groupMembers: {
type: Array as PropType<User[]>,
required: false
}
}
})
</script>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmmm I'm honestly not the biggest fan of combining space- and group-members in one and the same panel. Especially when we extend the functionality in the future to search/filter by specific attributes or make it possible to add/remove members.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I was about to write the same. We did this already with the Quota Modal which looks super cluttery.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

so some code duplication is better in this case, right? would be fine with me too

Copy link
Collaborator

@JammingBen JammingBen Apr 17, 2023

Choose a reason for hiding this comment

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

It's always a fine line between making components generic & reusable and duplicating code. Here I'd say a separate component with a bit of duplication is better. I suspect at least the group member panel will be expanded by some edit functionality in the future. That's the point where it definitely will get messy.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

lookacat marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -15,23 +15,23 @@
data-testid="space-members-role-managers"
>
<h3 class="oc-text-bold oc-text-medium" v-text="$gettext('Managers')" />
<members-role-section :members="filteredSpaceManagers" />
<members-role-section :space-members="filteredSpaceManagers" />
</div>
<div
v-if="filteredSpaceEditors.length"
class="oc-mb-m"
data-testid="space-members-role-editors"
>
<h3 class="oc-text-bold oc-text-medium" v-text="$gettext('Editors')" />
<members-role-section :members="filteredSpaceEditors" />
<members-role-section :space-members="filteredSpaceEditors" />
</div>
<div
v-if="filteredSpaceViewers.length"
class="oc-mb-m"
data-testid="space-members-role-viewers"
>
<h3 class="oc-text-bold oc-text-medium" v-text="$gettext('Viewers')" />
<members-role-section :members="filteredSpaceViewers" />
<members-role-section :space-members="filteredSpaceViewers" />
</div>
</div>
</div>
Expand All @@ -52,6 +52,7 @@ export default defineComponent({
const filterTerm = ref('')
const markInstance = ref(null)
const membersListRef = ref(null)

const filterMembers = (collection, term) => {
if (!(term || '').trim()) {
return collection
Expand All @@ -68,20 +69,23 @@ export default defineComponent({
}

const spaceMembers = computed(() => {
return [
...unref(resource).spaceRoles.manager.map((r) => ({
...r,
roleType: spaceRoleManager.name
})),
...unref(resource).spaceRoles.editor.map((r) => ({
...r,
roleType: spaceRoleEditor.name
})),
...unref(resource).spaceRoles.viewer.map((r) => ({
...r,
roleType: spaceRoleViewer.name
}))
].sort((a, b) => a.displayName.localeCompare(b.displayName))
if (resource) {
return [
...unref(resource).spaceRoles.manager.map((r) => ({
...r,
roleType: spaceRoleManager.name
})),
...unref(resource).spaceRoles.editor.map((r) => ({
...r,
roleType: spaceRoleEditor.name
})),
...unref(resource).spaceRoles.viewer.map((r) => ({
...r,
roleType: spaceRoleViewer.name
}))
].sort((a, b) => a.displayName.localeCompare(b.displayName))
}
return []
})

const filteredSpaceMembers = computed(() => {
Expand Down
lookacat marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
<template>
<ul class="oc-list">
<li
v-for="(member, index) in members"
v-for="(member, index) in spaceMembers"
:key="index"
class="oc-flex oc-flex-middle oc-mb-s"
data-testid="space-members-list"
>
<oc-avatar
v-if="member.kind === 'user'"
v-if="showOcAvatar(member)"
:user-name="member.displayName"
:width="36"
class="oc-mr-s"
Expand All @@ -31,16 +31,19 @@ import { SpaceRole } from 'web-client/src/helpers'
export default defineComponent({
name: 'MembersRoleSection',
props: {
members: {
spaceMembers: {
type: Array as PropType<SpaceRole[]>,
required: true
required: false
}
},
setup() {
setup(props) {
const groupIcon = computed(() => {
return ShareTypes.group.icon
})
return { groupIcon }
const showOcAvatar = (member) => {
return (member as any).kind === 'user'
}
return { groupIcon, showOcAvatar }
}
})
</script>
14 changes: 14 additions & 0 deletions packages/web-app-admin-settings/src/views/Groups.vue
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ import { useClientService, useStore } from 'web-pkg/src/composables'
import AppTemplate from '../components/AppTemplate.vue'
import { useSideBar } from 'web-pkg/src/composables/sideBar'
import { useGroupActionsDelete } from '../composables/actions/groups'
import MembersPanel from '../components/Groups/SideBar/MembersPanel.vue'

export default defineComponent({
components: {
Expand All @@ -93,6 +94,11 @@ export default defineComponent({
CreateGroupModal,
ContextActions
},
provide() {
return {
group: computed(() => this.selectedGroups[0])
}
},
setup() {
const store = useStore()
const groups = ref([])
Expand Down Expand Up @@ -192,6 +198,14 @@ export default defineComponent({
group: this.selectedGroups.length === 1 ? this.selectedGroups[0] : null,
onConfirm: this.editGroup
}
},
{
app: 'GroupMembers',
icon: 'group',
title: this.$gettext('Members'),
component: MembersPanel,
default: false,
enabled: this.selectedGroups.length === 1
}
].filter((p) => p.enabled)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import MembersPanel from '../../../../../src/components/Groups/SideBar/MembersPanel.vue'
Copy link
Member

@kulmann kulmann Apr 19, 2023

Choose a reason for hiding this comment

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

the import looks wrong (or at least weird 😆 )

import { defaultPlugins, shallowMount } from 'web-test-helpers'
import { mock } from 'jest-mock-extended'
import { Group } from 'web-client/src/generated'

const groupMock = mock<Group>({
id: '1',
groupTypes: [],
members: [{ displayName: 'Albert Einstein' }]
})
const selectors = {
membersRolePanelStub: 'members-role-section-stub'
}

describe('MembersPanel', () => {
it('should render all members accordingly to their role assignments', () => {
const { wrapper } = getWrapper()
expect(wrapper.html()).toMatchSnapshot()
})
it('should filter members accordingly to the entered search term', async () => {
const { wrapper } = getWrapper()
wrapper.vm.filterTerm = 'ein'
await wrapper.vm.$nextTick
expect(wrapper.findAll(selectors.membersRolePanelStub).length).toBe(1)
expect(
wrapper.findComponent<any>(selectors.membersRolePanelStub).props().groupMembers[0].displayName
).toEqual('Albert Einstein')
})
it('should display an empty result if no matching members found', async () => {
const { wrapper } = getWrapper()
wrapper.vm.filterTerm = 'no-match'
await wrapper.vm.$nextTick
expect(wrapper.findAll(selectors.membersRolePanelStub).length).toBe(0)
expect(wrapper.html()).toMatchSnapshot()
})
})

function getWrapper({ group = groupMock } = {}) {
return {
wrapper: shallowMount(MembersPanel, {
global: {
plugins: [...defaultPlugins()],
provide: { group: group }
}
})
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`MembersPanel should display an empty result if no matching members found 1`] = `
<div class="oc-ml-s">
<oc-text-input-stub class="oc-text-truncate oc-mr-s oc-mt-m" clearbuttonaccessiblelabel="" clearbuttonenabled="false" disabled="false" fixmessageline="false" id="oc-textinput-3" label="Filter members" modelvalue="no-match" type="text"></oc-text-input-stub>
<div>
<div>
<h3 class="oc-text-bold oc-text-medium">No members found</h3>
</div>
<!--v-if-->
</div>
</div>
`;

exports[`MembersPanel should render all members accordingly to their role assignments 1`] = `
<div class="oc-ml-s">
<oc-text-input-stub class="oc-text-truncate oc-mr-s oc-mt-m" clearbuttonaccessiblelabel="" clearbuttonenabled="false" disabled="false" fixmessageline="false" id="oc-textinput-1" label="Filter members" modelvalue="" type="text"></oc-text-input-stub>
<div>
<!--v-if-->
<div class="oc-mb-m">
<h3 class="oc-text-bold oc-text-medium">Members</h3>
<members-role-section-stub groupmembers="[object Object]"></members-role-section-stub>
</div>
</div>
</div>
`;
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ describe('MembersPanel', () => {
await wrapper.vm.$nextTick
expect(wrapper.findAll(selectors.membersRolePanelStub).length).toBe(1)
expect(
wrapper.findComponent<any>(selectors.membersRolePanelStub).props().members[0].displayName
wrapper.findComponent<any>(selectors.membersRolePanelStub).props().spaceMembers[0].displayName
).toEqual(userToFilterFor.displayName)
})
it('should display an empty result if no matching members found', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,15 @@ exports[`MembersPanel should render all members accordingly to their role assign
<!--v-if-->
<div class="oc-mb-m">
<h3 class="oc-text-bold oc-text-medium">Managers</h3>
<members-role-section-stub members="[object Object]"></members-role-section-stub>
<members-role-section-stub spacemembers="[object Object]"></members-role-section-stub>
</div>
<div class="oc-mb-m">
<h3 class="oc-text-bold oc-text-medium">Editors</h3>
<members-role-section-stub members="[object Object],[object Object]"></members-role-section-stub>
<members-role-section-stub spacemembers="[object Object],[object Object]"></members-role-section-stub>
</div>
<div class="oc-mb-m">
<h3 class="oc-text-bold oc-text-medium">Viewers</h3>
<members-role-section-stub members="[object Object]"></members-role-section-stub>
<members-role-section-stub spacemembers="[object Object]"></members-role-section-stub>
</div>
</div>
</div>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,3 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`MembersRoleSection should render all members accordingly 1`] = `
<ul class="oc-list">
<li class="oc-flex oc-flex-middle oc-mb-s">
<oc-avatar-stub accessiblelabel="" class="oc-mr-s" src="" username="einstein" width="36"></oc-avatar-stub> einstein
</li>
<li class="oc-flex oc-flex-middle oc-mb-s">
<oc-avatar-item-stub accessiblelabel="" background="var(--oc-color-swatch-passive-default)" class="oc-mr-s" icon="group" iconcolor="var(--oc-color-text-inverse)" iconfilltype="fill" iconsize="medium" name="group" width="36"></oc-avatar-item-stub> physic-lovers
</li>
</ul>
`;
exports[`MembersRoleSection should render all members accordingly 1`] = `<ul class="oc-list" members="undefined,undefined"></ul>`;