From a9db66791d6fbd9b43f1ea1bb189b371eb4dd863 Mon Sep 17 00:00:00 2001 From: Jannik Stehle Date: Tue, 2 Jul 2024 14:05:31 +0200 Subject: [PATCH] fix: trash bin breaking on parent folder navigation Fixes an issue where the trash bin would break when navigating via the parent folder of a resource. Also adds an interface for trashed resources to better distinguish them from regular resources. --- .../bugfix-trash-bin-break-on-navigation | 7 ++++ .../SideBar/Details/FileDetails.vue | 10 ++++-- .../src/helpers/resource/functions.ts | 8 +++-- .../web-client/src/helpers/resource/types.ts | 7 ++-- .../components/FilesList/ResourceTable.vue | 6 ++-- .../actions/files/useFileActionsRestore.ts | 5 +-- .../FilesList/ResourceTable.spec.ts | 4 --- .../files/useFileActionsEmptyTrashBin.spec.ts | 4 +-- .../files/useFileActionsRestore.spec.ts | 36 +++++++++---------- 9 files changed, 51 insertions(+), 36 deletions(-) create mode 100644 changelog/unreleased/bugfix-trash-bin-break-on-navigation diff --git a/changelog/unreleased/bugfix-trash-bin-break-on-navigation b/changelog/unreleased/bugfix-trash-bin-break-on-navigation new file mode 100644 index 00000000000..9f64593daa2 --- /dev/null +++ b/changelog/unreleased/bugfix-trash-bin-break-on-navigation @@ -0,0 +1,7 @@ +Bugfix: Trash bin breaking on navigation + +We've fixed a bug where the trash bin would break when navigating into the parent folder of a resource. + +https://github.com/owncloud/web/pull/11132 +https://github.com/owncloud/web/issues/11100 +https://github.com/owncloud/web/issues/10686 diff --git a/packages/web-app-files/src/components/SideBar/Details/FileDetails.vue b/packages/web-app-files/src/components/SideBar/Details/FileDetails.vue index 58fd94dfed6..67cbd483411 100644 --- a/packages/web-app-files/src/components/SideBar/Details/FileDetails.vue +++ b/packages/web-app-files/src/components/SideBar/Details/FileDetails.vue @@ -140,7 +140,7 @@ import { formatDateFromJSDate } from '@ownclouders/web-pkg' import upperFirst from 'lodash-es/upperFirst' -import { isShareResource, ShareTypes } from '@ownclouders/web-client' +import { isShareResource, isTrashResource, ShareTypes } from '@ownclouders/web-client' import { usePreviewService, useGetMatchingSpace } from '@ownclouders/web-pkg' import { getIndicators } from '@ownclouders/web-pkg' import { @@ -256,11 +256,15 @@ export default defineComponent({ }) const hasDeletionDate = computed(() => { - return unref(resource).ddate + return isTrashResource(unref(resource)) }) const capitalizedDeletionDate = computed(() => { - const displayDate = formatDateFromJSDate(new Date(unref(resource).ddate), language.current) + const item = unref(resource) + if (!isTrashResource(item)) { + return '' + } + const displayDate = formatDateFromJSDate(new Date(item.ddate), language.current) return upperFirst(displayDate) }) diff --git a/packages/web-client/src/helpers/resource/functions.ts b/packages/web-client/src/helpers/resource/functions.ts index 4be9344ec57..6d3e2e9840c 100644 --- a/packages/web-client/src/helpers/resource/functions.ts +++ b/packages/web-client/src/helpers/resource/functions.ts @@ -1,13 +1,17 @@ import path, { basename, dirname } from 'path' import { urlJoin } from '../../utils' import { DavPermission, DavProperty } from '../../webdav/constants' -import { Resource, ResourceIndicator, WebDavResponseResource } from './types' +import { Resource, ResourceIndicator, TrashResource, WebDavResponseResource } from './types' import { camelCase } from 'lodash-es' const fileExtensions = { complex: ['tar.bz2', 'tar.gz', 'tar.xz'] } +export const isTrashResource = (resource: Resource): resource is TrashResource => { + return Object.hasOwn(resource, 'ddate') +} + export const extractDomSelector = (str: string): string => { return str.replace(/[^A-Za-z0-9\-_]/g, '') } @@ -202,7 +206,7 @@ export function buildResource(resource: WebDavResponseResource): Resource { return r } -export function buildDeletedResource(resource: WebDavResponseResource): Resource { +export function buildDeletedResource(resource: WebDavResponseResource): TrashResource { const isFolder = resource.type === 'directory' const fullName = resource.props[DavProperty.TrashbinOriginalFilename]?.toString() const extension = extractExtensionFromFile({ name: fullName, type: resource.type } as Resource) diff --git a/packages/web-client/src/helpers/resource/types.ts b/packages/web-client/src/helpers/resource/types.ts index 9004d9832c9..359c8e2d6aa 100644 --- a/packages/web-client/src/helpers/resource/types.ts +++ b/packages/web-client/src/helpers/resource/types.ts @@ -78,7 +78,6 @@ export interface Resource { privateLink?: string owner?: Identity extension?: string - ddate?: string // necessary for incoming share resources and resources inside shares remoteItemId?: string @@ -90,7 +89,6 @@ export interface Resource { canShare?(args?: { user?: User; ability?: Ability }): boolean canRename?(args?: { user?: User; ability?: Ability }): boolean canBeDeleted?(args?: { user?: User; ability?: Ability }): boolean - canBeRestored?(): boolean canDeny?(): boolean canRemoveFromTrashbin?({ user }: { user?: User }): boolean canEditTags?(): boolean @@ -118,6 +116,11 @@ export interface FileResource extends Resource { __fileResource?: any } +export interface TrashResource extends Resource { + ddate: string + canBeRestored(): boolean +} + export interface WebDavResponseTusSupport { extension?: string[] maxSize?: number diff --git a/packages/web-pkg/src/components/FilesList/ResourceTable.vue b/packages/web-pkg/src/components/FilesList/ResourceTable.vue index 4b394131c4c..7fb7a24ce9e 100644 --- a/packages/web-pkg/src/components/FilesList/ResourceTable.vue +++ b/packages/web-pkg/src/components/FilesList/ResourceTable.vue @@ -243,7 +243,7 @@ import { ComponentPublicInstance } from 'vue' import { useWindowSize } from '@vueuse/core' -import { IncomingShareResource, Resource } from '@ownclouders/web-client' +import { IncomingShareResource, Resource, TrashResource } from '@ownclouders/web-client' import { extractDomSelector, SpaceResource } from '@ownclouders/web-client' import { ShareTypes, isShareResource } from '@ownclouders/web-client' @@ -756,9 +756,9 @@ export default defineComponent({ wrap: 'nowrap', width: 'shrink', accessibleLabelCallback: (item) => - this.formatDateRelative((item as Resource).ddate) + + this.formatDateRelative((item as TrashResource).ddate) + ' (' + - this.formatDate((item as Resource).ddate) + + this.formatDate((item as TrashResource).ddate) + ')' } ] as FieldType[] diff --git a/packages/web-pkg/src/composables/actions/files/useFileActionsRestore.ts b/packages/web-pkg/src/composables/actions/files/useFileActionsRestore.ts index c5a0fc3f4a8..acaf6e6717f 100644 --- a/packages/web-pkg/src/composables/actions/files/useFileActionsRestore.ts +++ b/packages/web-pkg/src/composables/actions/files/useFileActionsRestore.ts @@ -5,7 +5,8 @@ import { Resource, isProjectSpaceResource, extractExtensionFromFile, - SpaceResource + SpaceResource, + isTrashResource } from '@ownclouders/web-client' import { ResolveStrategy, @@ -213,7 +214,7 @@ export const useFileActionsRestore = () => { if (!isLocationTrashActive(router, 'files-trash-generic')) { return false } - if (!resources.every((r) => r.canBeRestored())) { + if (!resources.every((r) => isTrashResource(r) && r.canBeRestored())) { return false } 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 a6fff8f8665..617008a52ee 100644 --- a/packages/web-pkg/tests/unit/components/FilesList/ResourceTable.spec.ts +++ b/packages/web-pkg/tests/unit/components/FilesList/ResourceTable.spec.ts @@ -136,7 +136,6 @@ const resourcesWithAllFields = [ tags: ['space', 'tag'], mdate: getCurrentDate(), sdate: getCurrentDate(), - ddate: getCurrentDate(), owner, sharedBy, sharedWith, @@ -161,7 +160,6 @@ const resourcesWithAllFields = [ tags: [], mdate: getCurrentDate(), sdate: getCurrentDate(), - ddate: getCurrentDate(), owner, sharedBy, sharedWith, @@ -185,7 +183,6 @@ const resourcesWithAllFields = [ size: '237895', mdate: getCurrentDate(), sdate: getCurrentDate(), - ddate: getCurrentDate(), owner, sharedBy, sharedWith, @@ -243,7 +240,6 @@ const processingResourcesWithAllFields = [ tags: ['space', 'tag'], mdate: getCurrentDate(), sdate: getCurrentDate(), - ddate: getCurrentDate(), owner, sharedBy, sharedWith, diff --git a/packages/web-pkg/tests/unit/composables/actions/files/useFileActionsEmptyTrashBin.spec.ts b/packages/web-pkg/tests/unit/composables/actions/files/useFileActionsEmptyTrashBin.spec.ts index 5d5316db01e..458da1c71e2 100644 --- a/packages/web-pkg/tests/unit/composables/actions/files/useFileActionsEmptyTrashBin.spec.ts +++ b/packages/web-pkg/tests/unit/composables/actions/files/useFileActionsEmptyTrashBin.spec.ts @@ -3,7 +3,7 @@ import { useMessages, useModals } from '../../../../../src/composables/piniaStor import { mock } from 'vitest-mock-extended' import { getComposableWrapper, defaultComponentMocks, RouteLocation } from 'web-test-helpers' import { unref } from 'vue' -import { ProjectSpaceResource, Resource } from '@ownclouders/web-client' +import { ProjectSpaceResource, TrashResource } from '@ownclouders/web-client' import { FileActionOptions } from '../../../../../src/composables/actions' describe('emptyTrashBin', () => { @@ -23,7 +23,7 @@ describe('emptyTrashBin', () => { expect( unref(actions)[0].isVisible({ space, - resources: [{ canBeRestored: () => true }] as Resource[] + resources: [{ canBeRestored: () => true }] as TrashResource[] }) ).toBe(false) } diff --git a/packages/web-pkg/tests/unit/composables/actions/files/useFileActionsRestore.spec.ts b/packages/web-pkg/tests/unit/composables/actions/files/useFileActionsRestore.spec.ts index c7c0a01e616..ccbcbc4feb7 100644 --- a/packages/web-pkg/tests/unit/composables/actions/files/useFileActionsRestore.spec.ts +++ b/packages/web-pkg/tests/unit/composables/actions/files/useFileActionsRestore.spec.ts @@ -3,7 +3,7 @@ import { mock } from 'vitest-mock-extended' import { defaultComponentMocks, getComposableWrapper, RouteLocation } from 'web-test-helpers' import { useMessages, useResourcesStore } from '../../../../../src/composables/piniaStores' import { unref } from 'vue' -import { HttpError, Resource } from '@ownclouders/web-client' +import { HttpError, Resource, TrashResource } from '@ownclouders/web-client' import { ProjectSpaceResource, SpaceResource } from '@ownclouders/web-client' import { Drive } from '@ownclouders/web-client/graph/generated' import { AxiosResponse } from 'axios' @@ -16,12 +16,7 @@ describe('restore', () => { it('should be false when no resource is given', () => { getWrapper({ setup: ({ actions }, { space }) => { - expect( - unref(actions)[0].isVisible({ - space, - resources: [] as Resource[] - }) - ).toBe(false) + expect(unref(actions)[0].isVisible({ space, resources: [] })).toBe(false) } }) }) @@ -31,7 +26,7 @@ describe('restore', () => { expect( unref(actions)[0].isVisible({ space, - resources: [{ canBeRestored: () => true }] as Resource[] + resources: [{ canBeRestored: () => true, ddate: '2020-01-01' }] as TrashResource[] }) ).toBe(true) } @@ -43,7 +38,7 @@ describe('restore', () => { expect( unref(actions)[0].isVisible({ space, - resources: [{ canBeRestored: () => false }] as Resource[] + resources: [{ canBeRestored: () => false }] as TrashResource[] }) ).toBe(false) } @@ -53,7 +48,9 @@ describe('restore', () => { getWrapper({ invalidLocation: true, setup: ({ actions }, { space }) => { - expect(unref(actions)[0].isVisible({ space, resources: [{}] as Resource[] })).toBe(false) + expect(unref(actions)[0].isVisible({ space, resources: [{}] as TrashResource[] })).toBe( + false + ) } }) }) @@ -64,7 +61,7 @@ describe('restore', () => { expect( unref(actions)[0].isVisible({ space, - resources: [{ canBeRestored: () => true }] as Resource[] + resources: [{ canBeRestored: () => true }] as TrashResource[] }) ).toBe(false) } @@ -74,7 +71,7 @@ describe('restore', () => { describe('method "restoreResources"', () => { it('should show message on success', () => { - const resourcesToRestore = [{ id: '1', path: '/1' }] + const resourcesToRestore = [{ id: '1', path: '/1' }] as TrashResource[] getWrapper({ setup: ({ restoreResources }, { space }) => { @@ -93,7 +90,7 @@ describe('restore', () => { it('should show message on error', () => { vi.spyOn(console, 'error').mockImplementation(() => undefined) - const resourcesToRestore = [{ id: '1', path: '/1' }] + const resourcesToRestore = [{ id: '1', path: '/1' }] as TrashResource[] getWrapper({ setup: ({ restoreResources }, { space }) => { @@ -116,7 +113,7 @@ describe('restore', () => { it('should request parent folder on collecting restore conflicts', () => { getWrapper({ setup: async ({ collectConflicts }, { space, clientService }) => { - const resource = { id: '1', path: '1', name: '1' } as Resource + const resource = { id: '1', path: '1', name: '1' } as TrashResource await collectConflicts(space, [resource]) expect(clientService.webdav.listFiles).toHaveBeenCalledWith(expect.anything(), { @@ -129,8 +126,8 @@ describe('restore', () => { it('should find conflict within resources', () => { getWrapper({ setup: async ({ collectConflicts }, { space }) => { - const resourceOne = { id: '1', path: '1', name: '1' } as Resource - const resourceTwo = { id: '2', path: '1', name: '1' } as Resource + const resourceOne = { id: '1', path: '1', name: '1' } as TrashResource + const resourceTwo = { id: '2', path: '1', name: '1' } as TrashResource const { conflicts } = await collectConflicts(space, [resourceOne, resourceTwo]) expect(conflicts).toContain(resourceTwo) @@ -141,7 +138,7 @@ describe('restore', () => { it('should add files without conflict to resolved resources', () => { getWrapper({ setup: async ({ collectConflicts }, { space }) => { - const resource = { id: '1', path: '1', name: '1' } as Resource + const resource = { id: '1', path: '1', name: '1' } as TrashResource const { resolvedResources } = await collectConflicts(space, [resource]) expect(resolvedResources).toContain(resource) @@ -158,7 +155,10 @@ function getWrapper({ }: { invalidLocation?: boolean driveType?: string - restoreResult?: { successful: Resource[]; failed: { resource: Resource; error: HttpError }[] } + restoreResult?: { + successful: TrashResource[] + failed: { resource: TrashResource; error: HttpError }[] + } setup: ( instance: ReturnType, {