From e1b42e9aa01b0c6adab05afb9c5ee0cf9fbb41a9 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Mon, 22 May 2017 15:50:58 -0400 Subject: [PATCH] Add coverage for ReportFilter and AccountFilter (#3236) --- app/models/account.rb | 2 ++ app/models/account_filter.rb | 21 ++++++++++-------- app/models/user.rb | 2 ++ spec/models/account_filter_spec.rb | 35 ++++++++++++++++++++++++++++-- spec/models/report_filter_spec.rb | 3 ++- 5 files changed, 51 insertions(+), 12 deletions(-) diff --git a/app/models/account.rb b/app/models/account.rb index 7e4291d55..f5ac89257 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -89,6 +89,8 @@ class Account < ApplicationRecord scope :recent, -> { reorder(id: :desc) } scope :alphabetic, -> { order(domain: :asc, username: :asc) } scope :by_domain_accounts, -> { group(:domain).select(:domain, 'COUNT(*) AS accounts_count').order('accounts_count desc') } + scope :matches_username, ->(value) { where(arel_table[:username].matches("#{value}%")) } + scope :matches_display_name, ->(value) { where(arel_table[:display_name].matches("#{value}%")) } delegate :email, :current_sign_in_ip, diff --git a/app/models/account_filter.rb b/app/models/account_filter.rb index 186b38cd7..1a8cc5192 100644 --- a/app/models/account_filter.rb +++ b/app/models/account_filter.rb @@ -18,8 +18,6 @@ class AccountFilter private def scope_for(key, value) - accounts = Account.arel_table - case key.to_s when 'local' Account.local @@ -34,21 +32,26 @@ class AccountFilter when 'suspended' Account.suspended when 'username' - Account.where(accounts[:username].matches("#{value}%")) + Account.matches_username(value) when 'display_name' - Account.where(accounts[:display_name].matches("#{value}%")) + Account.matches_display_name(value) when 'email' - users = User.arel_table - Account.joins(:user).merge(User.where(users[:email].matches("#{value}%"))) + accounts_with_users.merge User.matches_email(value) when 'ip' - return Account.default_scoped unless valid_ip?(value) - matches_ip = User.where(current_sign_in_ip: value).or(User.where(last_sign_in_ip: value)) - Account.joins(:user).merge(matches_ip) + if valid_ip?(value) + accounts_with_users.merge User.with_recent_ip_address(value) + else + Account.default_scoped + end else raise "Unknown filter: #{key}" end end + def accounts_with_users + Account.joins(:user) + end + def valid_ip?(value) IPAddr.new(value) true diff --git a/app/models/user.rb b/app/models/user.rb index d0732ed59..9f2a49b6a 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -53,6 +53,8 @@ class User < ApplicationRecord scope :admins, -> { where(admin: true) } scope :confirmed, -> { where.not(confirmed_at: nil) } scope :inactive, -> { where(arel_table[:current_sign_in_at].lt(ACTIVE_DURATION.ago)) } + scope :matches_email, ->(value) { where(arel_table[:email].matches("#{value}%")) } + scope :with_recent_ip_address, ->(value) { where(arel_table[:current_sign_in_ip].eq(value).or(arel_table[:last_sign_in_ip].eq(value))) } before_validation :sanitize_languages diff --git a/spec/models/account_filter_spec.rb b/spec/models/account_filter_spec.rb index 52b4c4893..8441939c5 100644 --- a/spec/models/account_filter_spec.rb +++ b/spec/models/account_filter_spec.rb @@ -17,19 +17,50 @@ describe AccountFilter do end end + describe 'when an IP address is provided' do + it 'filters with IP when valid' do + filter = described_class.new(ip: '127.0.0.1') + allow(User).to receive(:with_recent_ip_address).and_return(User.none) + + filter.results + expect(User).to have_received(:with_recent_ip_address).with('127.0.0.1') + end + + it 'skips IP when invalid' do + filter = described_class.new(ip: '345.678.901.234') + expect(User).not_to receive(:with_recent_ip_address) + + filter.results + end + end + describe 'with valid params' do it 'combines filters on Account' do - filter = described_class.new(by_domain: 'test.com', silenced: true) + filter = described_class.new( + by_domain: 'test.com', + silenced: true, + username: 'test', + display_name: 'name', + email: 'user@example.com', + ) allow(Account).to receive(:where).and_return(Account.none) allow(Account).to receive(:silenced).and_return(Account.none) + allow(Account).to receive(:matches_display_name).and_return(Account.none) + allow(Account).to receive(:matches_username).and_return(Account.none) + allow(User).to receive(:matches_email).and_return(User.none) + filter.results + expect(Account).to have_received(:where).with(domain: 'test.com') expect(Account).to have_received(:silenced) + expect(Account).to have_received(:matches_username).with('test') + expect(Account).to have_received(:matches_display_name).with('name') + expect(User).to have_received(:matches_email).with('user@example.com') end describe 'that call account methods' do - %i(local remote silenced recent).each do |option| + %i(local remote silenced recent suspended).each do |option| it "delegates the #{option} option" do allow(Account).to receive(option).and_return(Account.none) filter = described_class.new({ option => true }) diff --git a/spec/models/report_filter_spec.rb b/spec/models/report_filter_spec.rb index fc0b7d7b5..099c0731d 100644 --- a/spec/models/report_filter_spec.rb +++ b/spec/models/report_filter_spec.rb @@ -19,12 +19,13 @@ describe ReportFilter do describe 'with valid params' do it 'combines filters on Report' do - filter = ReportFilter.new(account_id: '123', resolved: true) + filter = ReportFilter.new(account_id: '123', resolved: true, target_account_id: '456') allow(Report).to receive(:where).and_return(Report.none) allow(Report).to receive(:resolved).and_return(Report.none) filter.results expect(Report).to have_received(:where).with(account_id: '123') + expect(Report).to have_received(:where).with(target_account_id: '456') expect(Report).to have_received(:resolved) end end