From 4f6edc75963f4ea4e65a3512f0626b37e83b7dd7 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Fri, 10 Jan 2025 04:33:07 -0500 Subject: [PATCH] Use `in_order_of` in `trends/*` classes (#33531) --- app/models/trends/links.rb | 4 ++-- app/models/trends/query.rb | 7 +++++++ app/models/trends/statuses.rb | 4 ++-- app/models/trends/tags.rb | 5 +++-- spec/models/trends/links_spec.rb | 30 +++++++++++++++++++++++++++++ spec/models/trends/statuses_spec.rb | 25 ++++++++++++++++++++++++ spec/models/trends/tags_spec.rb | 25 ++++++++++++++++++++++++ 7 files changed, 94 insertions(+), 6 deletions(-) create mode 100644 spec/models/trends/links_spec.rb diff --git a/app/models/trends/links.rb b/app/models/trends/links.rb index 0f3ead43f88..57ad486631c 100644 --- a/app/models/trends/links.rb +++ b/app/models/trends/links.rb @@ -16,7 +16,7 @@ class Trends::Links < Trends::Base class Query < Trends::Query def to_arel scope = PreviewCard.joins(:trend).reorder(score: :desc) - scope = scope.reorder(language_order_clause.desc, score: :desc) if preferred_languages.present? + scope = scope.merge(language_order_clause) if preferred_languages.present? scope = scope.merge(PreviewCardTrend.allowed) if @allowed scope = scope.offset(@offset) if @offset.present? scope = scope.limit(@limit) if @limit.present? @@ -26,7 +26,7 @@ class Trends::Links < Trends::Base private def language_order_clause - Arel::Nodes::Case.new.when(PreviewCardTrend.arel_table[:language].in(preferred_languages)).then(1).else(0) + language_order_for(PreviewCardTrend) end end diff --git a/app/models/trends/query.rb b/app/models/trends/query.rb index 590e81f4fda..abed64042ec 100644 --- a/app/models/trends/query.rb +++ b/app/models/trends/query.rb @@ -94,6 +94,13 @@ class Trends::Query to_arel.to_a end + def language_order_for(trend_class) + trend_class + .reorder(nil) + .in_order_of(:language, [preferred_languages], filter: false) + .order(score: :desc) + end + def preferred_languages if @account&.chosen_languages.present? @account.chosen_languages diff --git a/app/models/trends/statuses.rb b/app/models/trends/statuses.rb index 1d2f02809b8..9c47dd486b9 100644 --- a/app/models/trends/statuses.rb +++ b/app/models/trends/statuses.rb @@ -15,7 +15,7 @@ class Trends::Statuses < Trends::Base class Query < Trends::Query def to_arel scope = Status.joins(:trend).reorder(score: :desc) - scope = scope.reorder(language_order_clause.desc, score: :desc) if preferred_languages.present? + scope = scope.merge(language_order_clause) if preferred_languages.present? scope = scope.merge(StatusTrend.allowed) if @allowed scope = scope.not_excluded_by_account(@account).not_domain_blocked_by_account(@account) if @account.present? scope = scope.offset(@offset) if @offset.present? @@ -26,7 +26,7 @@ class Trends::Statuses < Trends::Base private def language_order_clause - Arel::Nodes::Case.new.when(StatusTrend.arel_table[:language].in(preferred_languages)).then(1).else(0) + language_order_for(StatusTrend) end end diff --git a/app/models/trends/tags.rb b/app/models/trends/tags.rb index 18f2a9a9492..84e8dde11a5 100644 --- a/app/models/trends/tags.rb +++ b/app/models/trends/tags.rb @@ -15,7 +15,8 @@ class Trends::Tags < Trends::Base class Query < Trends::Query def to_arel - scope = Tag.joins(:trend).reorder(language_order_clause.desc, score: :desc) + scope = Tag.joins(:trend).reorder(score: :desc) + scope = scope.merge(language_order_clause) if preferred_languages.present? scope = scope.merge(TagTrend.allowed) if @allowed scope = scope.offset(@offset) if @offset.present? scope = scope.limit(@limit) if @limit.present? @@ -25,7 +26,7 @@ class Trends::Tags < Trends::Base private def language_order_clause - Arel::Nodes::Case.new.when(TagTrend.arel_table[:language].in(preferred_languages)).then(1).else(0) + language_order_for(TagTrend) end end diff --git a/spec/models/trends/links_spec.rb b/spec/models/trends/links_spec.rb new file mode 100644 index 00000000000..b0d41d4613a --- /dev/null +++ b/spec/models/trends/links_spec.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe Trends::Links do + describe 'Trends::Links::Query' do + subject { described_class.new.query } + + describe '#records' do + context 'with scored cards' do + let!(:higher_score) { Fabricate :preview_card_trend, score: 10, language: 'en' } + let!(:lower_score) { Fabricate :preview_card_trend, score: 1, language: 'es' } + + it 'returns higher score first' do + expect(subject.records) + .to eq([higher_score.preview_card, lower_score.preview_card]) + end + + context 'with preferred locale' do + before { subject.in_locale!('es') } + + it 'returns in language order' do + expect(subject.records) + .to eq([lower_score.preview_card, higher_score.preview_card]) + end + end + end + end + end +end diff --git a/spec/models/trends/statuses_spec.rb b/spec/models/trends/statuses_spec.rb index 7c30b5b9976..abb1535d04a 100644 --- a/spec/models/trends/statuses_spec.rb +++ b/spec/models/trends/statuses_spec.rb @@ -45,6 +45,31 @@ RSpec.describe Trends::Statuses do end end + describe 'Trends::Statuses::Query methods' do + subject { described_class.new.query } + + describe '#records' do + context 'with scored cards' do + let!(:higher_score) { Fabricate :status_trend, score: 10, language: 'en' } + let!(:lower_score) { Fabricate :status_trend, score: 1, language: 'es' } + + it 'returns higher score first' do + expect(subject.records) + .to eq([higher_score.status, lower_score.status]) + end + + context 'with preferred locale' do + before { subject.in_locale!('es') } + + it 'returns in language order' do + expect(subject.records) + .to eq([lower_score.status, higher_score.status]) + end + end + end + end + end + describe '#add' do let(:status) { Fabricate(:status) } diff --git a/spec/models/trends/tags_spec.rb b/spec/models/trends/tags_spec.rb index 936b441d92a..8f36b4a50d3 100644 --- a/spec/models/trends/tags_spec.rb +++ b/spec/models/trends/tags_spec.rb @@ -29,6 +29,31 @@ RSpec.describe Trends::Tags do end end + describe 'Trends::Tags::Query' do + subject { described_class.new.query } + + describe '#records' do + context 'with scored cards' do + let!(:higher_score) { Fabricate :tag_trend, score: 10, language: 'en' } + let!(:lower_score) { Fabricate :tag_trend, score: 1, language: 'es' } + + it 'returns higher score first' do + expect(subject.records) + .to eq([higher_score.tag, lower_score.tag]) + end + + context 'with preferred locale' do + before { subject.in_locale!('es') } + + it 'returns in language order' do + expect(subject.records) + .to eq([lower_score.tag, higher_score.tag]) + end + end + end + end + end + describe '#refresh' do let!(:today) { at_time } let!(:yesterday) { today - 1.day }