From 8900fea97e38590a0fd57ab20ceedebe8da592bb Mon Sep 17 00:00:00 2001
From: Jannik Stehle <jannik.stehle@kiteworks.com>
Date: Thu, 8 Aug 2024 11:24:58 +0200
Subject: [PATCH] fix: keyboard actions for deactivated resources

Prevents actions for disabled resources from being accessible via keyboard navigation. This is achieved by either disabling them (checkboxes) or hiding them (rename action, quick actions).
---
 ...ix-keyboard-actions-for-disabled-resources |  6 +++
 .../OcStatusIndicators.spec.ts                | 13 ++++++
 .../OcStatusIndicators/OcStatusIndicators.vue |  9 +++-
 .../components/FilesList/ResourceTable.vue    |  5 ++-
 .../components/FilesList/ResourceTiles.vue    |  6 +++
 .../actions/files/useFileActionsRename.ts     |  2 +-
 .../FilesList/ResourceTable.spec.ts           | 44 ++++++++++++++-----
 .../FilesList/ResourceTiles.spec.ts           | 34 +++++++++++++-
 8 files changed, 105 insertions(+), 14 deletions(-)
 create mode 100644 changelog/unreleased/bugfix-keyboard-actions-for-disabled-resources

diff --git a/changelog/unreleased/bugfix-keyboard-actions-for-disabled-resources b/changelog/unreleased/bugfix-keyboard-actions-for-disabled-resources
new file mode 100644
index 00000000000..9490406006b
--- /dev/null
+++ b/changelog/unreleased/bugfix-keyboard-actions-for-disabled-resources
@@ -0,0 +1,6 @@
+Bugfix: Keyboard actions for disabled resources
+
+We've fixed an issue where certain actions such as rename or select were still possible for disabled resources when navigating via keyboard.
+
+https://github.com/owncloud/web/pull/11342
+https://github.com/owncloud/web/issues/11335
diff --git a/packages/design-system/src/components/OcStatusIndicators/OcStatusIndicators.spec.ts b/packages/design-system/src/components/OcStatusIndicators/OcStatusIndicators.spec.ts
index 4a2bd64fb98..71a796c1b7f 100644
--- a/packages/design-system/src/components/OcStatusIndicators/OcStatusIndicators.spec.ts
+++ b/packages/design-system/src/components/OcStatusIndicators/OcStatusIndicators.spec.ts
@@ -46,4 +46,17 @@ describe('OcStatusIndicators', () => {
     })
     expect(wrapper.find(`#${indicator.id}`).exists()).toBeTruthy()
   })
+  it('does not render a button if disableHandler is set', () => {
+    const wrapper = mount(StatusIndicators, {
+      props: {
+        resource: fileResource,
+        indicators: [indicator],
+        disableHandler: true
+      },
+      global: {
+        plugins: [...defaultPlugins()]
+      }
+    })
+    expect(wrapper.find('button').exists()).toBeFalsy()
+  })
 })
diff --git a/packages/design-system/src/components/OcStatusIndicators/OcStatusIndicators.vue b/packages/design-system/src/components/OcStatusIndicators/OcStatusIndicators.vue
index 921886d4d0b..cc167724915 100644
--- a/packages/design-system/src/components/OcStatusIndicators/OcStatusIndicators.vue
+++ b/packages/design-system/src/components/OcStatusIndicators/OcStatusIndicators.vue
@@ -2,7 +2,7 @@
   <div class="oc-status-indicators">
     <template v-for="indicator in indicators">
       <oc-button
-        v-if="hasHandler(indicator)"
+        v-if="hasHandler(indicator) && !disableHandler"
         :id="indicator.id"
         :key="`${indicator.id}-handler`"
         v-oc-tooltip="$gettext(indicator.label)"
@@ -97,6 +97,13 @@ export default defineComponent({
     indicators: {
       type: Array as PropType<Indicator[]>,
       required: true
+    },
+    /**
+     * Disables the handler for all indicators. This is useful e.g. for disabled resources.
+     */
+    disableHandler: {
+      type: Boolean,
+      default: false
     }
   },
 
diff --git a/packages/web-pkg/src/components/FilesList/ResourceTable.vue b/packages/web-pkg/src/components/FilesList/ResourceTable.vue
index 0329ec28e02..651aefab239 100644
--- a/packages/web-pkg/src/components/FilesList/ResourceTable.vue
+++ b/packages/web-pkg/src/components/FilesList/ResourceTable.vue
@@ -41,6 +41,7 @@
         <oc-checkbox
           id="resource-table-select-all"
           size="large"
+          :disabled="resources.length === disabledResources.length"
           :label="allResourcesCheckboxLabel"
           :hide-label="true"
           :model-value="areAllResourcesSelected"
@@ -54,6 +55,7 @@
         :label="getResourceCheckboxLabel(item)"
         :hide-label="true"
         size="large"
+        :disabled="isResourceDisabled(item)"
         :model-value="isResourceSelected(item)"
         :outline="isLatestSelectedItem(item)"
         @click.stop="toggleSelection(item.id)"
@@ -155,6 +157,7 @@
         v-if="item.indicators.length"
         :resource="item"
         :indicators="item.indicators"
+        :disable-handler="isResourceDisabled(item)"
       />
     </template>
     <template #sdate="{ item }">
@@ -204,7 +207,7 @@
       </oc-button>
     </template>
     <template #actions="{ item }">
-      <div class="resource-table-actions">
+      <div v-if="!isResourceDisabled(item)" class="resource-table-actions">
         <!-- @slot Add quick actions before the `context-menu / three dot` button in the actions column -->
         <slot name="quickActions" :resource="item" />
         <context-menu-quick-action
diff --git a/packages/web-pkg/src/components/FilesList/ResourceTiles.vue b/packages/web-pkg/src/components/FilesList/ResourceTiles.vue
index e3199da1683..20d2ac46ab3 100644
--- a/packages/web-pkg/src/components/FilesList/ResourceTiles.vue
+++ b/packages/web-pkg/src/components/FilesList/ResourceTiles.vue
@@ -68,6 +68,7 @@
               :hide-label="true"
               size="large"
               class="oc-flex-inline oc-p-s"
+              :disabled="isResourceDisabled(resource)"
               :model-value="isResourceSelected(resource)"
               @click.stop.prevent="toggleTile([resource, $event])"
             />
@@ -87,6 +88,7 @@
           </template>
           <template #contextMenu>
             <context-menu-quick-action
+              v-if="!isResourceDisabled(resource)"
               :ref="(el) => (tileRefs.dropBtns[resource.id] = el as ContextMenuQuickActionRef)"
               :item="resource"
               class="resource-tiles-btn-action-dropdown"
@@ -313,6 +315,10 @@ export default defineComponent({
     }
 
     const isResourceClickable = (resource: Resource) => {
+      if (isResourceDisabled(resource)) {
+        return false
+      }
+
       if (resource.isFolder) {
         return true
       }
diff --git a/packages/web-pkg/src/composables/actions/files/useFileActionsRename.ts b/packages/web-pkg/src/composables/actions/files/useFileActionsRename.ts
index 45f31e0e14c..953ff9901fe 100644
--- a/packages/web-pkg/src/composables/actions/files/useFileActionsRename.ts
+++ b/packages/web-pkg/src/composables/actions/files/useFileActionsRename.ts
@@ -238,7 +238,7 @@ export const useFileActionsRename = () => {
         }
 
         const renameDisabled = resources.some((resource) => {
-          return !resource.canRename()
+          return !resource.canRename() || resource.processing
         })
         return !renameDisabled
       },
diff --git a/packages/web-pkg/tests/unit/components/FilesList/ResourceTable.spec.ts b/packages/web-pkg/tests/unit/components/FilesList/ResourceTable.spec.ts
index 763f6461ddf..26c7060fc3f 100644
--- a/packages/web-pkg/tests/unit/components/FilesList/ResourceTable.spec.ts
+++ b/packages/web-pkg/tests/unit/components/FilesList/ResourceTable.spec.ts
@@ -16,9 +16,11 @@ import { displayPositionedDropdown } from '../../../../src/helpers/contextMenuDr
 import { eventBus } from '../../../../src/services/eventBus'
 import { SideBarEventTopics } from '../../../../src/composables/sideBar'
 import { mock } from 'vitest-mock-extended'
-import { computed } from 'vue'
+import { computed, ref } from 'vue'
 import { Identity } from '@ownclouders/web-client/graph/generated'
 import { describe } from 'vitest'
+import { useFileActionsRename } from '../../../../src/composables/actions/files'
+import { FileAction } from '../../../../src/composables/actions/types'
 
 const mockUseEmbedMode = vi.fn().mockReturnValue({
   isLocationPicker: computed(() => false),
@@ -36,6 +38,11 @@ vi.mock('../../../../src/composables/resources', async (importOriginal) => ({
   useCanBeOpenedWithSecureView: vi.fn()
 }))
 
+vi.mock('../../../../src/composables/actions/files', async (importOriginal) => ({
+  ...(await importOriginal<any>()),
+  useFileActionsRename: vi.fn()
+}))
+
 const router = {
   push: vi.fn(),
   afterEach: vi.fn(),
@@ -491,16 +498,15 @@ describe('ResourceTable', () => {
       expect(spyDisplayPositionedDropdown).toHaveBeenCalledTimes(1)
     })
 
-    it('does not emit select event on clicking the three-dot icon in table row of a disabled resource', async () => {
-      const spyDisplayPositionedDropdown = vi.mocked(displayPositionedDropdown)
+    it('does not show the three-dot icon in table row of a disabled resource', () => {
       const { wrapper } = getMountedWrapper({ addProcessingResources: true })
-      await wrapper
-        .find(
-          '.oc-tbody-tr-rainforest .oc-table-data-cell-actions .resource-table-btn-action-dropdown'
-        )
-        .trigger('click')
-      expect(wrapper.emitted('update:selectedIds')).toBeUndefined()
-      expect(spyDisplayPositionedDropdown).toHaveBeenCalledTimes(0)
+      expect(
+        wrapper
+          .find(
+            '.oc-tbody-tr-rainforest .oc-table-data-cell-actions .resource-table-btn-action-dropdown'
+          )
+          .exists()
+      ).toBeFalsy()
     })
 
     it('removes invalid chars from item ids for usage in html template', async () => {
@@ -601,6 +607,16 @@ describe('ResourceTable', () => {
       expect(wrapper.findAll('.resource-table-shared-with .oc-avatar').length).toBe(1)
     })
   })
+  describe('rename action', () => {
+    it('shows if available', () => {
+      const { wrapper } = getMountedWrapper()
+      expect(wrapper.find('.resource-table-edit-name').exists()).toBeTruthy()
+    })
+    it('does not show if not available', () => {
+      const { wrapper } = getMountedWrapper({ hasRenameAction: false })
+      expect(wrapper.find('.resource-table-edit-name').exists()).toBeFalsy()
+    })
+  })
 })
 
 function getMountedWrapper({
@@ -608,12 +624,14 @@ function getMountedWrapper({
   userContextReady = true,
   addProcessingResources = false,
   canBeOpenedWithSecureView = true,
+  hasRenameAction = true,
   resources = resourcesWithAllFields
 }: {
   props?: PartialComponentProps<typeof ResourceTable>
   userContextReady?: boolean
   addProcessingResources?: boolean
   canBeOpenedWithSecureView?: boolean
+  hasRenameAction?: boolean
   resources?: Resource[]
 } = {}) {
   const capabilities = {
@@ -624,6 +642,12 @@ function getMountedWrapper({
     canBeOpenedWithSecureView: () => canBeOpenedWithSecureView
   })
 
+  vi.mocked(useFileActionsRename).mockReturnValue(
+    mock<ReturnType<typeof useFileActionsRename>>({
+      actions: ref([mock<FileAction>({ isVisible: () => hasRenameAction })])
+    })
+  )
+
   return {
     wrapper: mount(ResourceTable, {
       props: {
diff --git a/packages/web-pkg/tests/unit/components/FilesList/ResourceTiles.spec.ts b/packages/web-pkg/tests/unit/components/FilesList/ResourceTiles.spec.ts
index 4b753e53baf..896ac5de434 100644
--- a/packages/web-pkg/tests/unit/components/FilesList/ResourceTiles.spec.ts
+++ b/packages/web-pkg/tests/unit/components/FilesList/ResourceTiles.spec.ts
@@ -7,7 +7,9 @@ import { ComponentPublicInstance, computed } from 'vue'
 import { extractDomSelector } from '@ownclouders/web-client'
 import OcDrop from 'design-system/src/components/OcDrop/OcDrop.vue'
 import { useCanBeOpenedWithSecureView } from '../../../../src/composables/resources'
+import { displayPositionedDropdown } from '../../../../src/helpers/contextMenuDropdown'
 
+vi.mock('../../../../src/helpers/contextMenuDropdown')
 vi.mock('../../../../src/composables/viewMode', async (importOriginal) => ({
   ...(await importOriginal<any>()),
   useTileSize: vi.fn().mockReturnValue({
@@ -65,7 +67,7 @@ const resources = [
     canRename: vi.fn(),
     getDomSelector: () => extractDomSelector('forest'),
     canDownload: () => true
-  }
+  } as Resource
 ]
 
 describe('ResourceTiles component', () => {
@@ -198,6 +200,36 @@ describe('ResourceTiles component', () => {
         expect(wrapper.emitted('fileDropped')).toBeDefined()
       })
     })
+    describe('context menu', () => {
+      it('triggers the positioned dropdown on click', async () => {
+        const spyDisplayPositionedDropdown = vi.mocked(displayPositionedDropdown)
+        const { wrapper } = getWrapper({ props: { resources } })
+        const btn = wrapper.find('.resource-tiles-btn-action-dropdown')
+        await btn.trigger('click')
+        expect(spyDisplayPositionedDropdown).toHaveBeenCalled()
+      })
+      it('does not show for disabled resources', () => {
+        const { wrapper } = getWrapper({
+          props: { resources: [{ ...resources[0], processing: true }] }
+        })
+        expect(wrapper.find('.resource-tiles-btn-action-dropdown').exists()).toBeFalsy()
+      })
+    })
+    describe('checkboxes', () => {
+      it('update the selected ids on click', async () => {
+        const { wrapper } = getWrapper({ props: { resources } })
+        const checkbox = wrapper.find('input[type="checkbox"]')
+        await checkbox.trigger('click')
+        expect(wrapper.emitted('update:selectedIds')).toBeTruthy()
+      })
+      it('are disabled for disabled resources', () => {
+        const { wrapper } = getWrapper({
+          props: { resources: [{ ...resources[0], processing: true }] }
+        })
+        const checkbox = wrapper.find('input[type="checkbox"]')
+        expect(Object.keys(checkbox.attributes())).toContain('disabled')
+      })
+    })
   })
 
   it.each([