Conversation
BundleMonFiles updated (3)
Unchanged files (6)
Total files change +107.95KB +8.38% Groups updated (2)
Unchanged groups (1)
Final result: ✅ View report in BundleMon website ➡️ |
|
@KillianCourvoisier don't hesitate to ping us again when it's ready, I think github doesn't notify us again. You can avoid creating draft if it's not really necessary for testing or specific review of someone for something, because we don't read draft anymore by default |
|
@JF-Cozy Yes for sure, no problem, I just used the draft to ensure nobody would try to review it before it is done. But of course when done I'll ping you, thank you for the attention ! :) |
Crash--
left a comment
There was a problem hiding this comment.
I know the PR is not finished, but at the end, you'll create a PR with 3k lines of code, you should not do that.
We should add code, behind flag, and merge it. Regularly. Because here, it'll be a nightmare to read and if we ask you to change things, it's so easier to change little PR than big ones.
b1bebc5 to
3525efe
Compare
|
Hello @Crash-- , thank you for all the feedback, I'll try my best to do what you asked. |
b01de8c to
6eb5d8d
Compare
f5b35fb to
e725f3e
Compare
But better than ever. Keep pushing! |
|
Can you know before starting the import the total number of files to import? Because currently the progress bar makes no sense because it is increasing the target each time it enters in a new folder. Screen.Recording.2025-12-03.at.11.08.49.mov |
|
Thank you for the PR description that explain very well what is done in the PR @KillianCourvoisier. Do you have more general context about the feature? Like what is the goal of this feature? Why are we doing something different that Nextcloud konnector? Also what is the purpose of enabling/disabling imports? Are they updated in the background? |
|
@Crash-- , thank you for the review.
Yes it true, but I tought it could have waited for the next PR we were mentioning for this kind of correction, do you want me to do it all in here anyway ?
I'll search on my side, but can you point me a doc or something a could use to get what's expected ? And/or @zatteo maybe ?
It will be cleaned up @zatteo , thanks for the feedback too :)
There is no bulk treatment possible with the downstream API (at least for what i know ?) so I technically can, but it will be very greedy as I will have to "pre-scan" the entire folder recursively before launching the actual import. For example, as it is intended to be a one-shot import tool, most of the users will probably import their files from the root directory directly. For massive instances, this would be hundreds, maybe thousands of requests, just to pre-scan. The solution I found to avoid this was to implement the number of files "toProcess" for each directory I want to import. If you have any suggestion, please don't hesitate as it would be indeed better.
Yes, it is not intended, but i did not work at all on this page yet, it just a "placeholder". I can remove it until i get to work on it if it is a problem 👍
Yes, sure. For the toggle on import, it was just to act like a "I understand the risks ..." button to allow import tool on your instance. And no, there's no background, everything occurs in the client, if the user reload or go back from the imports/run page during the import, the import is stopped and could be relaunch. No persistence between pages |
|
Why the connector can't do what you explain? You can use the same route & API so we're able to do it also (of course you need to add code to the connector). But here, what happens here if you close your browser during the import? What happen if your browser lose internet connection? You'll not have this issue with a konnector / service. |
|
About styling, you can either: Use utility classes from cozy-ui So instead of having You can check every utility classes here and you can also check how it is used in our app codebase: Use stylus file Check how are used .styl files there are plenty in cozy-settings |
Because the konnector was simply designed to create a shared drive, not importing the documents. But it's true, it could be all in there probably .
I did not push the test to "kill the browser during import", but it stops the import on reload, so I assume it stops the execution when closing the tab totally. I'll try during the new corrections. @zatteo mucha gracias, I'll take a look on that, it'll help ! |
|
@KillianCourvoisier About the progress bar I think it has not real interest as it because it does help to know when it will ends (but I do not say also that you need to precalculate everything to make the progress bar work). |
|
@zatteo OK then, i can remove it. But I still think it could be welcome to know how many files are being treated, what do you think ? Maybe I could simply use a spinner, and let the number of processed files at least ? Let me know |
Good idea you can display the number of files already processed for example. It's clearly better than nothing it will show that something is happening. |
…eanup inline styles
|
I applied corrections to the code according to last review, however regarding i18n and stylus, I just did the NextcloudAccountDialog.jsx file to try to do what has been asked. I wanted to have you check the modifications before going any further. Is it what is expected ? If so, I'll make the changes for every other component 👍 |
It's exactly this for styling 👍 |
|
Ok, all i18n translation and usage are done too, I hope everything is fine now. Lemme know |
Crash--
left a comment
There was a problem hiding this comment.
Still super complicated to understand.
If I have to maintain that by myself, I'm not very confident.
src/components/Imports/Providers/nextcloud/destinationService.js
Outdated
Show resolved
Hide resolved
src/components/Imports/Providers/nextcloud/useNextcloudAccounts.js
Outdated
Show resolved
Hide resolved
src/components/Imports/Providers/nextcloud/useNextcloudImport.js
Outdated
Show resolved
Hide resolved
| err.status = 404 | ||
| err.path = clean | ||
| throw err | ||
| } |
There was a problem hiding this comment.
If I understand correctly:
- Let's say I call probePath with
/foo/bar/ - I'll call
listRemotewith/foo - Then I loop on all the result on
/footo see ifbarreally exist? - And if not exist, we can throw an error with `weird characters name' . (wtf?)
- And if exist, then we check the type, and if it's a directory, then we finally list the files of the directory.
So:
- If I have a lot of folder / files in
/foowhat happen? Do we've pagination on this route? Can it be very slow? Can't we already know the type of theentry? - Can we really call this route with a file?
There was a problem hiding this comment.
The current probePath behavior follows what I could find for NextcloudFilesCollection.
The collection only exposes a directory-level listing (find() lists children of a given parentPath), to check a path like /foo/bar I need to do as follows :
- Listing the parent directory (
/foo) - Locating the entry named
bar - Fetching its children only if it is a directory
I've done this to avoid introducing custom remote calls, as requested
On performance: the endpoint used by NextcloudFilesCollection seems to not support pagination, so the cost is the same whether the lookup happens inside our code or directly through the collection.
…irectoryExists and simplify path handling
|
Hello @Crash-- , I tried to address everything you pointed out on the last review, here is what I've changed :
I think it resolve most of the problem you were mentioning, I still have stuff to do like a health check of the account upon creation, to hide the listing/importing section if the account is not OK or change the progress bar to a spinner + processedFiles number to avoid confusion when the bar is dropping back on each new scanned folder but I was thinking it could have been for another PR. Let me know what you think 👍 |
|
I still think this code should belongs to nextcloud connector & not be part of this app. With connectors, we already have runs history, we have "stats" (files imported), we have "background fetching". I think it's a terrible idea to keep all the logic here. Maybe we can keep the modal to create the account if we don't want to use harvest... but that's it. |

This PR introduces the full integration of the new Data Imports feature into Cozy Settings.
A new entry has been added in the “Data” menu, with three dedicated pages:
/imports/imports/run/imports/historyThe entire feature is wrapped behind a Cozy feature flag (
settings.imports).If the flag is off, the menu entry and routes are fully hidden.
For now, only Nextcloud accounts are supported. Additional providers such as Google Drive and Dropbox will be added in future updates.