Enable multiple reactions on #index (including front-end render) (#19169)

* Revert "(Temporarily) Remove multiple reactions from #index (#19142)" This reverts commit a45d300639. * Clean up reaction category count mechanism for #index * Remove, unnecessary * Expose reaction categories in JSON response * Update test with new categories * Fix flaky sample / minimum floor * buildArticle (infinite scroll) with multiple reactions * If we're doing #index front-end, we aren't feature-flag'd * nbsp * react (home feed) has multiple_reactions * Update app/assets/javascripts/utilities/buildArticleHTML.js Co-authored-by: Rajat Talesra <rajat@forem.com> * Clean up Reactable#reaction_categories * Use 'multiple_reactions_icons_container' * Try adding a ReactionCount test * Adapt memory fix from 76dd53d * Try adding categories to fixture * Attempt to eager-load distinct public categories * Setting dependent to the default for rubocop * Try making image assets more public? * Revert "Fix flaky sample / minimum floor" --------- Co-authored-by: Rajat Talesra <rajat@forem.com> Co-authored-by: Mac Siri <mac@forem.com>
2023-03-03 15:17:19 +01:00 · 2023-03-03 15:17:19 +01:00 · 8a45a4bf67
commit 8a45a4bf67
parent 54a077a1ec
22 changed files with 162 additions and 47 deletions
--- a/app/assets/javascripts/utilities/buildArticleHTML.js
+++ b/app/assets/javascripts/utilities/buildArticleHTML.js
@ -112,14 +112,30 @@ function buildArticleHTML(article, currentUserId = null) {
    var reactionsText = reactionsCount === 1 ? 'reaction' : 'reactions';

    if (article.class_name !== 'User' && reactionsCount > 0) {
-      reactionsDisplay =
-        '<a href="' +
-        article.path +
-        '"' +
-        commentsAriaLabelText +
-        'class="crayons-btn crayons-btn--s crayons-btn--ghost crayons-btn--icon-left"><svg class="crayons-icon" width="24" height="24" xmlns="http://www.w3.org/2000/svg"><path d="M18.884 12.595l.01.011L12 19.5l-6.894-6.894.01-.01A4.875 4.875 0 0112 5.73a4.875 4.875 0 016.884 6.865zM6.431 7.037a3.375 3.375 0 000 4.773L12 17.38l5.569-5.569a3.375 3.375 0 10-4.773-4.773L9.613 10.22l-1.06-1.062 2.371-2.372a3.375 3.375 0 00-4.492.25v.001z"/></svg>' +
-        reactionsCount +
-        `<span class="hidden s:inline">&nbsp;${reactionsText}</span></a>`;
+      var icons = [];
+      for (var category of article.public_reaction_categories) {
+        var icon = `<img src="/assets/${category.icon}.svg" width="18" height="18" />`;
+        icons = icons.concat(
+          `<span class="crayons_icon_container">${icon}</span>`,
+        );
+      }
+      icons.reverse();
+
+      reactionsDisplay = `<a href="${
+        article.path
+      }" class="crayons-btn crayons-btn--s crayons-btn--ghost crayons-btn--icon-left" data-reaction-count data-reactable-id="${
+        article.id
+      }">
+                          <div class="multiple_reactions_aggregate">
+                            <span class="multiple_reactions_icons_container" dir="rtl">
+                                ${icons.join()}
+                            </span>
+                            <span class="aggregate_reactions_counter">
+                              <span class="hidden s:inline">${reactionsCount}&nbsp;${reactionsText}</span>
+                            </span>
+                        </span>
+                          </div>
+                        </a>`;
    }

    var picUrl;
--- a/app/javascript/articles/tests/utilities/articleUtilities.js
+++ b/app/javascript/articles/tests/utilities/articleUtilities.js
@ -128,6 +128,32 @@ export const articleWithReactions = {
  readable_publish_date: 'February 18',
  top_comments: [],
  public_reactions_count: 232,
+  public_reaction_categories: [
+    {
+      slug: 'like',
+      name: 'Like',
+      icon: 'sparkle-heart',
+      position: 1,
+    },
+    {
+      slug: 'unicorn',
+      name: 'Unicorn',
+      icon: 'multi-unicorn',
+      position: 2,
+    },
+    {
+      slug: 'raised_hands',
+      name: 'Raised Hands',
+      icon: 'raised-hands',
+      position: 4,
+    },
+    {
+      slug: 'fire',
+      name: 'Fire',
+      icon: 'fire',
+      position: 5,
+    },
+  ],
 };

 export const articleWithoutReactions = {
--- a/app/javascript/articles/components/ReactionsCount.jsx
+++ b/app/javascript/articles/components/ReactionsCount.jsx
@ -1,42 +1,65 @@
 import { h } from 'preact';
 import { articlePropTypes } from '../../common-prop-types';
-import { Button } from '../../crayons/Button';
 import { locale } from '../../utilities/locale';

 export const ReactionsCount = ({ article }) => {
  const totalReactions = article.public_reactions_count || 0;
-  const reactionsSVG = () => (
-    <svg
-      className="crayons-icon"
-      width="24"
-      height="24"
-      xmlns="http://www.w3.org/2000/svg"
-    >
-      <path d="M18.884 12.595l.01.011L12 19.5l-6.894-6.894.01-.01A4.875 4.875 0 0112 5.73a4.875 4.875 0 016.884 6.865zM6.431 7.037a3.375 3.375 0 000 4.773L12 17.38l5.569-5.569a3.375 3.375 0 10-4.773-4.773L9.613 10.22l-1.06-1.062 2.371-2.372a3.375 3.375 0 00-4.492.25v.001z" />
-    </svg>
-  );

  if (totalReactions === 0) {
    return;
  }

-  return (
-    <Button
-      variant="ghost"
-      size="s"
-      contentType="icon-left"
-      url={article.path}
-      icon={reactionsSVG}
-      tagName="a"
-    >
-      <span title="Number of reactions">
-        {totalReactions}
-        <span className="hidden s:inline">
-          &nbsp;
-          {`${totalReactions == 1 ? locale('core.reaction') : `${locale('core.reaction')}s`}`}
+  function buildIcons() {
+    const reversable = article.public_reaction_categories;
+    if (reversable === undefined) {
+      return;
+    }
+    reversable.reverse();
+
+    const icons = reversable.map((category) => {
+      const path = `/assets/${category.icon}.svg`;
+      const alt = category.name;
+      return (
+        <span className="crayons_icon_container" key={category.slug}>
+          <img src={path} width="18" height="18" alt={alt} />
+        </span>
+      );
+    });
+
+    return (
+      <span className="multiple_reactions_icons_container" dir="rtl">
+        {icons}
+      </span>
+    );
+  }
+
+  function buildCounter() {
+    const reactionText = `${
+      totalReactions == 1
+        ? locale('core.reaction')
+        : `${locale('core.reaction')}s`
+    }`;
+    return (
+      <span className="aggregate_reactions_counter">
+        <span className="hidden s:inline" title="Number of reactions">
+          {totalReactions}&nbsp;{reactionText}
        </span>
      </span>
-    </Button>
+    );
+  }
+
+  return (
+    <a
+      href="{article.path}"
+      className="crayons-btn crayons-btn--s crayons-btn--ghost crayons-btn--icon-left"
+      data-reaction-count
+      data-reactable-id="{article.id}"
+    >
+      <div className="multiple_reactions_aggregate">
+        {buildIcons()}
+        {buildCounter()}
+      </div>
+    </a>
  );
 };

--- a/app/javascript/articles/components/tests/ReactionsCount.test.jsx
+++ b/app/javascript/articles/components/tests/ReactionsCount.test.jsx
@ -23,4 +23,16 @@ describe('<ReactionsCount /> component', () => {

    expect(findByText('232 reactions')).toBeDefined();
  });
+
+  it('should display multiple reactions when there are reactions', async () => {
+    const { findByText } = render(
+      <ReactionsCount article={articleWithReactions} />,
+    );
+
+    expect(
+      findByText(
+        '<span class="multiple_reactions_icons_container" dir="rtl"><span class="crayons_icon_container"><img src="/assets/like.svg" alt="Like" width="18" height="18"></span>',
+      ),
+    ).toBeDefined();
+  });
 });
--- a/app/models/concerns/reactable.rb
+++ b/app/models/concerns/reactable.rb
@ -3,13 +3,24 @@ module Reactable

  included do
    has_many :reactions, as: :reactable, inverse_of: :reactable, dependent: :destroy
+    has_many :distinct_reaction_categories, -> { order(:category).merge(Reaction.distinct_categories) },
+             as: :reactable,
+             inverse_of: :reactable,
+             dependent: nil,
+             class_name: "Reaction"
  end

  def sync_reactions_count
    update_column(:public_reactions_count, reactions.public_category.size)
  end

-  def reaction_categories
-    reactions.distinct(:category).pluck(:category)
+  def public_reaction_categories
+    @public_reaction_categories ||= begin
+      # .map is intentional below - .pluck would break eager-loaded association!
+      reacted = distinct_reaction_categories.map(&:category)
+      ReactionCategory.for_view.select do |reaction_type|
+        reacted.include?(reaction_type.slug.to_s)
+      end
+    end
  end
 end
--- a/app/models/reaction.rb
+++ b/app/models/reaction.rb
@ -39,6 +39,7 @@ class Reaction < ApplicationRecord
  scope :unarchived, -> { where.not(status: "archived") }
  scope :from_user, ->(user) { where(user: user) }
  scope :readinglist_for_user, ->(user) { readinglist.unarchived.only_articles.from_user(user) }
+  scope :distinct_categories, -> { select("distinct(reactions.category) as category, reactable_id, reactable_type") }

  validates :category, inclusion: { in: ReactionCategory.all_slugs.map(&:to_s) }
  validates :reactable_type, inclusion: { in: REACTABLE_TYPES }
--- a/app/models/reaction_category.rb
+++ b/app/models/reaction_category.rb
@ -68,4 +68,13 @@ class ReactionCategory
  def visible_to_public?
    !privileged? && published?
  end
+
+  def as_json(_options = nil)
+    {
+      slug: slug,
+      name: name,
+      icon: icon,
+      position: position
+    }
+  end
 end
--- a/app/queries/homepage/articles_query.rb
+++ b/app/queries/homepage/articles_query.rb
@ -67,6 +67,7 @@ module Homepage
      @relation = @relation.where(user_id: user_id) if user_id.present?
      @relation = @relation.where(organization_id: organization_id) if organization_id.present?
      @relation = @relation.cached_tagged_with_any(tags) if tags.any?
+      @relation = @relation.includes(:distinct_reaction_categories)

      relation
    end
--- a/app/serializers/homepage/article_serializer.rb
+++ b/app/serializers/homepage/article_serializer.rb
@ -28,6 +28,7 @@ module Homepage
      :reading_time,
      :title,
      :user_id,
+      :public_reaction_categories,
    )

    attribute :video_duration_string, &:video_duration_in_minutes
--- a/app/services/articles/feeds/basic.rb
+++ b/app/services/articles/feeds/basic.rb
@ -15,6 +15,7 @@ module Articles
          .with_at_least_home_feed_minimum_score
          .limit(@number_of_articles)
          .limited_column_select.includes(top_comments: :user)
+          .includes(:distinct_reaction_categories)
        return articles unless @user

        articles = articles.where.not(user_id: UserBlock.cached_blocked_ids_for_blocker(@user.id))
--- a/app/services/articles/feeds/large_forem_experimental.rb
+++ b/app/services/articles/feeds/large_forem_experimental.rb
@ -103,6 +103,7 @@ module Articles
      def experimental_hot_story_grab
        start_time = Articles::Feeds.oldest_published_at_to_consider_for(user: @user)
        Article.published.limited_column_select.includes(top_comments: :user)
+          .includes(:distinct_reaction_categories)
          .where("published_at > ?", start_time)
          .page(@page).per(@number_of_articles)
          .order(score: :desc)
--- a/app/services/articles/feeds/tag.rb
+++ b/app/services/articles/feeds/tag.rb
@ -17,6 +17,7 @@ module Articles
          .published
          .limited_column_select
          .includes(top_comments: :user)
+          .includes(:distinct_reaction_categories)
          .page(page)
          .per(number_of_articles)
      end
--- a/app/services/articles/feeds/variant_query.rb
+++ b/app/services/articles/feeds/variant_query.rb
@ -134,6 +134,7 @@ module Articles
        Article.joins(join_fragment)
          .limited_column_select
          .includes(top_comments: :user)
+          .includes(:distinct_reaction_categories)
          .order(config.order_by.to_sql)
      end

--- a/app/views/articles/_single_story.html.erb
+++ b/app/views/articles/_single_story.html.erb
@ -120,13 +120,21 @@
      <div class="crayons-story__bottom">
        <div class="crayons-story__details">
          <% if story.public_reactions_count > 0 %>
-            <a href="<%= story.path %>" class="crayons-btn crayons-btn--s crayons-btn--ghost crayons-btn--icon-left" data-reaction-count data-reactable-id="<%= story.id %>" aria-label="<%= t("views.articles.comments.aria_label", title: story.title) %>">
-              <%= crayons_icon_tag("small-heart", title: t("views.reactions.summary.title")) %>
-              <%= t("views.reactions.summary.count_html",
-                    count: story.public_reactions_count,
-                    start: tag("span", { class: %w[hidden s:inline] }, true),
-                    end: "</span>".html_safe) %>
-            </a>
+          <a href="<%= story.path %>" class="crayons-btn crayons-btn--s crayons-btn--ghost crayons-btn--icon-left" data-reaction-count data-reactable-id="<%= story.id %>" aria-label="<%= t("views.articles.comments.aria_label", title: story.title) %>">
+            <div class="multiple_reactions_aggregate">
+              <span class="multiple_reactions_icons_container" dir="rtl">
+                <% story.public_reaction_categories.reverse_each do |reaction_type| %>
+                  <span class="crayons_icon_container">
+                    <%= image_tag "#{reaction_type.icon}.svg", size: "18x18" %>
+                  </span>
+                <% end %>
+              </span>
+              <span class="aggregate_reactions_counter"><%= t("views.reactions.summary.count_html",
+                                                              count: story.public_reactions_count,
+                                                              start: tag("span", { class: %w[hidden s:inline] }, true),
+                                                              end: "</span>".html_safe) %></span>
+            </div>
+          </a>
          <% end %>
          <% if story.comments_count > 0 %>
            <a href="<%= story.path %>#comments" class="crayons-btn crayons-btn--s crayons-btn--ghost crayons-btn--icon-left flex items-center" aria-label="<%= t("views.articles.comments.aria_label", title: story.title) %>">
--- a/app/views/stories/feeds/show.json.jbuilder
+++ b/app/views/stories/feeds/show.json.jbuilder
@ -7,6 +7,7 @@ article_methods_to_include = %i[
  readable_publish_date flare_tag class_name
  cloudinary_video_url video_duration_in_minutes published_at_int
  published_timestamp main_image_background_hex_color
+  public_reaction_categories
 ]

 json.array!(@stories) do |article|
--- a/app/assets/images/fire.svg
+++ b/app/assets/images/fire.svg
--- a/app/assets/images/lightbulb.svg
+++ b/app/assets/images/lightbulb.svg
--- a/app/assets/images/multi-unicorn.svg
+++ b/app/assets/images/multi-unicorn.svg
--- a/app/assets/images/raised-hands.svg
+++ b/app/assets/images/raised-hands.svg
--- a/app/assets/images/sparkle-heart.svg
+++ b/app/assets/images/sparkle-heart.svg
--- a/spec/models/article_spec.rb
+++ b/spec/models/article_spec.rb
@ -1342,7 +1342,7 @@ RSpec.describe Article do
    end
  end

-  describe "#reaction_categories reports unique associated reaction categories" do
+  describe "#public_reaction_categories reports unique associated reaction categories" do
    before do
      user2 = create(:user)
      user2.add_role(:trusted)
@ -1354,7 +1354,8 @@ RSpec.describe Article do
    end

    it "reports accurately" do
-      expect(article.reaction_categories).to contain_exactly("like", "readinglist", "vomit")
+      categories = article.public_reaction_categories
+      expect(categories.map(&:slug)).to contain_exactly(*%i[like])
    end
  end
 end
--- a/spec/services/homepage/fetch_articles_spec.rb
+++ b/spec/services/homepage/fetch_articles_spec.rb
@ -12,11 +12,12 @@ RSpec.describe Homepage::FetchArticles, type: :service do
      result = described_class.call.first

      keys = %i[
-        class_name cloudinary_video_url comments_count flare_tag id path public_reactions_count
+        class_name cloudinary_video_url comments_count flare_tag id path
+        public_reactions_count public_reaction_categories
        published_at_int readable_publish_date reading_time tag_list title
        user user_id video_duration_string
      ]
-      expect(result.keys.sort).to eq(keys)
+      expect(result.keys.sort).to contain_exactly(*keys)

      expect(result[:class_name]).to eq("Article")
      expect(result[:cloudinary_video_url]).to eq(article.cloudinary_video_url)