Fix media attachments without file being uploadable (#12562)

Fix #12554

diff --git a/app/models/media_attachment.rb b/app/models/media_attachment.rb
index 1fd0adfd02ba74976da02b9ddff224f8ac3aace8..42364641f39fc371c43a65ddc9c6362c71423549 100644 (file)
--- a/app/models/media_attachment.rb
+++ b/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) }

diff --git a/spec/fabricators/media_attachment_fabricator.rb b/spec/fabricators/media_attachment_fabricator.rb
index bb938e36d9825d6bbd9f1769d84e04731bf9889b..651927c2dd97f8a790e4dd09a5578a355323b46e 100644 (file)
--- a/spec/fabricators/media_attachment_fabricator.rb
+++ b/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
+    case attrs[:type]
+    when :gifv, :video
+      attachment_fixture('attachment.webm')
+    else
+      attachment_fixture('attachment.jpg')
+    end
   end
 end

diff --git a/spec/models/media_attachment_spec.rb b/spec/models/media_attachment_spec.rb
index a275621a134be4d3c37e4cbba151c79e5e7c9214..456bc42167b8b56db2e238a9dd9e15ced0dd9bef 100644 (file)
--- a/spec/models/media_attachment_spec.rb
+++ b/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')

diff --git a/spec/services/post_status_service_spec.rb b/spec/services/post_status_service_spec.rb
index bf06f50e9bd3dc68563181b8eda13175a36c0d7f..025a3da40c1dea17f29377234f5d83dcffb5c13b 100644 (file)
--- a/spec/services/post_status_service_spec.rb
+++ b/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(