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

[stable29] fix(files): Close sidebar and update fileid when current node is deleted #45659

Merged
merged 3 commits into from
Jun 7, 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
5 changes: 3 additions & 2 deletions apps/files/src/actions/deleteAction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,14 +143,15 @@ export const action = new FileAction({
.every(permission => (permission & Permission.DELETE) !== 0)
},

async exec(node: Node) {
async exec(node: Node, view: View, dir: string) {
try {
await axios.delete(node.encodedSource)

// Let's delete even if it's moved to the trashbin
// since it has been removed from the current view
// and changing the view will trigger a reload anyway.
// and changing the view will trigger a reload anyway.
emit('files:node:deleted', node)

return true
} catch (error) {
logger.error('Error while deleting a file', { error, source: node.source, node })
Expand Down
2 changes: 1 addition & 1 deletion apps/files/src/store/paths.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import logger from '../logger'
import { useFilesStore } from './files'

export const usePathsStore = function(...args) {
const files = useFilesStore()
const files = useFilesStore(...args)

const store = defineStore('paths', {
state: () => ({
Expand Down
36 changes: 36 additions & 0 deletions apps/files/src/views/FilesList.vue
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,14 @@ export default defineComponent({
return (this.$route?.query?.dir?.toString() || '/').replace(/^(.+)\/$/, '$1')
},

/**
* The current file id
*/
fileId(): number | null {
const number = Number.parseInt(this.$route?.params.fileid ?? '')
return Number.isNaN(number) ? null : number
},

/**
* The current folder.
*/
Expand Down Expand Up @@ -453,6 +461,8 @@ export default defineComponent({

mounted() {
this.fetchContent()

subscribe('files:node:deleted', this.onNodeDeleted)
subscribe('files:node:updated', this.onUpdatedNode)
subscribe('nextcloud:unified-search.search', this.onSearch)
subscribe('nextcloud:unified-search.reset', this.onSearch)
Expand All @@ -462,6 +472,7 @@ export default defineComponent({
},

unmounted() {
unsubscribe('files:node:deleted', this.onNodeDeleted)
unsubscribe('files:node:updated', this.onUpdatedNode)
unsubscribe('nextcloud:unified-search.search', this.onSearch)
unsubscribe('nextcloud:unified-search.reset', this.onSearch)
Expand Down Expand Up @@ -535,6 +546,31 @@ export default defineComponent({
return this.filesStore.getNode(fileId)
},

/**
* Handle the node deleted event to reset open file
* @param node The deleted node
*/
onNodeDeleted(node: Node) {
if (node.fileid && node.fileid === this.fileId) {
if (node.fileid === this.currentFolder?.fileid) {
// Handle the edge case that the current directory is deleted
// in this case we neeed to keept the current view but move to the parent directory
window.OCP.Files.Router.goToRoute(
null,
{ view: this.$route.params.view },
{ dir: this.currentFolder?.dirname ?? '/' },
)
} else {
// If the currently active file is deleted we need to remove the fileid and possible the `openfile` query
window.OCP.Files.Router.goToRoute(
null,
{ ...this.$route.params, fileid: undefined },
{ ...this.$route.query, openfile: undefined },
)
}
}
},

/**
* The upload manager have finished handling the queue
* @param {Upload} upload the uploaded data
Expand Down
15 changes: 14 additions & 1 deletion apps/files/src/views/Sidebar.vue
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@
import { getCurrentUser } from '@nextcloud/auth'
import { getCapabilities } from '@nextcloud/capabilities'
import { showError } from '@nextcloud/dialogs'
import { emit } from '@nextcloud/event-bus'
import { emit, subscribe, unsubscribe } from '@nextcloud/event-bus'
import { File, Folder, formatFileSize } from '@nextcloud/files'
import { encodePath } from '@nextcloud/paths'
import { generateRemoteUrl, generateUrl } from '@nextcloud/router'
Expand Down Expand Up @@ -306,10 +306,13 @@ export default {
},
},
created() {
subscribe('files:node:deleted', this.onNodeDeleted)

window.addEventListener('resize', this.handleWindowResize)
this.handleWindowResize()
},
beforeDestroy() {
unsubscribe('file:node:deleted', this.onNodeDeleted)
window.removeEventListener('resize', this.handleWindowResize)
},

Expand Down Expand Up @@ -509,6 +512,16 @@ export default {
this.resetData()
},

/**
* Handle if the current node was deleted
* @param {import('@nextcloud/files').Node} node The deleted node
*/
onNodeDeleted(node) {
if (this.fileInfo && node && this.fileInfo.id === node.fileid) {
this.close()
}
},

/**
* Allow to set the Sidebar as fullscreen from OCA.Files.Sidebar
*
Expand Down
94 changes: 94 additions & 0 deletions cypress/e2e/files/files-sidebar.cy.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
/**
* @copyright Copyright (c) 2024 Ferdinand Thiessen <opensource@fthiessen.de>
*
* @author Ferdinand Thiessen <opensource@fthiessen.de>
*
* @license AGPL-3.0-or-later
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
* published by the Free Software Foundation, either version 3 of the
* License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
*/

import type { User } from '@nextcloud/cypress'
import { getRowForFile, navigateToFolder, triggerActionForFile } from './FilesUtils'

describe('Files: Sidebar', { testIsolation: true }, () => {
let user: User
let fileId: number = 0

beforeEach(() => cy.createRandomUser().then(($user) => {
user = $user

cy.mkdir(user, '/folder')
cy.uploadContent(user, new Blob([]), 'text/plain', '/file').then((response) => {
fileId = Number.parseInt(response.headers['oc-fileid'] ?? '0')
})
cy.login(user)
}))

it('opens the sidebar', () => {
cy.visit('/apps/files')
getRowForFile('file').should('be.visible')

triggerActionForFile('file', 'details')

cy.get('[cy-data-sidebar]').should('be.visible')
})

it('changes the current fileid', () => {
cy.visit('/apps/files')
getRowForFile('file').should('be.visible')

triggerActionForFile('file', 'details')

cy.get('[cy-data-sidebar]').should('be.visible')
cy.url().should('contain', `apps/files/files/${fileId}`)
})

it('closes the sidebar on delete', () => {
cy.visit('/apps/files')
getRowForFile('file').should('be.visible')

// open the sidebar
triggerActionForFile('file', 'details')
// validate it is open
cy.get('[cy-data-sidebar]').should('be.visible')

triggerActionForFile('file', 'delete')
cy.get('[cy-data-sidebar]').should('not.exist')
})

it('changes the fileid on delete', () => {
cy.uploadContent(user, new Blob([]), 'text/plain', '/folder/other').then((response) => {
const otherFileId = Number.parseInt(response.headers['oc-fileid'] ?? '0')
cy.login(user)
cy.visit('/apps/files')

getRowForFile('folder').should('be.visible')
navigateToFolder('folder')
getRowForFile('other').should('be.visible')

// open the sidebar
triggerActionForFile('other', 'details')
// validate it is open
cy.get('[cy-data-sidebar]').should('be.visible')
cy.url().should('contain', `apps/files/files/${otherFileId}`)

triggerActionForFile('other', 'delete')
cy.get('[cy-data-sidebar]').should('not.exist')
// Ensure the URL is changed
cy.url().should('not.contain', `apps/files/files/${otherFileId}`)
})
})
})
6 changes: 5 additions & 1 deletion cypress/support/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
*/
/* eslint-disable n/no-unpublished-import */
// eslint-disable-next-line n/no-extraneous-import
import type { AxiosResponse } from 'axios'

import axios from '@nextcloud/axios'
import { addCommands, User } from '@nextcloud/cypress'
import { basename } from 'path'
Expand All @@ -33,10 +35,12 @@ addCommands()
declare global {
// eslint-disable-next-line @typescript-eslint/no-namespace
namespace Cypress {
// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unused-vars
interface Chainable<Subject = any> {
/**
* Enable or disable a given user
*/
// eslint-disable-next-line @typescript-eslint/no-explicit-any
enableUser(user: User, enable?: boolean): Cypress.Chainable<Cypress.Response<any>>,

/**
Expand Down
4 changes: 2 additions & 2 deletions dist/files-init.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion dist/files-init.js.map

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions dist/files-main.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion dist/files-main.js.map

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions dist/files-sidebar.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion dist/files-sidebar.js.map

Large diffs are not rendered by default.

Loading