Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: trash bin breaking on parent folder navigation #11132

Merged
merged 1 commit into from
Jul 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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