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(files): Ensure favorites set in sidebar work #50220

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
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
18 changes: 18 additions & 0 deletions apps/files/src/store/files.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,14 @@
getters: {
/**
* Get a file or folder by its source
* @param state

Check warning on line 27 in apps/files/src/store/files.ts

View workflow job for this annotation

GitHub Actions / NPM lint

Missing JSDoc @param "state" description
*/
getNode: (state) => (source: FileSource): Node|undefined => state.files[source],

/**
* Get a list of files or folders by their IDs
* Note: does not return undefined values
* @param state

Check warning on line 34 in apps/files/src/store/files.ts

View workflow job for this annotation

GitHub Actions / NPM lint

Missing JSDoc @param "state" description
*/
getNodes: (state) => (sources: FileSource[]): Node[] => sources
.map(source => state.files[source])
Expand All @@ -41,13 +41,13 @@
* Get files or folders by their file ID
* Multiple nodes can have the same file ID but different sources
* (e.g. in a shared context)
* @param state

Check warning on line 44 in apps/files/src/store/files.ts

View workflow job for this annotation

GitHub Actions / NPM lint

Missing JSDoc @param "state" description
*/
getNodesById: (state) => (fileId: number): Node[] => Object.values(state.files).filter(node => node.fileid === fileId),

/**
* Get the root folder of a service
* @param state

Check warning on line 50 in apps/files/src/store/files.ts

View workflow job for this annotation

GitHub Actions / NPM lint

Missing JSDoc @param "state" description
*/
getRoot: (state) => (service: Service): Folder|undefined => state.roots[service],
},
Expand All @@ -58,7 +58,7 @@
*
* @param service The service (files view)
* @param path The path relative within the service
* @return Array of cached nodes within the path

Check warning on line 61 in apps/files/src/store/files.ts

View workflow job for this annotation

GitHub Actions / NPM lint

Missing JSDoc @return type
*/
getNodesByPath(service: string, path?: string): Node[] {
const pathsStore = usePathsStore()
Expand Down Expand Up @@ -149,6 +149,21 @@
// Otherwise, it means we receive an event for a node that is not in the store
fetchNode(node).then(n => this.updateNodes([n]))
},

// Handlers for legacy sidebar (no real nodes support)
onAddFavorite(node: Node) {
const ourNode = this.getNode(node.source)
if (ourNode) {
Vue.set(ourNode.attributes, 'favorite', 1)
}
},

onRemoveFavorite(node: Node) {
const ourNode = this.getNode(node.source)
if (ourNode) {
Vue.set(ourNode.attributes, 'favorite', 0)
}
},
},
})

Expand All @@ -159,6 +174,9 @@
subscribe('files:node:deleted', fileStore.onDeletedNode)
subscribe('files:node:updated', fileStore.onUpdatedNode)
subscribe('files:node:moved', fileStore.onMovedNode)
// legacy sidebar
subscribe('files:favorites:added', fileStore.onAddFavorite)
subscribe('files:favorites:removed', fileStore.onRemoveFavorite)

fileStore._initialized = true
}
Expand Down
20 changes: 12 additions & 8 deletions apps/files/src/views/Sidebar.vue
Original file line number Diff line number Diff line change
Expand Up @@ -404,10 +404,10 @@ export default {
},

/**
* Toggle favourite state
* Toggle favorite state
* TODO: better implementation
*
* @param {boolean} state favourited or not
* @param {boolean} state is favorite or not
*/
async toggleStarred(state) {
try {
Expand All @@ -430,17 +430,21 @@ export default {
*/
const isDir = this.fileInfo.type === 'dir'
const Node = isDir ? Folder : File
emit(state ? 'files:favorites:added' : 'files:favorites:removed', new Node({
const node = new Node({
fileid: this.fileInfo.id,
source: this.davPath,
root: `/files/${getCurrentUser().uid}`,
source: `${davRemoteURL}${davRootPath}${this.file}`,
root: davRootPath,
mime: isDir ? undefined : this.fileInfo.mimetype,
}))
attributes: {
favorite: 1,
},
})
emit(state ? 'files:favorites:added' : 'files:favorites:removed', node)

this.fileInfo.isFavourited = state
} catch (error) {
showError(t('files', 'Unable to change the favourite state of the file'))
logger.error('Unable to change favourite state', { error })
showError(t('files', 'Unable to change the favorite state of the file'))
logger.error('Unable to change favorite state', { error })
}
},

Expand Down
4 changes: 2 additions & 2 deletions apps/files/src/views/favorites.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ export const registerFavoritesView = async () => {
favoriteFoldersViews.forEach(view => Navigation.register(view))

/**
* Update favourites navigation when a new folder is added
* Update favorites navigation when a new folder is added
*/
subscribe('files:favorites:added', (node: Node) => {
if (node.type !== FileType.Folder) {
Expand Down Expand Up @@ -99,7 +99,7 @@ export const registerFavoritesView = async () => {
})

/**
* Update favourites navigation when a folder is renamed
* Update favorites navigation when a folder is renamed
*/
subscribe('files:node:renamed', (node: Node) => {
if (node.type !== FileType.Folder) {
Expand Down
137 changes: 137 additions & 0 deletions cypress/e2e/files/favorites.cy.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
/*!
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

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

describe('files: Favorites', { testIsolation: true }, () => {
let user: User

beforeEach(() => {
cy.createRandomUser().then(($user) => {
user = $user
cy.uploadContent(user, new Blob([]), 'text/plain', '/file.txt')
cy.mkdir(user, '/new folder')
cy.login(user)
cy.visit('/apps/files')
})
})

it('Mark file as favorite', () => {
// See file exists
getRowForFile('file.txt')
.should('exist')

cy.intercept('POST', '**/apps/files/api/v1/files/file.txt').as('addToFavorites')
// Click actions
getActionButtonForFile('file.txt').click({ force: true })
// See action is called 'Add to favorites'
cy.get('[data-cy-files-list-row-action="favorite"] > button').last()
.should('exist')
.and('have.text', 'Add to favorites')
.click({ force: true })
cy.wait('@addToFavorites')
// See favorites star
getRowForFile('file.txt')
.findByRole('img', { name: 'Favorite' })
.should('exist')
})

it('Un-mark file as favorite', () => {
// See file exists
getRowForFile('file.txt')
.should('exist')

cy.intercept('POST', '**/apps/files/api/v1/files/file.txt').as('addToFavorites')
// toggle favorite
triggerActionForFile('file.txt', 'favorite')
cy.wait('@addToFavorites')

// See favorites star
getRowForFile('file.txt')
.findByRole('img', { name: 'Favorite' })
.should('be.visible')

// Remove favorite
// click action button
getActionButtonForFile('file.txt').click({ force: true })
// See action is called 'Remove from favorites'
cy.get('[data-cy-files-list-row-action="favorite"] > button').last()
.should('exist')
.and('have.text', 'Remove from favorites')
.click({ force: true })
cy.wait('@addToFavorites')
// See no favorites star anymore
getRowForFile('file.txt')
.findByRole('img', { name: 'Favorite' })
.should('not.exist')
})

it('See favorite folders in navigation', () => {
cy.intercept('POST', '**/apps/files/api/v1/files/new%20folder').as('addToFavorites')

// see navigation has no entry
cy.get('[data-cy-files-navigation-item="favorites"]')
.should('be.visible')
.contains('new folder')
.should('not.exist')

// toggle favorite
triggerActionForFile('new folder', 'favorite')
cy.wait('@addToFavorites')

// See in navigation
cy.get('[data-cy-files-navigation-item="favorites"]')
.should('be.visible')
.contains('new folder')
.should('exist')

// toggle favorite
triggerActionForFile('new folder', 'favorite')
cy.wait('@addToFavorites')

// See no longer in navigation
cy.get('[data-cy-files-navigation-item="favorites"]')
.should('be.visible')
.contains('new folder')
.should('not.exist')
})

it('Mark file as favorite using the sidebar', () => {
// See file exists
getRowForFile('new folder')
.should('exist')
// see navigation has no entry
cy.get('[data-cy-files-navigation-item="favorites"]')
.should('be.visible')
.contains('new folder')
.should('not.exist')

cy.intercept('PROPPATCH', '**/remote.php/dav/files/*/new%20folder').as('addToFavorites')
// open sidebar
triggerActionForFile('new folder', 'details')
// open actions
cy.get('[data-cy-sidebar]')
.findByRole('button', { name: 'Actions' })
.click()
// trigger menu button
cy.findAllByRole('menu')
.findByRole('menuitem', { name: 'Add to favorites' })
.should('be.visible')
.click()
cy.wait('@addToFavorites')

// See favorites star
getRowForFile('new folder')
.findByRole('img', { name: 'Favorite' })
.should('be.visible')

// See folder in navigation
cy.get('[data-cy-files-navigation-item="favorites"]')
.should('be.visible')
.contains('new folder')
.should('exist')
})
})
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