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

Refactor of the export csv button to export more than 500 documents max #993

Merged
merged 9 commits into from
Jan 27, 2023

Conversation

thomas-mauran
Copy link
Member

@thomas-mauran thomas-mauran commented Nov 8, 2022

Refactor of the export csv button to export more than 500 documents max

Refactoring the export csv function to know export more than 500 documents max

image

How should this be manually tested?

  • Step 1 : open a kuzzle backend with data
  • Step 2 : go on a collection with a lot of document
  • Step 3 : try the export button

Other changes

deleted the function convertToCSV() from src/services/cllectionHelper.ts

@netlify
Copy link

netlify bot commented Nov 8, 2022

Deploy Preview for heuristic-goodall-67c4b1 ready!

Name Link
🔨 Latest commit 22e3fed
🔍 Latest deploy log https://app.netlify.com/sites/heuristic-goodall-67c4b1/deploys/63d3c86669f39900087866c9
😎 Deploy Preview https://deploy-preview-993--heuristic-goodall-67c4b1.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

Copy link
Contributor

@Aschen Aschen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you improve the PR title and also assign it to you please?

src/components/Data/Documents/Views/Column/Column.vue Outdated Show resolved Hide resolved
src/components/Data/Documents/Views/Column/Column.vue Outdated Show resolved Hide resolved
src/components/Data/Documents/Views/Column/Column.vue Outdated Show resolved Hide resolved
src/components/Data/Documents/Views/Column/Column.vue Outdated Show resolved Hide resolved
src/components/Data/Documents/Views/Column/Column.vue Outdated Show resolved Hide resolved
src/components/Data/Documents/Views/Column/Column.vue Outdated Show resolved Hide resolved
And remove the exportCsvModal component since it's not used anymore
@thomas-mauran thomas-mauran self-assigned this Nov 9, 2022
@thomas-mauran thomas-mauran changed the title Feat/export csv Refactor of the export csv button to export more than 500 documents max Nov 9, 2022
@sebtiz13 sebtiz13 changed the base branch from master to 4-dev November 23, 2022 10:51
@sebtiz13 sebtiz13 self-requested a review November 23, 2022 10:51
@sebtiz13 sebtiz13 force-pushed the feat/export-csv branch 2 times, most recently from 6d55a56 to a4f1049 Compare November 23, 2022 16:06
@sebtiz13
Copy link
Member

Export doesn't work with huge collection (example: more than 10k documents).
The actual solution isn't really good, because the data are stored in client memory before conversion in blob to create download link, this can cause block the JS browser runtime, also this solution has an UX problem because with a large number of documents, user doesn't have indication of status (loading status or progress).

I think an better client solution it's to use Web Worker for non blocking process an split treatment in chunk.
IMHO the best solution, it's to implement a stream solution on backend and client just open a link, so the browser treat the download progress independently.

@Shiranuit
Copy link

Export doesn't work with huge collection (example: more than 10k documents). The actual solution isn't really good, because the data are stored in client memory before conversion in blob to create download link, this can cause block the JS browser runtime, also this solution has an UX problem because with a large number of documents, user doesn't have indication of status (loading status or progress).

I think an better client solution it's to use Web Worker for non blocking process an split treatment in chunk. IMHO the best solution, it's to implement a stream solution on backend and client just open a link, so the browser treat the download progress independently.

Using the export method is the proper way to download large amount of documents directly from the backend, this method is able to handle extremely large amount of data by streaming them directly from the backend, you should be able to download the file directly by letting the browser open the following url https://<kuzzle host>/<index>/<collection>/_export and configure every options of the export by putting things into the query params (lang, searchBody, scroll etc...)
The only thing I'm not sure how to do is how to provide the authentication token when going to the export url

@sebtiz13
Copy link
Member

@Shiranuit It's not possible to provide the authentication token in GET params of url ?
If it's not possible due to security reason (understandable), one solution can be add an method to generate one temporary token (this token can contain the params of export) to provide in request. This temporary token are valid just one time.
IMHO it's the better solution to export securely without allow export spamming.

@Aschen
Copy link
Contributor

Aschen commented Nov 29, 2022

The only thing I'm not sure how to do is how to provide the authentication token when going to the export url

If the SDK use the cookie auth then the auth cookie should be forwarded automatically no?

@Shiranuit
Copy link

Shiranuit commented Nov 29, 2022

@Shiranuit It's not possible to provide the authentication token in GET params of url ? If it's not possible due to security reason (understandable), one solution can be add an method to generate one temporary token (this token can contain the params of export) to provide in request. This temporary token are valid just one time. IMHO it's the better solution to export securely without allow export spamming.

@sebtiz13 Actually it's not possible to provide the authentication token directly in the URL, also it's not recommended even though query params are encrypted, the proper solution would be with cookies but it might not be possible if this is disabled by the backend

The only thing I'm not sure how to do is how to provide the authentication token when going to the export url

If the SDK use the cookie auth then the auth cookie should be forwarded automatically no?

@Aschen Yes it might work with the cookie but this will require the admin console to support login with cookies which is not the case right now, I dont think this will be that hard to implement, but this also means that if the cookies are disable on the Kuzzle instance the export will not be possible, and the admin-console should also handle this case

@Aschen
Copy link
Contributor

Aschen commented Nov 29, 2022

@Shiranuit I think cookie auth should be default, isn't it the case ?

Otherwise I agree but if the backend didn't enable cookies I don't see how we can properly authenticate the client..

@sebtiz13
Copy link
Member

sebtiz13 commented Nov 29, 2022

Ok like I say it's understandable not to be able to provide directly token in GET parameters, but what you think of solution of generate a temporary token for export ?
This token would only be valid for one export and not use the user token. So, off course this create a specific case, but don't cause security problem and it's simpler than cookie.

@Aschen
Copy link
Contributor

Aschen commented Nov 29, 2022

@sebtiz13 We could do this but it require some backend development for Kuzzle

@Aschen Aschen marked this pull request as draft January 19, 2023 13:33
Aschen and others added 3 commits January 19, 2023 16:34
* improving style
* hiding the modal when the dowmload button is clicked
@thomas-mauran
Copy link
Member Author

image

Here is what the css looks like

@thomas-mauran thomas-mauran requested a review from Aschen January 27, 2023 10:35
@thomas-mauran thomas-mauran marked this pull request as ready for review January 27, 2023 13:34
@thomas-mauran thomas-mauran merged commit d508dc1 into 4-dev Jan 27, 2023
This was referenced Jan 27, 2023
@rolljee rolljee deleted the feat/export-csv branch September 5, 2024 09:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants