From 38bec798117446603b304c0d14ca39bae5ee8be8 Mon Sep 17 00:00:00 2001
From: Chad Pytel <chad@thoughtbot.com>
Date: Fri, 7 Apr 2017 12:50:43 -0400
Subject: 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.
---
 spec/fabricators/media_attachment_fabricator.rb |  2 +-
 spec/services/post_status_service_spec.rb       | 41 +++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 1 deletion(-)

(limited to 'spec')

diff --git a/spec/fabricators/media_attachment_fabricator.rb b/spec/fabricators/media_attachment_fabricator.rb
index 59db2440d..dc91d708f 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 9ee4daf6f..acb922d64 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
-- 
cgit 


From d4c94fa004117fdb7226b1b846a12d12dc0542d9 Mon Sep 17 00:00:00 2001
From: Joël Quenneville <joelq@thoughtbot.com>
Date: Fri, 7 Apr 2017 14:18:30 -0400
Subject: DRY up reblog vs original status check

Checking reblog vs original status was happening in multiple places
across the app. For views, this logic was encapsulated in a helper
method named `proper_status` but in the other layers of the app, the
logic was duplicated.

Because the logic is used at all layers of the app, we extracted it into
a `Status#proper` method on the model and changed all uses of the logic
to use this method. There is now a single source of truth for this
condition.

We added test coverage to untested methods that got refactored.
---
 app/helpers/stream_entries_helper.rb       |  4 --
 app/lib/atom_serializer.rb                 |  2 +-
 app/models/account.rb                      |  4 +-
 app/models/status.rb                       |  6 ++-
 app/views/stream_entries/_status.html.haml |  2 +-
 spec/models/account_spec.rb                | 68 +++++++++++++++++++++++++++++-
 spec/models/status_spec.rb                 | 11 +++++
 7 files changed, 86 insertions(+), 11 deletions(-)

(limited to 'spec')

diff --git a/app/helpers/stream_entries_helper.rb b/app/helpers/stream_entries_helper.rb
index a26e912a3..38e63ed8d 100644
--- a/app/helpers/stream_entries_helper.rb
+++ b/app/helpers/stream_entries_helper.rb
@@ -34,10 +34,6 @@ module StreamEntriesHelper
     user_signed_in? && @favourited.key?(status.id) ? 'favourited' : ''
   end
 
-  def proper_status(status)
-    status.reblog? ? status.reblog : status
-  end
-
   def rtl?(text)
     return false if text.empty?
 
diff --git a/app/lib/atom_serializer.rb b/app/lib/atom_serializer.rb
index be1cced8b..b9dcee6b3 100644
--- a/app/lib/atom_serializer.rb
+++ b/app/lib/atom_serializer.rb
@@ -328,7 +328,7 @@ class AtomSerializer
 
   def serialize_status_attributes(entry, status)
     append_element(entry, 'summary', status.spoiler_text) unless status.spoiler_text.blank?
-    append_element(entry, 'content', Formatter.instance.format(status.reblog? ? status.reblog : status).to_str, type: 'html')
+    append_element(entry, 'content', Formatter.instance.format(status.proper).to_str, type: 'html')
 
     status.mentions.each do |mentioned|
       append_element(entry, 'link', nil, rel: :mentioned, 'ostatus:object-type': TagManager::TYPES[:person], href: TagManager.instance.uri_for(mentioned.account))
diff --git a/app/models/account.rb b/app/models/account.rb
index 6968607a2..cbba8b5b6 100644
--- a/app/models/account.rb
+++ b/app/models/account.rb
@@ -125,11 +125,11 @@ class Account < ApplicationRecord
   end
 
   def favourited?(status)
-    (status.reblog? ? status.reblog : status).favourites.where(account: self).count.positive?
+    status.proper.favourites.where(account: self).count.positive?
   end
 
   def reblogged?(status)
-    (status.reblog? ? status.reblog : status).reblogs.where(account: self).count.positive?
+    status.proper.reblogs.where(account: self).count.positive?
   end
 
   def keypair
diff --git a/app/models/status.rb b/app/models/status.rb
index 6948ad77c..7e3dd3e28 100644
--- a/app/models/status.rb
+++ b/app/models/status.rb
@@ -62,8 +62,12 @@ class Status < ApplicationRecord
     reply? ? :comment : :note
   end
 
+  def proper
+    reblog? ? reblog : self
+  end
+
   def content
-    reblog? ? reblog.text : text
+    proper.text
   end
 
   def target
diff --git a/app/views/stream_entries/_status.html.haml b/app/views/stream_entries/_status.html.haml
index cdd0dde3b..434c5c8da 100644
--- a/app/views/stream_entries/_status.html.haml
+++ b/app/views/stream_entries/_status.html.haml
@@ -16,7 +16,7 @@
           %strong= display_name(status.account)
         = t('stream_entries.reblogged')
 
-  = render partial: centered ? 'stream_entries/detailed_status' : 'stream_entries/simple_status', locals: { status: proper_status(status) }
+  = render partial: centered ? 'stream_entries/detailed_status' : 'stream_entries/simple_status', locals: { status: status.proper }
 
 - if include_threads
   = render partial: 'stream_entries/status', collection: @descendants, as: :status, locals: { is_successor: true }
diff --git a/spec/models/account_spec.rb b/spec/models/account_spec.rb
index d7f59adb8..93a45459d 100644
--- a/spec/models/account_spec.rb
+++ b/spec/models/account_spec.rb
@@ -99,11 +99,75 @@ RSpec.describe Account, type: :model do
   end
 
   describe '#favourited?' do
-    pending
+    let(:original_status) do
+      author = Fabricate(:account, username: 'original')
+      Fabricate(:status, account: author)
+    end
+
+    context 'when the status is a reblog of another status' do
+      let(:original_reblog) do
+        author = Fabricate(:account, username: 'original_reblogger')
+        Fabricate(:status, reblog: original_status, account: author)
+      end
+
+      it 'is is true when this account has favourited it' do
+        Fabricate(:favourite, status: original_reblog, account: subject)
+
+        expect(subject.favourited?(original_status)).to eq true
+      end
+
+      it 'is false when this account has not favourited it' do
+        expect(subject.favourited?(original_status)).to eq false
+      end
+    end
+
+    context 'when the status is an original status' do
+      it 'is is true when this account has favourited it' do
+        Fabricate(:favourite, status: original_status, account: subject)
+
+        expect(subject.favourited?(original_status)).to eq true
+      end
+
+      it 'is false when this account has not favourited it' do
+        expect(subject.favourited?(original_status)).to eq false
+      end
+    end
   end
 
   describe '#reblogged?' do
-    pending
+    let(:original_status) do
+      author = Fabricate(:account, username: 'original')
+      Fabricate(:status, account: author)
+    end
+
+    context 'when the status is a reblog of another status'do
+      let(:original_reblog) do
+        author = Fabricate(:account, username: 'original_reblogger')
+        Fabricate(:status, reblog: original_status, account: author)
+      end
+
+      it 'is true when this account has reblogged it' do
+        Fabricate(:status, reblog: original_reblog, account: subject)
+
+        expect(subject.reblogged?(original_reblog)).to eq true
+      end
+
+      it 'is false when this account has not reblogged it' do
+        expect(subject.reblogged?(original_reblog)).to eq false
+      end
+    end
+
+    context 'when the status is an original status' do
+      it 'is true when this account has reblogged it' do
+        Fabricate(:status, reblog: original_status, account: subject)
+
+        expect(subject.reblogged?(original_status)).to eq true
+      end
+
+      it 'is false when this account has not reblogged it' do
+        expect(subject.reblogged?(original_status)).to eq false
+      end
+    end
   end
 
   describe '.find_local' do
diff --git a/spec/models/status_spec.rb b/spec/models/status_spec.rb
index b9d079521..db244ebe7 100644
--- a/spec/models/status_spec.rb
+++ b/spec/models/status_spec.rb
@@ -97,4 +97,15 @@ RSpec.describe Status, type: :model do
   describe '#favourites_count' do
     pending
   end
+
+  describe '#proper' do
+    it 'is itself for original statuses' do
+      expect(subject.proper).to eq subject
+    end
+
+    it 'is the source status for reblogs' do
+      subject.reblog = other
+      expect(subject.proper).to eq other
+    end
+  end
 end
-- 
cgit 


From 13c0077003288416b8cacd5d339f8796bc347f39 Mon Sep 17 00:00:00 2001
From: Chad Pytel <chad@thoughtbot.com>
Date: Fri, 7 Apr 2017 13:48:38 -0400
Subject: 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 | 165 +++++++++++++++++++++++++-----
 2 files changed, 143 insertions(+), 23 deletions(-)

(limited to 'spec')

diff --git a/spec/fabricators/status_fabricator.rb b/spec/fabricators/status_fabricator.rb
index df222fc9d..8ec5f4ba7 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 acb922d64..3fe878f63 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"
 
-  it 'does not allow attaching more than 4 files' do
+    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"
 
-    expect do
-      PostStatusService.new.call(
+    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: [
-        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),
+      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
+      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
-- 
cgit 


From ad5ddd5e95062d0d5cd4bc56baff53698c598723 Mon Sep 17 00:00:00 2001
From: Chad Pytel <chad@thoughtbot.com>
Date: Fri, 7 Apr 2017 14:19:16 -0400
Subject: 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(-)

(limited to 'spec')

diff --git a/app/services/post_status_service.rb b/app/services/post_status_service.rb
index b8179f7dc..221aa42a3 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 742219df9..aa3a732f9 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: "&hellip;"
+  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 3fe878f63..0e39cd969 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
 
-- 
cgit 


From 4fdeac21f4520c5fffb06b1143bf4b1ac88d8533 Mon Sep 17 00:00:00 2001
From: Joël Quenneville <joelq@thoughtbot.com>
Date: Fri, 7 Apr 2017 15:30:36 -0400
Subject: Implement pending specs on Status

Implement the two pending specs on `Status`: `reblogs_count` and
`favourites_count`.
---
 spec/models/status_spec.rb | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

(limited to 'spec')

diff --git a/spec/models/status_spec.rb b/spec/models/status_spec.rb
index b9d079521..675e18418 100644
--- a/spec/models/status_spec.rb
+++ b/spec/models/status_spec.rb
@@ -91,10 +91,20 @@ RSpec.describe Status, type: :model do
   end
 
   describe '#reblogs_count' do
-    pending
+    it 'is the number of reblogs' do
+      Fabricate(:status, account: bob, reblog: subject)
+      Fabricate(:status, account: alice, reblog: subject)
+
+      expect(subject.reblogs_count).to eq 2
+    end
   end
 
   describe '#favourites_count' do
-    pending
+    it 'is the number of favorites' do
+      Fabricate(:favourite, account: bob, status: subject)
+      Fabricate(:favourite, account: alice, status: subject)
+
+      expect(subject.favourites_count).to eq 2
+    end
   end
 end
-- 
cgit