diff --git a/app/controllers/admin/reports_controller.rb b/app/controllers/admin/reports_controller.rb index 9aff9f597f..ac6f1b73b4 100644 --- a/app/controllers/admin/reports_controller.rb +++ b/app/controllers/admin/reports_controller.rb @@ -80,11 +80,10 @@ module Admin block: "start" ).broadcast - blob = ReportBlob.create_for_upload_later!(report_filename) - ReportJob.perform_later( report_class:, user: spree_current_user, params:, - format:, blob:, channel: ScopedChannel.for_id(params[:uuid]), + format:, filename: report_filename, + channel: ScopedChannel.for_id(params[:uuid]), ) head :no_content diff --git a/app/jobs/application_job.rb b/app/jobs/application_job.rb index 6411c58c0b..5fd395e89f 100644 --- a/app/jobs/application_job.rb +++ b/app/jobs/application_job.rb @@ -12,6 +12,6 @@ class ApplicationJob < ActiveJob::Base def enable_active_storage_urls ActiveStorage::Current.url_options ||= - Rails.application.config.action_controller.default_url_options + Rails.application.routes.default_url_options end end diff --git a/app/jobs/report_job.rb b/app/jobs/report_job.rb index 9d1e081433..c72cb98b2e 100644 --- a/app/jobs/report_job.rb +++ b/app/jobs/report_job.rb @@ -9,12 +9,12 @@ class ReportJob < ApplicationJob NOTIFICATION_TIME = 5.seconds - def perform(report_class:, user:, params:, format:, blob:, channel: nil) + def perform(report_class:, user:, params:, format:, filename:, channel: nil) start_time = Time.zone.now report = report_class.new(user, params, render: true) result = report.render_as(format) - blob.store(result) + blob = ReportBlob.create!(filename, result) execution_time = Time.zone.now - start_time diff --git a/app/models/report_blob.rb b/app/models/report_blob.rb index fb382fba31..b9d3927b68 100644 --- a/app/models/report_blob.rb +++ b/app/models/report_blob.rb @@ -5,34 +5,20 @@ class ReportBlob < ActiveStorage::Blob # AWS S3 limits URL expiry to one week. LIFETIME = 1.week - def self.create_for_upload_later!(filename) - # ActiveStorage discourages modifying a blob later but we need a blob - # before we know anything about the report file. It enables us to use the - # same blob in the controller to read the result. - create_before_direct_upload!( + def self.create!(filename, content) + create_and_upload!( + io: StringIO.new(content), filename:, - byte_size: 0, - checksum: "0", content_type: content_type(filename), - ).tap do |blob| - ActiveStorage::PurgeJob.set(wait: LIFETIME).perform_later(blob) - end + identify: false, + service_name: :local, + ) end def self.content_type(filename) MIME::Types.of(filename).first&.to_s || "application/octet-stream" end - def store(content) - io = StringIO.new(content) - upload(io, identify: false) - save! - end - - def content_stored? - @content_stored ||= reload.checksum != "0" - end - def result @result ||= download.force_encoding(Encoding::UTF_8) end diff --git a/config/environments/test.rb b/config/environments/test.rb index e51d8b19cd..e5cc306c18 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -56,6 +56,4 @@ Openfoodnetwork::Application.configure do config.active_support.deprecation = :stderr config.active_job.queue_adapter = :test - - config.active_storage.service = :test end diff --git a/config/storage.yml b/config/storage.yml index 271782041f..8e0bb583a8 100644 --- a/config/storage.yml +++ b/config/storage.yml @@ -1,10 +1,6 @@ local: service: Disk - root: <%= Rails.root.join("storage") %> - -test: - service: Disk - root: <%= Rails.root.join("tmp/storage") %> + root: <%= Rails.root.join(Rails.env.test? ? "tmp/storage" : "storage") %> test_amazon: service: S3 diff --git a/spec/jobs/report_job_spec.rb b/spec/jobs/report_job_spec.rb index c99f5fc38a..14807c6578 100644 --- a/spec/jobs/report_job_spec.rb +++ b/spec/jobs/report_job_spec.rb @@ -3,16 +3,17 @@ require 'spec_helper' describe ReportJob do + include CableReady::Broadcaster + let(:report_args) { - { report_class:, user:, params:, format:, - blob: } + { report_class:, user:, params:, format:, filename: } } let(:report_class) { Reporting::Reports::UsersAndEnterprises::Base } let(:user) { enterprise.owner } let(:enterprise) { create(:enterprise) } let(:params) { {} } let(:format) { :csv } - let(:blob) { ReportBlob.create_for_upload_later!("report.csv") } + let(:filename) { "report.csv" } it "generates a report" do job = perform_enqueued_jobs(only: ReportJob) do @@ -22,15 +23,30 @@ describe ReportJob do end it "enqueues a job for async processing" do - job = ReportJob.perform_later(**report_args) - expect(blob.content_stored?).to eq false + expect { + ReportJob.perform_later(**report_args) + }.to_not change { ActiveStorage::Blob.count } - perform_enqueued_jobs(only: ReportJob) + expect { + perform_enqueued_jobs(only: ReportJob) + }.to change { ActiveStorage::Blob.count } - expect(blob.content_stored?).to eq true expect_csv_report end + it "notifies Cable Ready when the report is done" do + channel = ScopedChannel.for_id("123") + with_channel = report_args.merge(channel:) + + ReportJob.perform_later(**with_channel) + + expect(cable_ready[channel]).to receive(:broadcast).and_call_original + + expect { + perform_enqueued_jobs(only: ReportJob) + }.to change { ActiveStorage::Blob.count } + end + it "triggers an email when the report is done" do # Setup test data which also triggers emails: report_args @@ -44,10 +60,9 @@ describe ReportJob do ReportJob.perform_later(**report_args) perform_enqueued_jobs(only: ReportJob) }.to enqueue_mail(ReportMailer, :report_ready).with( - params: { + params: hash_including( to: user.email, - blob:, - }, + ), args: [], ) end @@ -76,7 +91,7 @@ describe ReportJob do end def expect_csv_report - blob.reload + blob = ReportBlob.last expect(blob.filename.to_s).to eq "report.csv" expect(blob.content_type).to eq "text/csv" diff --git a/spec/mailers/report_mailer_spec.rb b/spec/mailers/report_mailer_spec.rb index 443617afd3..54a63845ea 100644 --- a/spec/mailers/report_mailer_spec.rb +++ b/spec/mailers/report_mailer_spec.rb @@ -10,7 +10,7 @@ describe ReportMailer do blob:, ).report_ready } - let(:blob) { ReportBlob.create_for_upload_later!("customers.csv") } + let(:blob) { ReportBlob.create!("customers.csv", "report content") } it "notifies about a report" do expect(email.subject).to eq "Report ready" diff --git a/spec/models/report_blob_spec.rb b/spec/models/report_blob_spec.rb index d16acc8fe0..1238ae1564 100644 --- a/spec/models/report_blob_spec.rb +++ b/spec/models/report_blob_spec.rb @@ -4,11 +4,10 @@ require 'spec_helper' describe ReportBlob, type: :model do it "preserves UTF-8 content" do - blob = ReportBlob.create_for_upload_later!("customers.html") content = "This works. ✓" expect do - blob.store(content) + blob = ReportBlob.create!("customers.html", content) content = blob.result end.to_not change { content.encoding }.from(Encoding::UTF_8) end diff --git a/spec/system/admin/reports_spec.rb b/spec/system/admin/reports_spec.rb index 33a29028da..2547e59c2f 100644 --- a/spec/system/admin/reports_spec.rb +++ b/spec/system/admin/reports_spec.rb @@ -80,14 +80,13 @@ describe ' email = ActionMailer::Base.deliveries.last expect(email.body).to have_link( "customers", - href: %r"^http://test\.host/rails/active_storage/disk/.*/customers_[0-9]+\.html$" + href: %r"^http://.*/rails/active_storage/disk/.*/customers_[0-9]+\.html$" ) # ActiveStorage links usually expire after 5 minutes. # But we want a longer expiry in emailed links. parsed_email = Capybara::Node::Simple.new(email.body.to_s) - email_link_href = parsed_email.find(:link, "customers")[:href] - report_link = email_link_href.sub("test.host", Rails.application.default_url_options[:host]) + report_link = parsed_email.find(:link, "customers")[:href] content = URI.parse(report_link).read expect(content).to match "\nFirst Name\n"