Fix batch context actions in admin settings

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

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

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) => {

packages/web-app-admin-settings/src/composables/actions/groups/useGroupActionsDelete.ts

@@ -94,6 +94,8 @@ export const useGroupActionsDelete = ({ store }: { store?: Store<any> }) => {
   ])
 
   return {
-    actions
+    actions,
+    // HACK: exported for unit tests:
+    deleteGroups
   }
 }

packages/web-app-admin-settings/src/composables/actions/users/useUserActionsDelete.ts

@@ -89,6 +89,8 @@ export const useUserActionsDelete = ({ store }: { store?: Store<any> }) => {
   ])
 
   return {
-    actions
+    actions,
+    // HACK: exported for unit tests:
+    deleteUsers
   }
 }

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'

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<Action>({ isEnabled: () => true })])
+        actions: computed(() => [mock<Action>({ isEnabled: () => true })]),
+        deleteGroups: null
       }))
       jest.mocked(useGroupActionsEdit).mockImplementation(() => ({
         actions: computed(() => [mock<Action>({ isEnabled: () => true })])

packages/web-app-admin-settings/tests/unit/composables/actions/useGeneralActionsResetLogo.spec.ts → 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()

packages/web-app-admin-settings/tests/unit/composables/actions/useGeneralActionsUploadLogo.spec.ts → 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' }]

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<Group>({ groupTypes: [] })], isEnabled: true },
+      {
+        resources: [mock<Group>({ groupTypes: [] }), mock<Group>({ 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<Group>({ 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<Group>({ 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<Group>({ 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<typeof useGroupActionsDelete>,
+    {
+      storeOptions,
+      clientService
+    }: {
+      storeOptions: typeof defaultStoreMockOptions
+      clientService: ReturnType<typeof defaultComponentMocks>['$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 }
+    )
+  }
+}

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<Group>({ groupTypes: [] })], isEnabled: true },
+      { resources: [], isEnabled: false },
+      {
+        resources: [mock<Group>({ groupTypes: [] }), mock<Group>({ 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<Group>({ groupTypes: ['ReadOnly'] })]
+          expect(unref(actions)[0].isEnabled({ resources })).toBeFalsy()
+        }
+      })
+    })
+  })
+})
+
+function getWrapper({
+  setup
+}: {
+  setup: (instance: ReturnType<typeof useGroupActionsEdit>) => void
+}) {
+  return {
+    wrapper: getComposableWrapper(() => {
+      const instance = useGroupActionsEdit()
+      setup(instance)
+    })
+  }
+}

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<User>()], isEnabled: true },
+      { resources: [mock<User>(), mock<User>()], 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<User>({ 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<User>({ 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<typeof useUserActionsDelete>,
+    {
+      storeOptions,
+      clientService
+    }: {
+      storeOptions: typeof defaultStoreMockOptions
+      clientService: ReturnType<typeof defaultComponentMocks>['$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 }
+    )
+  }
+}

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<User>()], isEnabled: true },
+      { resources: [], isEnabled: false },
+      { resources: [mock<User>(), mock<User>()], 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<typeof useUserActionsEdit>) => void
+}) {
+  return {
+    wrapper: getComposableWrapper(() => {
+      const instance = useUserActionsEdit()
+      setup(instance)
+    })
+  }
+}