Misc spec coverage improvements (#2821)

* Dont use raise_error by itself (avoids warning)

* Add coverage for AccountFilter

* Improve coverage and refactor for Subscription#lease_seconds

* Improve coverage and refactor for NotificationMailer

* Simplify assignment of min/max threshold on subscription
This commit is contained in:
Matt Jankowski 2017-05-05 14:56:00 -04:00 committed by GitHub
parent d08f1112d5
commit 484c9709b6
7 changed files with 128 additions and 15 deletions

View file

@ -7,7 +7,7 @@ class NotificationMailer < ApplicationMailer
@me = recipient @me = recipient
@status = notification.target_status @status = notification.target_status
I18n.with_locale(@me.user.locale || I18n.default_locale) do locale_for_account(@me) do
mail to: @me.user.email, subject: I18n.t('notification_mailer.mention.subject', name: @status.account.acct) mail to: @me.user.email, subject: I18n.t('notification_mailer.mention.subject', name: @status.account.acct)
end end
end end
@ -16,7 +16,7 @@ class NotificationMailer < ApplicationMailer
@me = recipient @me = recipient
@account = notification.from_account @account = notification.from_account
I18n.with_locale(@me.user.locale || I18n.default_locale) do locale_for_account(@me) do
mail to: @me.user.email, subject: I18n.t('notification_mailer.follow.subject', name: @account.acct) mail to: @me.user.email, subject: I18n.t('notification_mailer.follow.subject', name: @account.acct)
end end
end end
@ -26,7 +26,7 @@ class NotificationMailer < ApplicationMailer
@account = notification.from_account @account = notification.from_account
@status = notification.target_status @status = notification.target_status
I18n.with_locale(@me.user.locale || I18n.default_locale) do locale_for_account(@me) do
mail to: @me.user.email, subject: I18n.t('notification_mailer.favourite.subject', name: @account.acct) mail to: @me.user.email, subject: I18n.t('notification_mailer.favourite.subject', name: @account.acct)
end end
end end
@ -36,7 +36,7 @@ class NotificationMailer < ApplicationMailer
@account = notification.from_account @account = notification.from_account
@status = notification.target_status @status = notification.target_status
I18n.with_locale(@me.user.locale || I18n.default_locale) do locale_for_account(@me) do
mail to: @me.user.email, subject: I18n.t('notification_mailer.reblog.subject', name: @account.acct) mail to: @me.user.email, subject: I18n.t('notification_mailer.reblog.subject', name: @account.acct)
end end
end end
@ -45,7 +45,7 @@ class NotificationMailer < ApplicationMailer
@me = recipient @me = recipient
@account = notification.from_account @account = notification.from_account
I18n.with_locale(@me.user.locale || I18n.default_locale) do locale_for_account(@me) do
mail to: @me.user.email, subject: I18n.t('notification_mailer.follow_request.subject', name: @account.acct) mail to: @me.user.email, subject: I18n.t('notification_mailer.follow_request.subject', name: @account.acct)
end end
end end
@ -58,7 +58,7 @@ class NotificationMailer < ApplicationMailer
return if @notifications.empty? return if @notifications.empty?
I18n.with_locale(@me.user.locale || I18n.default_locale) do locale_for_account(@me) do
mail to: @me.user.email, mail to: @me.user.email,
subject: I18n.t( subject: I18n.t(
:subject, :subject,
@ -67,4 +67,12 @@ class NotificationMailer < ApplicationMailer
) )
end end
end end
private
def locale_for_account(account)
I18n.with_locale(account.user.locale || I18n.default_locale) do
yield
end
end
end end

View file

@ -1,4 +1,5 @@
# frozen_string_literal: true # frozen_string_literal: true
# == Schema Information # == Schema Information
# #
# Table name: subscriptions # Table name: subscriptions
@ -15,18 +16,20 @@
# #
class Subscription < ApplicationRecord class Subscription < ApplicationRecord
MIN_EXPIRATION = 3600 * 24 * 7 MIN_EXPIRATION = 7.days.seconds.to_i
MAX_EXPIRATION = 3600 * 24 * 30 MAX_EXPIRATION = 30.days.seconds.to_i
belongs_to :account, required: true belongs_to :account, required: true
validates :callback_url, presence: true validates :callback_url, presence: true
validates :callback_url, uniqueness: { scope: :account_id } validates :callback_url, uniqueness: { scope: :account_id }
scope :active, -> { where(confirmed: true).where('expires_at > ?', Time.now.utc) } scope :confirmed, -> { where(confirmed: true) }
scope :future_expiration, -> { where(arel_table[:expires_at].gt(Time.now.utc)) }
scope :active, -> { confirmed.future_expiration }
def lease_seconds=(str) def lease_seconds=(value)
self.expires_at = Time.now.utc + [[MIN_EXPIRATION, str.to_i].max, MAX_EXPIRATION].min.seconds self.expires_at = future_expiration(value)
end end
def lease_seconds def lease_seconds
@ -41,6 +44,17 @@ class Subscription < ApplicationRecord
private private
def future_expiration(value)
Time.now.utc + future_offset(value).seconds
end
def future_offset(seconds)
[
[MIN_EXPIRATION, seconds.to_i].max,
MAX_EXPIRATION,
].min
end
def set_min_expiration def set_min_expiration
self.lease_seconds = 0 unless expires_at self.lease_seconds = 0 unless expires_at
end end

View file

@ -62,4 +62,34 @@ RSpec.describe NotificationMailer, type: :mailer do
end end
end end
describe 'follow_request' do
let(:follow_request) { Fabricate(:follow_request, account: sender, target_account: receiver.account) }
let(:mail) { NotificationMailer.follow_request(receiver.account, Notification.create!(account: receiver.account, activity: follow_request)) }
it 'renders the headers' do
expect(mail.subject).to eq('Pending follower: bob')
expect(mail.to).to eq([receiver.email])
end
it 'renders the body' do
expect(mail.body.encoded).to match("bob has requested to follow you")
end
end
describe 'digest' do
before do
mention = Fabricate(:mention, account: receiver.account)
Fabricate(:notification, account: receiver.account, activity: mention)
end
let(:mail) { NotificationMailer.digest(receiver.account, since: 5.days.ago) }
it 'renders the headers' do
expect(mail.subject).to match('notification since your last')
expect(mail.to).to eq([receiver.email])
end
it 'renders the body' do
expect(mail.body.encoded).to match('brief summary')
end
end
end end

View file

@ -3,7 +3,7 @@ require 'rails_helper'
describe AccountFilter do describe AccountFilter do
describe 'with empty params' do describe 'with empty params' do
it 'defaults to alphabetic account list' do it 'defaults to alphabetic account list' do
filter = AccountFilter.new({}) filter = described_class.new({})
expect(filter.results).to eq Account.alphabetic expect(filter.results).to eq Account.alphabetic
end end
@ -11,7 +11,7 @@ describe AccountFilter do
describe 'with invalid params' do describe 'with invalid params' do
it 'raises with key error' do it 'raises with key error' do
filter = AccountFilter.new(wrong: true) filter = described_class.new(wrong: true)
expect { filter.results }.to raise_error(/wrong/) expect { filter.results }.to raise_error(/wrong/)
end end
@ -19,7 +19,7 @@ describe AccountFilter do
describe 'with valid params' do describe 'with valid params' do
it 'combines filters on Account' do it 'combines filters on Account' do
filter = AccountFilter.new(by_domain: 'test.com', silenced: true) filter = described_class.new(by_domain: 'test.com', silenced: true)
allow(Account).to receive(:where).and_return(Account.none) allow(Account).to receive(:where).and_return(Account.none)
allow(Account).to receive(:silenced).and_return(Account.none) allow(Account).to receive(:silenced).and_return(Account.none)
@ -27,5 +27,17 @@ describe AccountFilter do
expect(Account).to have_received(:where).with(domain: 'test.com') expect(Account).to have_received(:where).with(domain: 'test.com')
expect(Account).to have_received(:silenced) expect(Account).to have_received(:silenced)
end end
describe 'that call account methods' do
%i(local remote silenced recent).each do |option|
it "delegates the #{option} option" do
allow(Account).to receive(option).and_return(Account.none)
filter = described_class.new({ option => true })
filter.results
expect(Account).to have_received(option)
end
end
end
end end
end end

View file

@ -16,4 +16,52 @@ RSpec.describe Subscription, type: :model do
expect(subject.expired?).to be false expect(subject.expired?).to be false
end end
end end
describe 'lease_seconds' do
it 'returns the time remaing until expiration' do
datetime = 1.day.from_now
subscription = Subscription.new(expires_at: datetime)
travel_to(datetime - 12.hours) do
expect(subscription.lease_seconds).to eq(12.hours)
end
end
end
describe 'lease_seconds=' do
it 'sets expires_at to min expiration when small value is provided' do
subscription = Subscription.new
datetime = 1.day.from_now
too_low = Subscription::MIN_EXPIRATION - 1000
travel_to(datetime) do
subscription.lease_seconds = too_low
end
expected = datetime + Subscription::MIN_EXPIRATION.seconds
expect(subscription.expires_at).to be_within(1.0).of(expected)
end
it 'sets expires_at to value when valid value is provided' do
subscription = Subscription.new
datetime = 1.day.from_now
valid = Subscription::MIN_EXPIRATION + 1000
travel_to(datetime) do
subscription.lease_seconds = valid
end
expected = datetime + valid.seconds
expect(subscription.expires_at).to be_within(1.0).of(expected)
end
it 'sets expires_at to max expiration when large value is provided' do
subscription = Subscription.new
datetime = 1.day.from_now
too_high = Subscription::MAX_EXPIRATION + 1000
travel_to(datetime) do
subscription.lease_seconds = too_high
end
expected = datetime + Subscription::MAX_EXPIRATION.seconds
expect(subscription.expires_at).to be_within(1.0).of(expected)
end
end
end end

View file

@ -25,6 +25,7 @@ RSpec.configure do |config|
config.include Devise::Test::ControllerHelpers, type: :controller config.include Devise::Test::ControllerHelpers, type: :controller
config.include Devise::Test::ControllerHelpers, type: :view config.include Devise::Test::ControllerHelpers, type: :view
config.include Paperclip::Shoulda::Matchers config.include Paperclip::Shoulda::Matchers
config.include ActiveSupport::Testing::TimeHelpers
config.before :each, type: :feature do config.before :each, type: :feature do
https = ENV['LOCAL_HTTPS'] == 'true' https = ENV['LOCAL_HTTPS'] == 'true'

View file

@ -33,6 +33,6 @@ RSpec.describe SubscribeService do
it 'fails loudly if PuSH hub is unavailable' do it 'fails loudly if PuSH hub is unavailable' do
stub_request(:post, 'http://hub.example.com/').to_return(status: 503) stub_request(:post, 'http://hub.example.com/').to_return(status: 503)
expect { subject.call(account) }.to raise_error expect { subject.call(account) }.to raise_error(/Subscription attempt failed/)
end end
end end