Merge branch 'refactor-factories' into develop

spec/controllers/admin_request_controller_spec.rb

@@ -260,7 +260,7 @@
     end
 
     it 'redirects after destroying a request with incoming_messages' do
-      incoming_message = FactoryBot.create(:incoming_message_with_html_attachment,
+      incoming_message = FactoryBot.create(:incoming_message, :with_html_attachment,
                                            info_request: info_request)
       delete :destroy, params: { id: info_request.id }
 

spec/controllers/attachments_controller_spec.rb

@@ -104,7 +104,7 @@ def show(params = {})
         )
       end
 
-      context 'when masked attachment is avaliable before timing out' do
+      context 'when masked attachment is available before timing out' do
         before do
           allow(IncomingMessage).to receive(
             :get_attachment_by_url_part_number_and_filename!

spec/controllers/request_controller_spec.rb

@@ -546,8 +546,7 @@ def expect_hidden(hidden_template)
   end
 
   let(:info_request) do
-    FactoryBot.
-      create(:info_request_with_incoming_attachments, prominence: prominence)
+    FactoryBot.create(:info_request_with_pdf_attachment, prominence: prominence)
   end
 
   context 'when the request is hidden' do

spec/factories/foi_attchments.rb

@@ -4,17 +4,16 @@
     sequence(:url_part_number) { |n| n + 1 }
     display_size { '0K' }
     masked_at { 1.day.ago }
+    content_type { 'text/plain' }
+    filename { 'attachment.txt' }
 
     transient do
       body { 'hereisthemaskedtext' }
     end
 
     after(:build) do |foi_attachment, evaluator|
       body = evaluator.body
-      foi_attachment.hexdigest = Digest::MD5.hexdigest(body) if body
-
-      next unless body && foi_attachment.filename && foi_attachment.content_type
-
+      foi_attachment.hexdigest = Digest::MD5.hexdigest(body)
       foi_attachment.file.attach(
         io: StringIO.new(body),
         filename: foi_attachment.filename,

spec/factories/incoming_messages.rb

@@ -23,22 +23,29 @@
 #
 
 FactoryBot.define do
-
   factory :incoming_message do
     info_request
     association :raw_email, strategy: :create
     last_parsed { 1.week.ago }
     sent_at { 1.week.ago }
 
-    after(:build) do |incoming_message, _evaluator|
-      incoming_message.foi_attachments << build(
-        :body_text,
-        incoming_message: incoming_message,
-        url_part_number: 1
-      )
+    transient do
+      foi_attachments_factories { [] }
+    end
+
+    after(:build) do |incoming_message, evaluator|
+      foi_attachments_factories = [[:body_text]]
+      foi_attachments_factories += evaluator.foi_attachments_factories
+      foi_attachments_factories.each.with_index(1) do |factory, index|
+        incoming_message.foi_attachments << build(
+          *factory,
+          incoming_message: incoming_message,
+          url_part_number: index
+        )
+      end
 
-      incoming_message.raw_email.incoming_message = incoming_message
-      incoming_message.raw_email.data = "somedata"
+      mail = build_incoming_message_mail(incoming_message)
+      incoming_message.raw_email.data = mail
     end
 
     trait :unparsed do
@@ -50,47 +57,27 @@
       prominence { 'hidden' }
     end
 
-    factory :plain_incoming_message do
-      last_parsed { nil }
-      sent_at { nil }
-
-      after(:create) do |incoming_message, _evaluator|
-        data = load_file_fixture('incoming-request-plain.email')
-        data.gsub!('EMAIL_FROM', 'Bob Responder <[email protected]>')
-        incoming_message.raw_email.data = data
-        incoming_message.raw_email.save!
-      end
+    trait :with_html_attachment do
+      foi_attachments_factories { [[:html_attachment]] }
     end
 
-    factory :incoming_message_with_html_attachment do
-      after(:build) do |incoming_message, _evaluator|
-        incoming_message.foi_attachments << build(
-          :html_attachment,
-          incoming_message: incoming_message,
-          url_part_number: 2
-        )
-      end
+    trait :with_pdf_attachment do
+      foi_attachments_factories { [[:pdf_attachment]] }
     end
+  end
 
-    factory :incoming_message_with_attachments do
-      # foi_attachments_count is declared as an ignored attribute and available in
-      # attributes on the factory, as well as the callback via the evaluator
-      transient do
-        foi_attachments_count { 2 }
-      end
+  factory :plain_incoming_message, class: IncomingMessage do
+    info_request
+    association :raw_email, strategy: :create
+    last_parsed { nil }
+    sent_at { nil }
 
-      # the after(:build) yields two values; the incoming_message instance itself and the
-      # evaluator, which stores all values from the factory, including ignored
-      # attributes;
-      after(:build) do |incoming_message, evaluator|
-        evaluator.foi_attachments_count.times do |count|
-          incoming_message.foi_attachments << build(
-            :pdf_attachment,
-            incoming_message: incoming_message,
-            url_part_number: count + 2
-          )
-        end
-      end
+    after(:create) do |incoming_message, _evaluator|
+      data = load_file_fixture('incoming-request-plain.email')
+      data.gsub!('EMAIL_FROM', 'Bob Responder <[email protected]>')
+      incoming_message.raw_email.data = data
+      incoming_message.raw_email.save!
+      incoming_message.extract_attachments!
     end
   end
 end

spec/factories/info_request_events.rb

@@ -57,20 +57,20 @@
 
     factory :response_event do
       transient do
-        incoming_message_factory { :incoming_message }
+        incoming_message_factory { [:incoming_message] }
       end
 
       event_type { 'response' }
 
       after(:build) do |event, evaluator|
         event.incoming_message ||= build(
-          evaluator.incoming_message_factory, info_request: event.info_request
+          *evaluator.incoming_message_factory, info_request: event.info_request
         )
         event.info_request = event.incoming_message.info_request
       end
 
       trait :with_attachments do
-        incoming_message_factory { :incoming_message_with_attachments }
+        incoming_message_factory { [:incoming_message, :with_pdf_attachment] }
       end
     end
 

spec/factories/info_requests.rb

@@ -60,11 +60,11 @@
 
     trait :with_incoming do
       transient do
-        incoming_message_factory { :incoming_message }
+        incoming_message_factory { [:incoming_message] }
       end
 
       after(:create) do |info_request, evaluator|
-        incoming_message = create(evaluator.incoming_message_factory,
+        incoming_message = create(*evaluator.incoming_message_factory,
                                   info_request: info_request)
         info_request.log_event(
           'response',
@@ -74,17 +74,17 @@
     end
 
     trait :with_plain_incoming do
-      incoming_message_factory { :plain_incoming_message }
+      incoming_message_factory { [:plain_incoming_message] }
       with_incoming
     end
 
     trait :with_incoming_with_html_attachment do
-      incoming_message_factory { :incoming_message_with_html_attachment }
+      incoming_message_factory { [:incoming_message, :with_html_attachment] }
       with_incoming
     end
 
-    trait :with_incoming_with_attachments do
-      incoming_message_factory { :incoming_message_with_attachments }
+    trait :with_incoming_with_pdf_attachment do
+      incoming_message_factory { [:incoming_message, :with_pdf_attachment] }
       with_incoming
     end
 
@@ -284,9 +284,9 @@
 
     factory :info_request_with_plain_incoming, traits: [:with_plain_incoming]
     factory :info_request_with_html_attachment, traits: [:with_incoming_with_html_attachment]
-    factory :info_request_with_incoming_attachments, traits: [:with_incoming_with_attachments]
+    factory :info_request_with_pdf_attachment, traits: [:with_incoming_with_pdf_attachment]
     factory :info_request_with_internal_review_request, traits: [:with_internal_review_request]
-    factory :embargoed_request, traits: [:embargoed, :with_incoming_with_attachments]
+    factory :embargoed_request, traits: [:embargoed, :with_incoming_with_pdf_attachment]
     factory :embargo_expiring_request, traits: [:embargo_expiring]
     factory :re_embargoed_request, traits: [:re_embargoed]
     factory :embargo_expired_request, traits: [:embargo_expired]

spec/integration/download_request_spec.rb

@@ -53,11 +53,11 @@ def sleep_and_receive_mail(name, info_request)
 
         # Non-owner can download zip with incoming and attachments
         non_owner = login(FactoryBot.create(:user))
-        info_request = FactoryBot.create(:info_request_with_incoming_attachments)
+        info_request = FactoryBot.create(:info_request_with_pdf_attachment)
         rebuild_raw_emails(info_request)
 
         inspect_zip_download(non_owner, info_request) do |zip|
-          expect(zip.count).to eq(3)
+          expect(zip.count).to eq(2)
           expect(zip.read('correspondence.pdf')).to match('hereisthetext')
         end
 
@@ -72,7 +72,7 @@ def sleep_and_receive_mail(name, info_request)
 
         # Admin retains the requester only things
         inspect_zip_download(admin, info_request) do |zip|
-          expect(zip.count).to eq(3)
+          expect(zip.count).to eq(2)
           expect(zip.read('correspondence.pdf')).to match('hereisthetext')
         end
 
@@ -88,7 +88,7 @@ def sleep_and_receive_mail(name, info_request)
         # Requester retains the requester only things
         owner = login(info_request.user)
         inspect_zip_download(owner, info_request) do |zip|
-          expect(zip.count).to eq(3)
+          expect(zip.count).to eq(2)
           expect(zip.read('correspondence.pdf')).to match('hereisthetext')
         end
 
@@ -299,11 +299,11 @@ def sleep_and_receive_mail(name, info_request)
 
         # Non-owner can download zip with outgoing
         non_owner = login(FactoryBot.create(:user))
-        info_request = FactoryBot.create(:info_request_with_incoming_attachments)
+        info_request = FactoryBot.create(:info_request_with_pdf_attachment)
         rebuild_raw_emails(info_request)
 
         inspect_zip_download(non_owner, info_request) do |zip|
-          expect(zip.count).to eq(3)
+          expect(zip.count).to eq(2)
           expect(zip.read('correspondence.txt')).to match('hereisthetext')
         end
 
@@ -318,7 +318,7 @@ def sleep_and_receive_mail(name, info_request)
 
         # Admin retains the requester only things
         inspect_zip_download(admin, info_request) do |zip|
-          expect(zip.count).to eq(3)
+          expect(zip.count).to eq(2)
           expect(zip.read('correspondence.txt')).to match('hereisthetext')
         end
 
@@ -334,7 +334,7 @@ def sleep_and_receive_mail(name, info_request)
         # Requester retains the requester only things
         owner = login(info_request.user)
         inspect_zip_download(owner, info_request) do |zip|
-          expect(zip.count).to eq(3)
+          expect(zip.count).to eq(2)
           expect(zip.read('correspondence.txt')).to match('hereisthetext')
         end
 

spec/integration/errors_spec.rb

@@ -92,7 +92,7 @@
     end
 
     it 'should render a 403 with text body for attempts at directory listing for attachments' do
-      info_request = FactoryBot.create(:info_request_with_incoming_attachments)
+      info_request = FactoryBot.create(:info_request_with_pdf_attachment)
       id = info_request.id
       prefix = id.to_s[0..2]
       msg_id = info_request.incoming_messages.first.id

spec/models/foi_attachment_spec.rb

@@ -136,7 +136,7 @@
         allow(FoiAttachmentMaskJob).to receive(:perform_now).and_return(false)
       end
 
-      it 'raises missing attachment expection' do
+      it 'raises missing attachment exception' do
         expect { foi_attachment.body }.to raise_error(
           FoiAttachment::MissingAttachment,
           "job already queued (ID=#{foi_attachment.id})"
@@ -223,7 +223,7 @@
           FactoryBot.create(:body_text, prominence: 'hidden')
         end
 
-        it 'raises missing attachment expection' do
+        it 'raises missing attachment exception' do
           expect { unmasked_body }.to raise_error(
             FoiAttachment::MissingAttachment,
             "prominence not public (ID=#{foi_attachment.id})"
@@ -233,10 +233,12 @@
 
       context 'when attachment file is unattached' do
         let(:foi_attachment) do
-          FactoryBot.create(:body_text, filename: nil)
+          FactoryBot.create(:body_text)
         end
 
-        it 'raises missing attachment expection' do
+        it 'raises missing attachment exception' do
+          foi_attachment.file.purge
+
           expect { unmasked_body }.to raise_error(
             FoiAttachment::MissingAttachment,
             "file not attached (ID=#{foi_attachment.id})"
@@ -251,7 +253,7 @@
           ).and_return(nil)
         end
 
-        it 'raises missing attachment expection' do
+        it 'raises missing attachment exception' do
           expect { unmasked_body }.to raise_error(
             FoiAttachment::MissingAttachment,
             "unable to find original (ID=#{foi_attachment.id})"
@@ -312,7 +314,7 @@
   describe '#main_body_part?' do
     subject { attachment.main_body_part? }
 
-    let(:message) { FactoryBot.build(:incoming_message_with_attachments) }
+    let(:message) { FactoryBot.build(:incoming_message, :with_pdf_attachment) }
 
     context 'when the attachment is the main body' do
       let(:attachment) { message.get_main_body_text_part }

spec/models/incoming_message_spec.rb

@@ -612,7 +612,7 @@
 
   context 'with attachments' do
     let(:incoming_with_attachment) {
-      FactoryBot.create(:incoming_message_with_html_attachment)
+      FactoryBot.create(:incoming_message, :with_html_attachment)
     }
 
     it 'destroys the incoming message' do

spec/models/info_request_event_spec.rb

@@ -897,7 +897,7 @@
 
       it 'should return a space separated list of the attachment file types' do
         info_request = ire.info_request
-        incoming = FactoryBot.create(:incoming_message_with_attachments,
+        incoming = FactoryBot.create(:incoming_message, :with_pdf_attachment,
                                      info_request: info_request)
         ire.incoming_message = incoming
         expect(ire.send(:filetype)).to eq('pdf')

spec/models/info_request_spec.rb

@@ -133,7 +133,7 @@
 
     context 'when there are incoming messages with attachments' do
       let(:info_request) do
-        FactoryBot.create(:info_request_with_incoming_attachments)
+        FactoryBot.create(:info_request_with_pdf_attachment)
       end
 
       it { is_expected.to be_many }