From e545148fd594e1683163abd7f71e52c7affb9f96 Mon Sep 17 00:00:00 2001
From: Ben Halpern <bendhalpern@gmail.com>
Date: Tue, 21 Nov 2023 11:10:10 -0500
Subject: [PATCH] Add active discussions to sidebar (#20372)

* Add active discussions to sidebar

* Move featured tags to before signin-guard

* Move featured tags back to view

* Fix test
---
 app/assets/stylesheets/components/cards.scss  |  3 --
 app/controllers/sidebars_controller.rb        | 15 +++++++
 app/models/settings/general.rb                |  1 +
 app/views/sidebars/_homepage_content.html.erb | 37 +++++++++++++-----
 config/environments/development.rb            |  1 +
 config/locales/views/main/en.yml              |  1 +
 spec/requests/sidebars_spec.rb                | 39 +++++++++++++++++++
 7 files changed, 85 insertions(+), 12 deletions(-)

diff --git a/app/assets/stylesheets/components/cards.scss b/app/assets/stylesheets/components/cards.scss
index 3d4d0da5c..1e09009b8 100644
--- a/app/assets/stylesheets/components/cards.scss
+++ b/app/assets/stylesheets/components/cards.scss
@@ -35,9 +35,6 @@
   &__header {
     padding: var(--su-3) var(--su-4);
     border-bottom: 1px solid var(--body-bg);
-    display: flex;
-    justify-content: space-between;
-    align-items: center;
   }
 
   &__body {
diff --git a/app/controllers/sidebars_controller.rb b/app/controllers/sidebars_controller.rb
index 349f22670..66486b970 100644
--- a/app/controllers/sidebars_controller.rb
+++ b/app/controllers/sidebars_controller.rb
@@ -3,6 +3,7 @@ class SidebarsController < ApplicationController
 
   def show
     get_latest_campaign_articles
+    get_active_discussions if user_signed_in?
   end
 
   private
@@ -11,4 +12,18 @@ class SidebarsController < ApplicationController
     @campaign_articles_count = Campaign.current.count
     @latest_campaign_articles = Campaign.current.plucked_article_attributes
   end
+
+  def get_active_discussions
+    tag_names = current_user.cached_followed_tag_names
+    @active_discussions = Article.published
+      .where("published_at > ?", 1.week.ago)
+      .where("comments_count > ?", 0)
+      .with_at_least_home_feed_minimum_score
+      .cached_tagged_with_any(tag_names)
+      .or(Article.featured.published.where("published_at > ?", 1.week.ago)
+        .with_at_least_home_feed_minimum_score)
+      .order("last_comment_at DESC")
+      .limit(5)
+      .pluck(:path, :title, :comments_count, :created_at)
+  end
 end
diff --git a/app/models/settings/general.rb b/app/models/settings/general.rb
index 17715f8f7..923760aa4 100644
--- a/app/models/settings/general.rb
+++ b/app/models/settings/general.rb
@@ -102,6 +102,7 @@ module Settings
     setting :twitter_hashtag, type: :string
 
     # Tags
+    setting :display_sidebar_active_discussions, type: :boolean, default: true
     setting :sidebar_tags, type: :array, default: %w[]
 
     # Broadcast
diff --git a/app/views/sidebars/_homepage_content.html.erb b/app/views/sidebars/_homepage_content.html.erb
index c898db879..225253513 100644
--- a/app/views/sidebars/_homepage_content.html.erb
+++ b/app/views/sidebars/_homepage_content.html.erb
@@ -1,26 +1,45 @@
 <aside class="side-bar sidebar-additional showing grid gap-4" id="sidebar-additional">
-  <% cache(release_adjusted_cache_key("main-article-right-sidebar-discussions-#{params[:timeframe]}-#{user_signed_in?}"), expires_in: (params[:timeframe].blank? ? 120 : 360).seconds) do %>
-    <% @sidebar_billboard = Billboard.for_display(area: "sidebar_right", user_signed_in: user_signed_in?) %>
-    <% if @sidebar_billboard %>
-      <%= render partial: "shared/billboard", locals: { billboard: @sidebar_billboard, data_context_type: BillboardEvent::CONTEXT_TYPE_HOME } %>
+  <% @sidebar_billboard = Billboard.for_display(area: "sidebar_right", user_signed_in: user_signed_in?) %>
+  <% if @sidebar_billboard %>
+    <%= render partial: "shared/billboard", locals: { billboard: @sidebar_billboard, data_context_type: BillboardEvent::CONTEXT_TYPE_HOME } %>
+  <% end %>
+  <% if user_signed_in? && @active_discussions.any? %>
+    <% if Settings::General.display_sidebar_active_discussions %>
+      <section class="crayons-card crayons-card--secondary crayons-layout__content" id="active-discussions">
+        <header class="crayons-card__header">
+          <h3 class="crayons-subtitle-2">
+            <%= t("views.main.side.active_discussions") %>
+          </h3>
+        </header>
+        <div>
+          <% @active_discussions.each do |plucked_article| %>
+            <%= render "articles/widget_list_item", plucked_article: plucked_article, show_comment_count: true %>
+          <% end %>
+        </div>
+      </section>
     <% end %>
+  <% end %>
+
+  <% cache(release_adjusted_cache_key("main-article-right-sidebar-discussions-#{params[:timeframe]}-#{user_signed_in?}"), expires_in: (params[:timeframe].blank? ? 120 : 360).seconds) do %>
 
     <%= render "articles/sidebar_campaign" if Campaign.current.show_in_sidebar? %>
     <%= render "articles/sidebar_listings" if Listing.feature_enabled? %>
 
-    <% Settings::General.sidebar_tags.each do |tag| %>
+    <% Tag.where(name: Settings::General.sidebar_tags).find_each do |tag| %>
       <section class="crayons-card crayons-card--secondary crayons-layout__content">
         <header class="crayons-card__header">
-          <h3 class="crayons-subtitle-2"><a href="/t/<%= tag %>" class="crayons-link">#<%= tag %></a></h3>
+          <h3 class="crayons-subtitle-2"><a href="/t/<%= tag.name %>" class="crayons-link">#<%= tag.name %></a></h3>
+          <div class="fs-xs color-base-70">
+            <%= tag.short_summary %>
+          </div>
         </header>
-
         <div>
-          <% if tag == "help" %>
+          <% if tag.name == "help" %>
             <% Article.active_help.limit(5).pluck(:path, :title, :comments_count, :created_at).each do |plucked_article| %>
               <%= render "articles/widget_list_item", plucked_article: plucked_article, show_comment_count: true %>
             <% end %>
           <% else %>
-            <% active_threads(tags: [tag], time_ago: Timeframe.datetime(params[:timeframe]), count: 5).each do |plucked_article| %>
+            <% active_threads(tags: [tag.name], time_ago: Timeframe.datetime(params[:timeframe]), count: 5).each do |plucked_article| %>
               <%= render "articles/widget_list_item", plucked_article: plucked_article, show_comment_count: true %>
             <% end %>
           <% end %>
diff --git a/config/environments/development.rb b/config/environments/development.rb
index ab5261d45..a0bd82e02 100644
--- a/config/environments/development.rb
+++ b/config/environments/development.rb
@@ -92,6 +92,7 @@ Rails.application.configure do
   end
 
   config.hosts << "www.example.com"
+  config.hosts << /[a-zA-Z0-9\-]+\.preview\.app\.github\.dev\z/
 
   config.app_domain = ENV.fetch("APP_DOMAIN", "localhost:3000")
 
diff --git a/config/locales/views/main/en.yml b/config/locales/views/main/en.yml
index 060d78691..02c6d6dd2 100644
--- a/config/locales/views/main/en.yml
+++ b/config/locales/views/main/en.yml
@@ -44,6 +44,7 @@ en:
         Terms of use: Terms of use
         Videos: Videos
       side:
+        active_discussions: "Active discussions"
         listings:
           heading: Listings
           all: See all
diff --git a/spec/requests/sidebars_spec.rb b/spec/requests/sidebars_spec.rb
index ca03d850b..3bc146c27 100644
--- a/spec/requests/sidebars_spec.rb
+++ b/spec/requests/sidebars_spec.rb
@@ -4,10 +4,49 @@ RSpec.describe "Sidebars" do
   describe "GET /sidebars/home" do
     it "includes relevant parts" do
       listing = create(:listing, published: true)
+      create(:tag, name: "rubymagoo")
       allow(Settings::General).to receive(:sidebar_tags).and_return(["rubymagoo"])
       get "/sidebars/home"
       expect(response.body).to include("rubymagoo")
       expect(response.body).to include(CGI.escapeHTML(listing.title))
     end
+
+    context "when active discussions exist" do
+      let(:tag) { create(:tag, name: "testmagoo") }
+      let(:user) { create(:user) }
+      let!(:article) do
+        create(:article, tag_list: tag.name, last_comment_at: 1.day.ago,
+                         score: 10, comments_count: 5, created_at: 3.days.ago)
+      end
+
+      before do
+        user.follow(tag)
+      end
+
+      it "does not include active article if not signed in" do
+        get "/sidebars/home"
+        expect(response.body).not_to include("active-discussions")
+      end
+
+      it "does show active discussions if signed in and user follows tag" do
+        sign_in user
+        get "/sidebars/home"
+        expect(response.body).to include(CGI.escapeHTML(article.title))
+      end
+
+      it "includes an article without the proper tags if featured" do
+        second_article = create(:article, featured: true)
+        sign_in user
+        get "/sidebars/home"
+        expect(response.body).to include(CGI.escapeHTML(second_article.title))
+      end
+
+      it "does not include non-featured non-tagg-followed article" do
+        second_article = create(:article)
+        sign_in user
+        get "/sidebars/home"
+        expect(response.body).not_to include(CGI.escapeHTML(second_article.title))
+      end
+    end
   end
 end