-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Remote file paths #4537
Merged
Merged
Remote file paths #4537
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
02037c1
rename directories to directoryStack
mifi c05a8c4
implement relativePath for remote files
mifi f3fc489
make relativePath work like with local files
mifi 6409eee
Merge branch 'main' into remote-file-paths
mifi 76b9f85
apple code review suggestions
mifi 0de8c06
fix merge conflict issue
mifi 32cc925
restore relativePath null behavior
mifi 002fe47
Apply suggestions from code review
mifi File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,6 +32,15 @@ function isOriginAllowed (origin, allowedOrigin) { | |
.some((pattern) => pattern?.test(origin) || pattern?.test(`${origin}/`)) // allowing for trailing '/' | ||
} | ||
|
||
function formatBreadcrumbs (breadcrumbs) { | ||
return breadcrumbs.slice(1).map((directory) => directory.name).join('/') | ||
} | ||
|
||
function prependPath (path, component) { | ||
if (!path) return component | ||
return `${path}/${component}` | ||
} | ||
|
||
/** | ||
* Class to easily generate generic views for Provider plugins | ||
*/ | ||
|
@@ -74,7 +83,7 @@ export default class ProviderView extends View { | |
authenticated: false, | ||
files: [], | ||
folders: [], | ||
directories: [], | ||
breadcrumbs: [], | ||
filterInput: '', | ||
isSearchVisible: false, | ||
currentSelection: [], | ||
|
@@ -86,69 +95,91 @@ export default class ProviderView extends View { | |
// Nothing. | ||
} | ||
|
||
#updateFilesAndFolders (res, files, folders) { | ||
this.nextPagePath = res.nextPagePath | ||
res.items.forEach((item) => { | ||
async #list ({ requestPath, absDirPath, signal }) { | ||
const { username, nextPagePath, items } = await this.provider.list(requestPath, { signal }) | ||
this.username = username || this.username | ||
|
||
return { | ||
items: items.map((item) => ({ | ||
...item, | ||
absDirPath, | ||
})), | ||
nextPagePath, | ||
} | ||
} | ||
|
||
async #listFilesAndFolders ({ requestPath, breadcrumbs, signal }) { | ||
const absDirPath = formatBreadcrumbs(breadcrumbs) | ||
|
||
const { items, nextPagePath } = await this.#list({ requestPath, absDirPath, signal }) | ||
|
||
this.nextPagePath = nextPagePath | ||
|
||
const files = [] | ||
const folders = [] | ||
|
||
items.forEach((item) => { | ||
if (item.isFolder) { | ||
folders.push(item) | ||
} else { | ||
files.push(item) | ||
} | ||
}) | ||
|
||
this.plugin.setPluginState({ folders, files }) | ||
return { files, folders } | ||
} | ||
|
||
/** | ||
* Based on folder ID, fetch a new folder and update it to state | ||
* Select a folder based on its id: fetches the folder and then updates state with its contents | ||
* TODO rename to something better like selectFolder or navigateToFolder (breaking change?) | ||
* | ||
* @param {string} id Folder id | ||
* @param {string} requestPath | ||
* the path we need to use when sending list request to companion (for some providers it's different from ID) | ||
* @param {string} name used in the UI and to build the absDirPath | ||
* @returns {Promise} Folders/files in folder | ||
*/ | ||
async getFolder (id, name) { | ||
async getFolder (requestPath, name) { | ||
const controller = new AbortController() | ||
const cancelRequest = () => { | ||
controller.abort() | ||
this.clearSelection() | ||
} | ||
const getNewBreadcrumbsDirectories = () => { | ||
const state = this.plugin.getPluginState() | ||
const index = state.directories.findIndex((dir) => id === dir.id) | ||
|
||
if (index !== -1) { | ||
return state.directories.slice(0, index + 1) | ||
} | ||
return state.directories.concat([{ id, title: name }]) | ||
} | ||
|
||
this.plugin.uppy.on('dashboard:close-panel', cancelRequest) | ||
this.plugin.uppy.on('cancel-all', cancelRequest) | ||
this.setLoading(true) | ||
|
||
this.setLoading(true) | ||
try { | ||
const folders = [] | ||
const files = [] | ||
this.nextPagePath = id | ||
this.lastCheckbox = undefined | ||
|
||
let { breadcrumbs } = this.plugin.getPluginState() | ||
|
||
const index = breadcrumbs.findIndex((dir) => requestPath === dir.requestPath) | ||
|
||
if (index !== -1) { | ||
// means we navigated back to a known directory (already in the stack), so cut the stack off there | ||
breadcrumbs = breadcrumbs.slice(0, index + 1) | ||
} else { | ||
// we have navigated into a new (unknown) folder, add it to the stack | ||
breadcrumbs = [...breadcrumbs, { requestPath, name }] | ||
} | ||
|
||
let files = [] | ||
let folders = [] | ||
do { | ||
const res = await this.provider.list(this.nextPagePath, { signal: controller.signal }) | ||
const { files: newFiles, folders: newFolders } = await this.#listFilesAndFolders({ | ||
requestPath, breadcrumbs, signal: controller.signal, | ||
}) | ||
|
||
for (const f of res.items) { | ||
if (f.isFolder) folders.push(f) | ||
else files.push(f) | ||
} | ||
files = files.concat(newFiles) | ||
folders = folders.concat(newFolders) | ||
|
||
this.nextPagePath = res.nextPagePath | ||
if (res.username) this.username = res.username | ||
this.setLoading(this.plugin.uppy.i18n('loadedXFiles', { numFiles: files.length + folders.length })) | ||
} while ( | ||
this.nextPagePath && this.opts.loadAllFiles | ||
this.opts.loadAllFiles && this.nextPagePath | ||
) | ||
|
||
const directories = getNewBreadcrumbsDirectories(this.nextPagePath) | ||
|
||
this.plugin.setPluginState({ files, folders, directories, filterInput: '' }) | ||
this.lastCheckbox = undefined | ||
this.plugin.setPluginState({ folders, files, breadcrumbs, filterInput: '' }) | ||
} catch (err) { | ||
if (err.cause?.name === 'AbortError') { | ||
// Expected, user clicked “cancel” | ||
|
@@ -191,7 +222,7 @@ export default class ProviderView extends View { | |
authenticated: false, | ||
files: [], | ||
folders: [], | ||
directories: [], | ||
breadcrumbs: [], | ||
filterInput: '', | ||
} | ||
this.plugin.setPluginState(newState) | ||
|
@@ -250,16 +281,22 @@ export default class ProviderView extends View { | |
} | ||
|
||
async handleScroll (event) { | ||
const path = this.nextPagePath || null | ||
const requestPath = this.nextPagePath || null | ||
|
||
if (this.shouldHandleScroll(event) && path) { | ||
if (this.shouldHandleScroll(event) && requestPath) { | ||
this.isHandlingScroll = true | ||
|
||
try { | ||
const response = await this.provider.list(path) | ||
const { files, folders } = this.plugin.getPluginState() | ||
const { files, folders, breadcrumbs } = this.plugin.getPluginState() | ||
|
||
this.#updateFilesAndFolders(response, files, folders) | ||
const { files: newFiles, folders: newFolders } = await this.#listFilesAndFolders({ | ||
requestPath, breadcrumbs, | ||
}) | ||
|
||
const combinedFiles = files.concat(newFiles) | ||
const combinedFolders = folders.concat(newFolders) | ||
|
||
this.plugin.setPluginState({ folders: combinedFolders, files: combinedFiles }) | ||
} catch (error) { | ||
this.handleError(error) | ||
} finally { | ||
|
@@ -268,11 +305,11 @@ export default class ProviderView extends View { | |
} | ||
} | ||
|
||
async recursivelyListAllFiles (path, queue, onFiles) { | ||
let curPath = path | ||
async #recursivelyListAllFiles ({ requestPath, absDirPath, relDirPath, queue, onFiles }) { | ||
let curPath = requestPath | ||
|
||
while (curPath) { | ||
const res = await this.provider.list(curPath) | ||
const res = await this.#list({ requestPath: curPath, absDirPath }) | ||
curPath = res.nextPagePath | ||
|
||
const files = res.items.filter((item) => !item.isFolder) | ||
|
@@ -282,7 +319,13 @@ export default class ProviderView extends View { | |
|
||
// recursively queue call to self for each folder | ||
const promises = folders.map(async (folder) => queue.add(async () => ( | ||
this.recursivelyListAllFiles(folder.requestPath, queue, onFiles) | ||
this.#recursivelyListAllFiles({ | ||
requestPath: folder.requestPath, | ||
absDirPath: prependPath(absDirPath, folder.name), | ||
relDirPath: prependPath(relDirPath, folder.name), | ||
queue, | ||
onFiles, | ||
}) | ||
))) | ||
await Promise.all(promises) // in case we get an error | ||
} | ||
|
@@ -296,9 +339,17 @@ export default class ProviderView extends View { | |
const messages = [] | ||
const newFiles = [] | ||
|
||
for (const file of currentSelection) { | ||
if (file.isFolder) { | ||
const { requestPath, name } = file | ||
for (const selectedItem of currentSelection) { | ||
const { requestPath } = selectedItem | ||
|
||
const withRelDirPath = (newItem) => ({ | ||
...newItem, | ||
// calculate the file's path relative to the user's selected item's path | ||
// see https://github.com/transloadit/uppy/pull/4537#issuecomment-1614236655 | ||
relDirPath: newItem.absDirPath.replace(selectedItem.absDirPath, '').replace(/^\//, ''), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. since you added There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not sure how that would work |
||
}) | ||
|
||
if (selectedItem.isFolder) { | ||
let isEmpty = true | ||
let numNewFiles = 0 | ||
|
||
|
@@ -313,36 +364,42 @@ export default class ProviderView extends View { | |
// the folder was already added. This checks if all files are duplicate, | ||
// if that's the case, we don't add the files. | ||
if (!this.plugin.uppy.checkIfFileAlreadyExists(id)) { | ||
newFiles.push(newFile) | ||
newFiles.push(withRelDirPath(newFile)) | ||
numNewFiles++ | ||
this.setLoading(this.plugin.uppy.i18n('addedNumFiles', { numFiles: numNewFiles })) | ||
} | ||
isEmpty = false | ||
} | ||
} | ||
|
||
await this.recursivelyListAllFiles(requestPath, queue, onFiles) | ||
await this.#recursivelyListAllFiles({ | ||
requestPath, | ||
absDirPath: prependPath(selectedItem.absDirPath, selectedItem.name), | ||
relDirPath: selectedItem.name, | ||
queue, | ||
onFiles, | ||
}) | ||
await queue.onIdle() | ||
|
||
let message | ||
if (isEmpty) { | ||
message = this.plugin.uppy.i18n('emptyFolderAdded') | ||
} else if (numNewFiles === 0) { | ||
message = this.plugin.uppy.i18n('folderAlreadyAdded', { | ||
folder: name, | ||
folder: selectedItem.name, | ||
}) | ||
} else { | ||
// TODO we don't really know at this point whether any files were actually added | ||
// (only later after addFiles has been called) so we should probably rewrite this. | ||
// Example: If all files fail to add due to restriction error, it will still say "Added 100 files from folder" | ||
message = this.plugin.uppy.i18n('folderAdded', { | ||
smart_count: numNewFiles, folder: name, | ||
smart_count: numNewFiles, folder: selectedItem.name, | ||
}) | ||
} | ||
|
||
messages.push(message) | ||
} else { | ||
newFiles.push(file) | ||
newFiles.push(withRelDirPath(selectedItem)) | ||
} | ||
} | ||
|
||
|
@@ -380,7 +437,7 @@ export default class ProviderView extends View { | |
const headerProps = { | ||
showBreadcrumbs: targetViewOptions.showBreadcrumbs, | ||
getFolder: this.getFolder, | ||
directories: this.plugin.getPluginState().directories, | ||
breadcrumbs: this.plugin.getPluginState().breadcrumbs, | ||
pluginIcon: this.plugin.icon, | ||
title: this.plugin.title, | ||
logout: this.logout, | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need this todo? Name is not that bad.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getX
sounds to me like pure function (no side effects) that simply returns something. but this function does a whole lot more than that: it changes the breadcrumbs, it lists folders/files and then updates the stateThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately, almost everything in Uppy is state and almost all functions perform things on state. But yes since it's a public function I don't think we can change it now.