diff --git a/changelog/unreleased/bugfix-admin-settings-batch-context-actions b/changelog/unreleased/bugfix-admin-settings-batch-context-actions new file mode 100644 index 00000000000..ec6e1b8e1a0 --- /dev/null +++ b/changelog/unreleased/bugfix-admin-settings-batch-context-actions @@ -0,0 +1,6 @@ +Bugfix: Batch context actions in admin settings + +Several issues when triggering batch actions via the context menu for users/groups/spaces in the admin-settings have been fixed. Some actions were showing wrongly ("edit"), some actions were resetting the current selection ("show details"). + +https://github.com/owncloud/web/issues/8549 +https://github.com/owncloud/web/pull/8785 diff --git a/packages/web-app-admin-settings/src/components/Groups/GroupsList.vue b/packages/web-app-admin-settings/src/components/Groups/GroupsList.vue index 33e847b8a9b..a0ef5e1f893 100644 --- a/packages/web-app-admin-settings/src/components/Groups/GroupsList.vue +++ b/packages/web-app-admin-settings/src/components/Groups/GroupsList.vue @@ -149,7 +149,9 @@ export default defineComponent({ } const rowClicked = (data) => { const group = data[0] - selectGroup(group) + if (!isGroupSelected(group)) { + selectGroup(group) + } } const showContextMenuOnBtnClick = (data, group) => { const { dropdown, event } = data diff --git a/packages/web-app-admin-settings/src/components/Spaces/SpacesList.vue b/packages/web-app-admin-settings/src/components/Spaces/SpacesList.vue index 2ef30df6458..4ba47e44138 100644 --- a/packages/web-app-admin-settings/src/components/Spaces/SpacesList.vue +++ b/packages/web-app-admin-settings/src/components/Spaces/SpacesList.vue @@ -366,7 +366,9 @@ export default defineComponent({ const fileClicked = (data) => { const space = data[0] - selectSpace(space) + if (!isSpaceSelected(space)) { + selectSpace(space) + } } const showContextMenuOnBtnClick = (data, space) => { diff --git a/packages/web-app-admin-settings/src/composables/actions/groups/useGroupActionsDelete.ts b/packages/web-app-admin-settings/src/composables/actions/groups/useGroupActionsDelete.ts index a22ad109756..e7d506fa9f2 100644 --- a/packages/web-app-admin-settings/src/composables/actions/groups/useGroupActionsDelete.ts +++ b/packages/web-app-admin-settings/src/composables/actions/groups/useGroupActionsDelete.ts @@ -94,6 +94,8 @@ export const useGroupActionsDelete = ({ store }: { store?: Store }) => { ]) return { - actions + actions, + // HACK: exported for unit tests: + deleteGroups } } diff --git a/packages/web-app-admin-settings/src/composables/actions/users/useUserActionsDelete.ts b/packages/web-app-admin-settings/src/composables/actions/users/useUserActionsDelete.ts index 4aa5af9349d..94ec93282f3 100644 --- a/packages/web-app-admin-settings/src/composables/actions/users/useUserActionsDelete.ts +++ b/packages/web-app-admin-settings/src/composables/actions/users/useUserActionsDelete.ts @@ -89,6 +89,8 @@ export const useUserActionsDelete = ({ store }: { store?: Store }) => { ]) return { - actions + actions, + // HACK: exported for unit tests: + deleteUsers } } diff --git a/packages/web-app-admin-settings/src/composables/actions/users/useUserActionsEdit.ts b/packages/web-app-admin-settings/src/composables/actions/users/useUserActionsEdit.ts index 81ae4c56e52..3b8847a0ad2 100644 --- a/packages/web-app-admin-settings/src/composables/actions/users/useUserActionsEdit.ts +++ b/packages/web-app-admin-settings/src/composables/actions/users/useUserActionsEdit.ts @@ -14,7 +14,7 @@ export const useUserActionsEdit = () => { label: () => $gettext('Edit'), handler: () => eventBus.publish(SideBarEventTopics.openWithPanel, 'EditPanel'), isEnabled: ({ resources }) => { - return resources.length > 0 + return resources.length === 1 }, componentType: 'button', class: 'oc-users-actions-edit-trigger' diff --git a/packages/web-app-admin-settings/tests/unit/components/Groups/ContextActions.spec.ts b/packages/web-app-admin-settings/tests/unit/components/Groups/ContextActions.spec.ts index 5b9b6611c6e..948ed35c06d 100644 --- a/packages/web-app-admin-settings/tests/unit/components/Groups/ContextActions.spec.ts +++ b/packages/web-app-admin-settings/tests/unit/components/Groups/ContextActions.spec.ts @@ -48,7 +48,8 @@ describe('ContextActions', () => { it('render enabled actions', () => { const enabledComposables = [useGroupActionsDelete, useGroupActionsEdit] jest.mocked(useGroupActionsDelete).mockImplementation(() => ({ - actions: computed(() => [mock({ isEnabled: () => true })]) + actions: computed(() => [mock({ isEnabled: () => true })]), + deleteGroups: null })) jest.mocked(useGroupActionsEdit).mockImplementation(() => ({ actions: computed(() => [mock({ isEnabled: () => true })]) diff --git a/packages/web-app-admin-settings/tests/unit/composables/actions/useGeneralActionsResetLogo.spec.ts b/packages/web-app-admin-settings/tests/unit/composables/actions/general/useGeneralActionsResetLogo.spec.ts similarity index 87% rename from packages/web-app-admin-settings/tests/unit/composables/actions/useGeneralActionsResetLogo.spec.ts rename to packages/web-app-admin-settings/tests/unit/composables/actions/general/useGeneralActionsResetLogo.spec.ts index 26fe4a5fb76..416fa2cb8b4 100644 --- a/packages/web-app-admin-settings/tests/unit/composables/actions/useGeneralActionsResetLogo.spec.ts +++ b/packages/web-app-admin-settings/tests/unit/composables/actions/general/useGeneralActionsResetLogo.spec.ts @@ -1,4 +1,4 @@ -import { useGeneralActionsResetLogo } from '../../../../src/composables/actions/general/useGeneralActionsResetLogo' +import { useGeneralActionsResetLogo } from '../../../../../src/composables/actions/general/useGeneralActionsResetLogo' import { mock } from 'jest-mock-extended' import { unref } from 'vue' import { @@ -15,8 +15,8 @@ jest.useFakeTimers() describe('resetLogo', () => { describe('handler', () => { - it('should show message on request success', async () => { - const { wrapper } = getWrapper({ + it('should show message on request success', () => { + getWrapper({ setup: async ({ actions }, { storeOptions, clientService, router }) => { clientService.httpAuthenticated.delete.mockResolvedValue(() => mockAxiosResolve()) await unref(actions)[0].handler() @@ -27,9 +27,9 @@ describe('resetLogo', () => { }) }) - it('should show message on request error', async () => { + it('should show message on request error', () => { jest.spyOn(console, 'error').mockImplementation(() => undefined) - const { wrapper } = getWrapper({ + getWrapper({ setup: async ({ actions }, { storeOptions, clientService, router }) => { clientService.httpAuthenticated.delete.mockRejectedValue(() => mockAxiosReject()) await unref(actions)[0].handler() diff --git a/packages/web-app-admin-settings/tests/unit/composables/actions/useGeneralActionsUploadLogo.spec.ts b/packages/web-app-admin-settings/tests/unit/composables/actions/general/useGeneralActionsUploadLogo.spec.ts similarity index 87% rename from packages/web-app-admin-settings/tests/unit/composables/actions/useGeneralActionsUploadLogo.spec.ts rename to packages/web-app-admin-settings/tests/unit/composables/actions/general/useGeneralActionsUploadLogo.spec.ts index 21e3b4bfe9d..2e09f181813 100644 --- a/packages/web-app-admin-settings/tests/unit/composables/actions/useGeneralActionsUploadLogo.spec.ts +++ b/packages/web-app-admin-settings/tests/unit/composables/actions/general/useGeneralActionsUploadLogo.spec.ts @@ -1,4 +1,4 @@ -import { useGeneralActionsUploadLogo } from '../../../../src/composables/actions/general/useGeneralActionsUploadLogo' +import { useGeneralActionsUploadLogo } from '../../../../../src/composables/actions/general/useGeneralActionsUploadLogo' import { mock } from 'jest-mock-extended' import { VNodeRef } from 'vue' import { @@ -15,8 +15,8 @@ jest.useFakeTimers() describe('uploadImage', () => { describe('method "uploadImage"', () => { - it('should show message on request success', async () => { - const { wrapper } = getWrapper({ + it('should show message on request success', () => { + getWrapper({ setup: async ({ uploadImage }, { storeOptions, clientService, router }) => { clientService.httpAuthenticated.post.mockResolvedValue(() => mockAxiosResolve()) await uploadImage({ @@ -31,9 +31,9 @@ describe('uploadImage', () => { }) }) - it('should show message on request error', async () => { + it('should show message on request error', () => { jest.spyOn(console, 'error').mockImplementation(() => undefined) - const { wrapper } = getWrapper({ + getWrapper({ setup: async ({ uploadImage }, { storeOptions, clientService, router }) => { clientService.httpAuthenticated.post.mockRejectedValue(() => mockAxiosReject()) await uploadImage({ @@ -48,10 +48,10 @@ describe('uploadImage', () => { }) }) - it('should show message on invalid mimeType', async () => { + it('should show message on invalid mimeType', () => { jest.spyOn(console, 'error').mockImplementation(() => undefined) - const { wrapper } = getWrapper({ - setup: async ({ uploadImage }, { storeOptions, clientService, router }) => { + getWrapper({ + setup: async ({ uploadImage }, { storeOptions, clientService }) => { await uploadImage({ currentTarget: { files: [{ name: 'text.txt', type: 'text/plain' }] diff --git a/packages/web-app-admin-settings/tests/unit/composables/actions/groups/useGroupActionsDelete.spec.ts b/packages/web-app-admin-settings/tests/unit/composables/actions/groups/useGroupActionsDelete.spec.ts new file mode 100644 index 00000000000..190df169cc5 --- /dev/null +++ b/packages/web-app-admin-settings/tests/unit/composables/actions/groups/useGroupActionsDelete.spec.ts @@ -0,0 +1,94 @@ +import { useGroupActionsDelete } from '../../../../../src/composables/actions/groups/useGroupActionsDelete' +import { mock } from 'jest-mock-extended' +import { unref } from 'vue' +import { Group } from 'web-client/src/generated' +import { eventBus } from 'web-pkg/src' +import { + createStore, + defaultComponentMocks, + defaultStoreMockOptions, + getComposableWrapper +} from 'web-test-helpers' + +describe('useGroupActionsDelete', () => { + describe('method "isEnabled"', () => { + it.each([ + { resources: [], isEnabled: false }, + { resources: [mock({ groupTypes: [] })], isEnabled: true }, + { + resources: [mock({ groupTypes: [] }), mock({ groupTypes: [] })], + isEnabled: true + } + ])('should only return true if 1 or more groups are selected', ({ resources, isEnabled }) => { + getWrapper({ + setup: ({ actions }) => { + expect(unref(actions)[0].isEnabled({ resources })).toEqual(isEnabled) + } + }) + }) + it('should return false for read-only groups', () => { + getWrapper({ + setup: ({ actions }) => { + const resources = [mock({ groupTypes: ['ReadOnly'] })] + expect(unref(actions)[0].isEnabled({ resources })).toBeFalsy() + } + }) + }) + }) + describe('method "deleteGroups"', () => { + it('should successfully delete all given gropups and reload the groups list', () => { + const eventSpy = jest.spyOn(eventBus, 'publish') + getWrapper({ + setup: async ({ deleteGroups }, { storeOptions, clientService }) => { + const group = mock({ id: '1' }) + await deleteGroups([group]) + expect(clientService.graphAuthenticated.groups.deleteGroup).toHaveBeenCalledWith(group.id) + expect(storeOptions.actions.hideModal).toHaveBeenCalled() + expect(eventSpy).toHaveBeenCalledWith('app.admin-settings.list.load') + } + }) + }) + it('should handle errors and not reload the groups list in such case', () => { + jest.spyOn(console, 'error').mockImplementation(() => undefined) + const eventSpy = jest.spyOn(eventBus, 'publish') + getWrapper({ + setup: async ({ deleteGroups }, { storeOptions, clientService }) => { + clientService.graphAuthenticated.groups.deleteGroup.mockRejectedValue({}) + const group = mock({ id: '1' }) + await deleteGroups([group]) + expect(clientService.graphAuthenticated.groups.deleteGroup).toHaveBeenCalledWith(group.id) + expect(storeOptions.actions.hideModal).toHaveBeenCalled() + expect(eventSpy).not.toHaveBeenCalledWith('app.admin-settings.list.load') + } + }) + }) + }) +}) + +function getWrapper({ + setup +}: { + setup: ( + instance: ReturnType, + { + storeOptions, + clientService + }: { + storeOptions: typeof defaultStoreMockOptions + clientService: ReturnType['$clientService'] + } + ) => void +}) { + const storeOptions = defaultStoreMockOptions + const store = createStore(storeOptions) + const mocks = defaultComponentMocks() + return { + wrapper: getComposableWrapper( + () => { + const instance = useGroupActionsDelete({ store }) + setup(instance, { storeOptions, clientService: mocks.$clientService }) + }, + { store, mocks } + ) + } +} diff --git a/packages/web-app-admin-settings/tests/unit/composables/actions/groups/useGroupActionsEdit.spec.ts b/packages/web-app-admin-settings/tests/unit/composables/actions/groups/useGroupActionsEdit.spec.ts new file mode 100644 index 00000000000..3900f6a9a57 --- /dev/null +++ b/packages/web-app-admin-settings/tests/unit/composables/actions/groups/useGroupActionsEdit.spec.ts @@ -0,0 +1,45 @@ +import { useGroupActionsEdit } from '../../../../../src/composables/actions/groups/useGroupActionsEdit' +import { mock } from 'jest-mock-extended' +import { unref } from 'vue' +import { Group } from 'web-client/src/generated' +import { getComposableWrapper } from 'web-test-helpers' + +describe('useGroupActionsEdit', () => { + describe('method "isEnabled"', () => { + it.each([ + { resources: [mock({ groupTypes: [] })], isEnabled: true }, + { resources: [], isEnabled: false }, + { + resources: [mock({ groupTypes: [] }), mock({ groupTypes: [] })], + isEnabled: false + } + ])('should only return true for one group', ({ resources, isEnabled }) => { + getWrapper({ + setup: ({ actions }) => { + expect(unref(actions)[0].isEnabled({ resources })).toEqual(isEnabled) + } + }) + }) + it('should return false for read-only groups', () => { + getWrapper({ + setup: ({ actions }) => { + const resources = [mock({ groupTypes: ['ReadOnly'] })] + expect(unref(actions)[0].isEnabled({ resources })).toBeFalsy() + } + }) + }) + }) +}) + +function getWrapper({ + setup +}: { + setup: (instance: ReturnType) => void +}) { + return { + wrapper: getComposableWrapper(() => { + const instance = useGroupActionsEdit() + setup(instance) + }) + } +} diff --git a/packages/web-app-admin-settings/tests/unit/composables/actions/users/useUserActionsDelete.spec.ts b/packages/web-app-admin-settings/tests/unit/composables/actions/users/useUserActionsDelete.spec.ts new file mode 100644 index 00000000000..8e1a3b16934 --- /dev/null +++ b/packages/web-app-admin-settings/tests/unit/composables/actions/users/useUserActionsDelete.spec.ts @@ -0,0 +1,83 @@ +import { useUserActionsDelete } from '../../../../../src/composables/actions/users/useUserActionsDelete' +import { mock } from 'jest-mock-extended' +import { unref } from 'vue' +import { User } from 'web-client/src/generated' +import { eventBus } from 'web-pkg/src' +import { + createStore, + defaultComponentMocks, + defaultStoreMockOptions, + getComposableWrapper +} from 'web-test-helpers' + +describe('useUserActionsDelete', () => { + describe('method "isEnabled"', () => { + it.each([ + { resources: [], isEnabled: false }, + { resources: [mock()], isEnabled: true }, + { resources: [mock(), mock()], isEnabled: true } + ])('should only return true if 1 or more users are selected', ({ resources, isEnabled }) => { + getWrapper({ + setup: ({ actions }) => { + expect(unref(actions)[0].isEnabled({ resources })).toEqual(isEnabled) + } + }) + }) + }) + describe('method "deleteUsers"', () => { + it('should successfully delete all given users and reload the users list', () => { + const eventSpy = jest.spyOn(eventBus, 'publish') + getWrapper({ + setup: async ({ deleteUsers }, { storeOptions, clientService }) => { + const user = mock({ id: '1' }) + await deleteUsers([user]) + expect(clientService.graphAuthenticated.users.deleteUser).toHaveBeenCalledWith(user.id) + expect(storeOptions.actions.hideModal).toHaveBeenCalled() + expect(eventSpy).toHaveBeenCalledWith('app.admin-settings.list.load') + } + }) + }) + it('should handle errors and not reload the users list in such case', () => { + jest.spyOn(console, 'error').mockImplementation(() => undefined) + const eventSpy = jest.spyOn(eventBus, 'publish') + getWrapper({ + setup: async ({ deleteUsers }, { storeOptions, clientService }) => { + clientService.graphAuthenticated.users.deleteUser.mockRejectedValue({}) + const user = mock({ id: '1' }) + await deleteUsers([user]) + expect(clientService.graphAuthenticated.users.deleteUser).toHaveBeenCalledWith(user.id) + expect(storeOptions.actions.hideModal).toHaveBeenCalled() + expect(eventSpy).not.toHaveBeenCalledWith('app.admin-settings.list.load') + } + }) + }) + }) +}) + +function getWrapper({ + setup +}: { + setup: ( + instance: ReturnType, + { + storeOptions, + clientService + }: { + storeOptions: typeof defaultStoreMockOptions + clientService: ReturnType['$clientService'] + } + ) => void +}) { + const storeOptions = defaultStoreMockOptions + const store = createStore(storeOptions) + const mocks = defaultComponentMocks() + return { + wrapper: getComposableWrapper( + () => { + const instance = useUserActionsDelete({ store }) + setup(instance, { storeOptions, clientService: mocks.$clientService }) + }, + { store, mocks } + ) + } +} diff --git a/packages/web-app-admin-settings/tests/unit/composables/actions/users/useUserActionsEdit.spec.ts b/packages/web-app-admin-settings/tests/unit/composables/actions/users/useUserActionsEdit.spec.ts new file mode 100644 index 00000000000..71ea837e941 --- /dev/null +++ b/packages/web-app-admin-settings/tests/unit/composables/actions/users/useUserActionsEdit.spec.ts @@ -0,0 +1,34 @@ +import { useUserActionsEdit } from '../../../../../src/composables/actions/users/useUserActionsEdit' +import { mock } from 'jest-mock-extended' +import { unref } from 'vue' +import { User } from 'web-client/src/generated' +import { getComposableWrapper } from 'web-test-helpers' + +describe('useUserActionsEdit', () => { + describe('method "isEnabled"', () => { + it.each([ + { resources: [mock()], isEnabled: true }, + { resources: [], isEnabled: false }, + { resources: [mock(), mock()], isEnabled: false } + ])('should only return true for one user', ({ resources, isEnabled }) => { + getWrapper({ + setup: ({ actions }) => { + expect(unref(actions)[0].isEnabled({ resources })).toEqual(isEnabled) + } + }) + }) + }) +}) + +function getWrapper({ + setup +}: { + setup: (instance: ReturnType) => void +}) { + return { + wrapper: getComposableWrapper(() => { + const instance = useUserActionsEdit() + setup(instance) + }) + } +}