Fix media attachments without file being uploadable (#12562)

Fix #12554

app/models/media_attachment.rb

@@ -142,6 +142,7 @@ class MediaAttachment < ApplicationRecord
 
   validates :account, presence: true
   validates :description, length: { maximum: MAX_DESCRIPTION_LENGTH }, if: :local?
+  validates :file, presence: true, if: :local?
 
   scope :attached,   -> { where.not(status_id: nil).or(where.not(scheduled_status_id: nil)) }
   scope :unattached, -> { where(status_id: nil, scheduled_status_id: nil) }

spec/fabricators/media_attachment_fabricator.rb

@@ -1,16 +1,12 @@
 Fabricator(:media_attachment) do
   account
+
   file do |attrs|
-    [
     case attrs[:type]
-      when :gifv
-        attachment_fixture ['attachment.gif', 'attachment.webm'].sample
-      when :image
-        attachment_fixture 'attachment.jpg'
-      when nil
-        attachment_fixture ['attachment.gif', 'attachment.jpg', 'attachment.webm'].sample
-      end,
-      nil
-    ].sample
+    when :gifv, :video
+      attachment_fixture('attachment.webm')
+    else
+      attachment_fixture('attachment.jpg')
+    end
   end
 end

spec/models/media_attachment_spec.rb

@@ -31,14 +31,6 @@ RSpec.describe MediaAttachment, type: :model do
     context 'file is blank' do
       let(:file) { nil }
 
-      context 'remote_url is blank' do
-        let(:remote_url) { '' }
-
-        it 'returns false' do
-          is_expected.to be false
-        end
-      end
-
       context 'remote_url is present' do
         let(:remote_url) { 'remote_url' }
 
@@ -153,6 +145,11 @@ RSpec.describe MediaAttachment, type: :model do
     end
   end
 
+  it 'is invalid without file' do
+    media = MediaAttachment.new(account: Fabricate(:account))
+    expect(media.valid?).to be false
+  end
+
   describe 'descriptions for remote attachments' do
     it 'are cut off at 1500 characters' do
       media = Fabricate(:media_attachment, description: 'foo' * 1000, remote_url: 'http://example.com/blah.jpg')

spec/services/post_status_service_spec.rb

@@ -212,14 +212,18 @@ RSpec.describe PostStatusService, type: :service do
 
   it 'does not allow attaching both videos and images' do
     account = Fabricate(:account)
+    video   = Fabricate(:media_attachment, type: :video, account: account)
+    image   = Fabricate(:media_attachment, type: :image, account: account)
+
+    video.update(type: :video)
 
     expect do
       subject.call(
         account,
         text: "test status update",
         media_ids: [
-          Fabricate(:media_attachment, type: :video, account: account),
-          Fabricate(:media_attachment, type: :image, account: account),
+          video,
+          image,
         ].map(&:id),
       )
     end.to raise_error(