[full-ci] Resource ids in urls (#7725)

Co-authored-by: Jannik Stehle <[email protected]> Co-authored-by: Dominik Schmidt <[email protected]> Co-authored-by: Jan <[email protected]>
owncloud · Oct 10, 2022 · cc0eec8 · cc0eec8
1 parent 601af15
commit cc0eec8
Show file tree

Hide file tree

Showing 79 changed files with 952 additions and 698 deletions.
diff --git a/changelog/unreleased/enhancement-enable-renaming-received-shares b/changelog/unreleased/enhancement-enable-renaming-received-shares
@@ -0,0 +1,7 @@
+Enhancement: Enable renaming on received shares
+
+As a receiver the user can rename a share which will only take effect for the respective user
+but won't change the name for the sharee or other share receivers.
+
+https://github.com/owncloud/web/pull/7725
+https://github.com/owncloud/web/issues/6247
diff --git a/changelog/unreleased/enhancement-id-based-routing b/changelog/unreleased/enhancement-id-based-routing
@@ -0,0 +1,14 @@
+Enhancement: Id based routing
+
+We now include fileIds in the URL query to be able to
+- resolve files and spaces correctly and
+- resolve the correct relative path of a file if it was changed (this might be the case for bookmarks)
+The fileIds in the URL can be disabled by setting `options.routing.idBased` to `false` in the `config.json`.
+
+Note: It's recommended to keep the default of fileIds being used in routing. Otherwise it's not possible
+to resolve spaces with name clashes correctly.
+
+https://github.com/owncloud/web/issues/6247
+https://github.com/owncloud/web/pull/7725
+https://github.com/owncloud/web/issues/7714
+https://github.com/owncloud/web/issues/7715
diff --git a/docs/getting-started.md b/docs/getting-started.md
@@ -60,6 +60,8 @@ substring of a value of the authenticated user. Examples are `/Shares`, `/{{.Id}
 - `options.cernFeatures` Enabling this will activate CERN-specific features. Defaults to `false`.
 - `options.hoverableQuickActions` Set this option to `true` to hide the quick actions (buttons appearing on file rows), and only show them when the user
 hovers the row with his mouse. Defaults to `false`.
+- `option.routing` This accepts an object with the following fields to customize the routing behaviour:
+  - `options.routing.idBased` Enable or disable fileIds being added to the URL. Defaults to `true` because otherwise e.g. spaces with name clashes can't be resolved correctly. Only disable this if you can guarantee server side that spaces of the same namespace can't have name clashes.   
 
 ### Sentry
 

diff --git a/packages/web-app-draw-io/src/App.vue b/packages/web-app-draw-io/src/App.vue
@@ -61,10 +61,17 @@ export default defineComponent({
       return `${this.config.url}?${query}`
     }
   },
+  watch: {
+    currentFileContext: {
+      handler: function () {
+        this.load()
+      },
+      immediate: true
+    }
+  },
   created() {
     this.filePath = this.currentFileContext.path
     this.fileExtension = this.filePath.split('.').pop()
-    this.checkPermissions()
     window.addEventListener('message', (event) => {
       if (event.data.length > 0) {
         const payload = JSON.parse(event.data)
@@ -114,8 +121,9 @@ export default defineComponent({
     async checkPermissions() {
       try {
         const resource = await this.getFileInfo(this.currentFileContext, {
-          davProperties: [DavProperty.Permissions]
+          davProperties: [DavProperty.FileId, DavProperty.Permissions]
         })
+        this.replaceInvalidFileRoute(this.currentFileContext, resource)
         this.isReadOnly = ![DavPermission.Updateable, DavPermission.FileUpdateable].some(
           (p) => (resource.permissions || '').indexOf(p) > -1
         )
@@ -124,24 +132,26 @@ export default defineComponent({
         this.errorPopup(error)
       }
     },
-    load() {
-      this.getFileContents(this.currentFileContext)
-        .then((resp) => {
-          this.currentETag = resp.headers.ETag
-          this.$refs.drawIoEditor.contentWindow.postMessage(
-            JSON.stringify({
-              action: 'load',
-              xml: resp.body,
-              autosave: this.config.autosave
-            }),
-            '*'
-          )
-        })
-        .catch((error) => {
-          this.errorPopup(error)
-        })
+    async loadFileContent() {
+      try {
+        const response = await this.getFileContents(this.currentFileContext)
+        this.currentETag = response.headers.ETag
+        this.$refs.drawIoEditor.contentWindow.postMessage(
+          JSON.stringify({
+            action: 'load',
+            xml: response.body,
+            autosave: this.config.autosave
+          }),
+          '*'
+        )
+      } catch (error) {
+        this.errorPopup(error)
+      }
+    },
+    async load() {
+      await Promise.all([this.checkPermissions(), this.loadFileContent()])
     },
-    async importVisio() {
+    importVisio() {
       const getDescription = () =>
         this.$gettextInterpolate(
           this.$gettext('The diagram will open as a new .drawio file: %{file}'),

diff --git a/packages/web-app-files/src/components/AppBar/CreateAndUpload.vue b/packages/web-app-files/src/components/AppBar/CreateAndUpload.vue
@@ -183,6 +183,11 @@ export default defineComponent({
       type: Boolean,
       default: false,
       required: false
+    },
+    itemId: {
+      type: [String, Number],
+      required: false,
+      default: null
     }
   },
   setup(props) {
@@ -212,7 +217,8 @@ export default defineComponent({
       }),
       ...useUploadHelpers({
         space: computed(() => props.space),
-        currentFolder: computed(() => props.item)
+        currentFolder: computed(() => props.item),
+        currentFolderId: computed(() => props.itemId)
       }),
       ...useRequest(),
       ...useGraphClient(),
@@ -333,9 +339,10 @@ export default defineComponent({
           return
         }
 
+        const { spaceId, currentFolder, currentFolderId } = file.meta
         if (!['public', 'share'].includes(file.meta.driveType)) {
           if (this.hasSpaces) {
-            const driveResponse = await this.graphClient.drives.getDrive(file.meta.spaceId)
+            const driveResponse = await this.graphClient.drives.getDrive(spaceId)
             this.UPDATE_SPACE_FIELD({
               id: driveResponse.data.id,
               field: 'spaceQuota',
@@ -347,8 +354,10 @@ export default defineComponent({
           }
         }
 
-        const fileIsInCurrentPath =
-          file.meta.spaceId === this.space.id && file.meta.currentFolder === this.item
+        const sameFolder = this.itemId
+          ? currentFolderId === this.itemId
+          : currentFolder === this.item
+        const fileIsInCurrentPath = spaceId === this.space.id && sameFolder
         if (fileIsInCurrentPath) {
           bus.publish('app.files.list.load')
         }
@@ -501,12 +510,11 @@ export default defineComponent({
         this.UPSERT_RESOURCE(resource)
 
         if (this.newFileAction) {
-          const fileId = resource.fileId
-
           this.$_fileActions_openEditor(
             this.newFileAction,
             this.space.getDriveAliasAndItem(resource),
-            fileId,
+            resource.webDavPath,
+            resource.fileId,
             EDITOR_MODE_CREATE
           )
           this.hideModal()
@@ -615,14 +623,15 @@ export default defineComponent({
       return null
     },
 
-    async onFilesSelected(filesToUpload: File[]) {
+    onFilesSelected(filesToUpload: File[]) {
       const uploader = new ResourcesUpload(
         filesToUpload,
         this.files,
         this.inputFilesToUppyFiles,
         this.$uppyService,
         this.space,
         this.item,
+        this.itemId,
         this.spaces,
         this.hasSpaces,
         this.createDirectoryTree,

diff --git a/packages/web-app-files/src/components/FilesList/NotFoundMessage.vue b/packages/web-app-files/src/components/FilesList/NotFoundMessage.vue
@@ -51,10 +51,10 @@ import {
   isLocationPublicActive,
   isLocationSpacesActive
 } from '../../router'
-import { useRouter, useStore } from 'web-pkg/src/composables'
+import { useRouter } from 'web-pkg/src/composables'
 import { defineComponent, PropType } from '@vue/composition-api'
-import { Resource } from 'web-client'
 import { SpaceResource } from 'web-client/src/helpers'
+import { createFileRouteOptions } from 'web-pkg/src/helpers/router'
 
 export default defineComponent({
   name: 'NotFoundMessage',
@@ -67,26 +67,20 @@ export default defineComponent({
   },
   setup(props) {
     const router = useRouter()
-    const store = useStore()
     const isProjectSpace = props.space?.driveType === 'project'
     return {
       showPublicLinkButton: isLocationPublicActive(router, 'files-public-link'),
       showHomeButton: isLocationSpacesActive(router, 'files-spaces-generic') && !isProjectSpace,
       showSpacesButton: isLocationSpacesActive(router, 'files-spaces-generic') && isProjectSpace,
       homeRoute: createLocationSpaces('files-spaces-generic', {
         params: {
-          driveAliasAndItem: props.space?.getDriveAliasAndItem({
-            path: store.getters.homeFolder
-          } as Resource)
-        }
-      }),
-      publicLinkRoute: createLocationPublic('files-public-link', {
-        params: {
-          driveAliasAndItem: props.space?.getDriveAliasAndItem({
-            path: ''
-          } as Resource)
+          driveAliasAndItem: 'personal'
         }
       }),
+      publicLinkRoute: createLocationPublic(
+        'files-public-link',
+        createFileRouteOptions(props.space, {})
+      ),
       spacesRoute: createLocationSpaces('files-spaces-projects')
     }
   }

diff --git a/packages/web-app-files/src/components/FilesList/ResourceTable.vue b/packages/web-app-files/src/components/FilesList/ResourceTable.vue
@@ -192,7 +192,6 @@ import maxSize from 'popper-max-size-modifier'
 import { mapGetters, mapActions, mapState } from 'vuex'
 import { EVENT_TROW_MOUNTED, EVENT_FILE_DROPPED } from '../../constants'
 import { SortDir } from '../../composables'
-import * as path from 'path'
 import { determineSortFields } from '../../helpers/ui/resourceTable'
 import {
   useCapabilityProjectSpacesEnabled,
@@ -208,6 +207,9 @@ import { formatDateFromJSDate, formatRelativeDateFromJSDate } from 'web-pkg/src/
 import { SideBarEventTopics } from '../../composables/sideBar'
 import { buildShareSpaceResource, extractDomSelector, SpaceResource } from 'web-client/src/helpers'
 import { configurationManager } from 'web-pkg/src/configuration'
+import { CreateTargetRouteOptions } from '../../helpers/folderLink'
+import { createFileRouteOptions } from 'web-pkg/src/helpers/router'
+import { basename, dirname } from 'path'
 
 export default defineComponent({
   mixins: [Rename],
@@ -589,7 +591,7 @@ export default defineComponent({
         .length
     },
     openRenameDialog(item) {
-      this.$_rename_trigger({ resources: [item] }, this.getMatchingSpace(item.storageId))
+      this.$_rename_trigger({ resources: [item] }, this.getMatchingSpace(item))
     },
     openTagsSidebar() {
       bus.publish(SideBarEventTopics.open)
@@ -606,45 +608,42 @@ export default defineComponent({
       bus.publish(SideBarEventTopics.openWithPanel, panelToOpen)
     },
     folderLink(file) {
-      return this.createFolderLink(file.path, file)
+      return this.createFolderLink({ path: file.path, fileId: file.fileId, resource: file })
     },
     parentFolderLink(file) {
       if (file.shareId && file.path === '/') {
         return createLocationShares('files-shares-with-me')
       }
 
-      return this.createFolderLink(path.dirname(file.path), file)
+      return this.createFolderLink({
+        path: dirname(file.path),
+        ...(file.parentFolderId && { fileId: file.parentFolderId }),
+        resource: file
+      })
     },
-    createFolderLink(p, resource) {
+    createFolderLink(options: CreateTargetRouteOptions) {
       if (this.targetRouteCallback) {
-        return this.targetRouteCallback(p, resource)
+        return this.targetRouteCallback(options)
       }
 
+      const { path, fileId, resource } = options
+      let space
       if (resource.shareId) {
-        const space = buildShareSpaceResource({
+        space = buildShareSpaceResource({
           shareId: resource.shareId,
-          shareName: path.basename(resource.shareRoot),
+          shareName: basename(resource.shareRoot),
           serverUrl: configurationManager.serverUrl
         })
-        return createLocationSpaces('files-spaces-generic', {
-          params: {
-            driveAliasAndItem: space.getDriveAliasAndItem({ path: p } as Resource)
-          },
-          query: {
-            shareId: resource.shareId
-          }
-        })
+      } else {
+        space = this.getMatchingSpace(resource)
       }
-
-      const matchingSpace = this.getMatchingSpace(resource.storageId)
-      if (!matchingSpace) {
+      if (!space) {
         return {}
       }
-      return createLocationSpaces('files-spaces-generic', {
-        params: {
-          driveAliasAndItem: matchingSpace.getDriveAliasAndItem({ path: p } as Resource)
-        }
-      })
+      return createLocationSpaces(
+        'files-spaces-generic',
+        createFileRouteOptions(space, { path, fileId })
+      )
     },
     fileDragged(file) {
       this.addSelectedResource(file)
@@ -751,7 +750,7 @@ export default defineComponent({
       this.emitSelect(this.resources.map((resource) => resource.id))
     },
     emitFileClick(resource) {
-      let space = this.getMatchingSpace(resource.storageId)
+      let space = this.getMatchingSpace(resource)
       if (!space) {
         space = buildShareSpaceResource({
           shareId: resource.shareId,
@@ -817,12 +816,20 @@ export default defineComponent({
         ownerName: resource.owner[0].displayName
       })
     },
-    getMatchingSpace(storageId): SpaceResource {
-      return this.space || this.spaces.find((space) => space.id === storageId)
+    getMatchingSpace(resource: Resource): SpaceResource {
+      return (
+        this.space ||
+        this.spaces.find((space) => space.id === resource.storageId) ||
+        buildShareSpaceResource({
+          shareId: resource.shareId,
+          shareName: resource.name,
+          serverUrl: configurationManager.serverUrl
+        })
+      )
     },
     getDefaultParentFolderName(resource) {
       if (this.hasProjectSpaces) {
-        const matchingSpace = this.getMatchingSpace(resource.storageId)
+        const matchingSpace = this.getMatchingSpace(resource)
         if (matchingSpace?.driveType === 'project') {
           return matchingSpace.name
         }
@@ -835,7 +842,7 @@ export default defineComponent({
       if (resource.shareId) {
         return resource.path === '/'
           ? this.$gettext('Shared with me')
-          : path.basename(resource.shareRoot)
+          : basename(resource.shareRoot)
       }
 
       return this.$gettext('Personal')

diff --git a/packages/web-app-files/src/components/Search/List.vue b/packages/web-app-files/src/components/Search/List.vue
@@ -86,6 +86,7 @@ import SideBar from '../../components/SideBar/SideBar.vue'
 import { buildShareSpaceResource, SpaceResource } from 'web-client/src/helpers'
 import { useStore } from 'web-pkg/src/composables'
 import { configurationManager } from 'web-pkg/src/configuration'
+import { basename } from 'path'
 
 const visibilityObserver = new VisibilityObserver()
 
@@ -120,7 +121,7 @@ export default defineComponent({
       if (resource.shareId) {
         return buildShareSpaceResource({
           shareId: resource.shareId,
-          shareName: resource.name,
+          shareName: basename(resource.shareRoot),
           serverUrl: configurationManager.serverUrl
         })
       }