diff --git a/packages/design-system/src/components/OcTable/OcTable.spec.ts b/packages/design-system/src/components/OcTable/OcTable.spec.ts index be99fe03642..bd9b9b15c56 100644 --- a/packages/design-system/src/components/OcTable/OcTable.spec.ts +++ b/packages/design-system/src/components/OcTable/OcTable.spec.ts @@ -233,6 +233,23 @@ describe('OcTable', () => { }) }) + it('re-renders a row when its itemKey changes', async () => { + const wrapper = mount(Table, { + props: { + fields, + data, + itemKey: (item: { id: string; doubled: number }) => `${item.id}-${item.doubled}` + }, + global: { plugins: defaultPlugins() } + }) + const row = wrapper.find(`[data-item-id="${data[0].id}"]`).element + + await wrapper.setProps({ data: [{ ...data[0], doubled: 4 }, ...data.slice(1)] }) + + expect(wrapper.find(`[data-item-id="${data[0].id}"]`).element).not.toBe(row) + expect(wrapper.find(`[data-item-id="${data[1].id}"]`).exists()).toBe(true) + }) + it('emits contextmenu-clicked event upon right click on table row', async () => { const wrapper = shallowMount(Table, { props: { diff --git a/packages/design-system/src/components/OcTable/OcTable.vue b/packages/design-system/src/components/OcTable/OcTable.vue index 3b9c67e0b22..1d36830c23a 100644 --- a/packages/design-system/src/components/OcTable/OcTable.vue +++ b/packages/design-system/src/components/OcTable/OcTable.vue @@ -55,7 +55,7 @@ string + /** + * @docs A function to get the key for each row. The row is re-rendered when its key changes. Falls back to the dom selector. + */ + itemKey?: (item: Item) => string /** * @docs Determines if the table should be lazy loaded. * @default false @@ -294,6 +298,7 @@ const { hover = false, idKey = 'id', itemDomSelector, + itemKey, lazy = false, scrollContainer = undefined, paddingX = 'small', diff --git a/packages/web-app-files/src/composables/useFileUpload.ts b/packages/web-app-files/src/composables/useFileUpload.ts index 3778d15f5de..a8cfe8f9f8d 100644 --- a/packages/web-app-files/src/composables/useFileUpload.ts +++ b/packages/web-app-files/src/composables/useFileUpload.ts @@ -8,7 +8,11 @@ import { useExtensionRegistry } from '@opencloud-eu/web-pkg' import { computed, onMounted, onBeforeUnmount, unref, watch, Ref } from 'vue' -import { SpaceResource, isPublicSpaceResource } from '@opencloud-eu/web-client' +import { + SpaceResource, + isPublicSpaceResource, + isShareSpaceResource +} from '@opencloud-eu/web-client' import { useService, useUpload, UppyService, UploadResult } from '@opencloud-eu/web-pkg' import { HandleUpload } from '../HandleUpload' import { useGettext } from 'vue3-gettext' @@ -81,9 +85,14 @@ export const useFileUpload = (space: Ref) => { path: unref(currentFolder).path }) - const existingIds = new Set(resourcesStore.resources.map((r) => r.id)) - const newResources = children.filter((child) => !existingIds.has(child.id)) - resourcesStore.upsertResources(newResources) + if (isShareSpaceResource(unref(space))) { + // TODO: remove when server returns share id for federated shares in propfind response + children.forEach((r) => (r.remoteItemId = unref(space).id)) + } + + const existingEtags = new Map(resourcesStore.resources.map((r) => [r.id, r.etag])) + const changedResources = children.filter((child) => existingEtags.get(child.id) !== child.etag) + resourcesStore.upsertResources(changedResources) } onMounted(() => { diff --git a/packages/web-app-files/tests/unit/composables/useFileUpload.spec.ts b/packages/web-app-files/tests/unit/composables/useFileUpload.spec.ts new file mode 100644 index 00000000000..ac99281db6b --- /dev/null +++ b/packages/web-app-files/tests/unit/composables/useFileUpload.spec.ts @@ -0,0 +1,87 @@ +import { mock } from 'vitest-mock-extended' +import { ref } from 'vue' +import { Resource, SpaceResource } from '@opencloud-eu/web-client' +import { UploadResult, useResourcesStore } from '@opencloud-eu/web-pkg' +import { defaultComponentMocks, getComposableWrapper } from '@opencloud-eu/web-test-helpers' +import { useFileUpload } from '../../../src/composables' + +vi.mock('@opencloud-eu/web-pkg', async (importOriginal) => ({ + ...(await importOriginal()), + useUpload: vi.fn() +})) + +describe('useFileUpload', () => { + it('updates existing resources that were replaced by the upload', async () => { + const existing = { id: 'file', path: '/folder/file.txt', etag: 'old', size: 20 } as Resource + const replaced = { id: 'file', path: '/folder/file.txt', etag: 'new', size: 50 } as Resource + const { resourcesStore, completeUpload } = setup({ + space: mock({ id: 'space-1', driveType: 'personal' }), + existing: [existing], + children: [replaced] + }) + + await completeUpload() + + expect(resourcesStore.upsertResources).toHaveBeenCalledWith([replaced]) + }) + + it('keeps the share id of resources in a share space', async () => { + const replaced = { id: 'file', path: '/folder/file.txt', etag: 'new' } as Resource + const { resourcesStore, completeUpload } = setup({ + space: mock({ id: 'space-1', driveType: 'share' }), + existing: [{ id: 'file', path: '/folder/file.txt', etag: 'old' } as Resource], + children: [replaced] + }) + + await completeUpload() + + expect(resourcesStore.upsertResources).toHaveBeenCalledWith([ + expect.objectContaining({ id: 'file', remoteItemId: 'space-1' }) + ]) + }) +}) + +function setup({ + space, + existing, + children +}: { + space: SpaceResource + existing: Resource[] + children: Resource[] +}) { + const currentFolder = mock({ id: 'folder', path: '/folder' }) + + const mocks = defaultComponentMocks() + mocks.$clientService.webdav.listFiles.mockResolvedValue({ resource: currentFolder, children }) + let onUploadComplete: (result: UploadResult) => Promise + mocks.$uppyService.subscribe.mockImplementation((_, callback) => { + onUploadComplete = callback as typeof onUploadComplete + return 'sub' + }) + + let resourcesStore: ReturnType + getComposableWrapper( + () => { + resourcesStore = useResourcesStore() + useFileUpload(ref(space)) + }, + { + mocks, + provide: mocks, + pluginOptions: { + piniaOptions: { resourcesStore: { currentFolder, resources: existing } } + } + } + ) + + return { + resourcesStore, + completeUpload: () => + onUploadComplete( + mock({ + successful: [{ meta: { spaceId: space.id, driveType: space.driveType } }] + }) + ) + } +} diff --git a/packages/web-pkg/src/components/FilesList/ResourceTable.vue b/packages/web-pkg/src/components/FilesList/ResourceTable.vue index e53784f9e39..94745587999 100644 --- a/packages/web-pkg/src/components/FilesList/ResourceTable.vue +++ b/packages/web-pkg/src/components/FilesList/ResourceTable.vue @@ -20,6 +20,7 @@ :drag-drop="dragDrop" :hover="hover" :item-dom-selector="resourceDomSelector" + :item-key="resourceKey" :sort-by="sortBy" :sort-dir="sortDir" :lazy="lazy" @@ -393,6 +394,11 @@ defineSlots<{ footer?: () => unknown }>() +// re-render a row when its resource changed, so its preview gets loaded again +function resourceKey(resource: Resource) { + return `${resourceDomSelector(resource)}-${resource.etag}` +} + const router = useRouter() const { getExtensionActions } = useFileActions() const capabilityStore = useCapabilityStore() diff --git a/packages/web-pkg/src/components/FilesList/ResourceTiles.vue b/packages/web-pkg/src/components/FilesList/ResourceTiles.vue index d61db94e9d6..c21c189eb58 100644 --- a/packages/web-pkg/src/components/FilesList/ResourceTiles.vue +++ b/packages/web-pkg/src/components/FilesList/ResourceTiles.vue @@ -47,7 +47,7 @@
  • { - if (!unref(currentFolder)?.size || unref(currentFolder)?.size === '0') { - const accumulatedSize = unref(resources) - .map((r) => (r.size ? parseInt(r.size.toString()) : 0)) - .reduce((x, y) => x + y, 0) - return formatFileSize(accumulatedSize, currentLanguage) - } - return formatFileSize(unref(currentFolder).size, currentLanguage) + const accumulatedSize = unref(resources) + .map((r) => (r.size ? parseInt(r.size.toString()) : 0)) + .reduce((x, y) => x + y, 0) + return formatFileSize(accumulatedSize, currentLanguage) }) const resourceContentsText = computed(() => { 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 fcadb9cc6fa..fc3e62bb7c2 100644 --- a/packages/web-pkg/tests/unit/components/FilesList/ResourceTiles.spec.ts +++ b/packages/web-pkg/tests/unit/components/FilesList/ResourceTiles.spec.ts @@ -106,6 +106,16 @@ describe('ResourceTiles component', () => { expect(wrapper.html()).toMatchSnapshot() }) + it('re-renders a tile when its resource changed, so its preview gets loaded again', async () => { + const resource = { ...spacesResources[0], etag: 'old' } + const { wrapper } = getWrapper({ props: { resources: [resource] } }) + expect(wrapper.emitted('itemVisible')).toHaveLength(1) + + await wrapper.setProps({ resources: [{ ...resource, etag: 'new' }] }) + + expect(wrapper.emitted('itemVisible')).toHaveLength(2) + }) + it('renders a footer slot', () => { const { wrapper } = getWrapper({ slots: { footer: 'Hello, ResourceTiles footer!' } }) expect(wrapper.html()).toMatchSnapshot() diff --git a/packages/web-pkg/tests/unit/resources/useResourceContents.spec.ts b/packages/web-pkg/tests/unit/resources/useResourceContents.spec.ts index 75ae2722cd5..fa3acc9eff5 100644 --- a/packages/web-pkg/tests/unit/resources/useResourceContents.spec.ts +++ b/packages/web-pkg/tests/unit/resources/useResourceContents.spec.ts @@ -106,6 +106,18 @@ describe('resourceContents', () => { }) }) }) + it('should sum up the resources instead of using the possibly stale size of the current folder', () => { + getWrapper({ + currentFolder: mock({ size: 3 }), + resources: [ + mock({ isFolder: false, size: 3, type: 'file', name: 'file1' }), + mock({ isFolder: false, size: 2000000, type: 'file', name: 'file2' }) + ], + setup: ({ resourceContentsText }) => { + expect(unref(resourceContentsText)).toBe('2 items with 2 MB in total (2 files, 0 folders)') + } + }) + }) it.each` size | expectedSize ${1} | ${'1 B'} @@ -162,10 +174,12 @@ describe('resourceContents', () => { function getWrapper({ areHiddenFilesShown = true, currentRouteName = 'files-spaces-generic', + currentFolder = null, resources = [], setup }: { areHiddenFilesShown?: boolean + currentFolder?: Resource currentRouteName?: string resources: Resource[] setup: (instance: ReturnType) => void @@ -186,7 +200,7 @@ function getWrapper({ mocks, pluginOptions: { piniaOptions: { - resourcesStore: { resources, areHiddenFilesShown } + resourcesStore: { resources, areHiddenFilesShown, currentFolder } } }, provide: mocks