From 465ee7792ff48905088efdf3df6f718081b5e244 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Thu, 7 Apr 2022 18:06:15 +0200 Subject: Fix pagination header on empty trends responses in REST API (#17986) --- app/models/trends/query.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'app/models/trends') diff --git a/app/models/trends/query.rb b/app/models/trends/query.rb index 231b65228..f19df162f 100644 --- a/app/models/trends/query.rb +++ b/app/models/trends/query.rb @@ -59,7 +59,7 @@ class Trends::Query @records end - delegate :each, :empty?, :first, :last, to: :records + delegate :each, :empty?, :first, :last, :size, to: :records def to_ary records.dup -- cgit From fd9a9b07c2cd19ef08d15e138fd3fc59fc5318b6 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Fri, 8 Apr 2022 17:10:53 +0200 Subject: Fix trends returning less results per page when filtered in REST API (#17996) - Change filtering and pagination to occur in SQL instead of Redis - Change rank/score displayed on trends in admin UI to be locale-specific --- app/models/trends/base.rb | 8 ++++---- app/models/trends/query.rb | 11 ++++++---- app/models/trends/statuses.rb | 24 +++++----------------- .../admin/trends/links/_preview_card.html.haml | 4 ++-- app/views/admin/trends/statuses/_status.html.haml | 4 ++-- 5 files changed, 20 insertions(+), 31 deletions(-) (limited to 'app/models/trends') diff --git a/app/models/trends/base.rb b/app/models/trends/base.rb index 7ed13228d..38a49246b 100644 --- a/app/models/trends/base.rb +++ b/app/models/trends/base.rb @@ -37,12 +37,12 @@ class Trends::Base Trends::Query.new(key_prefix, klass) end - def score(id) - redis.zscore("#{key_prefix}:all", id) || 0 + def score(id, locale: nil) + redis.zscore([key_prefix, 'all', locale].compact.join(':'), id) || 0 end - def rank(id) - redis.zrevrank("#{key_prefix}:allowed", id) + def rank(id, locale: nil) + redis.zrevrank([key_prefix, 'allowed', locale].compact.join(':'), id) end def currently_trending_ids(allowed, limit) diff --git a/app/models/trends/query.rb b/app/models/trends/query.rb index f19df162f..cd5571bc6 100644 --- a/app/models/trends/query.rb +++ b/app/models/trends/query.rb @@ -14,8 +14,8 @@ class Trends::Query @records = [] @loaded = false @allowed = false - @limit = -1 - @offset = 0 + @limit = nil + @offset = nil end def allowed! @@ -73,7 +73,10 @@ class Trends::Query if tmp_ids.empty? klass.none else - klass.joins("join unnest(array[#{tmp_ids.join(',')}]) with ordinality as x (id, ordering) on #{klass.table_name}.id = x.id").reorder('x.ordering') + scope = klass.joins("join unnest(array[#{tmp_ids.join(',')}]) with ordinality as x (id, ordering) on #{klass.table_name}.id = x.id").reorder('x.ordering') + scope = scope.offset(@offset) if @offset.present? + scope = scope.limit(@limit) if @limit.present? + scope end end @@ -93,7 +96,7 @@ class Trends::Query end def ids - redis.zrevrange(key, @offset, @limit.positive? ? @limit - 1 : @limit).map(&:to_i) + redis.zrevrange(key, 0, -1).map(&:to_i) end def perform_queries diff --git a/app/models/trends/statuses.rb b/app/models/trends/statuses.rb index e785413ec..dc5309554 100644 --- a/app/models/trends/statuses.rb +++ b/app/models/trends/statuses.rb @@ -22,25 +22,11 @@ class Trends::Statuses < Trends::Base private def apply_scopes(scope) - scope.includes(:account) - end - - def perform_queries - return super if @account.nil? - - statuses = super - account_ids = statuses.map(&:account_id) - account_domains = statuses.map(&:account_domain) - - preloaded_relations = { - blocking: Account.blocking_map(account_ids, @account.id), - blocked_by: Account.blocked_by_map(account_ids, @account.id), - muting: Account.muting_map(account_ids, @account.id), - following: Account.following_map(account_ids, @account.id), - domain_blocking_by_domain: Account.domain_blocking_map_by_domain(account_domains, @account.id), - } - - statuses.reject { |status| StatusFilter.new(status, @account, preloaded_relations).filtered? } + if @account.nil? + scope + else + scope.not_excluded_by_account(@account).not_domain_blocked_by_account(@account) + end end end diff --git a/app/views/admin/trends/links/_preview_card.html.haml b/app/views/admin/trends/links/_preview_card.html.haml index 2e6a0c62f..7d4897c7e 100644 --- a/app/views/admin/trends/links/_preview_card.html.haml +++ b/app/views/admin/trends/links/_preview_card.html.haml @@ -18,9 +18,9 @@ = t('admin.trends.links.shared_by_over_week', count: preview_card.history.reduce(0) { |sum, day| sum + day.accounts }) - - if preview_card.trendable? && (rank = Trends.links.rank(preview_card.id)) + - if preview_card.trendable? && (rank = Trends.links.rank(preview_card.id, locale: params[:locale].presence)) • - %abbr{ title: t('admin.trends.tags.current_score', score: Trends.links.score(preview_card.id)) }= t('admin.trends.tags.trending_rank', rank: rank + 1) + %abbr{ title: t('admin.trends.tags.current_score', score: Trends.links.score(preview_card.id, locale: params[:locale].presence)) }= t('admin.trends.tags.trending_rank', rank: rank + 1) - if preview_card.decaying? • diff --git a/app/views/admin/trends/statuses/_status.html.haml b/app/views/admin/trends/statuses/_status.html.haml index 0c463c6b1..e4d75bbb9 100644 --- a/app/views/admin/trends/statuses/_status.html.haml +++ b/app/views/admin/trends/statuses/_status.html.haml @@ -25,9 +25,9 @@ - if status.trendable? && !status.account.discoverable? • = t('admin.trends.statuses.not_discoverable') - - if status.trendable? && (rank = Trends.statuses.rank(status.id)) + - if status.trendable? && (rank = Trends.statuses.rank(status.id, locale: params[:locale].presence)) • - %abbr{ title: t('admin.trends.tags.current_score', score: Trends.statuses.score(status.id)) }= t('admin.trends.tags.trending_rank', rank: rank + 1) + %abbr{ title: t('admin.trends.tags.current_score', score: Trends.statuses.score(status.id, locale: params[:locale].presence)) }= t('admin.trends.tags.trending_rank', rank: rank + 1) - elsif status.requires_review? • = t('admin.trends.pending_review') -- cgit From 68273a7c6d6c630b6c88764579580682e12eebce Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Fri, 8 Apr 2022 19:35:31 +0200 Subject: Fix dangling language-specific trends (#17997) - Change score half-life for trending statuses from 2 to 6 hours - Change score threshold for trimming old items from 1 to 0.3 --- app/models/trends/base.rb | 4 ++-- app/models/trends/links.rb | 5 +++-- app/models/trends/statuses.rb | 7 ++++--- 3 files changed, 9 insertions(+), 7 deletions(-) (limited to 'app/models/trends') diff --git a/app/models/trends/base.rb b/app/models/trends/base.rb index 38a49246b..200f8d635 100644 --- a/app/models/trends/base.rb +++ b/app/models/trends/base.rb @@ -65,8 +65,8 @@ class Trends::Base end def trim_older_items - redis.zremrangebyscore("#{key_prefix}:all", '-inf', '(1') - redis.zremrangebyscore("#{key_prefix}:allowed", '-inf', '(1') + redis.zremrangebyscore("#{key_prefix}:all", '-inf', '(0.3') + redis.zremrangebyscore("#{key_prefix}:allowed", '-inf', '(0.3') end def score_at_rank(rank) diff --git a/app/models/trends/links.rb b/app/models/trends/links.rb index 62308e706..5f046643a 100644 --- a/app/models/trends/links.rb +++ b/app/models/trends/links.rb @@ -30,7 +30,6 @@ class Trends::Links < Trends::Base def refresh(at_time = Time.now.utc) preview_cards = PreviewCard.where(id: (recently_used_ids(at_time) + currently_trending_ids(false, -1)).uniq) calculate_scores(preview_cards, at_time) - trim_older_items end def request_review @@ -101,6 +100,8 @@ class Trends::Links < Trends::Base }) end + trim_older_items + # Clean up localized sets by calculating the intersection with the main # set. We do this instead of just deleting the localized sets to avoid # having moments where the API returns empty results @@ -108,7 +109,7 @@ class Trends::Links < Trends::Base redis.pipelined do Trends.available_locales.each do |locale| redis.zinterstore("#{key_prefix}:all:#{locale}", ["#{key_prefix}:all:#{locale}", "#{key_prefix}:all"], aggregate: 'max') - redis.zinterstore("#{key_prefix}:allowed:#{locale}", ["#{key_prefix}:allowed:#{locale}", "#{key_prefix}:all"], aggregate: 'max') + redis.zinterstore("#{key_prefix}:allowed:#{locale}", ["#{key_prefix}:allowed:#{locale}", "#{key_prefix}:allowed"], aggregate: 'max') end end end diff --git a/app/models/trends/statuses.rb b/app/models/trends/statuses.rb index dc5309554..d9b07446e 100644 --- a/app/models/trends/statuses.rb +++ b/app/models/trends/statuses.rb @@ -6,7 +6,7 @@ class Trends::Statuses < Trends::Base self.default_options = { threshold: 5, review_threshold: 3, - score_halflife: 2.hours.freeze, + score_halflife: 6.hours.freeze, } class Query < Trends::Query @@ -48,7 +48,6 @@ class Trends::Statuses < Trends::Base def refresh(at_time = Time.now.utc) statuses = Status.where(id: (recently_used_ids(at_time) + currently_trending_ids(false, -1)).uniq).includes(:account, :media_attachments) calculate_scores(statuses, at_time) - trim_older_items end def request_review @@ -111,13 +110,15 @@ class Trends::Statuses < Trends::Base }) end + trim_older_items + # Clean up localized sets by calculating the intersection with the main # set. We do this instead of just deleting the localized sets to avoid # having moments where the API returns empty results Trends.available_locales.each do |locale| redis.zinterstore("#{key_prefix}:all:#{locale}", ["#{key_prefix}:all:#{locale}", "#{key_prefix}:all"], aggregate: 'max') - redis.zinterstore("#{key_prefix}:allowed:#{locale}", ["#{key_prefix}:allowed:#{locale}", "#{key_prefix}:all"], aggregate: 'max') + redis.zinterstore("#{key_prefix}:allowed:#{locale}", ["#{key_prefix}:allowed:#{locale}", "#{key_prefix}:allowed"], aggregate: 'max') end end end -- cgit