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

fix(utils): Prevent incorrect CSV data appending due to shared variable #4368

Merged
merged 4 commits into from
Aug 29, 2024

Conversation

ERosendo
Copy link
Contributor

@ERosendo ERosendo commented Aug 28, 2024

This PR addresses a bug that was leading to inaccurate data in the exported CSV file. To address this issue, we've refactored the generate_docket_entries_csv_data function to avoid using a common variable within the inner loop.

This PR implements a rate limiter to control the frequency of CSV exports per hour. Additionally, the de_list template has been modified to display a loading spinner while the export request is in progress. If the rate limit is reached, an error message will be shown to inform the user about the issue.

Here are some gifs showing the spinner and the error message:

Small screen size:

Regular Download

Screen Recording 2024-08-28 at 5 30 34 PM

Ratelimit Error

Screen Recording 2024-08-28 at 5 37 10 PM


Medium screen size:

Regular Download

Screen Recording 2024-08-28 at 6 02 58 PM

Ratelimit Error

Screen Recording 2024-08-28 at 5 38 01 PM


Large screen size:

Regular Download

Screen Recording 2024-08-28 at 6 04 21 PM

Ratelimit Error

Screen Recording 2024-08-28 at 5 38 48 PM

Fixes #4362

- Refactored `generate_docket_entries_csv_data` to avoid using a common variable within the inner loop.
- This change addresses a bug that was causing duplicate or incorrect data in the generated CSV file.
@ERosendo ERosendo force-pushed the 4362-feat-upgrade-export-docket-feature branch from fe516d9 to 3cacc71 Compare August 28, 2024 20:13
@ERosendo ERosendo force-pushed the 4362-feat-upgrade-export-docket-feature branch from 3cacc71 to 71a66fc Compare August 28, 2024 20:39
@ERosendo ERosendo force-pushed the 4362-feat-upgrade-export-docket-feature branch from 71a66fc to c3c2486 Compare August 28, 2024 22:08
@ERosendo ERosendo marked this pull request as ready for review August 28, 2024 22:23
@ERosendo ERosendo requested a review from mlissner August 28, 2024 22:23
Copy link
Member

@mlissner mlissner left a comment

Choose a reason for hiding this comment

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

Nice. A couple little things, but looks good to me otherwise. Thank you!

cl/opinion_page/templates/includes/de_list.html Outdated Show resolved Hide resolved
cl/opinion_page/templates/includes/de_list.html Outdated Show resolved Hide resolved
This commit moves the spinner icon to the right side of the button text and uses a custom class to control its visibility, replacing the previous opacity-based approach.
@ERosendo
Copy link
Contributor Author

@mike Thanks for your comments.

Here are updated gifs for medium and large screen sizes:

Medium screen size:

Regular Download

Screen Recording 2024-08-28 at 8 06 26 PM

Ratelimit Error

Screen Recording 2024-08-28 at 8 02 25 PM


Large screen size:

Regular Download

Screen Recording 2024-08-28 at 8 08 15 PM

Ratelimit Error

Screen Recording 2024-08-28 at 8 10 53 PM

@ERosendo ERosendo requested a review from mlissner August 29, 2024 00:12
@mlissner mlissner merged commit 553a772 into main Aug 29, 2024
13 checks passed
@mlissner mlissner deleted the 4362-feat-upgrade-export-docket-feature branch August 29, 2024 00:30
@mlissner
Copy link
Member

Nice, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

Upgrades for export docket feature
2 participants