diff --git a/app/controllers/additional_content_boxes_controller.rb b/app/controllers/additional_content_boxes_controller.rb index 2f69b905b..636c28ede 100644 --- a/app/controllers/additional_content_boxes_controller.rb +++ b/app/controllers/additional_content_boxes_controller.rb @@ -8,16 +8,18 @@ class AdditionalContentBoxesController < ApplicationController article_ids = params[:article_id].split(",") @article = Article.find(article_ids[0]) - @suggested_articles = Suggester::Articles::Classic. - new(@article, not_ids: article_ids).get(2) + + @suggested_articles = Suggester::Articles::Classic.new(@article, not_ids: article_ids).get(2) + if (!user_signed_in? || params[:state] == "include_sponsors") && @article.user.permit_adjacent_sponsors && randomize - @boosted_article = Suggester::Articles::Boosted.new( - (@article.decorate.cached_tag_list_array + @article.boosted_additional_tags.split).sample, - not_ids: (article_ids + @suggested_articles.pluck(:id)), area: "additional_articles", - ).suggest + + tag = (@article.decorate.cached_tag_list_array + @article.boosted_additional_tags.split).sample + not_ids = article_ids + @suggested_articles.pluck(:id) + @boosted_article = Suggester::Articles::Boosted.new(tag, not_ids: not_ids, area: "additional_articles").suggest end + set_surrogate_key_header "additional_content_boxes_" + params.to_s render "boxes", layout: false end diff --git a/app/controllers/dashboards_controller.rb b/app/controllers/dashboards_controller.rb index 7ef37d1f3..40d4fd966 100644 --- a/app/controllers/dashboards_controller.rb +++ b/app/controllers/dashboards_controller.rb @@ -16,12 +16,17 @@ class DashboardsController < ApplicationController if params[:which] == "organization" && params[:org_id] && (@user.org_admin?(params[:org_id]) || @user.any_admin?) target = @organizations.find_by(id: params[:org_id]) @organization = target + @articles = target.articles + else + # if the target is a user, we need to eager load the organization + @articles = target.articles.includes(:organization) end - @articles = target.articles.includes(:organization).sorting(params[:sort]).decorate + @articles = @articles.sorting(params[:sort]).decorate # Updates analytics in background if appropriate - Articles::UpdateAnalyticsWorker.perform_async(current_user.id) if @articles && SiteConfig.ga_fetch_rate < 50 # Rate limit concerned, sometimes we throttle down. + update_analytics = @articles && SiteConfig.ga_fetch_rate < 50 # Rate limited, sometimes we throttle down + Articles::UpdateAnalyticsWorker.perform_async(current_user.id) if update_analytics end def following_tags diff --git a/app/controllers/notifications_controller.rb b/app/controllers/notifications_controller.rb index 1ca1e4e5d..18768ca2e 100644 --- a/app/controllers/notifications_controller.rb +++ b/app/controllers/notifications_controller.rb @@ -26,12 +26,20 @@ class NotificationsController < ApplicationController @user.notifications end - @notifications = @notifications.includes(:notifiable).without_past_aggregations.order(notified_at: :desc) + @notifications = @notifications.without_past_aggregations.order(notified_at: :desc) # if offset based pagination is invoked by the frontend code, we filter out all earlier ones @notifications = @notifications.where("notified_at < ?", notified_at_offset) if notified_at_offset - @notifications = NotificationDecorator.decorate_collection(@notifications.limit(num)) + @notifications = @notifications.limit(num) + + # after notifications are paginated/limited, we check if any of the notifiables is "Comment" with no action + # because its respective view is the only one that actually uses ".notifiable" to render, + # this way we save useless eager loadings + notifiable_eager_loading_params = { notifiable_type: %w[Comment], action: [nil, ""] } + @notifications = @notifications.includes(:notifiable) if @notifications.exists?(notifiable_eager_loading_params) + + @notifications = NotificationDecorator.decorate_collection(@notifications) @last_user_reaction = @user.reactions.last&.id @last_user_comment = @user.comments.last&.id diff --git a/app/services/article_api_index_service.rb b/app/services/article_api_index_service.rb index a1a26ef1e..12d91f7e0 100644 --- a/app/services/article_api_index_service.rb +++ b/app/services/article_api_index_service.rb @@ -74,18 +74,29 @@ class ArticleApiIndexService end def top_articles - Article.published.order("positive_reactions_count DESC").where("published_at > ?", top.to_i.days.ago). + Article.published.includes(:user, :organization). + where("published_at > ?", top.to_i.days.ago). + order("positive_reactions_count DESC"). page(page).per(per_page || DEFAULT_PER_PAGE) end def state_articles(state) - if state == "fresh" - Article.published. - where("positive_reactions_count < ? AND featured_number > ? AND score > ?", 2, 7.hours.ago.to_i, -2) - elsif state == "rising" - Article.published. - where("positive_reactions_count > ? AND positive_reactions_count < ? AND featured_number > ?", 19, 33, 3.days.ago.to_i) - end + articles = Article.published.includes(:user, :organization) + + articles = if state == "fresh" + articles.where( + "positive_reactions_count < ? AND featured_number > ? AND score > ?", 2, 7.hours.ago.to_i, -2 + ) + elsif state == "rising" + articles.where( + "positive_reactions_count > ? AND positive_reactions_count < ? AND featured_number > ?", + 19, 33, 3.days.ago.to_i + ) + else + Article.none + end + + articles.page(page).per(per_page || DEFAULT_PER_PAGE) end def collection_articles(collection_id) diff --git a/app/views/additional_content_boxes/boxes.html.erb b/app/views/additional_content_boxes/boxes.html.erb index 472581ed5..ea8ad380c 100644 --- a/app/views/additional_content_boxes/boxes.html.erb +++ b/app/views/additional_content_boxes/boxes.html.erb @@ -5,6 +5,7 @@ classification_text: "From one of our Community Sponsors", follow_cue: @boosted_article.organization&.tag_line || @boosted_article.organization&.tag_line %> <% end %> + <% if @suggested_articles.any? %> <% @suggested_articles.each do |article| %> <%= render "additional_content_boxes/article_box", diff --git a/spec/requests/additional_content_boxes_spec.rb b/spec/requests/additional_content_boxes_spec.rb index 1ca66a2f8..ffa682920 100644 --- a/spec/requests/additional_content_boxes_spec.rb +++ b/spec/requests/additional_content_boxes_spec.rb @@ -7,29 +7,29 @@ RSpec.describe "AdditionalContentBoxes", type: :request do describe "GET /additional_content_boxes" do it "returns an article if there is a published/featured one" do - suggestion = build_stubbed(:article, published: true, featured: true, path: "blah") - suggestion.title = "Title of the article" - - fake = instance_double(Suggester::Articles::Classic) - allow(Suggester::Articles::Classic).to receive(:new).and_return(fake) - allow(fake).to receive(:get).and_return([suggestion]) + suggestion = create(:article, published: true, published_at: 12.months.ago, featured: true, path: "blah") + suggestion.update(body_markdown: "foobar", title: "Title of the article", positive_reactions_count: 100) get "/additional_content_boxes?article_id=#{regular_article.id}&state=include_sponsors" - expect(response.body).to include CGI.escapeHTML(suggestion.title) + expect(response.body).to include(CGI.escapeHTML(suggestion.title)) end it "returns no article if not published/featured" do suggestion = create(:article) + get "/additional_content_boxes?article_id=#{regular_article.id}&state=include_sponsors" - expect(response.body).not_to include CGI.escapeHTML(suggestion.title) + expect(response.body).not_to include(CGI.escapeHTML(suggestion.title)) end it "returns boosted article if available" do organization = create(:organization) create(:article, published: true, featured: true) - boosted_sugg = create(:article, tags: [tag.name], featured: true, boosted_additional_articles: true, organization_id: organization.id) + boosted_sugg = create( + :article, tags: [tag.name], featured: true, boosted_additional_articles: true, organization_id: organization.id + ) + get "/additional_content_boxes?article_id=#{regular_article.id}&state=include_sponsors" - expect(response.body).to include CGI.escapeHTML(boosted_sugg.title) + expect(response.body).to include(CGI.escapeHTML(boosted_sugg.title)) end it "returns 422 status if article_id is missing" do diff --git a/spec/requests/api/v0/articles_spec.rb b/spec/requests/api/v0/articles_spec.rb index b73521334..3d472537a 100644 --- a/spec/requests/api/v0/articles_spec.rb +++ b/spec/requests/api/v0/articles_spec.rb @@ -167,6 +167,37 @@ RSpec.describe "Api::V0::Articles", type: :request do expect(json_response.length).to eq(1) end end + + context "with state param" do + it "returns fresh articles" do + article.update_columns(positive_reactions_count: 1, score: 1) + + get api_articles_path(state: "fresh") + expect(response.parsed_body.size).to eq(1) + end + + it "returns rising articles" do + article.update_columns(positive_reactions_count: 32, score: 1, featured_number: 2.days.ago.to_i) + + get api_articles_path(state: "rising") + expect(response.parsed_body.size).to eq(1) + end + + it "returns nothing if the state is unknown" do + get api_articles_path(state: "foobar") + + expect(response.parsed_body).to be_empty + end + + it "supports pagination" do + create_list(:article, 2, tags: "discuss", positive_reactions_count: 1, score: 1) + + get api_articles_path(state: "fresh"), params: { page: 1, per_page: 2 } + expect(json_response.length).to eq(2) + get api_articles_path(state: "fresh"), params: { page: 2, per_page: 2 } + expect(json_response.length).to eq(1) + end + end end describe "GET /api/articles/:id" do