Optimizations to show page (forem#22751)

* Optimize article queries in a few places

* Adjust optimizations

* Adjust comment

Author: benhalpern

app/controllers/stories_controller.rb

@@ -345,7 +345,8 @@ def assign_article_show_variables
       # considering non cross posted articles with a more recent publication date
       @collection_articles = @article.collection.articles
         .published.from_subforem
-        .order(Arel.sql("COALESCE(crossposted_at, published_at) ASC"))
+        .select(:id, :path, :title, :slug, :published_at, :crossposted_at, :user_id, :organization_id, :cached_tag_list, :subforem_id, :main_image)
+        .order(Arel.sql("COALESCE(articles.crossposted_at, articles.published_at) ASC"))
     end
 
     @comments_to_show_count = @article.cached_tag_list_array.include?("discuss") ? 50 : 30

app/services/articles/get_user_stickies.rb

@@ -1,14 +1,14 @@
 module Articles
   class GetUserStickies
     def self.call(article, author)
-      article_tags = article.cached_tag_list_array - ["discuss"]
+      article_tags = article.cached_tag_list.to_s.split(", ") - ["discuss"]
 
       author
         .articles
         .published.from_subforem
         .cached_tagged_with_any(article_tags)
         .unscope(:select)
-        .minimal_feed_column_select
+        .select(:id, :path, :title, :cached_tag_list, :organization_id, :user_id, :subforem_id) # Columns needed for _sticky_nav and path generation
         .where.not(id: article.id)
         .order(published_at: :desc)
         .limit(3)

app/services/articles/suggest_stickies.rb

@@ -40,7 +40,7 @@ def call
     attr_accessor :article, :reaction_count_num, :comment_count_num
 
     def tag_articles
-      article_tags = article.cached_tag_list_array - ["discuss"]
+      article_tags = article.cached_tag_list.to_s.split(", ") - ["discuss"]
 
       scope = Article
         .where("public_reactions_count > ? OR comments_count > ?", reaction_count_num, comment_count_num)
@@ -68,7 +68,7 @@ def apply_common_scope(scope:, tags:)
       scope.published.from_subforem
         .cached_tagged_with_any(tags)
         .unscope(:select)
-        .minimal_feed_column_select
+        .select(:id, :path, :title, :cached_tag_list, :cached_user, :organization_id, :user_id) # Columns needed for _sticky_nav (includes cached_user for avatar)
         .where.not(id: article.id)
         .not_authored_by(article.user_id)
         .where("published_at > ?", 5.days.ago)

spec/requests/stories_performance_fix_spec.rb

@@ -0,0 +1,114 @@
+require "rails_helper"
+
+RSpec.describe "StoriesPerformanceFix", type: :request do
+  let(:user) { create(:user) }
+
+  describe "GET /:username/:slug (article show with collection)" do
+    let(:collection) { create(:collection, user: user) }
+    let(:article) do
+      create(:article, user: user, published: true, collection: collection,
+             cached_tag_list: "ruby, rails", main_image: "https://example.com/image.png")
+    end
+    let!(:other_collection_articles) do
+      create_list(:article, 3, user: user, published: true, collection: collection,
+                  cached_tag_list: "ruby", main_image: "https://example.com/other.png")
+    end
+
+    before do
+      # Create articles to trigger sticky nav suggestions
+      create_list(:article, 3, user: user, published: true, cached_tag_list: "ruby")
+      career_articles = create_list(:article, 3, published: true, cached_tag_list: "career",
+                                    public_reactions_count: 50)
+      career_articles.each { |a| a.update_columns(published_at: 1.day.ago) }
+    end
+
+    it "renders the article show page with collection without MissingAttributeError" do
+      # The collection partial uses: id, path, title, slug, published_at, crossposted_at,
+      # user_id, organization_id, cached_tag_list, subforem_id, main_image
+      get article.path
+      follow_redirect! if response.status == 301
+      expect(response).to have_http_status(:ok)
+    end
+
+    it "renders sticky nav with user stickies (GetUserStickies)" do
+      # GetUserStickies uses: id, path, title, cached_tag_list, organization_id, user_id, subforem_id
+      get article.path
+      follow_redirect! if response.status == 301
+      expect(response).to have_http_status(:ok)
+    end
+
+    it "renders sticky nav with trending articles (SuggestStickies) when no user stickies" do
+      # Remove all same-author articles so GetUserStickies returns empty
+      user.articles.where.not(id: article.id).destroy_all
+      # SuggestStickies uses: id, path, title, cached_tag_list, cached_user, organization_id, user_id
+      get article.path
+      follow_redirect! if response.status == 301
+      expect(response).to have_http_status(:ok)
+    end
+  end
+
+  describe "GET /:username (user profile with articles)" do
+    let!(:articles) do
+      create_list(:article, 3, user: user, published: true, cached_tag_list: "ruby",
+                  main_image: "https://example.com/image.png")
+    end
+
+    it "renders user profile page without MissingAttributeError" do
+      # limited_column_select provides: path, title, id, published, comments_count,
+      # public_reactions_count, cached_tag_list, main_image, main_image_background_hex_color,
+      # updated_at, slug, video, user_id, organization_id, video_source_url, video_code,
+      # video_thumbnail_url, video_closed_caption_track_url, cached_user, cached_organization,
+      # published_at, crossposted_at, description, reading_time, video_duration_in_seconds,
+      # score, last_comment_at, main_image_height, type_of, edited_at, processed_html, subforem_id
+      get "/#{user.username}"
+      expect(response).to have_http_status(:ok)
+    end
+
+    it "renders all article titles on the profile page" do
+      get "/#{user.username}"
+      expect(response).to have_http_status(:ok)
+      articles.each do |a|
+        expect(response.body).to include(a.title)
+      end
+    end
+
+    it "renders article reading time without MissingAttributeError" do
+      get "/#{user.username}"
+      expect(response).to have_http_status(:ok)
+      expect(response.body).to include("min read")
+    end
+
+    context "with pinned articles" do
+      let!(:pin) { create(:profile_pin, profile: user, pinnable: articles.first) }
+
+      it "renders pinned articles without MissingAttributeError" do
+        get "/#{user.username}"
+        expect(response).to have_http_status(:ok)
+        expect(response.body).to include("Pinned")
+        expect(response.body).to include(articles.first.title)
+      end
+    end
+  end
+
+  describe "GET /:org_slug (organization profile with articles)" do
+    let(:org) { create(:organization) }
+    let!(:org_membership) { create(:organization_membership, user: user, organization: org) }
+    let!(:org_articles) do
+      create_list(:article, 3, organization: org, user: user, published: true,
+                  cached_tag_list: "devops", main_image: "https://example.com/org.png")
+    end
+
+    it "renders organization profile page without MissingAttributeError" do
+      get "/#{org.slug}"
+      expect(response).to have_http_status(:ok)
+    end
+
+    it "renders org article cards with all required columns" do
+      get "/#{org.slug}"
+      expect(response).to have_http_status(:ok)
+      org_articles.each do |a|
+        expect(response.body).to include(a.title)
+      end
+    end
+  end
+end

spec/services/articles/get_user_stickies_spec.rb

@@ -0,0 +1,51 @@
+require "rails_helper"
+
+RSpec.describe Articles::GetUserStickies, type: :service do
+  let(:user) { create(:user) }
+  let(:article) { create(:article, user: user, published: true, cached_tag_list: "ruby") }
+  let!(:other_articles) { create_list(:article, 3, user: user, published: true, cached_tag_list: "ruby") }
+
+  describe ".call" do
+    it "returns articles with limited attributes needed by _sticky_nav" do
+      result = described_class.call(article, user)
+      expect(result).not_to be_empty
+
+      sticky = result.first
+      # Columns explicitly selected for _sticky_nav partial:
+      expect { sticky.id }.not_to raise_error
+      expect { sticky.path }.not_to raise_error
+      expect { sticky.title }.not_to raise_error
+      expect { sticky.cached_tag_list }.not_to raise_error
+      expect { sticky.organization_id }.not_to raise_error
+      expect { sticky.user_id }.not_to raise_error
+      expect { sticky.subforem_id }.not_to raise_error
+    end
+
+    it "allows decorating for cached_tag_list_array (used in view)" do
+      result = described_class.call(article, user)
+      sticky = result.first
+      # _sticky_nav calls article.decorate.cached_tag_list_array
+      expect { sticky.decorate.cached_tag_list_array }.not_to raise_error
+      expect(sticky.decorate.cached_tag_list_array).to be_a(Array)
+    end
+
+    it "excludes the current article" do
+      result = described_class.call(article, user)
+      expect(result.map(&:id)).not_to include(article.id)
+    end
+
+    it "does not load unnecessary columns" do
+      result = described_class.call(article, user)
+      sticky = result.first
+      expect { sticky.body_markdown }.to raise_error(ActiveModel::MissingAttributeError)
+      expect { sticky.processed_html }.to raise_error(ActiveModel::MissingAttributeError)
+    end
+
+    it "handles article with nil cached_tag_list" do
+      article.update_column(:cached_tag_list, nil)
+      result = described_class.call(article.reload, user)
+      # Should not raise, just returns empty or unfiltered results
+      expect(result).to be_a(ActiveRecord::Relation)
+    end
+  end
+end

spec/services/articles/suggest_stickies_spec.rb

@@ -1,11 +1,88 @@
 require "rails_helper"
 
-RSpec.describe Articles::SuggestStickies, type: :services do
-  let(:article) { create(:article).decorate }
+RSpec.describe Articles::SuggestStickies, type: :service do
+  let(:user) { create(:user) }
+  let(:article) { create(:article, user: user, published: true, cached_tag_list: "ruby") }
+
+  # Create articles with SUGGESTION_TAGS that meet the reaction/comment thresholds.
+  # We create first, then update published_at via update_column to bypass validation.
+  let!(:career_articles) do
+    articles = create_list(:article, 3, published: true, cached_tag_list: "career",
+                           public_reactions_count: 50, comments_count: 10)
+    articles.each { |a| a.update_columns(published_at: 1.day.ago) }
+    articles
+  end
+  let!(:discuss_articles) do
+    articles = create_list(:article, 2, published: true, cached_tag_list: "discuss",
+                           public_reactions_count: 50, comments_count: 10)
+    articles.each { |a| a.update_columns(published_at: 1.day.ago) }
+    articles
+  end
 
   describe ".call" do
-    subject(:method_call) { described_class.call(article) }
+    it "returns articles with limited attributes needed by _sticky_nav" do
+      result = described_class.call(article)
+      expect(result).not_to be_empty
+
+      sticky = result.first
+      # Columns explicitly selected for _sticky_nav (suggest stickies section):
+      expect { sticky.id }.not_to raise_error
+      expect { sticky.path }.not_to raise_error
+      expect { sticky.title }.not_to raise_error
+      expect { sticky.cached_tag_list }.not_to raise_error
+      expect { sticky.cached_user }.not_to raise_error
+      expect { sticky.organization_id }.not_to raise_error
+      expect { sticky.user_id }.not_to raise_error
+    end
+
+    it "allows accessing cached_user avatar data (used in view)" do
+      result = described_class.call(article)
+      expect(result).not_to be_empty
+
+      sticky = result.first
+      # _sticky_nav calls article.cached_user.profile_image_url_for and cached_user.name
+      cached_user = sticky.cached_user
+      expect(cached_user).not_to be_nil
+      expect { cached_user.name }.not_to raise_error
+    end
+
+    it "allows decorating for cached_tag_list_array (used in view)" do
+      result = described_class.call(article)
+      expect(result).not_to be_empty
+
+      sticky = result.first
+      # _sticky_nav calls article.decorate.cached_tag_list_array
+      expect { sticky.decorate.cached_tag_list_array }.not_to raise_error
+    end
+
+    it "does not include the current article" do
+      result = described_class.call(article)
+      expect(result.map(&:id)).not_to include(article.id)
+    end
+
+    it "does not include articles by the same author" do
+      result = described_class.call(article)
+      expect(result.map(&:user_id)).not_to include(article.user_id)
+    end
+
+    it "does not load unnecessary columns (proves optimization)" do
+      result = described_class.call(article)
+      next if result.empty?
+
+      sticky = result.first
+      expect { sticky.body_markdown }.to raise_error(ActiveModel::MissingAttributeError)
+      expect { sticky.processed_html }.to raise_error(ActiveModel::MissingAttributeError)
+    end
+
+    it "returns an array" do
+      result = described_class.call(article)
+      expect(result).to be_a(Array)
+    end
 
-    it { is_expected.to be_a Array }
+    it "handles article with nil cached_tag_list" do
+      article.update_column(:cached_tag_list, nil)
+      result = described_class.call(article.reload)
+      expect(result).to be_a(Array)
+    end
   end
 end