From af578e8ce0aabdbe9c0cd3d72d6fa2cc30b7fc66 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Thu, 2 Mar 2023 10:21:04 -0500 Subject: [PATCH] Fix deprecation warning about merging conditions (#23618) --- app/models/account_filter.rb | 24 +++++++++++++++++++++--- spec/models/account_filter_spec.rb | 26 ++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 3 deletions(-) diff --git a/app/models/account_filter.rb b/app/models/account_filter.rb index d27bb46fc..1666ea883 100644 --- a/app/models/account_filter.rb +++ b/app/models/account_filter.rb @@ -17,13 +17,13 @@ class AccountFilter attr_reader :params def initialize(params) - @params = params + @params = params.to_h.symbolize_keys end def results scope = Account.includes(:account_stat, user: [:ips, :invite_request]).without_instance_actor.reorder(nil) - params.each do |key, value| + relevant_params.each do |key, value| next if key.to_s == 'page' scope.merge!(scope_for(key, value)) if value.present? @@ -34,6 +34,16 @@ class AccountFilter private + def relevant_params + params.tap do |args| + args.delete(:origin) if origin_is_remote_and_domain_present? + end + end + + def origin_is_remote_and_domain_present? + params[:origin] == 'remote' && params[:by_domain].present? + end + def scope_for(key, value) case key.to_s when 'origin' @@ -94,7 +104,15 @@ class AccountFilter def order_scope(value) case value.to_s when 'active' - accounts_with_users.left_joins(:account_stat).order(Arel.sql('coalesce(users.current_sign_in_at, account_stats.last_status_at, to_timestamp(0)) desc, accounts.id desc')) + accounts_with_users + .left_joins(:account_stat) + .order( + Arel.sql( + <<~SQL.squish + COALESCE(users.current_sign_in_at, account_stats.last_status_at, to_timestamp(0)) DESC, accounts.id DESC + SQL + ) + ) when 'recent' Account.recent else diff --git a/spec/models/account_filter_spec.rb b/spec/models/account_filter_spec.rb index 853d20a0c..3032260fe 100644 --- a/spec/models/account_filter_spec.rb +++ b/spec/models/account_filter_spec.rb @@ -18,4 +18,30 @@ describe AccountFilter do expect { filter.results }.to raise_error(/wrong/) end end + + describe 'with origin and by_domain interacting' do + let!(:local_account) { Fabricate(:account, domain: nil) } + let!(:remote_account_one) { Fabricate(:account, domain: 'example.org') } + let(:remote_account_two) { Fabricate(:account, domain: 'other.domain') } + + it 'works with domain first and origin remote' do + filter = described_class.new(by_domain: 'example.org', origin: 'remote') + expect(filter.results).to match_array [remote_account_one] + end + + it 'works with domain last and origin remote' do + filter = described_class.new(origin: 'remote', by_domain: 'example.org') + expect(filter.results).to match_array [remote_account_one] + end + + it 'works with domain first and origin local' do + filter = described_class.new(by_domain: 'example.org', origin: 'local') + expect(filter.results).to match_array [local_account] + end + + it 'works with domain last and origin local' do + filter = described_class.new(origin: 'local', by_domain: 'example.org') + expect(filter.results).to match_array [remote_account_one] + end + end end