-
-
Notifications
You must be signed in to change notification settings - Fork 236
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
Offline CSV generation #3146
Offline CSV generation #3146
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3146 +/- ##
==========================================
- Coverage 83.49% 83.48% -0.01%
==========================================
Files 250 251 +1
Lines 15639 15721 +82
Branches 2925 2938 +13
==========================================
+ Hits 13058 13125 +67
- Misses 1657 1665 +8
- Partials 924 931 +7
Continue to review full report at Codecov.
|
b7dc527
to
63bd527
Compare
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.
Not so much anything wrong, but a question and a test nice to have.
my $token = 'access_token=' . $counciluser->id . '-1234567890abcdefgh'; | ||
$mech->get_ok("/dashboard/csv/$name?$token"); | ||
is $mech->res->code, 202; | ||
} |
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.
Should we not also check that if all files are generated it reflects that too. If for no other reason that to check we don't get endless request loops if someone leaves the tab open.
my $cfg = FixMyStreet->config('PHOTO_STORAGE_OPTIONS'); | ||
my $dir = $cfg ? $cfg->{UPLOAD_DIR} : FixMyStreet->config('UPLOAD_DIR'); | ||
$dir = path($dir, "dashboard_csv")->absolute(FixMyStreet->path_to()); | ||
my $subdir = $self->user ? $self->user->id : 0; |
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.
Should there not always be a user here?
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.
No, kiitc uses the cobrand hook to allow non-authenticated access to the CSV export.
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.
Though they will still be using export=1 and not be large, so wouldn't hit this. But still, might as well be safe.
Include a status page, the option for access token requests to use this system, and a script for manual generation.
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.
🔢
Fixes https:/mysociety/fixmystreet-commercial/issues/1926
From the command line, can call with export=2 instead of 1 to get the 202 flow, otherwise is as before.