Adjust VariantQuery to dynamically account for recent last_comment_at (#20323)

* Adjust VariantQuery to dynamically account for recent last_comment_at

* Adjust test line which was temporarily adjusted
This commit is contained in:
Ben Halpern 2023-11-07 08:22:22 -05:00 committed by GitHub
parent 472588379d
commit 64fd4e7800
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 35 additions and 8 deletions

View file

@ -58,7 +58,11 @@ module Articles
user: @user,
days_since_published: config.max_days_since_published,
)
@query_parameters = { oldest_published_at: oldest_published_at }
@query_parameters = {
oldest_published_at: oldest_published_at,
conditional_lookback: oldest_published_at - 12.hours,
conditional_comment_timeframe: 6.hours.ago
}
configure!
end
@ -261,14 +265,13 @@ module Articles
end
def build_sql_with_where_clauses(only_featured:, must_have_main_image:, omit_article_ids:)
where_clauses = "articles.published = true AND articles.published_at > :oldest_published_at"
# See Articles.published scope discussion regarding the query planner
# Hardcode the values for lookback_hours and comment_hours.
where_clauses = "articles.published = true"
where_clauses += " AND articles.published_at < :now"
where_clauses += " AND articles.score >= 0" # We only want positive values here.
# Without the compact, if we have `omit_article_ids: [nil]` we
# have the following SQL clause: `articles.id NOT IN (NULL)`
# which will immediately omit EVERYTHING from the query.
where_clauses += " AND articles.score >= 0"
where_clauses += " AND ((articles.published_at > :oldest_published_at)
OR (articles.published_at > :conditional_lookback
AND articles.last_comment_at > :conditional_comment_timeframe))"
where_clauses += " AND articles.id NOT IN (:omit_article_ids)" unless omit_article_ids.compact.empty?
where_clauses += " AND articles.featured = true" if only_featured
where_clauses += " AND articles.main_image IS NOT NULL" if must_have_main_image

View file

@ -34,6 +34,30 @@ RSpec.describe Articles::Feeds::VariantQuery, type: :service do
expect(query_call).to be_a(ActiveRecord::Relation)
expect(query_call.to_a).not_to match_array(article)
end
it "does not return an article published 27 hours before last page view if last comment at is too old" do
create_list(:page_view, 5, user: user) # Recent pageviews
article = create(:article)
article.update_columns(published_at: 27.hours.ago, last_comment_at: 22.hours.ago)
expect(query_call).to be_a(ActiveRecord::Relation)
expect(query_call.to_a).not_to include(article)
end
it "does return article published 27 hours before last page view if last comment is within 6 hours" do
create_list(:page_view, 5, user: user) # Recent pageviews
article = create(:article)
article.update_columns(published_at: 27.hours.ago, last_comment_at: 4.hours.ago)
expect(query_call).to be_a(ActiveRecord::Relation)
expect(query_call.to_a).to include(article)
end
it "does not return article published 38 hours before last page view if last comment is within 6 hours" do
create_list(:page_view, 5, user: user) # Recent pageviews
article = create(:article)
article.update_columns(published_at: 38.hours.ago, last_comment_at: 4.hours.ago)
expect(query_call).to be_a(ActiveRecord::Relation)
expect(query_call.to_a).not_to include(article)
end
end
describe "#featured_story_and_default_home_feed" do