From a95877e8ff655371cfd06383bd9dabc26e7f6653 Mon Sep 17 00:00:00 2001 From: rhymes Date: Wed, 5 Feb 2020 14:21:20 +0100 Subject: [PATCH] Use cleverer patterns to fetch/iterate AR objects when checking presence (#5833) [deploy] --- app/services/notifications/update.rb | 4 +++- app/services/users/delete_articles.rb | 2 +- app/views/additional_content_boxes/boxes.html.erb | 3 ++- app/views/articles/_sidebar.html.erb | 3 ++- app/views/articles/_sidebar_additional.html.erb | 6 +++--- app/views/articles/tag_index.html.erb | 4 +++- .../articles/tags/_sidebar_additional.html.erb | 13 +++++++------ app/views/internal/articles/index.html.erb | 2 +- app/views/internal/podcasts/edit.html.erb | 2 +- app/views/moderations/mod.html.erb | 2 +- .../organizations/_sidebar_additional.html.erb | 5 ++++- app/views/organizations/show.html.erb | 4 +++- app/views/partnerships/_form.html.erb | 4 ++-- app/views/users/_comments_section.html.erb | 2 +- app/views/users/_main_feed.html.erb | 6 +++--- app/views/users/_organizations_area.html.erb | 4 +++- 16 files changed, 40 insertions(+), 26 deletions(-) diff --git a/app/services/notifications/update.rb b/app/services/notifications/update.rb index 77359287b..728b0b7ce 100644 --- a/app/services/notifications/update.rb +++ b/app/services/notifications/update.rb @@ -19,7 +19,9 @@ module Notifications notifiable_type: notifiable.class.name, action: action, ) - return if notifications.blank? + # as we only select the first notification right after, there is no need + # to load all of them in memory with `.blank?`, thus we choose `.none?` + return if notifications.none? new_json_data = notifications.first.json_data || {} new_json_data[notifiable.class.name.downcase] = public_send("#{notifiable.class.name.downcase}_data", notifiable) diff --git a/app/services/users/delete_articles.rb b/app/services/users/delete_articles.rb index 34e86fc13..f520fcf3c 100644 --- a/app/services/users/delete_articles.rb +++ b/app/services/users/delete_articles.rb @@ -3,7 +3,7 @@ module Users module_function def call(user, cache_buster = CacheBuster) - return unless user.articles.any? + return if user.articles.blank? virtual_articles = user.articles.map { |article| Article.new(article.attributes) } user.articles.find_each do |article| diff --git a/app/views/additional_content_boxes/boxes.html.erb b/app/views/additional_content_boxes/boxes.html.erb index ea8ad380c..6d1236ad5 100644 --- a/app/views/additional_content_boxes/boxes.html.erb +++ b/app/views/additional_content_boxes/boxes.html.erb @@ -6,7 +6,8 @@ follow_cue: @boosted_article.organization&.tag_line || @boosted_article.organization&.tag_line %> <% end %> -<% if @suggested_articles.any? %> +<%# the pattern .present?/.each has the advantage of issuing only a single SQL query to load objects in memory %> +<% if @suggested_articles.present? %> <% @suggested_articles.each do |article| %> <%= render "additional_content_boxes/article_box", article: article, diff --git a/app/views/articles/_sidebar.html.erb b/app/views/articles/_sidebar.html.erb index 89745ca97..b1cdfc234 100644 --- a/app/views/articles/_sidebar.html.erb +++ b/app/views/articles/_sidebar.html.erb @@ -11,7 +11,8 @@ <% @sponsorships = Sponsorship.gold.live.includes(:organization).order(featured_number: :asc) %> - <% if @sponsorships.any? %> + <%# the pattern .present?/.each has the advantage of issuing only a single SQL query to load objects in memory %> + <% if @sponsorships.present? %>
<% @sponsorships.each do |sponsorship| %> <%= render "articles/single_sponsor", sponsorship: sponsorship %> diff --git a/app/views/articles/_sidebar_additional.html.erb b/app/views/articles/_sidebar_additional.html.erb index 608302663..30eb8d106 100644 --- a/app/views/articles/_sidebar_additional.html.erb +++ b/app/views/articles/_sidebar_additional.html.erb @@ -161,15 +161,15 @@ <% end %> <% unless user_signed_in? %> <% cache("seo-boostable-posts-homepage-#{params[:timeframe]}-xoxo", expires_in: 18.hours) do %> - <% @boostable_posts = Article.seo_boostable(nil, Timeframer.new(params[:timeframe]).datetime) %> - <% if @boostable_posts.any? %> + <% boostable_posts = Article.seo_boostable(nil, Timeframer.new(params[:timeframe]).datetime) %> + <% if boostable_posts.present? %>

trending guides/resources

- <% @boostable_posts.each do |plucked_article| %> + <% boostable_posts.each do |plucked_article| %> <%= render "articles/widget_list_item", plucked_article: plucked_article, show_comment_count: false %> <% end %>
diff --git a/app/views/articles/tag_index.html.erb b/app/views/articles/tag_index.html.erb index d347ee889..0ebb306eb 100644 --- a/app/views/articles/tag_index.html.erb +++ b/app/views/articles/tag_index.html.erb @@ -104,7 +104,9 @@
- <% if @stories.any? %> + <%# articles/tags/main_feed will iterate on stories with .each_with_index, + thus by using .present? here we preload the items %> + <% if @stories.present? %> <%= render "articles/tags/main_feed" %> <% end %>
diff --git a/app/views/articles/tags/_sidebar_additional.html.erb b/app/views/articles/tags/_sidebar_additional.html.erb index 00ad665fa..aa260de92 100644 --- a/app/views/articles/tags/_sidebar_additional.html.erb +++ b/app/views/articles/tags/_sidebar_additional.html.erb @@ -1,14 +1,15 @@