Skip to content

Commit

Permalink
fix: space reactivity on space navigation
Browse files Browse the repository at this point in the history
Fixes an issue where the current space did not update when navigating from one space into another directly, e.g. via search results.
  • Loading branch information
JammingBen committed May 21, 2024
1 parent ee52645 commit 636009d
Show file tree
Hide file tree
Showing 16 changed files with 114 additions and 86 deletions.
6 changes: 6 additions & 0 deletions changelog/unreleased/bugfix-space-not-updating
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Bugfix: Space not updating on navigation

We've fixed an issue where the current space did not update when navigating from one space into another directly, e.g. via search results. That could lead to uploads and file actions not working properly.

https://github.com/owncloud/web/issues/10941
https://github.com/owncloud/web/pull/10816
18 changes: 9 additions & 9 deletions packages/web-app-files/src/HandleUpload.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export interface HandleUploadOptions {
store: Store<any>
uppyService: UppyService
id?: string
space?: SpaceResource
space?: Ref<SpaceResource>
quotaCheckEnabled?: boolean
directoryTreeCreateEnabled?: boolean
conflictHandlingEnabled?: boolean
Expand All @@ -47,7 +47,7 @@ export class HandleUpload extends BasePlugin {
hasSpaces: Ref<boolean>
language: Language
route: Ref<RouteLocationNormalizedLoaded>
space: SpaceResource
space: Ref<SpaceResource>
store: Store<any>
uppyService: UppyService
quotaCheckEnabled: boolean
Expand Down Expand Up @@ -107,7 +107,7 @@ export class HandleUpload extends BasePlugin {
// public file drop
const publicLinkToken = queryItemAsString(unref(this.route).params.token)
let endpoint = urlJoin(
this.clientService.webdav.getPublicFileUrl(this.space, publicLinkToken),
this.clientService.webdav.getPublicFileUrl(unref(this.space), publicLinkToken),
{ trailingSlash: true }
)

Expand Down Expand Up @@ -148,7 +148,7 @@ export class HandleUpload extends BasePlugin {
topLevelFolderId = topLevelFolderIds[topLevelDirectory]
}

const webDavUrl = this.space.getWebDavUrl({
const webDavUrl = unref(this.space).getWebDavUrl({
path: currentFolderPath.split('/').map(encodeURIComponent).join('/')
})

Expand All @@ -164,10 +164,10 @@ export class HandleUpload extends BasePlugin {
name: file.name,
mtime: (file.data as any).lastModified / 1000,
// current path & space
spaceId: this.space.id,
spaceName: this.space.name,
driveAlias: this.space.driveAlias,
driveType: this.space.driveType,
spaceId: unref(this.space).id,
spaceName: unref(this.space).name,
driveAlias: unref(this.space).driveAlias,
driveType: unref(this.space).driveType,
currentFolder: currentFolderPath,
currentFolderId,
// upload data
Expand Down Expand Up @@ -266,7 +266,7 @@ export class HandleUpload extends BasePlugin {
*/
async createDirectoryTree(filesToUpload: UppyResource[]): Promise<UppyResource[]> {
const { webdav } = this.clientService
const space = this.space
const space = unref(this.space)
const { id: currentFolderId, path: currentFolderPath } = this.currentFolder

const routeName = filesToUpload[0].meta.routeName
Expand Down
23 changes: 14 additions & 9 deletions packages/web-app-files/src/components/AppBar/CreateAndUpload.vue
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@
<oc-button
:disabled="uploadOrFileCreationBlocked"
class="paste-files-btn"
@click="pasteFileAction({ space })"
@click="pasteFileAction"
>
<oc-icon fill-type="line" name="clipboard" />
<span v-text="$gettext('Paste here')" />
Expand Down Expand Up @@ -262,6 +262,8 @@ export default defineComponent({
const hasSpaces = useCapabilitySpacesEnabled(store)
const areFileExtensionsShown = computed(() => unref(store.state.Files.areFileExtensionsShown))
const space = computed(() => props.space)
useUpload({ uppyService })
if (!uppyService.getPlugin('HandleUpload')) {
Expand All @@ -270,7 +272,7 @@ export default defineComponent({
hasSpaces,
language,
route,
space: props.space,
space,
store,
uppyService
})
Expand All @@ -279,23 +281,26 @@ export default defineComponent({
let uploadCompletedSub
const { actions: pasteFileActions } = useFileActionsPaste({ store })
const pasteFileAction = unref(pasteFileActions)[0].handler
const pasteFileAction = () => {
return unref(pasteFileActions)[0].handler({ space: unref(space) })
}
const { actions: createNewFolder } = useFileActionsCreateNewFolder({
store,
space: props.space
space
})
const createNewFolderAction = computed(() => unref(createNewFolder)[0].handler)
const { actions: createNewShortcut } = useFileActionsCreateNewShortcut({ space: props.space })
const { actions: createNewShortcut } = useFileActionsCreateNewShortcut({ space })
const createNewShortcutAction = computed(() => unref(createNewShortcut)[0].handler)
const newFileHandlers = computed(() => store.getters.newFileHandlers)
const { actions: createNewFileActions } = useFileActionsCreateNewFile({
store,
space: props.space,
space,
newFileHandlers: newFileHandlers
})
Expand All @@ -309,7 +314,7 @@ export default defineComponent({
const { actions: createNewFileMimeTypeActions } = useFileActionsCreateNewFile({
store,
space: props.space,
space,
mimetypesAllowedForCreation: mimetypesAllowedForCreation
})
Expand Down Expand Up @@ -383,10 +388,10 @@ export default defineComponent({
}
const sameFolder =
props.itemId && !isShareSpaceResource(props.space)
props.itemId && !isShareSpaceResource(unref(space))
? props.itemId.toString().startsWith(currentFolderId.toString())
: currentFolder === props.item
const fileIsInCurrentPath = spaceId === props.space.id && sameFolder
const fileIsInCurrentPath = spaceId === unref(space).id && sameFolder
if (fileIsInCurrentPath) {
eventBus.publish('app.files.list.load')
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,15 +45,17 @@ export default defineComponent({
setup(props) {
const { $gettext } = useGettext()
const store = useStore()
const space = computed(() => props.space)
const contextMenuLabel = computed(() => $gettext('Show context menu'))
const currentFolder = computed(() => store.getters['Files/currentFolder'])
const actionOptions = computed(() => ({
space: props.space,
space: unref(space),
resources: [currentFolder.value]
}))
const { actions: createNewFolderAction } = useFileActionsCreateNewFolder({
store,
space: props.space
space
})
const { actions: showDetailsAction } = useFileActionsShowDetails({ store })
const { actions: pasteAction } = useFileActionsPaste({ store })
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,12 @@ import { useStore } from '@ownclouders/web-pkg'
import { useGettext } from 'vue3-gettext'
import { unref } from 'vue'
import { useFileActionsPaste } from '@ownclouders/web-pkg'
import { Ref } from 'vue'

export const useKeyboardTableSpaceActions = (keyActions: KeyboardActions, space: SpaceResource) => {
export const useKeyboardTableSpaceActions = (
keyActions: KeyboardActions,
space: Ref<SpaceResource>
) => {
const store = useStore()
const language = useGettext()

Expand All @@ -15,19 +19,19 @@ export const useKeyboardTableSpaceActions = (keyActions: KeyboardActions, space:
keyActions.bindKeyAction({ modifier: ModifierKey.Ctrl, primary: Key.C }, () => {
store.dispatch('Files/copySelectedFiles', {
...language,
space: space,
space: unref(space),
resources: store.getters['Files/selectedFiles']
})
})

keyActions.bindKeyAction({ modifier: ModifierKey.Ctrl, primary: Key.V }, () => {
pasteFileAction({ space: space })
pasteFileAction({ space: unref(space) })
})

keyActions.bindKeyAction({ modifier: ModifierKey.Ctrl, primary: Key.X }, () => {
store.dispatch('Files/cutSelectedFiles', {
...language,
space: space,
space: unref(space),
resources: store.getters['Files/selectedFiles']
})
})
Expand Down
42 changes: 22 additions & 20 deletions packages/web-app-files/src/views/spaces/GenericSpace.vue
Original file line number Diff line number Diff line change
Expand Up @@ -269,9 +269,12 @@ export default defineComponent({
const hasShareJail = useCapabilityShareJailEnabled()
const { breadcrumbsFromPath, concatBreadcrumbs } = useBreadcrumbsFromPath()
const { openWithDefaultApp } = useOpenWithDefaultApp()
const space = computed(() => props.space)
const { actions: createNewFolder } = useFileActionsCreateNewFolder({
store,
space: props.space
space
})
const { isEnabled: isEmbedModeEnabled } = useEmbedMode()
Expand All @@ -291,16 +294,16 @@ export default defineComponent({
path,
fileId
}: CreateTargetRouteOptions): RouteLocationNamedRaw => {
const { params, query } = createFileRouteOptions(props.space, { path, fileId })
if (isPublicSpaceResource(props.space)) {
const { params, query } = createFileRouteOptions(unref(space), { path, fileId })
if (isPublicSpaceResource(unref(space))) {
return createLocationPublic('files-public-link', { params, query })
}
return createLocationSpaces('files-spaces-generic', { params, query })
}
const hasSpaceHeader = computed(() => {
// for now the space header is only available in the root of a project space.
return props.space.driveType === 'project' && props.item === '/'
return unref(space).driveType === 'project' && props.item === '/'
})
const folderNotFound = computed(() => store.getters['Files/currentFolder'] === null)
Expand All @@ -310,7 +313,7 @@ export default defineComponent({
)
const titleSegments = computed(() => {
const segments = [props.space.name]
const segments = [unref(space).name]
if (props.item !== '/') {
segments.unshift(basename(props.item))
}
Expand All @@ -321,16 +324,15 @@ export default defineComponent({
const route = useRoute()
const breadcrumbs = computed(() => {
const space = props.space
const rootBreadcrumbItems: BreadcrumbItem[] = []
if (isProjectSpaceResource(space)) {
if (isProjectSpaceResource(unref(space))) {
rootBreadcrumbItems.push({
id: uuidv4(),
text: $gettext('Spaces'),
to: createLocationSpaces('files-spaces-projects'),
isStaticNav: true
})
} else if (isShareSpaceResource(space)) {
} else if (isShareSpaceResource(unref(space))) {
rootBreadcrumbItems.push(
{
id: uuidv4(),
Expand All @@ -348,30 +350,30 @@ export default defineComponent({
}
let spaceBreadcrumbItem: BreadcrumbItem
let { params, query } = createFileRouteOptions(space, { fileId: space.fileId })
let { params, query } = createFileRouteOptions(unref(space), { fileId: unref(space).fileId })
query = omit({ ...unref(route).query, ...query }, 'page')
if (isPersonalSpaceResource(space)) {
if (isPersonalSpaceResource(unref(space))) {
spaceBreadcrumbItem = {
id: uuidv4(),
text: space.name,
...(space.isOwner(store.getters.user) && {
text: unref(space).name,
...(unref(space).isOwner(store.getters.user) && {
to: createLocationSpaces('files-spaces-generic', {
params,
query
})
})
}
} else if (isShareSpaceResource(space)) {
} else if (isShareSpaceResource(unref(space))) {
spaceBreadcrumbItem = {
id: uuidv4(),
allowContextActions: true,
text: space.name,
text: unref(space).name,
to: createLocationSpaces('files-spaces-generic', {
params,
query: omit(query, 'fileId')
})
}
} else if (isPublicSpaceResource(space)) {
} else if (isPublicSpaceResource(unref(space))) {
spaceBreadcrumbItem = {
id: uuidv4(),
text: $gettext('Public link'),
Expand All @@ -385,7 +387,7 @@ export default defineComponent({
spaceBreadcrumbItem = {
id: uuidv4(),
allowContextActions: !unref(hasSpaceHeader),
text: space.name,
text: unref(space).name,
to: createLocationSpaces('files-spaces-generic', {
params,
query
Expand Down Expand Up @@ -445,7 +447,7 @@ export default defineComponent({
resourcesViewDefaults.viewMode
)
useKeyboardTableMouseActions(keyActions, resourcesViewDefaults.viewMode)
useKeyboardTableSpaceActions(keyActions, props.space)
useKeyboardTableSpaceActions(keyActions, space)
const performLoaderTask = async (
sameRoute: boolean,
Expand All @@ -456,9 +458,9 @@ export default defineComponent({
return
}
const options: FolderLoaderOptions = { loadShares: !isPublicSpaceResource(props.space) }
const options: FolderLoaderOptions = { loadShares: !isPublicSpaceResource(unref(space)) }
await resourcesViewDefaults.loadResourcesTask.perform(
props.space,
unref(space),
path || props.item,
fileId || props.itemId,
options
Expand All @@ -473,7 +475,7 @@ export default defineComponent({
if (unref(openWithDefaultAppQuery) === 'true') {
openWithDefaultApp({
space: props.space,
space: unref(space),
resource: unref(resourcesViewDefaults.selectedResources)[0]
})
}
Expand Down
29 changes: 16 additions & 13 deletions packages/web-app-files/tests/unit/HandleUpload.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,10 @@ describe('HandleUpload', () => {

expect(processedFiles[0].tus.endpoint).toEqual('/')
expect(processedFiles[0].meta.name).toEqual(fileToUpload.name)
expect(processedFiles[0].meta.spaceId).toEqual(mocks.opts.space.id)
expect(processedFiles[0].meta.spaceName).toEqual(mocks.opts.space.name)
expect(processedFiles[0].meta.driveAlias).toEqual(mocks.opts.space.driveAlias)
expect(processedFiles[0].meta.driveType).toEqual(mocks.opts.space.driveType)
expect(processedFiles[0].meta.spaceId).toEqual(unref(mocks.opts.space).id)
expect(processedFiles[0].meta.spaceName).toEqual(unref(mocks.opts.space).name)
expect(processedFiles[0].meta.driveAlias).toEqual(unref(mocks.opts.space).driveAlias)
expect(processedFiles[0].meta.driveType).toEqual(unref(mocks.opts.space).driveType)
expect(processedFiles[0].meta.currentFolder).toEqual(currentFolder.path)
expect(processedFiles[0].meta.currentFolderId).toEqual(currentFolder.id)
expect(processedFiles[0].meta.tusEndpoint).toEqual(currentFolder.path)
Expand Down Expand Up @@ -74,10 +74,10 @@ describe('HandleUpload', () => {
isFolder: true,
type: 'folder',
meta: expect.objectContaining({
spaceId: mocks.opts.space.id,
spaceName: mocks.opts.space.name,
driveAlias: mocks.opts.space.driveAlias,
driveType: mocks.opts.space.driveType,
spaceId: unref(mocks.opts.space).id,
spaceName: unref(mocks.opts.space).name,
driveAlias: unref(mocks.opts.space).driveAlias,
driveType: unref(mocks.opts.space).driveType,
currentFolder: currentFolder.path,
currentFolderId: currentFolder.id,
relativeFolder: '',
Expand All @@ -89,10 +89,13 @@ describe('HandleUpload', () => {
})
)
expect(mocks.opts.clientService.webdav.createFolder).toHaveBeenCalledTimes(1)
expect(mocks.opts.clientService.webdav.createFolder).toHaveBeenCalledWith(mocks.opts.space, {
path: relativeFolder,
fetchFolder: true
})
expect(mocks.opts.clientService.webdav.createFolder).toHaveBeenCalledWith(
unref(mocks.opts.space),
{
path: relativeFolder,
fetchFolder: true
}
)
expect(result.length).toBe(1)
})
it('filters out files whose folders could not be created', async () => {
Expand Down Expand Up @@ -280,7 +283,7 @@ const getWrapper = ({
language: mock<Language>(),
route: ref(route),
store,
space: mock<SpaceResource>(),
space: ref(mock<SpaceResource>()),
uppyService: mock<UppyService>(),
conflictHandlingEnabled,
directoryTreeCreateEnabled,
Expand Down
Loading

0 comments on commit 636009d

Please sign in to comment.