port tootsuite#12562 to monsterfork: Fix media attachments without file being uploadable

Fix #12554

4 files changed, 20 insertions, 22 deletions

diff --git a/app/models/media_attachment.rb b/app/models/media_attachment.rb
index d2db899ab..f8e925942 100644
--- a/app/models/media_attachment.rb
+++ b/app/models/media_attachment.rb
@@ -121,7 +121,8 @@ class MediaAttachment < ApplicationRecord
   include Attachmentable
 
   validates :account, presence: true
-  validates :description, length: { maximum: 6666 }, if: :local?
+  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 bb938e36d..651927c2d 100644
--- 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 af22a9812..e823f1713 100644
--- 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 666 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 e120d78e8..9a579a693 100644
--- a/spec/services/post_status_service_spec.rb
+++ b/spec/services/post_status_service_spec.rb
@@ -214,14 +214,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(