Merge pull request #4230 from tchak/fix-procedure-logo-migration-task

Fix and test migrate_procedure_logo task

app/models/procedure.rb

@@ -469,7 +469,8 @@ class Procedure < ApplicationRecord
       if Rails.application.secrets.fog[:enabled]
         RemoteDownloader.new(logo.filename).url
       else
-        LocalDownloader.new(logo.path, 'logo').url
+        # FIXME: this is horrible but used only in dev and will be removed after migration
+        File.join(LOCAL_DOWNLOAD_URL, logo.url)
       end
     end
   end

lib/tasks/2019_08_20_migrate_procedure_logo.rake

@@ -1,8 +1,8 @@
 namespace :'2019_08_20_migrate_procedure_logo' do
   task run: :environment do
-    procedures = Procedure.where
-      .not(logo: nil)
-      .left_joins(:logo_new_attachment)
+    procedures = Procedure.unscope(where: :hidden_at)
+      .where.not(logo: nil)
+      .left_joins(:logo_active_storage_attachment)
       .where('active_storage_attachments.id IS NULL')
       .order(:created_at)
 
@@ -18,7 +18,7 @@ namespace :'2019_08_20_migrate_procedure_logo' do
         response = Typhoeus.get(uri)
         if response.success?
           filename = procedure.logo.filename || procedure.logo_identifier
-          procedure.logo_new.attach(
+          procedure.logo_active_storage.attach(
             io: StringIO.new(response.body),
             filename: filename,
             content_type: procedure.logo.content_type,

spec/factories/procedure.rb

@@ -46,6 +46,10 @@ FactoryBot.define do
       logo_active_storage { Rack::Test::UploadedFile.new("./spec/fixtures/files/logo_test_procedure.png", 'image/png') }
     end
 
+    trait :with_legacy_logo do
+      logo { Rack::Test::UploadedFile.new("./spec/fixtures/files/logo_test_procedure.png", 'image/png') }
+    end
+
     trait :with_path do
       path { generate(:published_path) }
     end

spec/lib/tasks/2019_08_20_migrate_procedure_logo_spec.rb

@@ -0,0 +1,60 @@
+describe '2019_08_20_migrate_procedure_logo.rake' do
+  let(:rake_task) { Rake::Task['2019_08_20_migrate_procedure_logo:run'] }
+
+  let(:procedures) do
+    [
+      create(:procedure),
+      create(:procedure, :with_legacy_logo),
+      create(:procedure, :with_legacy_logo)
+    ]
+  end
+
+  let(:run_task) do
+    rake_task.invoke
+    procedures.each(&:reload)
+  end
+
+  before do
+    procedures.each do |procedure|
+      if procedure.logo.present?
+        stub_request(:get, procedure.logo_url)
+          .to_return(status: 200, body: File.read(procedure.logo.path))
+      end
+    end
+  end
+
+  after do
+    ENV['LIMIT'] = nil
+    rake_task.reenable
+  end
+
+  it 'should migrate logo' do
+    expect(procedures.map(&:logo_active_storage).map(&:attached?)).to eq([false, false, false])
+
+    run_task
+
+    expect(Procedure.where(logo: nil).count).to eq(1)
+    expect(procedures.map(&:logo_active_storage).map(&:attached?)).to eq([false, true, true])
+  end
+
+  it 'should migrate logo within limit' do
+    expect(procedures.map(&:logo_active_storage).map(&:attached?)).to eq([false, false, false])
+
+    ENV['LIMIT'] = '1'
+    run_task
+
+    expect(Procedure.where(logo: nil).count).to eq(1)
+    expect(procedures.map(&:logo_active_storage).map(&:attached?)).to eq([false, true, false])
+  end
+
+  context 'when a procedure is hidden' do
+    let(:hidden_procedure) { create(:procedure, :hidden, :with_legacy_logo) }
+    let(:procedures) { [hidden_procedure] }
+
+    it 'should migrate logo' do
+      run_task
+
+      expect(hidden_procedure.logo_active_storage.attached?).to be true
+    end
+  end
+end