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

[stable31] fix(files): properly update store on files conversions success #50651

Merged
merged 2 commits into from
Feb 4, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
10 changes: 5 additions & 5 deletions apps/files/src/actions/convertAction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { t } from '@nextcloud/l10n'

import AutoRenewSvg from '@mdi/svg/svg/autorenew.svg?raw'

import { convertFile, convertFiles, getParentFolder } from './convertUtils'
import { convertFile, convertFiles } from './convertUtils'

type ConversionsProvider = {
from: string,
Expand All @@ -33,17 +33,17 @@ export const registerConvertActions = () => {
return nodes.every(node => from === node.mime)
},

async exec(node: Node, view: View, dir: string) {
async exec(node: Node) {
// If we're here, we know that the node has a fileid
convertFile(node.fileid as number, to, getParentFolder(view, dir))
convertFile(node.fileid as number, to)

// Silently terminate, we'll handle the UI in the background
return null
},

async execBatch(nodes: Node[], view: View, dir: string) {
async execBatch(nodes: Node[]) {
const fileIds = nodes.map(node => node.fileid).filter(Boolean) as number[]
convertFiles(fileIds, to, getParentFolder(view, dir))
convertFiles(fileIds, to)

// Silently terminate, we'll handle the UI in the background
return Array(nodes.length).fill(null)
Expand Down
97 changes: 48 additions & 49 deletions apps/files/src/actions/convertUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,46 +2,57 @@
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/
import type { AxiosResponse } from '@nextcloud/axios'
import type { Folder, View } from '@nextcloud/files'
import type { AxiosResponse, AxiosError } from '@nextcloud/axios'
import type { OCSResponse } from '@nextcloud/typings/ocs'

import { emit } from '@nextcloud/event-bus'
import { generateOcsUrl } from '@nextcloud/router'
import { showError, showLoading, showSuccess } from '@nextcloud/dialogs'
import { t } from '@nextcloud/l10n'
import axios from '@nextcloud/axios'
import axios, { isAxiosError } from '@nextcloud/axios'
import PQueue from 'p-queue'

import { fetchNode } from '../services/WebdavClient.ts'
import logger from '../logger'
import { useFilesStore } from '../store/files'
import { getPinia } from '../store'
import { usePathsStore } from '../store/paths'

const queue = new PQueue({ concurrency: 5 })
type ConversionResponse = {
path: string
fileId: number
}

interface PromiseRejectedResult<T> {
status: 'rejected'
reason: T
}

type PromiseSettledResult<T, E> = PromiseFulfilledResult<T> | PromiseRejectedResult<E>;
type ConversionSuccess = AxiosResponse<OCSResponse<ConversionResponse>>
type ConversionError = AxiosError<OCSResponse<ConversionResponse>>

const queue = new PQueue({ concurrency: 5 })
const requestConversion = function(fileId: number, targetMimeType: string): Promise<AxiosResponse> {
return axios.post(generateOcsUrl('/apps/files/api/v1/convert'), {
fileId,
targetMimeType,
})
}

export const convertFiles = async function(fileIds: number[], targetMimeType: string, parentFolder: Folder | null) {
export const convertFiles = async function(fileIds: number[], targetMimeType: string) {
const conversions = fileIds.map(fileId => queue.add(() => requestConversion(fileId, targetMimeType)))

// Start conversion
const toast = showLoading(t('files', 'Converting files…'))

// Handle results
try {
const results = await Promise.allSettled(conversions)
const failed = results.filter(result => result.status === 'rejected')
const results = await Promise.allSettled(conversions) as PromiseSettledResult<ConversionSuccess, ConversionError>[]
const failed = results.filter(result => result.status === 'rejected') as PromiseRejectedResult<ConversionError>[]
if (failed.length > 0) {
const messages = failed.map(result => result.reason?.response?.data?.ocs?.meta?.message) as string[]
const messages = failed.map(result => result.reason?.response?.data?.ocs?.meta?.message)
logger.error('Failed to convert files', { fileIds, targetMimeType, messages })

// If all failed files have the same error message, show it
if (new Set(messages).size === 1) {
if (new Set(messages).size === 1 && typeof messages[0] === 'string') {
showError(t('files', 'Failed to convert files: {message}', { message: messages[0] }))
return
}
Expand Down Expand Up @@ -74,15 +85,27 @@ export const convertFiles = async function(fileIds: number[], targetMimeType: st
// All files converted
showSuccess(t('files', 'Files successfully converted'))

// Trigger a reload of the file list
if (parentFolder) {
emit('files:node:updated', parentFolder)
}
// Extract files that are within the current directory
// in batch mode, you might have files from different directories
// ⚠️, let's get the actual current dir, as the one from the action
// might have changed as the user navigated away
const currentDir = window.OCP.Files.Router.query.dir as string
const newPaths = results
.filter(result => result.status === 'fulfilled')
.map(result => result.value.data.ocs.data.path)
.filter(path => path.startsWith(currentDir))

// Fetch the new files
logger.debug('Files to fetch', { newPaths })
const newFiles = await Promise.all(newPaths.map(path => fetchNode(path)))

// Inform the file list about the new files
newFiles.forEach(file => emit('files:node:created', file))

// Switch to the new files
const firstSuccess = results[0] as PromiseFulfilledResult<AxiosResponse>
const firstSuccess = results[0] as PromiseFulfilledResult<ConversionSuccess>
const newFileId = firstSuccess.value.data.ocs.data.fileId
window.OCP.Files.Router.goToRoute(null, { ...window.OCP.Files.Router.params, fileid: newFileId }, window.OCP.Files.Router.query)
window.OCP.Files.Router.goToRoute(null, { ...window.OCP.Files.Router.params, fileid: newFileId.toString() }, window.OCP.Files.Router.query)
} catch (error) {
// Should not happen as we use allSettled and handle errors above
showError(t('files', 'Failed to convert files'))
Expand All @@ -93,24 +116,23 @@ export const convertFiles = async function(fileIds: number[], targetMimeType: st
}
}

export const convertFile = async function(fileId: number, targetMimeType: string, parentFolder: Folder | null) {
export const convertFile = async function(fileId: number, targetMimeType: string) {
const toast = showLoading(t('files', 'Converting file…'))

try {
const result = await queue.add(() => requestConversion(fileId, targetMimeType)) as AxiosResponse
const result = await queue.add(() => requestConversion(fileId, targetMimeType)) as AxiosResponse<OCSResponse<ConversionResponse>>
showSuccess(t('files', 'File successfully converted'))

// Trigger a reload of the file list
if (parentFolder) {
emit('files:node:updated', parentFolder)
}
// Inform the file list about the new file
const newFile = await fetchNode(result.data.ocs.data.path)
emit('files:node:created', newFile)

// Switch to the new file
const newFileId = result.data.ocs.data.fileId
window.OCP.Files.Router.goToRoute(null, { ...window.OCP.Files.Router.params, fileid: newFileId }, window.OCP.Files.Router.query)
window.OCP.Files.Router.goToRoute(null, { ...window.OCP.Files.Router.params, fileid: newFileId.toString() }, window.OCP.Files.Router.query)
} catch (error) {
// If the server returned an error message, show it
if (error.response?.data?.ocs?.meta?.message) {
if (isAxiosError(error) && error.response?.data?.ocs?.meta?.message) {
showError(t('files', 'Failed to convert file: {message}', { message: error.response.data.ocs.meta.message }))
return
}
Expand All @@ -122,26 +144,3 @@ export const convertFile = async function(fileId: number, targetMimeType: string
toast.hideToast()
}
}

/**
* Get the parent folder of a path
*
* TODO: replace by the parent node straight away when we
* update the Files actions api accordingly.
*
* @param view The current view
* @param path The path to the file
* @returns The parent folder
*/
export const getParentFolder = function(view: View, path: string): Folder | null {
const filesStore = useFilesStore(getPinia())
const pathsStore = usePathsStore(getPinia())

const parentSource = pathsStore.getPath(view.id, path)
if (!parentSource) {
return null
}

const parentFolder = filesStore.getNode(parentSource) as Folder | undefined
return parentFolder ?? null
}
13 changes: 7 additions & 6 deletions apps/files/src/services/WebdavClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,18 @@
* SPDX-FileCopyrightText: 2023 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/
import { davGetClient, davGetDefaultPropfind, davResultToNode, davRootPath } from '@nextcloud/files'
import type { FileStat, ResponseDataDetailed } from 'webdav'
import type { Node } from '@nextcloud/files'

export const client = davGetClient()
import { getClient, getDefaultPropfind, getRootPath, resultToNode } from '@nextcloud/files/dav'

export const fetchNode = async (node: Node): Promise<Node> => {
const propfindPayload = davGetDefaultPropfind()
const result = await client.stat(`${davRootPath}${node.path}`, {
export const client = getClient()

export const fetchNode = async (path: string): Promise<Node> => {
const propfindPayload = getDefaultPropfind()
const result = await client.stat(`${getRootPath()}${path}`, {
details: true,
data: propfindPayload,
}) as ResponseDataDetailed<FileStat>
return davResultToNode(result.data)
return resultToNode(result.data)
}
4 changes: 2 additions & 2 deletions apps/files/src/store/files.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ export const useFilesStore = function(...args) {
// If we have multiple nodes with the same file ID, we need to update all of them
const nodes = this.getNodesById(node.fileid)
if (nodes.length > 1) {
await Promise.all(nodes.map(fetchNode)).then(this.updateNodes)
await Promise.all(nodes.map(node => fetchNode(node.path))).then(this.updateNodes)
logger.debug(nodes.length + ' nodes updated in store', { fileid: node.fileid })
return
}
Expand All @@ -147,7 +147,7 @@ export const useFilesStore = function(...args) {
}

// Otherwise, it means we receive an event for a node that is not in the store
fetchNode(node).then(n => this.updateNodes([n]))
fetchNode(node.path).then(n => this.updateNodes([n]))
},

// Handlers for legacy sidebar (no real nodes support)
Expand Down
2 changes: 1 addition & 1 deletion apps/files/src/views/Sidebar.vue
Original file line number Diff line number Diff line change
Expand Up @@ -491,7 +491,7 @@ export default {
this.loading = true

try {
this.node = await fetchNode({ path: this.file })
this.node = await fetchNode(this.file)
this.fileInfo = FileInfo(this.node)
// adding this as fallback because other apps expect it
this.fileInfo.dir = this.file.split('/').slice(0, -1).join('/')
Expand Down
4 changes: 2 additions & 2 deletions apps/files_sharing/src/mixins/SharesMixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import { getCurrentUser } from '@nextcloud/auth'
import { showError, showSuccess } from '@nextcloud/dialogs'
import { ShareType } from '@nextcloud/sharing'
import { emit } from '@nextcloud/event-bus'
import { fetchNode } from '../services/WebdavClient.ts'

import PQueue from 'p-queue'
import debounce from 'debounce'
Expand All @@ -20,6 +19,7 @@ import logger from '../services/logger.ts'
import {
BUNDLED_PERMISSIONS,
} from '../lib/SharePermissionsToolBox.js'
import { fetchNode } from '../../../files/src/services/WebdavClient.ts'

export default {
mixins: [SharesRequests],
Expand Down Expand Up @@ -164,7 +164,7 @@ export default {
async getNode() {
const node = { path: this.path }
try {
this.node = await fetchNode(node)
this.node = await fetchNode(node.path)
logger.info('Fetched node:', { node: this.node })
} catch (error) {
logger.error('Error:', error)
Expand Down
18 changes: 0 additions & 18 deletions apps/files_sharing/src/services/WebdavClient.ts

This file was deleted.