Skip to content

Commit

Permalink
fix(files): Do not download files with openfile query flag
Browse files Browse the repository at this point in the history
Instead of downloading files, if there is no other default action,
we should just open the details tab.

Signed-off-by: Ferdinand Thiessen <[email protected]>
  • Loading branch information
susnux authored and backportbot[bot] committed Feb 6, 2025
1 parent 3e93249 commit 9e81253
Show file tree
Hide file tree
Showing 5 changed files with 253 additions and 36 deletions.
55 changes: 35 additions & 20 deletions apps/files/src/components/FilesListVirtual.vue
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@
import type { Node as NcNode } from '@nextcloud/files'
import type { ComponentPublicInstance, PropType } from 'vue'
import type { UserConfig } from '../types'
import type { Node as NcNode } from '@nextcloud/files'
import type { ComponentPublicInstance, PropType } from 'vue'
import type { Location } from 'vue-router'

import { getFileListHeaders, Folder, View, getFileActions, FileType } from '@nextcloud/files'
import { showError } from '@nextcloud/dialogs'
Expand Down Expand Up @@ -280,30 +283,42 @@ export default defineComponent({
* Handle opening a file (e.g. by ?openfile=true)
* @param fileId File to open
*/
handleOpenFile(fileId: number|null) {
if (fileId === null) {
return
}

async handleOpenFile(fileId: number) {
const node = this.nodes.find(n => n.fileid === fileId) as NcNode
if (node === undefined || node.type === FileType.Folder) {
if (node === undefined) {
return
}

logger.debug('Opening file ' + node.path, { node })
this.openFileId = fileId
const defaultAction = getFileActions()
// Get only default actions (visible and hidden)
.filter(action => !!action?.default)
// Find actions that are either always enabled or enabled for the current node
.filter((action) => !action.enabled || action.enabled([node], this.currentView))
// Sort enabled default actions by order
.sort((a, b) => (a.order || 0) - (b.order || 0))
// Get the first one
.at(0)
// Some file types do not have a default action (e.g. they can only be downloaded)
// So if there is an enabled default action, so execute it
defaultAction?.exec(node, this.currentView, this.currentFolder.path)
if (node.type === FileType.File) {
const defaultAction = getFileActions()
// Get only default actions (visible and hidden)
.filter((action) => !!action?.default)
// Find actions that are either always enabled or enabled for the current node
.filter((action) => !action.enabled || action.enabled([node], this.currentView))
.filter((action) => action.id !== 'download')
// Sort enabled default actions by order
.sort((a, b) => (a.order || 0) - (b.order || 0))
// Get the first one
.at(0)

// Some file types do not have a default action (e.g. they can only be downloaded)
// So if there is an enabled default action, so execute it
if (defaultAction) {
logger.debug('Opening file ' + node.path, { node })
return await defaultAction.exec(node, this.currentView, this.currentFolder.path)
}
}
// The file is either a folder or has no default action other than downloading
// in this case we need to open the details instead and remove the route from the history
const query = this.$route.query
delete query.openfile
query.opendetails = ''

logger.debug('Ignore `openfile` query and replacing with `opendetails` for ' + node.path, { node })
await this.$router.replace({
...(this.$route as Location),
query,
})
},

onDragOver(event: DragEvent) {
Expand Down
35 changes: 28 additions & 7 deletions apps/files/src/router/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,41 @@
* SPDX-License-Identifier: AGPL-3.0-or-later
*/
import type { RawLocation, Route } from 'vue-router'
import type { ErrorHandler } from 'vue-router/types/router.d.ts'

import { generateUrl } from '@nextcloud/router'
import queryString from 'query-string'
import Router from 'vue-router'
import Router, { isNavigationFailure, NavigationFailureType } from 'vue-router'
import Vue from 'vue'
import logger from '../logger'

Vue.use(Router)

// Prevent router from throwing errors when we're already on the page we're trying to go to
const originalPush = Router.prototype.push as (to, onComplete?, onAbort?) => Promise<Route>
Router.prototype.push = function push(to: RawLocation, onComplete?: ((route: Route) => void) | undefined, onAbort?: ErrorHandler | undefined): Promise<Route> {
if (onComplete || onAbort) return originalPush.call(this, to, onComplete, onAbort)
return originalPush.call(this, to).catch(err => err)
const originalPush = Router.prototype.push
Router.prototype.push = (function(this: Router, ...args: Parameters<typeof originalPush>) {
if (args.length > 1) {
return originalPush.call(this, ...args)
}
return originalPush.call<Router, [RawLocation], Promise<Route>>(this, args[0]).catch(ignoreDuplicateNavigation)
}) as typeof originalPush

const originalReplace = Router.prototype.replace
Router.prototype.replace = (function(this: Router, ...args: Parameters<typeof originalReplace>) {
if (args.length > 1) {
return originalReplace.call(this, ...args)
}
return originalReplace.call<Router, [RawLocation], Promise<Route>>(this, args[0]).catch(ignoreDuplicateNavigation)
}) as typeof originalReplace

/**
* Ignore duplicated-navigation error but forward real exceptions
* @param error The thrown error
*/
function ignoreDuplicateNavigation(error: unknown): void {
if (isNavigationFailure(error, NavigationFailureType.duplicated)) {
logger.debug('Ignoring duplicated navigation from vue-router', { error })
} else {
throw error
}
}

const router = new Router({
Expand Down
180 changes: 180 additions & 0 deletions cypress/e2e/files/router-query.cy.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,180 @@
/*!
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

import type { User } from '@nextcloud/cypress'
import { join } from 'path'
import { getRowForFileId } from './FilesUtils.ts'

/**
* Check that the sidebar is opened for a specific file
* @param name The name of the file
*/
function sidebarIsOpen(name: string): void {
cy.get('[data-cy-sidebar]')
.should('be.visible')
.findByRole('heading', { name })
.should('be.visible')
}

/**
* Skip a test without viewer installed
*/
function skipIfViewerDisabled(this: Mocha.Context): void {
cy.runOccCommand('app:list --enabled --output json')
.then((exec) => exec.stdout)
.then((output) => JSON.parse(output))
.then((obj) => 'viewer' in obj.enabled)
.then((enabled) => {
if (!enabled) {
this.skip()
}
})
}

/**
* Check a file was not downloaded
* @param filename The expected filename
*/
function fileNotDownloaded(filename: string): void {
const downloadsFolder = Cypress.config('downloadsFolder')
cy.readFile(join(downloadsFolder, filename)).should('not.exist')
}

describe('Check router query flags:', function() {
let user: User
let imageId: number
let archiveId: number
let folderId: number

before(() => {
cy.createRandomUser().then(($user) => {
user = $user
cy.uploadFile(user, 'image.jpg')
.then((response) => { imageId = Number.parseInt(response.headers['oc-fileid']) })
cy.mkdir(user, '/folder')
.then((response) => { folderId = Number.parseInt(response.headers['oc-fileid']) })
cy.uploadContent(user, new Blob([]), 'application/zstd', '/archive.zst')
.then((response) => { archiveId = Number.parseInt(response.headers['oc-fileid']) })
cy.login(user)
})
})

describe('"opendetails"', () => {
it('open details for known file type', () => {
cy.visit(`/apps/files/files/${imageId}?opendetails`)

// see sidebar
sidebarIsOpen('image.jpg')

// but no viewer
cy.findByRole('dialog', { name: 'image.jpg' })
.should('not.exist')

// and no download
fileNotDownloaded('image.jpg')
})

it('open details for unknown file type', () => {
cy.visit(`/apps/files/files/${archiveId}?opendetails`)

// see sidebar
sidebarIsOpen('archive.zst')

// but no viewer
cy.findByRole('dialog', { name: 'archive.zst' })
.should('not.exist')

// and no download
fileNotDownloaded('archive.zst')
})

it('open details for folder', () => {
cy.visit(`/apps/files/files/${folderId}?opendetails`)

// see sidebar
sidebarIsOpen('folder')

// but no viewer
cy.findByRole('dialog', { name: 'folder' })
.should('not.exist')

// and no download
fileNotDownloaded('folder')
})
})

describe('"openfile"', function() {
/** Check the viewer is open and shows the image */
function viewerShowsImage(): void {
cy.findByRole('dialog', { name: 'image.jpg' })
.should('be.visible')
.find(`img[src*="fileId=${imageId}"]`)
.should('be.visible')
}

it('opens files with default action', function() {
skipIfViewerDisabled.call(this)

cy.visit(`/apps/files/files/${imageId}?openfile`)
viewerShowsImage()
})

it('opens files with default action using explicit query state', function() {
skipIfViewerDisabled.call(this)

cy.visit(`/apps/files/files/${imageId}?openfile=true`)
viewerShowsImage()
})

it('does not open files with default action when using explicitly query value `false`', function() {
skipIfViewerDisabled.call(this)

cy.visit(`/apps/files/files/${imageId}?openfile=false`)
getRowForFileId(imageId)
.should('be.visible')
.and('have.class', 'files-list__row--active')

cy.findByRole('dialog', { name: 'image.jpg' })
.should('not.exist')
})

it('does not open folders but shows details', () => {
cy.visit(`/apps/files/files/${folderId}?openfile`)

// See the URL was replaced
cy.url()
.should('match', /[?&]opendetails(&|=|$)/)
.and('not.match', /openfile/)

// See the sidebar is correctly opened
cy.get('[data-cy-sidebar]')
.should('be.visible')
.findByRole('heading', { name: 'folder' })
.should('be.visible')

// see the folder was not changed
getRowForFileId(imageId).should('exist')
})

it('does not open unknown file types but shows details', () => {
cy.visit(`/apps/files/files/${archiveId}?openfile`)

// See the URL was replaced
cy.url()
.should('match', /[?&]opendetails(&|=|$)/)
.and('not.match', /openfile/)

// See the sidebar is correctly opened
cy.get('[data-cy-sidebar]')
.should('be.visible')
.findByRole('heading', { name: 'archive.zst' })
.should('be.visible')

// See no file was downloaded
const downloadsFolder = Cypress.config('downloadsFolder')
cy.readFile(join(downloadsFolder, 'archive.zst')).should('not.exist')
})
})
})
15 changes: 8 additions & 7 deletions cypress/support/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,12 +110,12 @@ Cypress.Commands.add('enableUser', (user: User, enable = true) => {
*/
Cypress.Commands.add('uploadFile', (user, fixture = 'image.jpg', mimeType = 'image/jpeg', target = `/${fixture}`) => {
// get fixture
return cy.fixture(fixture, 'base64').then(async file => {
// convert the base64 string to a blob
const blob = Cypress.Blob.base64StringToBlob(file, mimeType)

cy.uploadContent(user, blob, mimeType, target)
})
return cy.fixture(fixture, 'base64')
.then((file) => (
// convert the base64 string to a blob
Cypress.Blob.base64StringToBlob(file, mimeType)
))
.then((blob) => cy.uploadContent(user, blob, mimeType, target))
})

Cypress.Commands.add('setFileAsFavorite', (user: User, target: string, favorite = true) => {
Expand Down Expand Up @@ -154,7 +154,7 @@ Cypress.Commands.add('setFileAsFavorite', (user: User, target: string, favorite

Cypress.Commands.add('mkdir', (user: User, target: string) => {
// eslint-disable-next-line cypress/unsafe-to-chain-command
cy.clearCookies()
return cy.clearCookies()
.then(async () => {
try {
const rootPath = `${Cypress.env('baseUrl')}/remote.php/dav/files/${encodeURIComponent(user.userId)}`
Expand All @@ -168,6 +168,7 @@ Cypress.Commands.add('mkdir', (user: User, target: string) => {
},
})
cy.log(`Created directory ${target}`, response)
return response
} catch (error) {
cy.log('error', error)
throw new Error('Unable to create directory')
Expand Down
4 changes: 2 additions & 2 deletions cypress/support/cypress-e2e.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ declare global {
* Upload a file from the fixtures folder to a given user storage.
* **Warning**: Using this function will reset the previous session
*/
uploadFile(user: User, fixture?: string, mimeType?: string, target?: string): Cypress.Chainable<void>,
uploadFile(user: User, fixture?: string, mimeType?: string, target?: string): Cypress.Chainable<AxiosResponse>,

/**
* Upload a raw content to a given user storage.
Expand All @@ -38,7 +38,7 @@ declare global {
* Create a new directory
* **Warning**: Using this function will reset the previous session
*/
mkdir(user: User, target: string): Cypress.Chainable<void>,
mkdir(user: User, target: string): Cypress.Chainable<AxiosResponse>,

/**
* Set a file as favorite (or remove from favorite)
Expand Down

0 comments on commit 9e81253

Please sign in to comment.