From 81cc86bb1ffb662843938379eeb522e3a6f11b79 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Thu, 23 Jan 2020 21:40:03 +0100 Subject: [PATCH] Fix media attachments without file being uploadable (#12562) Fix #12554 --- app/models/media_attachment.rb | 1 + .../fabricators/media_attachment_fabricator.rb | 18 +++++++----------- spec/models/media_attachment_spec.rb | 13 +++++-------- spec/services/post_status_service_spec.rb | 8 ++++++-- 4 files changed, 19 insertions(+), 21 deletions(-) diff --git a/app/models/media_attachment.rb b/app/models/media_attachment.rb index 1fd0adfd02..42364641f3 100644 --- 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 bb938e36d9..651927c2dd 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 a275621a13..456bc42167 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 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 bf06f50e9b..025a3da40c 100644 --- 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(