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

Reports: Download all #620

Merged
merged 7 commits into from
Apr 25, 2021
Merged

Reports: Download all #620

merged 7 commits into from
Apr 25, 2021

Conversation

franpb14
Copy link
Collaborator

To solve this I have done as described in the step #596 (using rubyzip and calling the reports). A simple demonstration of how it works is given below.
Issue596

The new update allows the administrator to download a zip containing all the current CSV documents. Issue #596.
if you try to download the zip repeatedly, this error may appear, so now the algorithm retries it if it happens. It also make sure to close Tempfile.
All the new methods that I implemented are now covered with the exception of the rescue block. Issue #596
@franpb14 franpb14 requested a review from markets April 20, 2021 19:03
@markets markets mentioned this pull request Apr 20, 2021
3 tasks
@markets markets changed the title Third step of Issue #596 completed Reports: Download all Apr 24, 2021
Copy link
Collaborator

@markets markets left a comment

Choose a reason for hiding this comment

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

@franpb14 I pushed a couple of commits (most interesting one is 1914af8) with some refactoring. Basically to make pull collections more DRY. The rest seems fine to me ✅ 👍🏼

@sseerrggii ready for testing 👀

Thanks!

@sseerrggii
Copy link
Contributor

🟢 Tested!!

Good job @franpb14 Really useful the GIF for the new feature 👍

Thanks @markets for boosting the PR 😉

@sseerrggii sseerrggii merged commit aa1b5e5 into develop Apr 25, 2021
@sseerrggii sseerrggii deleted the feat/download-all-admins branch April 25, 2021 17:49
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.

3 participants