Skip to content

Commit

Permalink
Revert "Enable multiple reactions on #index (including front-end rend…
Browse files Browse the repository at this point in the history
…er)" (forem#19197)
  • Loading branch information
jaw6 authored Mar 3, 2023
1 parent 8a45a4b commit 727705d
Show file tree
Hide file tree
Showing 22 changed files with 47 additions and 162 deletions.
File renamed without changes
File renamed without changes
File renamed without changes
File renamed without changes
File renamed without changes
32 changes: 8 additions & 24 deletions app/assets/javascripts/utilities/buildArticleHTML.js
Original file line number Diff line number Diff line change
Expand Up @@ -112,30 +112,14 @@ function buildArticleHTML(article, currentUserId = null) {
var reactionsText = reactionsCount === 1 ? 'reaction' : 'reactions';

if (article.class_name !== 'User' && reactionsCount > 0) {
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>`;
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 picUrl;
Expand Down
26 changes: 0 additions & 26 deletions app/javascript/articles/__tests__/utilities/articleUtilities.js
Original file line number Diff line number Diff line change
Expand Up @@ -128,32 +128,6 @@ 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 = {
Expand Down
75 changes: 26 additions & 49 deletions app/javascript/articles/components/ReactionsCount.jsx
Original file line number Diff line number Diff line change
@@ -1,65 +1,42 @@
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;
}

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>
);
}

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}"
<Button
variant="ghost"
size="s"
contentType="icon-left"
url={article.path}
icon={reactionsSVG}
tagName="a"
>
<div className="multiple_reactions_aggregate">
{buildIcons()}
{buildCounter()}
</div>
</a>
<span title="Number of reactions">
{totalReactions}
<span className="hidden s:inline">
&nbsp;
{`${totalReactions == 1 ? locale('core.reaction') : `${locale('core.reaction')}s`}`}
</span>
</span>
</Button>
);
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,16 +23,4 @@ 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();
});
});
15 changes: 2 additions & 13 deletions app/models/concerns/reactable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,24 +3,13 @@ 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 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
def reaction_categories
reactions.distinct(:category).pluck(:category)
end
end
1 change: 0 additions & 1 deletion app/models/reaction.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ 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 }
Expand Down
9 changes: 0 additions & 9 deletions app/models/reaction_category.rb
Original file line number Diff line number Diff line change
Expand Up @@ -68,13 +68,4 @@ def negative?
def visible_to_public?
!privileged? && published?
end

def as_json(_options = nil)
{
slug: slug,
name: name,
icon: icon,
position: position
}
end
end
1 change: 0 additions & 1 deletion app/queries/homepage/articles_query.rb
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@ def filter
@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
Expand Down
1 change: 0 additions & 1 deletion app/serializers/homepage/article_serializer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ def self.serialized_collection_from(relation:)
:reading_time,
:title,
:user_id,
:public_reaction_categories,
)

attribute :video_duration_string, &:video_duration_in_minutes
Expand Down
1 change: 0 additions & 1 deletion app/services/articles/feeds/basic.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ def default_home_feed(**_kwargs)
.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))
Expand Down
1 change: 0 additions & 1 deletion app/services/articles/feeds/large_forem_experimental.rb
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,6 @@ def featured_story_from(stories:, must_have_main_image:)
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)
Expand Down
1 change: 0 additions & 1 deletion app/services/articles/feeds/tag.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ def self.call(tag = nil, number_of_articles: Article::DEFAULT_FEED_PAGINATION_WI
.published
.limited_column_select
.includes(top_comments: :user)
.includes(:distinct_reaction_categories)
.page(page)
.per(number_of_articles)
end
Expand Down
1 change: 0 additions & 1 deletion app/services/articles/feeds/variant_query.rb
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,6 @@ def call(only_featured: false, must_have_main_image: false, limit: default_limit
Article.joins(join_fragment)
.limited_column_select
.includes(top_comments: :user)
.includes(:distinct_reaction_categories)
.order(config.order_by.to_sql)
end

Expand Down
22 changes: 7 additions & 15 deletions app/views/articles/_single_story.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -120,21 +120,13 @@
<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) %>">
<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>
<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>
<% 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) %>">
Expand Down
1 change: 0 additions & 1 deletion app/views/stories/feeds/show.json.jbuilder
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ 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|
Expand Down
5 changes: 2 additions & 3 deletions spec/models/article_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1342,7 +1342,7 @@ def foo():
end
end

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

it "reports accurately" do
categories = article.public_reaction_categories
expect(categories.map(&:slug)).to contain_exactly(*%i[like])
expect(article.reaction_categories).to contain_exactly("like", "readinglist", "vomit")
end
end
end
5 changes: 2 additions & 3 deletions spec/services/homepage/fetch_articles_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,11 @@
result = described_class.call.first

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

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

0 comments on commit 727705d

Please sign in to comment.