Skip to content

Commit

Permalink
fix: trash bin breaking on parent folder navigation
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Jannik Stehle committed Jul 2, 2024
1 parent c905da6 commit a9db667
Show file tree
Hide file tree
Showing 9 changed files with 51 additions and 36 deletions.
7 changes: 7 additions & 0 deletions changelog/unreleased/bugfix-trash-bin-break-on-navigation
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
})
Expand Down
8 changes: 6 additions & 2 deletions packages/web-client/src/helpers/resource/functions.ts
Original file line number Diff line number Diff line change
@@ -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, '')
}
Expand Down Expand Up @@ -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)
Expand Down
7 changes: 5 additions & 2 deletions packages/web-client/src/helpers/resource/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions packages/web-pkg/src/components/FilesList/ResourceTable.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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[]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ import {
Resource,
isProjectSpaceResource,
extractExtensionFromFile,
SpaceResource
SpaceResource,
isTrashResource
} from '@ownclouders/web-client'
import {
ResolveStrategy,
Expand Down Expand Up @@ -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
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,6 @@ const resourcesWithAllFields = [
tags: ['space', 'tag'],
mdate: getCurrentDate(),
sdate: getCurrentDate(),
ddate: getCurrentDate(),
owner,
sharedBy,
sharedWith,
Expand All @@ -161,7 +160,6 @@ const resourcesWithAllFields = [
tags: [],
mdate: getCurrentDate(),
sdate: getCurrentDate(),
ddate: getCurrentDate(),
owner,
sharedBy,
sharedWith,
Expand All @@ -185,7 +183,6 @@ const resourcesWithAllFields = [
size: '237895',
mdate: getCurrentDate(),
sdate: getCurrentDate(),
ddate: getCurrentDate(),
owner,
sharedBy,
sharedWith,
Expand Down Expand Up @@ -243,7 +240,6 @@ const processingResourcesWithAllFields = [
tags: ['space', 'tag'],
mdate: getCurrentDate(),
sdate: getCurrentDate(),
ddate: getCurrentDate(),
owner,
sharedBy,
sharedWith,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand All @@ -23,7 +23,7 @@ describe('emptyTrashBin', () => {
expect(
unref(actions)[0].isVisible({
space,
resources: [{ canBeRestored: () => true }] as Resource[]
resources: [{ canBeRestored: () => true }] as TrashResource[]
})
).toBe(false)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -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)
}
})
})
Expand All @@ -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)
}
Expand All @@ -43,7 +38,7 @@ describe('restore', () => {
expect(
unref(actions)[0].isVisible({
space,
resources: [{ canBeRestored: () => false }] as Resource[]
resources: [{ canBeRestored: () => false }] as TrashResource[]
})
).toBe(false)
}
Expand All @@ -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
)
}
})
})
Expand All @@ -64,7 +61,7 @@ describe('restore', () => {
expect(
unref(actions)[0].isVisible({
space,
resources: [{ canBeRestored: () => true }] as Resource[]
resources: [{ canBeRestored: () => true }] as TrashResource[]
})
).toBe(false)
}
Expand All @@ -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 }) => {
Expand All @@ -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 }) => {
Expand All @@ -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(), {
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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<typeof useFileActionsRestore>,
{
Expand Down

0 comments on commit a9db667

Please sign in to comment.