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

Allow CSV files for previous runs to be downloaded via the UI #318

Merged
merged 2 commits into from
Feb 1, 2021

Conversation

adrianna-chang-shopify
Copy link
Contributor

@adrianna-chang-shopify adrianna-chang-shopify commented Jan 29, 2021

This PR leverages Active Storage's download link helper to provide download links for CSV attachments associated with previous runs for CSV Tasks. Note that we have to use Rails.application.routes.url_helpers.rails_blob_path instead of rails_blob_path because the download URL is associated with the host app, not the engine (and additionally, we don't specify Active Storage as a dependency of the engine, so this helper isn't even available to us).

Since we're using run.csv_file.attached? to check whether to show the download link, I've added preloading for the CSV file attachments when we generate the ActiveRecord::Relation for Runs, as @etiennebarrie suggested. (And we can use with_attached_csv_file, since this scope ships out-of-the-box with ActiveStorage)

To 🎩

  • Go to http://localhost:3000/maintenance_tasks/tasks/Maintenance::ImportPostsTask/
  • Upload the sample.csv from test/fixtures/files
  • Click the Download CSV link that appears when the Task finishes

@adrianna-chang-shopify adrianna-chang-shopify force-pushed the show-uploaded-csvs-for-csv-tasks branch 3 times, most recently from 4bebdd2 to 552d363 Compare January 30, 2021 20:04
Copy link
Member

@etiennebarrie etiennebarrie left a comment

Choose a reason for hiding this comment

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

It was indeed neat to have a nicer file name. Not sure it's worth the added complexity though.

app/views/maintenance_tasks/tasks/show.html.erb Outdated Show resolved Hide resolved
@adrianna-chang-shopify adrianna-chang-shopify merged commit 77f5c01 into main Feb 1, 2021
@adrianna-chang-shopify adrianna-chang-shopify deleted the show-uploaded-csvs-for-csv-tasks branch February 1, 2021 18:32
@adrianna-chang-shopify adrianna-chang-shopify added the CSV Requirements for CSV feature label Feb 11, 2021
@adrianna-chang-shopify adrianna-chang-shopify temporarily deployed to rubygems February 11, 2021 21:05 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CSV Requirements for CSV feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants