From 38bec798117446603b304c0d14ca39bae5ee8be8 Mon Sep 17 00:00:00 2001 From: Chad Pytel Date: Fri, 7 Apr 2017 12:50:43 -0400 Subject: [PATCH 1/3] Add specs for media attachment validations There are currently not specs for the two media validations that are performed by `PostStatusService`. This adds specs for the validations that ensure that you cannot attach more than four files, and that a status cannot have both image and video attachments. --- .../media_attachment_fabricator.rb | 2 +- spec/services/post_status_service_spec.rb | 41 +++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/spec/fabricators/media_attachment_fabricator.rb b/spec/fabricators/media_attachment_fabricator.rb index 59db2440..dc91d708 100644 --- a/spec/fabricators/media_attachment_fabricator.rb +++ b/spec/fabricators/media_attachment_fabricator.rb @@ -1,3 +1,3 @@ Fabricator(:media_attachment) do - + account end diff --git a/spec/services/post_status_service_spec.rb b/spec/services/post_status_service_spec.rb index 9ee4daf6..acb922d6 100644 --- a/spec/services/post_status_service_spec.rb +++ b/spec/services/post_status_service_spec.rb @@ -7,4 +7,45 @@ RSpec.describe PostStatusService do it 'creates a new response status' it 'processes mentions' it 'pings PuSH hubs' + + it 'does not allow attaching more than 4 files' do + account = Fabricate(:account) + + expect do + PostStatusService.new.call( + account, + "test status update", + nil, + media_ids: [ + Fabricate(:media_attachment, account: account), + Fabricate(:media_attachment, account: account), + Fabricate(:media_attachment, account: account), + Fabricate(:media_attachment, account: account), + Fabricate(:media_attachment, account: account), + ].map(&:id), + ) + end.to raise_error( + Mastodon::ValidationError, + 'Cannot attach more than 4 files', + ) + end + + it 'does not allow attaching both videos and images' do + account = Fabricate(:account) + + expect do + PostStatusService.new.call( + account, + "test status update", + nil, + media_ids: [ + Fabricate(:media_attachment, type: :video, account: account), + Fabricate(:media_attachment, type: :image, account: account), + ].map(&:id), + ) + end.to raise_error( + Mastodon::ValidationError, + 'Cannot attach a video to a toot that already contains images', + ) + end end From 13c0077003288416b8cacd5d339f8796bc347f39 Mon Sep 17 00:00:00 2001 From: Chad Pytel Date: Fri, 7 Apr 2017 13:48:38 -0400 Subject: [PATCH 2/3] Add specs for PostStatusService This implements all pending specs, and adds additional coverage for the following functionality: * Normal status creation * Creating a reply status * Creating a sensitive status * Creating a status with spoiler text * A status with no spoiler text gets an empty string for spoiler text * Creating a status with custom visibility * Creating a status for an application * Processing mentions * Processing Hashtags * Pinging PuSH hubs * Crawling links * Attaching media --- spec/fabricators/status_fabricator.rb | 1 + spec/services/post_status_service_spec.rb | 169 ++++++++++++++++++---- 2 files changed, 145 insertions(+), 25 deletions(-) diff --git a/spec/fabricators/status_fabricator.rb b/spec/fabricators/status_fabricator.rb index df222fc9..8ec5f4ba 100644 --- a/spec/fabricators/status_fabricator.rb +++ b/spec/fabricators/status_fabricator.rb @@ -1,3 +1,4 @@ Fabricator(:status) do + account text "Lorem ipsum dolor sit amet" end diff --git a/spec/services/post_status_service_spec.rb b/spec/services/post_status_service_spec.rb index acb922d6..3fe878f6 100644 --- a/spec/services/post_status_service_spec.rb +++ b/spec/services/post_status_service_spec.rb @@ -3,27 +3,142 @@ require 'rails_helper' RSpec.describe PostStatusService do subject { PostStatusService.new } - it 'creates a new status' - it 'creates a new response status' - it 'processes mentions' - it 'pings PuSH hubs' + it 'creates a new status' do + account = Fabricate(:account) + text = "test status update" + + status = subject.call(account, text) + + expect(status).to be_persisted + expect(status.text).to eq text + end + + it 'creates a new response status' do + in_reply_to_status = Fabricate(:status) + account = Fabricate(:account) + text = "test status update" + + status = subject.call(account, text, in_reply_to_status) + + expect(status).to be_persisted + expect(status.text).to eq text + expect(status.thread).to eq in_reply_to_status + end + + it 'creates a sensitive status' do + status = create_status_with_options(sensitive: true) + + expect(status).to be_persisted + expect(status).to be_sensitive + end + + it 'creates a status with spoiler text' do + spoiler_text = "spoiler text" + + status = create_status_with_options(spoiler_text: spoiler_text) + + expect(status).to be_persisted + expect(status.spoiler_text).to eq spoiler_text + end + + it 'creates a status with empty default spoiler text' do + status = create_status_with_options(spoiler_text: nil) + + expect(status).to be_persisted + expect(status.spoiler_text).to eq '' + end + + it 'creates a status with the given visibility' do + status = create_status_with_options(visibility: :private) + + expect(status).to be_persisted + expect(status.visibility).to eq "private" + end + + it 'creates a status for the given application' do + application = Fabricate(:application) + + status = create_status_with_options(application: application) + + expect(status).to be_persisted + expect(status.application).to eq application + end + + it 'processes mentions' do + mention_service = double(:process_mentions_service) + allow(mention_service).to receive(:call) + allow(ProcessMentionsService).to receive(:new).and_return(mention_service) + account = Fabricate(:account) + + status = subject.call(account, "test status update") + + expect(ProcessMentionsService).to have_received(:new) + expect(mention_service).to have_received(:call).with(status) + end + + it 'processes hashtags' do + hashtags_service = double(:process_hashtags_service) + allow(hashtags_service).to receive(:call) + allow(ProcessHashtagsService).to receive(:new).and_return(hashtags_service) + account = Fabricate(:account) + + status = subject.call(account, "test status update") + + expect(ProcessHashtagsService).to have_received(:new) + expect(hashtags_service).to have_received(:call).with(status) + end + + it 'pings PuSH hubs' do + allow(DistributionWorker).to receive(:perform_async) + allow(Pubsubhubbub::DistributionWorker).to receive(:perform_async) + account = Fabricate(:account) + + status = subject.call(account, "test status update") + + expect(DistributionWorker).to have_received(:perform_async).with(status.id) + expect(Pubsubhubbub::DistributionWorker). + to have_received(:perform_async).with(status.stream_entry.id) + end + + it 'crawls links' do + allow(LinkCrawlWorker).to receive(:perform_async) + account = Fabricate(:account) + + status = subject.call(account, "test status update") + + expect(LinkCrawlWorker).to have_received(:perform_async).with(status.id) + end + + it 'attaches the given media to the created status' do + account = Fabricate(:account) + media = Fabricate(:media_attachment) + + status = subject.call( + account, + "test status update", + nil, + media_ids: [media.id], + ) + + expect(media.reload.status).to eq status + end it 'does not allow attaching more than 4 files' do account = Fabricate(:account) expect do - PostStatusService.new.call( - account, - "test status update", - nil, - media_ids: [ - Fabricate(:media_attachment, account: account), - Fabricate(:media_attachment, account: account), - Fabricate(:media_attachment, account: account), - Fabricate(:media_attachment, account: account), - Fabricate(:media_attachment, account: account), - ].map(&:id), - ) + subject.call( + account, + "test status update", + nil, + media_ids: [ + Fabricate(:media_attachment, account: account), + Fabricate(:media_attachment, account: account), + Fabricate(:media_attachment, account: account), + Fabricate(:media_attachment, account: account), + Fabricate(:media_attachment, account: account), + ].map(&:id), + ) end.to raise_error( Mastodon::ValidationError, 'Cannot attach more than 4 files', @@ -34,18 +149,22 @@ RSpec.describe PostStatusService do account = Fabricate(:account) expect do - PostStatusService.new.call( - account, - "test status update", - nil, - media_ids: [ - Fabricate(:media_attachment, type: :video, account: account), - Fabricate(:media_attachment, type: :image, account: account), - ].map(&:id), - ) + subject.call( + account, + "test status update", + nil, + media_ids: [ + Fabricate(:media_attachment, type: :video, account: account), + Fabricate(:media_attachment, type: :image, account: account), + ].map(&:id), + ) end.to raise_error( Mastodon::ValidationError, 'Cannot attach a video to a toot that already contains images', ) end + + def create_status_with_options(options = {}) + subject.call(Fabricate(:account), "test", nil, options) + end end From ad5ddd5e95062d0d5cd4bc56baff53698c598723 Mon Sep 17 00:00:00 2001 From: Chad Pytel Date: Fri, 7 Apr 2017 14:19:16 -0400 Subject: [PATCH 3/3] Use I18n for media attachment validation errors These are currently user facing errors, but are not localized. This adds the ability for these messages to be localized. --- app/services/post_status_service.rb | 4 ++-- config/locales/en.yml | 4 ++++ spec/services/post_status_service_spec.rb | 4 ++-- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/app/services/post_status_service.rb b/app/services/post_status_service.rb index b8179f7d..221aa42a 100644 --- a/app/services/post_status_service.rb +++ b/app/services/post_status_service.rb @@ -37,11 +37,11 @@ class PostStatusService < BaseService def validate_media!(media_ids) return if media_ids.nil? || !media_ids.is_a?(Enumerable) - raise Mastodon::ValidationError, 'Cannot attach more than 4 files' if media_ids.size > 4 + raise Mastodon::ValidationError, I18n.t('media_attachments.validations.too_many') if media_ids.size > 4 media = MediaAttachment.where(status_id: nil).where(id: media_ids.take(4).map(&:to_i)) - raise Mastodon::ValidationError, 'Cannot attach a video to a toot that already contains images' if media.size > 1 && media.find(&:video?) + raise Mastodon::ValidationError, I18n.t('media_attachments.validations.images_and_video') if media.size > 1 && media.find(&:video?) media end diff --git a/config/locales/en.yml b/config/locales/en.yml index 742219df..aa3a732f 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -163,3 +163,7 @@ en: invalid_otp_token: Invalid two-factor code will_paginate: page_gap: "…" + media_attachments: + validations: + too_many: Cannot attach more than 4 files + images_and_video: Cannot attach a video to a status that already contains images diff --git a/spec/services/post_status_service_spec.rb b/spec/services/post_status_service_spec.rb index 3fe878f6..0e39cd96 100644 --- a/spec/services/post_status_service_spec.rb +++ b/spec/services/post_status_service_spec.rb @@ -141,7 +141,7 @@ RSpec.describe PostStatusService do ) end.to raise_error( Mastodon::ValidationError, - 'Cannot attach more than 4 files', + I18n.t('media_attachments.validations.too_many'), ) end @@ -160,7 +160,7 @@ RSpec.describe PostStatusService do ) end.to raise_error( Mastodon::ValidationError, - 'Cannot attach a video to a toot that already contains images', + I18n.t('media_attachments.validations.images_and_video'), ) end