Skip to content

Commit

Permalink
Merge pull request #11132 from owncloud/fix/trash-bin-breaking
Browse files Browse the repository at this point in the history
fix: trash bin breaking on parent folder navigation
  • Loading branch information
JammingBen authored Jul 2, 2024
2 parents 45078c7 + a9db667 commit a94351d
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 @@ -141,7 +141,7 @@ import {
useResourceContents
} 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 @@ -258,11 +258,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 a94351d

Please sign in to comment.