From aed51f99a9a821ab028f87f8dcb9b0d09b6b98e4 Mon Sep 17 00:00:00 2001 From: Jonas Jabari Date: Mon, 24 Jun 2024 11:59:40 +0200 Subject: [PATCH 01/43] [#40437] Easy emoji reactions, e.g "thumbs up" on activity entries https://community.openproject.org/work_packages/40437 From 53aa61c8382814b523029565b21dc94a0a1b494a Mon Sep 17 00:00:00 2001 From: Jonas Jabari Date: Mon, 1 Jul 2024 13:26:42 +0200 Subject: [PATCH 02/43] introduce simple emoji reactions --- .../activities_tab/index_component.rb | 2 +- .../item_component/add_reactions.html.erb | 33 ++++++++ .../journals/item_component/add_reactions.rb | 53 ++++++++++++ .../item_component/reactions.html.erb | 21 +++++ .../journals/item_component/reactions.rb | 72 +++++++++++++++++ .../journals/item_component/show.html.erb | 8 ++ .../emoji_reactions/base_contract.rb | 81 +++++++++++++++++++ .../emoji_reactions/create_contract.rb | 32 ++++++++ .../emoji_reactions/delete_contract.rb | 36 +++++++++ .../activities_tab_controller.rb | 50 ++++++++++-- app/models/concerns/reactable.rb | 69 ++++++++++++++++ app/models/emoji_reaction.rb | 56 +++++++++++++ app/models/journal.rb | 1 + .../emoji_reactions/create_service.rb | 32 ++++++++ .../emoji_reactions/delete_service.rb | 32 ++++++++ .../emoji_reactions/set_attributes_service.rb | 32 ++++++++ config/initializers/permissions.rb | 2 +- config/locales/en.yml | 8 ++ config/routes.rb | 1 + .../20240624103354_create_emoji_reactions.rb | 12 +++ 20 files changed, 626 insertions(+), 7 deletions(-) create mode 100644 app/components/work_packages/activities_tab/journals/item_component/add_reactions.html.erb create mode 100644 app/components/work_packages/activities_tab/journals/item_component/add_reactions.rb create mode 100644 app/components/work_packages/activities_tab/journals/item_component/reactions.html.erb create mode 100644 app/components/work_packages/activities_tab/journals/item_component/reactions.rb create mode 100644 app/contracts/emoji_reactions/base_contract.rb create mode 100644 app/contracts/emoji_reactions/create_contract.rb create mode 100644 app/contracts/emoji_reactions/delete_contract.rb create mode 100644 app/models/concerns/reactable.rb create mode 100644 app/models/emoji_reaction.rb create mode 100644 app/services/emoji_reactions/create_service.rb create mode 100644 app/services/emoji_reactions/delete_service.rb create mode 100644 app/services/emoji_reactions/set_attributes_service.rb create mode 100644 db/migrate/20240624103354_create_emoji_reactions.rb diff --git a/app/components/work_packages/activities_tab/index_component.rb b/app/components/work_packages/activities_tab/index_component.rb index a22f51a5f2c2..9abed8e499e0 100644 --- a/app/components/work_packages/activities_tab/index_component.rb +++ b/app/components/work_packages/activities_tab/index_component.rb @@ -53,7 +53,7 @@ def wrapper_data_attributes "work-packages--activities-tab--index-update-streams-url-value": update_streams_work_package_activities_url(work_package), "work-packages--activities-tab--index-sorting-value": journal_sorting, "work-packages--activities-tab--index-filter-value": filter, - "work-packages--activities-tab--index-polling-interval-in-ms-value": 60000 # protoypical implementation + "work-packages--activities-tab--index-polling-interval-in-ms-value": 10000 # protoypical implementation } end diff --git a/app/components/work_packages/activities_tab/journals/item_component/add_reactions.html.erb b/app/components/work_packages/activities_tab/journals/item_component/add_reactions.html.erb new file mode 100644 index 000000000000..4c43bb9eafb5 --- /dev/null +++ b/app/components/work_packages/activities_tab/journals/item_component/add_reactions.html.erb @@ -0,0 +1,33 @@ +<%= + component_wrapper do + if journal.available_untaken_emojis.any? + render(Primer::Alpha::Overlay.new( + title: I18n.t("reactions.action_title"), + padding: :condensed, + anchor_side: :outside_top, + visually_hide_title: true + )) do |d| + # d.with_header(title: "React") + d.with_show_button(icon: "smiley", "aria-label": I18n.t("reactions.add_reaction"), title: I18n.t("reactions.add_reaction")) + d.with_body(pt: 2) do + flex_layout do |add_reactions_container| + journal.available_untaken_emojis.each do |emoji| + add_reactions_container.with_column(mr: 2) do + render(Primer::Beta::Button.new( + scheme: :invisible, + id: "#{journal.id}-#{emoji}", + tag: :a, + href: toggle_reaction_work_package_activity_path(journal.journable.id, id: journal.id, emoji: emoji), + data: { "turbo-stream": true, "turbo-method": :put}, + "aria-label": I18n.t("reactions.react_with", emoji_shortcode: EmojiReaction.shortcode(emoji)), + )) do + emoji.html_safe + end + end + end + end + end + end + end + end +%> \ No newline at end of file diff --git a/app/components/work_packages/activities_tab/journals/item_component/add_reactions.rb b/app/components/work_packages/activities_tab/journals/item_component/add_reactions.rb new file mode 100644 index 000000000000..071965999192 --- /dev/null +++ b/app/components/work_packages/activities_tab/journals/item_component/add_reactions.rb @@ -0,0 +1,53 @@ +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) 2012-2023 the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +module WorkPackages + module ActivitiesTab + module Journals + class ItemComponent::AddReactions < ApplicationComponent + include ApplicationHelper + include OpPrimer::ComponentHelpers + include OpTurbo::Streamable + + def initialize(journal:) + super + + @journal = journal + end + + private + + attr_reader :journal + + def wrapper_uniq_by + journal.id + end + end + end + end +end diff --git a/app/components/work_packages/activities_tab/journals/item_component/reactions.html.erb b/app/components/work_packages/activities_tab/journals/item_component/reactions.html.erb new file mode 100644 index 000000000000..4916837ce607 --- /dev/null +++ b/app/components/work_packages/activities_tab/journals/item_component/reactions.html.erb @@ -0,0 +1,21 @@ +<%= + component_wrapper do + if journal.detailed_grouped_emoji_reactions.any? + flex_layout do |reactions_container| + journal.detailed_grouped_emoji_reactions.each do |emoji, data| + reactions_container.with_column(mr: 2) do + render(Primer::Beta::Button.new( + id: "#{journal.id}-#{emoji}", + tag: :a, + href: toggle_reaction_work_package_activity_path(journal.journable.id, id: journal.id, emoji: emoji), + data: { "turbo-stream": true, "turbo-method": :put} + )) do |button| + button.with_tooltip(text: tooltip_text(emoji, data[:users])) + "#{emoji} #{data[:count]}".html_safe + end + end + end + end + end + end +%> \ No newline at end of file diff --git a/app/components/work_packages/activities_tab/journals/item_component/reactions.rb b/app/components/work_packages/activities_tab/journals/item_component/reactions.rb new file mode 100644 index 000000000000..1efdb987733c --- /dev/null +++ b/app/components/work_packages/activities_tab/journals/item_component/reactions.rb @@ -0,0 +1,72 @@ +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) 2012-2023 the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +module WorkPackages + module ActivitiesTab + module Journals + class ItemComponent::Reactions < ApplicationComponent + include ApplicationHelper + include OpPrimer::ComponentHelpers + include OpTurbo::Streamable + + def initialize(journal:) + super + + @journal = journal + end + + private + + attr_reader :journal + + def wrapper_uniq_by + journal.id + end + + def tooltip_text(emoji, users) + max_displayed_users_count = 5 + + user_count = users.length + displayed_users = users.take(max_displayed_users_count).map { |u| u[:name] } + + result = if user_count <= max_displayed_users_count + displayed_users.join(', ') + elsif user_count == max_displayed_users_count + 1 + "#{displayed_users.join(', ')} #{I18n.t("reactions.and_n_others_singular", n: 1)}" + else + "#{displayed_users.join(', ')} #{I18n.t("reactions.and_n_others_plural", n: user_count - max_displayed_users_count)}" + end + + result += " " + + result += I18n.t("reactions.reacted_with", emoji_shortcode: EmojiReaction.shortcode(emoji)) + end + end + end + end +end diff --git a/app/components/work_packages/activities_tab/journals/item_component/show.html.erb b/app/components/work_packages/activities_tab/journals/item_component/show.html.erb index 797b27b85820..1fa394348029 100644 --- a/app/components/work_packages/activities_tab/journals/item_component/show.html.erb +++ b/app/components/work_packages/activities_tab/journals/item_component/show.html.erb @@ -18,6 +18,14 @@ end end end + journal_container.with_row(mt: 3, flex_layout: true) do |reactions_container| + reactions_container.with_column do + render(WorkPackages::ActivitiesTab::Journals::ItemComponent::Reactions.new(journal:)) + end + reactions_container.with_column do + render(WorkPackages::ActivitiesTab::Journals::ItemComponent::AddReactions.new(journal:)) + end + end end end end diff --git a/app/contracts/emoji_reactions/base_contract.rb b/app/contracts/emoji_reactions/base_contract.rb new file mode 100644 index 000000000000..821f801aa49e --- /dev/null +++ b/app/contracts/emoji_reactions/base_contract.rb @@ -0,0 +1,81 @@ +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) 2012-2024 the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +module EmojiReactions + class BaseContract < ::ModelContract + attribute :emoji + attribute :user_id + attribute :reactable_id + attribute :reactable_type + + validate :manage_emoji_reactions_permission? + validate :validate_user_exists + validate :validate_acting_user + validate :validate_reactable_exists + validate :validate_emoji_type + + def self.model + EmojiReaction + end + + private + + def validate_user_exists + errors.add :user, :error_not_found unless User.exists?(model.user_id) + end + + def validate_acting_user + errors.add :user, :error_unauthorized unless model.user_id == user.id + end + + def validate_reactable_exists + errors.add :reactable, :error_not_found unless model.reactable.present? + end + + def validate_emoji_type + errors.add :emoji, :inclusion unless EmojiReaction::AVAILABLE_EMOJIS.include?(model.emoji) + end + + def manage_emoji_reactions_permission? + unless manage_emoji_reactions? + errors.add :base, :error_unauthorized + end + end + + def manage_emoji_reactions? + case model.reactable + when WorkPackage + user.allowed_in_work_package?(:add_work_package_notes, model.reactable) + when Journal + user.allowed_in_work_package?(:add_work_package_notes, model.reactable.journable) + else + false + end + end + end +end diff --git a/app/contracts/emoji_reactions/create_contract.rb b/app/contracts/emoji_reactions/create_contract.rb new file mode 100644 index 000000000000..b3cf59f48a71 --- /dev/null +++ b/app/contracts/emoji_reactions/create_contract.rb @@ -0,0 +1,32 @@ +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) 2012-2024 the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +module EmojiReactions + class CreateContract < BaseContract + end +end diff --git a/app/contracts/emoji_reactions/delete_contract.rb b/app/contracts/emoji_reactions/delete_contract.rb new file mode 100644 index 000000000000..385fe7bb5b26 --- /dev/null +++ b/app/contracts/emoji_reactions/delete_contract.rb @@ -0,0 +1,36 @@ +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) 2012-2024 the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +module EmojiReactions + class DeleteContract < ::DeleteContract + delete_permission -> { + # The user can delete the reaction if they created it + model.user_id == user.id + } + end +end diff --git a/app/controllers/work_packages/activities_tab_controller.rb b/app/controllers/work_packages/activities_tab_controller.rb index a8fdc66e1158..bf450701616f 100644 --- a/app/controllers/work_packages/activities_tab_controller.rb +++ b/app/controllers/work_packages/activities_tab_controller.rb @@ -33,7 +33,7 @@ class WorkPackages::ActivitiesTabController < ApplicationController before_action :find_work_package before_action :find_project - before_action :find_journal, only: %i[edit cancel_edit update] + before_action :find_journal, only: %i[edit cancel_edit update toggle_reaction] before_action :authorize def index @@ -66,7 +66,8 @@ def update_filter end def update_streams - generate_time_based_update_streams(params[:last_update_timestamp], params[:filter]) + generate_time_based_journal_update_streams(params[:last_update_timestamp], params[:filter]) + generate_time_based_reaction_update_streams(params[:last_update_timestamp]) respond_with_turbo_streams end @@ -104,7 +105,8 @@ def create ### if call.success? && call.result - generate_time_based_update_streams(params[:last_update_timestamp], params[:filter]) + generate_time_based_journal_update_streams(params[:last_update_timestamp], params[:filter]) + generate_time_based_reaction_update_streams(params[:last_update_timestamp]) end clear_form_via_turbo_stream @@ -156,6 +158,31 @@ def update_sorting respond_with_turbo_streams end + def toggle_reaction + if @journal.emoji_reactions.exists?(user: User.current, emoji: params[:emoji]) + call = EmojiReactions::DeleteService.new( + user: User.current, + model: @journal.emoji_reactions.find_by(user: User.current, emoji: params[:emoji]) + ).call + else + call = EmojiReactions::CreateService.new( + user: User.current + ).call( + user: User.current, + reactable: @journal, + emoji: params[:emoji] + ) + end + + if call.success? + update_journal_via_turbo_stream(@journal) + else + # TODO: handle errors + end + + respond_with_turbo_streams + end + private def find_work_package @@ -178,8 +205,8 @@ def journal_params params.require(:journal).permit(:notes) end - def generate_time_based_update_streams(last_update_timestamp, filter) - # TODO: prototypical implementation + def generate_time_based_journal_update_streams(last_update_timestamp, filter) + # TODO: prototypical implementation! journals = @work_package.journals if filter == "only_comments" @@ -206,6 +233,19 @@ def generate_time_based_update_streams(last_update_timestamp, filter) end end + def generate_time_based_reaction_update_streams(last_update_timestamp) + # Current limitation: Only shows added reactions, not removed ones + EmojiReaction + .where(reactable_id: @work_package.journal_ids) + .where("updated_at > ?", last_update_timestamp).find_each do |reaction| + update_via_turbo_stream( + component: WorkPackages::ActivitiesTab::Journals::ItemComponent::Reactions.new( + journal: reaction.reactable + ) + ) + end + end + def append_or_prepend_latest_journal_via_turbo_stream(journal, latest_journal) if latest_journal.created_at.to_date == journal.created_at.to_date target_component = WorkPackages::ActivitiesTab::Journals::DayComponent.new( diff --git a/app/models/concerns/reactable.rb b/app/models/concerns/reactable.rb new file mode 100644 index 000000000000..5eb2ad85e716 --- /dev/null +++ b/app/models/concerns/reactable.rb @@ -0,0 +1,69 @@ +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) 2012-2024 the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +module Reactable + extend ActiveSupport::Concern + + included do + has_many :emoji_reactions, as: :reactable, dependent: :destroy + end + + def add_reaction(user, emoji) + emoji_reactions.create(user: user, emoji: emoji) + end + + def remove_reaction(user, emoji) + emoji_reactions.find_by(user: user, emoji: emoji)&.destroy + end + + def grouped_emoji_reactions + emoji_reactions.group(:emoji).count + end + + def detailed_grouped_emoji_reactions + emoji_reactions + .select('emoji, COUNT(*) as count, ARRAY_AGG(user_id) as user_ids') + .group(:emoji) + .map do |result| + users = User.where(id: result.user_ids).select(:id, :firstname, :lastname) + { + emoji: result.emoji, + count: result.count, + users: users.map { |u| { id: u.id, name: u.name } } + } + end.index_by { |r| r[:emoji] } + end + + def available_emojis + EmojiReaction.available_emojis + end + + def available_untaken_emojis + EmojiReaction.available_emojis - detailed_grouped_emoji_reactions.keys + end +end diff --git a/app/models/emoji_reaction.rb b/app/models/emoji_reaction.rb new file mode 100644 index 000000000000..ef998557ee6d --- /dev/null +++ b/app/models/emoji_reaction.rb @@ -0,0 +1,56 @@ +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) 2012-2024 the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +class EmojiReaction < ApplicationRecord + EMOJI_MAP = { + thumbs_up: "👍", + thumbs_down: "👎", + grinning_face_with_smiling_eyes: "😄", + confused_face: "😕", + heart: "❤", + party_popper: "🎉", + rocket: "🚀", + eyes: "👀" + }.freeze + + AVAILABLE_EMOJIS = EMOJI_MAP.values.freeze + + belongs_to :user + belongs_to :reactable, polymorphic: true + + validates :emoji, presence: true, inclusion: { in: AVAILABLE_EMOJIS } + validates :user_id, uniqueness: { scope: [:reactable_type, :reactable_id, :emoji] } + + def self.available_emojis + AVAILABLE_EMOJIS + end + + def self.shortcode(html_code) + EMOJI_MAP.key(html_code)&.to_s&.prepend(':')&.concat(':') || html_code + end +end diff --git a/app/models/journal.rb b/app/models/journal.rb index 1a9229a397a3..9a62e756eeee 100644 --- a/app/models/journal.rb +++ b/app/models/journal.rb @@ -34,6 +34,7 @@ class Journal < ApplicationRecord include ::JournalFormatter include ::Acts::Journalized::FormatHooks include Journal::Timestamps + include Reactable register_journal_formatter :diff, OpenProject::JournalFormatter::Diff register_journal_formatter :attachment, OpenProject::JournalFormatter::Attachment diff --git a/app/services/emoji_reactions/create_service.rb b/app/services/emoji_reactions/create_service.rb new file mode 100644 index 000000000000..5fd2f0a683b0 --- /dev/null +++ b/app/services/emoji_reactions/create_service.rb @@ -0,0 +1,32 @@ +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) 2012-2024 the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +module EmojiReactions + class CreateService < ::BaseServices::Create + end +end diff --git a/app/services/emoji_reactions/delete_service.rb b/app/services/emoji_reactions/delete_service.rb new file mode 100644 index 000000000000..aa4d5c63fa7a --- /dev/null +++ b/app/services/emoji_reactions/delete_service.rb @@ -0,0 +1,32 @@ +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) 2012-2024 the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +module EmojiReactions + class DeleteService < ::BaseServices::Delete + end +end diff --git a/app/services/emoji_reactions/set_attributes_service.rb b/app/services/emoji_reactions/set_attributes_service.rb new file mode 100644 index 000000000000..d37f853468eb --- /dev/null +++ b/app/services/emoji_reactions/set_attributes_service.rb @@ -0,0 +1,32 @@ +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) 2012-2024 the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +module EmojiReactions + class SetAttributesService < ::BaseServices::SetAttributes + end +end diff --git a/config/initializers/permissions.rb b/config/initializers/permissions.rb index 14ae47ef67db..909966926901 100644 --- a/config/initializers/permissions.rb +++ b/config/initializers/permissions.rb @@ -206,7 +206,7 @@ work_packages_api: [:get], "work_packages/reports": %i[report report_details], "work_packages/activities_tab": %i[index create update_streams edit cancel_edit update - update_sorting update_filter] + update_sorting update_filter toggle_reaction] }, permissible_on: %i[work_package project], contract_actions: { work_packages: %i[read] } diff --git a/config/locales/en.yml b/config/locales/en.yml index 501216bef429..738962d4a9fa 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -501,6 +501,14 @@ Project attributes and sections are defined in the Date: Tue, 2 Jul 2024 21:06:27 +0200 Subject: [PATCH 03/43] implemented feedback provided by @psatyal and sorting fix --- .../journals/item_component/add_reactions.html.erb | 2 +- .../journals/item_component/reactions.html.erb | 1 + .../activities_tab/journals/item_component/reactions.rb | 8 ++++++++ .../activities_tab/journals/item_component/show.html.erb | 4 ++-- app/models/concerns/reactable.rb | 5 +++-- 5 files changed, 15 insertions(+), 5 deletions(-) diff --git a/app/components/work_packages/activities_tab/journals/item_component/add_reactions.html.erb b/app/components/work_packages/activities_tab/journals/item_component/add_reactions.html.erb index 4c43bb9eafb5..bbabb4d5fcdd 100644 --- a/app/components/work_packages/activities_tab/journals/item_component/add_reactions.html.erb +++ b/app/components/work_packages/activities_tab/journals/item_component/add_reactions.html.erb @@ -8,7 +8,7 @@ visually_hide_title: true )) do |d| # d.with_header(title: "React") - d.with_show_button(icon: "smiley", "aria-label": I18n.t("reactions.add_reaction"), title: I18n.t("reactions.add_reaction")) + d.with_show_button(icon: "smiley", "aria-label": I18n.t("reactions.add_reaction"), title: I18n.t("reactions.add_reaction"), mr: 2) d.with_body(pt: 2) do flex_layout do |add_reactions_container| journal.available_untaken_emojis.each do |emoji| diff --git a/app/components/work_packages/activities_tab/journals/item_component/reactions.html.erb b/app/components/work_packages/activities_tab/journals/item_component/reactions.html.erb index 4916837ce607..809dda4b4e6c 100644 --- a/app/components/work_packages/activities_tab/journals/item_component/reactions.html.erb +++ b/app/components/work_packages/activities_tab/journals/item_component/reactions.html.erb @@ -5,6 +5,7 @@ journal.detailed_grouped_emoji_reactions.each do |emoji, data| reactions_container.with_column(mr: 2) do render(Primer::Beta::Button.new( + bg: counter_color(data[:users]), id: "#{journal.id}-#{emoji}", tag: :a, href: toggle_reaction_work_package_activity_path(journal.journable.id, id: journal.id, emoji: emoji), diff --git a/app/components/work_packages/activities_tab/journals/item_component/reactions.rb b/app/components/work_packages/activities_tab/journals/item_component/reactions.rb index 1efdb987733c..2c35315dc305 100644 --- a/app/components/work_packages/activities_tab/journals/item_component/reactions.rb +++ b/app/components/work_packages/activities_tab/journals/item_component/reactions.rb @@ -48,6 +48,14 @@ def wrapper_uniq_by journal.id end + def reacted_by_current_user?(users) + users.any? { |u| u[:id] == User.current.id } + end + + def counter_color(users) + reacted_by_current_user?(users) ? :accent : :default + end + def tooltip_text(emoji, users) max_displayed_users_count = 5 diff --git a/app/components/work_packages/activities_tab/journals/item_component/show.html.erb b/app/components/work_packages/activities_tab/journals/item_component/show.html.erb index 1fa394348029..c1b8e1ef0256 100644 --- a/app/components/work_packages/activities_tab/journals/item_component/show.html.erb +++ b/app/components/work_packages/activities_tab/journals/item_component/show.html.erb @@ -20,10 +20,10 @@ end journal_container.with_row(mt: 3, flex_layout: true) do |reactions_container| reactions_container.with_column do - render(WorkPackages::ActivitiesTab::Journals::ItemComponent::Reactions.new(journal:)) + render(WorkPackages::ActivitiesTab::Journals::ItemComponent::AddReactions.new(journal:)) end reactions_container.with_column do - render(WorkPackages::ActivitiesTab::Journals::ItemComponent::AddReactions.new(journal:)) + render(WorkPackages::ActivitiesTab::Journals::ItemComponent::Reactions.new(journal:)) end end end diff --git a/app/models/concerns/reactable.rb b/app/models/concerns/reactable.rb index 5eb2ad85e716..8da29c301c24 100644 --- a/app/models/concerns/reactable.rb +++ b/app/models/concerns/reactable.rb @@ -49,6 +49,7 @@ def detailed_grouped_emoji_reactions emoji_reactions .select('emoji, COUNT(*) as count, ARRAY_AGG(user_id) as user_ids') .group(:emoji) + .order('emoji ASC') .map do |result| users = User.where(id: result.user_ids).select(:id, :firstname, :lastname) { @@ -60,10 +61,10 @@ def detailed_grouped_emoji_reactions end def available_emojis - EmojiReaction.available_emojis + EmojiReaction.available_emojis.sort end def available_untaken_emojis - EmojiReaction.available_emojis - detailed_grouped_emoji_reactions.keys + (EmojiReaction.available_emojis - detailed_grouped_emoji_reactions.keys).sort end end From 7da1bf53d84c92fab668a2c3b95db1a05146e19c Mon Sep 17 00:00:00 2001 From: Jonas Jabari Date: Tue, 2 Jul 2024 21:38:24 +0200 Subject: [PATCH 04/43] avoid n+1 queries --- app/models/concerns/reactable.rb | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/app/models/concerns/reactable.rb b/app/models/concerns/reactable.rb index 8da29c301c24..a6b658aa858d 100644 --- a/app/models/concerns/reactable.rb +++ b/app/models/concerns/reactable.rb @@ -46,18 +46,25 @@ def grouped_emoji_reactions end def detailed_grouped_emoji_reactions - emoji_reactions + # TODO: Refactor this to be database agnostic + # fetch all emoji reactions and group them by emoji with their count and user ids + emoji_groups = emoji_reactions .select('emoji, COUNT(*) as count, ARRAY_AGG(user_id) as user_ids') .group(:emoji) .order('emoji ASC') - .map do |result| - users = User.where(id: result.user_ids).select(:id, :firstname, :lastname) - { - emoji: result.emoji, - count: result.count, - users: users.map { |u| { id: u.id, name: u.name } } - } - end.index_by { |r| r[:emoji] } + + # avoid N+1 queries by preloading all reacting users + user_ids = emoji_groups.flat_map(&:user_ids).uniq + users = User.where(id: user_ids).select(:id, :firstname, :lastname).index_by(&:id) + + # convert the result to a hash indexed by emoji suitable for rendering + emoji_groups.map do |result| + { + emoji: result.emoji, + count: result.count, + users: result.user_ids.map { |id| { id: id, name: users[id].name } } + } + end.index_by { |r| r[:emoji] } end def available_emojis From 3c9da7740509ee2e465015f37ad613b4eb0cb3bc Mon Sep 17 00:00:00 2001 From: Jonas Jabari Date: Wed, 3 Jul 2024 09:29:22 +0200 Subject: [PATCH 05/43] fixing focus loss on mobile --- .../dynamic/work-packages/activities-tab/index.controller.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/index.controller.ts b/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/index.controller.ts index 0ff411fe5c13..b433f8c6c755 100644 --- a/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/index.controller.ts +++ b/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/index.controller.ts @@ -171,6 +171,10 @@ export default class IndexController extends Controller { this.addEventListenersToCkEditorInstance(); + this.focusEditor(); + } + + focusEditor() { const ckEditorInstance = this.getCkEditorInstance(); if (ckEditorInstance) { setTimeout(() => ckEditorInstance.editing.view.focus(), 10); @@ -239,6 +243,7 @@ export default class IndexController extends Controller { if (this.journalsContainerTarget) { this.clearEditor(); + this.focusEditor(); if (this.journalsContainerTarget) { this.journalsContainerTarget.style.marginBottom = ''; this.journalsContainerTarget.classList.add('with-input-compensation'); From 81d62c30592a8ff14c0c9383c79a016f82500dce Mon Sep 17 00:00:00 2001 From: Jonas Jabari Date: Wed, 10 Jul 2024 15:54:07 +0200 Subject: [PATCH 06/43] merged and fixed base branch --- app/controllers/work_packages/activities_tab_controller.rb | 7 ++++++- .../work-packages/activities-tab/index.controller.ts | 7 ------- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/app/controllers/work_packages/activities_tab_controller.rb b/app/controllers/work_packages/activities_tab_controller.rb index 83a568c7b5aa..2c5ff8ae1e49 100644 --- a/app/controllers/work_packages/activities_tab_controller.rb +++ b/app/controllers/work_packages/activities_tab_controller.rb @@ -184,7 +184,12 @@ def toggle_reaction end if call.success? - update_journal_via_turbo_stream(@journal) + update_via_turbo_stream( + component: WorkPackages::ActivitiesTab::Journals::ItemComponent::Show.new( + journal: @journal, + filter: params[:filter]&.to_sym || :all + ) + ) else # TODO: handle errors end diff --git a/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/index.controller.ts b/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/index.controller.ts index 89cffe46770c..91205465bb1b 100644 --- a/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/index.controller.ts +++ b/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/index.controller.ts @@ -246,13 +246,6 @@ export default class IndexController extends Controller { } } - focusEditor() { - const ckEditorInstance = this.getCkEditorInstance(); - if (ckEditorInstance) { - setTimeout(() => ckEditorInstance.editing.view.focus(), 10); - } - } - quote(event:Event) { event.preventDefault(); const target = event.currentTarget as HTMLElement; From 71966de704caaf48394aea652332773aa97d3760 Mon Sep 17 00:00:00 2001 From: Jonas Jabari Date: Tue, 17 Sep 2024 15:19:15 +0200 Subject: [PATCH 07/43] fixed comment rendering --- .../journals/item_component/show.html.erb | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/app/components/work_packages/activities_tab/journals/item_component/show.html.erb b/app/components/work_packages/activities_tab/journals/item_component/show.html.erb index 0b9aa9bee815..ca5bae75d1de 100644 --- a/app/components/work_packages/activities_tab/journals/item_component/show.html.erb +++ b/app/components/work_packages/activities_tab/journals/item_component/show.html.erb @@ -6,13 +6,13 @@ render(Primer::Box.new(mt: 1)) do format_text(journal, :notes) end - journal_container.with_row(mt: 3, flex_layout: true) do |reactions_container| - reactions_container.with_column do - render(WorkPackages::ActivitiesTab::Journals::ItemComponent::AddReactions.new(journal:)) - end - reactions_container.with_column do - render(WorkPackages::ActivitiesTab::Journals::ItemComponent::Reactions.new(journal:)) - end + end + journal_container.with_row(mt: 3, flex_layout: true) do |reactions_container| + reactions_container.with_column do + render(WorkPackages::ActivitiesTab::Journals::ItemComponent::AddReactions.new(journal:)) + end + reactions_container.with_column do + render(WorkPackages::ActivitiesTab::Journals::ItemComponent::Reactions.new(journal:)) end end end From 338174f27efaab1c57d506e737eab6d0b1dd2e0b Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Mon, 7 Oct 2024 13:40:48 +0300 Subject: [PATCH 08/43] rubocop auto-amendments --- .../journals/item_component/reactions.rb | 16 +++++++++------- app/contracts/emoji_reactions/base_contract.rb | 2 +- app/models/concerns/reactable.rb | 10 +++++----- app/models/emoji_reaction.rb | 4 ++-- .../20240624103354_create_emoji_reactions.rb | 3 ++- 5 files changed, 19 insertions(+), 16 deletions(-) diff --git a/app/components/work_packages/activities_tab/journals/item_component/reactions.rb b/app/components/work_packages/activities_tab/journals/item_component/reactions.rb index 2c35315dc305..8fd1605a5e13 100644 --- a/app/components/work_packages/activities_tab/journals/item_component/reactions.rb +++ b/app/components/work_packages/activities_tab/journals/item_component/reactions.rb @@ -60,19 +60,21 @@ def tooltip_text(emoji, users) max_displayed_users_count = 5 user_count = users.length - displayed_users = users.take(max_displayed_users_count).map { |u| u[:name] } + displayed_users = users.take(max_displayed_users_count).pluck(:name) result = if user_count <= max_displayed_users_count - displayed_users.join(', ') - elsif user_count == max_displayed_users_count + 1 - "#{displayed_users.join(', ')} #{I18n.t("reactions.and_n_others_singular", n: 1)}" - else - "#{displayed_users.join(', ')} #{I18n.t("reactions.and_n_others_plural", n: user_count - max_displayed_users_count)}" - end + displayed_users.join(", ") + elsif user_count == max_displayed_users_count + 1 + "#{displayed_users.join(', ')} #{I18n.t('reactions.and_n_others_singular', n: 1)}" + else + "#{displayed_users.join(', ')} #{I18n.t('reactions.and_n_others_plural', + n: user_count - max_displayed_users_count)}" + end result += " " result += I18n.t("reactions.reacted_with", emoji_shortcode: EmojiReaction.shortcode(emoji)) + result end end end diff --git a/app/contracts/emoji_reactions/base_contract.rb b/app/contracts/emoji_reactions/base_contract.rb index 821f801aa49e..8a0658291d79 100644 --- a/app/contracts/emoji_reactions/base_contract.rb +++ b/app/contracts/emoji_reactions/base_contract.rb @@ -54,7 +54,7 @@ def validate_acting_user end def validate_reactable_exists - errors.add :reactable, :error_not_found unless model.reactable.present? + errors.add :reactable, :error_not_found if model.reactable.blank? end def validate_emoji_type diff --git a/app/models/concerns/reactable.rb b/app/models/concerns/reactable.rb index a6b658aa858d..ef5eb26e433e 100644 --- a/app/models/concerns/reactable.rb +++ b/app/models/concerns/reactable.rb @@ -34,11 +34,11 @@ module Reactable end def add_reaction(user, emoji) - emoji_reactions.create(user: user, emoji: emoji) + emoji_reactions.create(user:, emoji:) end def remove_reaction(user, emoji) - emoji_reactions.find_by(user: user, emoji: emoji)&.destroy + emoji_reactions.find_by(user:, emoji:)&.destroy end def grouped_emoji_reactions @@ -49,9 +49,9 @@ def detailed_grouped_emoji_reactions # TODO: Refactor this to be database agnostic # fetch all emoji reactions and group them by emoji with their count and user ids emoji_groups = emoji_reactions - .select('emoji, COUNT(*) as count, ARRAY_AGG(user_id) as user_ids') + .select("emoji, COUNT(*) as count, ARRAY_AGG(user_id) as user_ids") .group(:emoji) - .order('emoji ASC') + .order("emoji ASC") # avoid N+1 queries by preloading all reacting users user_ids = emoji_groups.flat_map(&:user_ids).uniq @@ -62,7 +62,7 @@ def detailed_grouped_emoji_reactions { emoji: result.emoji, count: result.count, - users: result.user_ids.map { |id| { id: id, name: users[id].name } } + users: result.user_ids.map { |id| { id:, name: users[id].name } } } end.index_by { |r| r[:emoji] } end diff --git a/app/models/emoji_reaction.rb b/app/models/emoji_reaction.rb index ef998557ee6d..15c8468f37da 100644 --- a/app/models/emoji_reaction.rb +++ b/app/models/emoji_reaction.rb @@ -44,13 +44,13 @@ class EmojiReaction < ApplicationRecord belongs_to :reactable, polymorphic: true validates :emoji, presence: true, inclusion: { in: AVAILABLE_EMOJIS } - validates :user_id, uniqueness: { scope: [:reactable_type, :reactable_id, :emoji] } + validates :user_id, uniqueness: { scope: %i[reactable_type reactable_id emoji] } def self.available_emojis AVAILABLE_EMOJIS end def self.shortcode(html_code) - EMOJI_MAP.key(html_code)&.to_s&.prepend(':')&.concat(':') || html_code + EMOJI_MAP.key(html_code)&.to_s&.prepend(":")&.concat(":") || html_code end end diff --git a/db/migrate/20240624103354_create_emoji_reactions.rb b/db/migrate/20240624103354_create_emoji_reactions.rb index 365aa4b7603f..d0f6ac11456f 100644 --- a/db/migrate/20240624103354_create_emoji_reactions.rb +++ b/db/migrate/20240624103354_create_emoji_reactions.rb @@ -7,6 +7,7 @@ def change t.timestamps end - add_index :emoji_reactions, [:user_id, :reactable_type, :reactable_id, :emoji], unique: true, name: 'index_emoji_reactions_uniqueness' + add_index :emoji_reactions, %i[user_id reactable_type reactable_id emoji], unique: true, + name: "index_emoji_reactions_uniqueness" end end From 0f17cee64ab9698f7a2c0536f973fd6ba0830504 Mon Sep 17 00:00:00 2001 From: Jonas Jabari Date: Tue, 8 Oct 2024 09:58:43 +0200 Subject: [PATCH 09/43] applied invisible style to reactions where the current user itself has not reacted as requested by @psatyal --- .../journals/item_component/reactions.html.erb | 2 ++ .../activities_tab/journals/item_component/reactions.rb | 6 +++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/app/components/work_packages/activities_tab/journals/item_component/reactions.html.erb b/app/components/work_packages/activities_tab/journals/item_component/reactions.html.erb index 809dda4b4e6c..2a6404dbc167 100644 --- a/app/components/work_packages/activities_tab/journals/item_component/reactions.html.erb +++ b/app/components/work_packages/activities_tab/journals/item_component/reactions.html.erb @@ -5,6 +5,8 @@ journal.detailed_grouped_emoji_reactions.each do |emoji, data| reactions_container.with_column(mr: 2) do render(Primer::Beta::Button.new( + scheme: button_scheme(data[:users]), + color: :default, bg: counter_color(data[:users]), id: "#{journal.id}-#{emoji}", tag: :a, diff --git a/app/components/work_packages/activities_tab/journals/item_component/reactions.rb b/app/components/work_packages/activities_tab/journals/item_component/reactions.rb index 8fd1605a5e13..537c5d7f8878 100644 --- a/app/components/work_packages/activities_tab/journals/item_component/reactions.rb +++ b/app/components/work_packages/activities_tab/journals/item_component/reactions.rb @@ -53,7 +53,11 @@ def reacted_by_current_user?(users) end def counter_color(users) - reacted_by_current_user?(users) ? :accent : :default + reacted_by_current_user?(users) ? :accent : nil + end + + def button_scheme(users) + reacted_by_current_user?(users) ? :default : :invisible end def tooltip_text(emoji, users) From 14108bb4504dca6676cbd3160d0c424260c07534 Mon Sep 17 00:00:00 2001 From: Jonas Jabari Date: Tue, 8 Oct 2024 11:44:55 +0200 Subject: [PATCH 10/43] fixed merge --- app/controllers/work_packages/activities_tab_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/work_packages/activities_tab_controller.rb b/app/controllers/work_packages/activities_tab_controller.rb index 5a847b39f047..2864a27adf18 100644 --- a/app/controllers/work_packages/activities_tab_controller.rb +++ b/app/controllers/work_packages/activities_tab_controller.rb @@ -323,7 +323,7 @@ def generate_time_based_update_streams(last_update_timestamp) rerender_journals_with_updated_notification(journals, last_update_timestamp) - append_or_prepend_new_journals(journals, last_update_timestamp) + append_or_prepend_journals(journals, last_update_timestamp) if journals.any? remove_potential_empty_state From 6ef94ef390d2b467ea0088fd7bc2736098e6da2a Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Tue, 8 Oct 2024 18:45:15 +0300 Subject: [PATCH 11/43] =?UTF-8?q?tests[Op#40437]:=20begin=20emoji=20reacti?= =?UTF-8?q?ons=20feature=20tests=20=F0=9F=91=8D=F0=9F=8F=BE?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../item_component/add_reactions.html.erb | 14 ++- .../item_component/reactions.html.erb | 8 +- .../work_package/emoji_reactions_spec.rb | 95 +++++++++++++++++++ .../work_packages/emoji_reactions.rb | 61 ++++++++++++ 4 files changed, 170 insertions(+), 8 deletions(-) create mode 100644 spec/features/activities/work_package/emoji_reactions_spec.rb create mode 100644 spec/support/components/work_packages/emoji_reactions.rb diff --git a/app/components/work_packages/activities_tab/journals/item_component/add_reactions.html.erb b/app/components/work_packages/activities_tab/journals/item_component/add_reactions.html.erb index bbabb4d5fcdd..00410f2c9b37 100644 --- a/app/components/work_packages/activities_tab/journals/item_component/add_reactions.html.erb +++ b/app/components/work_packages/activities_tab/journals/item_component/add_reactions.html.erb @@ -7,9 +7,15 @@ anchor_side: :outside_top, visually_hide_title: true )) do |d| - # d.with_header(title: "React") - d.with_show_button(icon: "smiley", "aria-label": I18n.t("reactions.add_reaction"), title: I18n.t("reactions.add_reaction"), mr: 2) - d.with_body(pt: 2) do + d.with_show_button( + icon: "smiley", + "aria-label": I18n.t("reactions.add_reaction"), + title: I18n.t("reactions.add_reaction"), + mr: 2, + test_selector: "add-reaction-button" + ) + + d.with_body(pt: 2) do flex_layout do |add_reactions_container| journal.available_untaken_emojis.each do |emoji| add_reactions_container.with_column(mr: 2) do @@ -30,4 +36,4 @@ end end end -%> \ No newline at end of file +%> diff --git a/app/components/work_packages/activities_tab/journals/item_component/reactions.html.erb b/app/components/work_packages/activities_tab/journals/item_component/reactions.html.erb index 2a6404dbc167..289f6a65d013 100644 --- a/app/components/work_packages/activities_tab/journals/item_component/reactions.html.erb +++ b/app/components/work_packages/activities_tab/journals/item_component/reactions.html.erb @@ -1,7 +1,7 @@ <%= component_wrapper do - if journal.detailed_grouped_emoji_reactions.any? - flex_layout do |reactions_container| + if journal.detailed_grouped_emoji_reactions.any? + flex_layout(test_selector: 'emoji-reactions') do |reactions_container| journal.detailed_grouped_emoji_reactions.each do |emoji, data| reactions_container.with_column(mr: 2) do render(Primer::Beta::Button.new( @@ -11,7 +11,7 @@ id: "#{journal.id}-#{emoji}", tag: :a, href: toggle_reaction_work_package_activity_path(journal.journable.id, id: journal.id, emoji: emoji), - data: { "turbo-stream": true, "turbo-method": :put} + data: { "turbo-stream": true, "turbo-method": :put } )) do |button| button.with_tooltip(text: tooltip_text(emoji, data[:users])) "#{emoji} #{data[:count]}".html_safe @@ -21,4 +21,4 @@ end end end -%> \ No newline at end of file +%> diff --git a/spec/features/activities/work_package/emoji_reactions_spec.rb b/spec/features/activities/work_package/emoji_reactions_spec.rb new file mode 100644 index 000000000000..08003ebc33ff --- /dev/null +++ b/spec/features/activities/work_package/emoji_reactions_spec.rb @@ -0,0 +1,95 @@ +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) 2012-2024 the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +require "spec_helper" + +RSpec.describe "Emoji reactions on work package activity", :js, :with_cuprite, + with_flag: { primerized_work_package_activities: true } do + let(:project) { create(:project) } + let(:admin) { create(:admin) } + let(:member) { create_user_as_project_member } + let(:viewer) { create_user_with_view_work_packages_permission } + let(:viewer_with_commenting_permission) { create_user_with_view_and_commenting_permission } + + let(:first_comment) do + create(:work_package_journal, user: admin, notes: "First comment by admin", journable: work_package, + version: 2) + end + + let(:wp_page) { Pages::FullWorkPackage.new(work_package, project) } + let(:activity_tab) { Components::WorkPackages::EmojiReactions.new(work_package) } + + context "when user is the work package author" do + current_user { member } + + let(:work_package) do + create(:work_package, project:, author: member, subject: "Test work package") + end + + before do + first_comment + + wp_page.visit! + wp_page.wait_for_activity_tab + end + + it "can add an emoji reactions" do + activity_tab.can_add_emoji_reaction_for_journal(first_comment, "👍") + end + end + + context "when user only has `view_work_packages` permissions" + context "when a user has `add_work_package_notes` and `edit_own_work_package_notes` permission" + context "with anonymouse user" + + def create_user_as_project_member + member_role = create(:project_role, + permissions: %i[view_work_packages edit_work_packages add_work_packages work_package_assigned + add_work_package_notes]) + create(:user, firstname: "A", lastname: "Member", + member_with_roles: { project => member_role }) + end + + def create_user_with_view_work_packages_permission + viewer_role = create(:project_role, permissions: %i[view_work_packages]) + create(:user, + firstname: "A", + lastname: "Viewer", + member_with_roles: { project => viewer_role }) + end + + def create_user_with_view_and_commenting_permission + viewer_role_with_commenting_permission = create(:project_role, + permissions: %i[view_work_packages add_work_package_notes + edit_own_work_package_notes]) + create(:user, + firstname: "A", + lastname: "Viewer", + member_with_roles: { project => viewer_role_with_commenting_permission }) + end +end diff --git a/spec/support/components/work_packages/emoji_reactions.rb b/spec/support/components/work_packages/emoji_reactions.rb new file mode 100644 index 000000000000..5d4ab44134f6 --- /dev/null +++ b/spec/support/components/work_packages/emoji_reactions.rb @@ -0,0 +1,61 @@ +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +module Components + module WorkPackages + class EmojiReactions < Activities + def can_add_emoji_reaction_for_journal(journal, emoji, expected_emoji_count: 1) + within_journal_entry(journal) do + click_on "Add reaction" + click_on emoji + + page.within_test_selector("emoji-reactions") do + expect(page).to have_text("#{emoji} #{expected_emoji_count}") + end + end + end + + def can_remove_emoji_reaction_for_journal(journal, emoji) + within_journal_entry(journal) do + page.within_test_selector("emoji-reactions") do + click_on emoji + expect(page).to have_no_text(emoji) + end + end + end + + def expect_add_reactions_button + expect(page).to have_test_selector("add-reactions-button") + end + + def expect_no_add_reactions_button + expect(page).not_to have_test_selector("add-reactions-button") + end + end + end +end From a3d0747f40154bf82b4112c07bf3d46821c62c33 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Thu, 10 Oct 2024 19:09:53 +0300 Subject: [PATCH 12/43] feat[Op#40437]: Introduce view only reactions covered by feature specs Users who do not have commenting permissions: typically the `add_work_package_notes` and anonymous users cannot add new reactions but they can view any existing reactions. Futher, the "Add reactions" button is also hidden from them. In view only state, the emoji reactions render as primer invisible buttons in disabled state. > They can respond to user input and provide feedback, but it does not trigger any actions. > See: https://primer.style/components/button#inactive The view only reactions mimic primer inactive buttons but without the muted styling and cursor change --- app/components/_index.sass | 1 + .../item_component/add_reactions.html.erb | 2 +- .../journals/item_component/add_reactions.rb | 9 ++- .../item_component/reactions.html.erb | 9 ++- .../journals/item_component/reactions.rb | 18 +++++ .../journals/item_component/reactions.sass | 2 + spec/factories/emoji_reactions_factory.rb | 39 +++++++++ .../work_package/emoji_reactions_spec.rb | 80 +++++++++++++++++-- .../work_packages/emoji_reactions.rb | 29 ++++++- 9 files changed, 175 insertions(+), 14 deletions(-) create mode 100644 app/components/work_packages/activities_tab/journals/item_component/reactions.sass create mode 100644 spec/factories/emoji_reactions_factory.rb diff --git a/app/components/_index.sass b/app/components/_index.sass index 5b9080f61808..2590c3e4eddb 100644 --- a/app/components/_index.sass +++ b/app/components/_index.sass @@ -3,6 +3,7 @@ @import "work_packages/activities_tab/journals/index_component" @import "work_packages/activities_tab/journals/item_component" @import "work_packages/activities_tab/journals/item_component/details" +@import "work_packages/activities_tab/journals/item_component/reactions" @import "shares/modal_body_component" @import "shares/invite_user_form_component" @import "work_packages/details/tab_component" diff --git a/app/components/work_packages/activities_tab/journals/item_component/add_reactions.html.erb b/app/components/work_packages/activities_tab/journals/item_component/add_reactions.html.erb index 00410f2c9b37..0161d7998a14 100644 --- a/app/components/work_packages/activities_tab/journals/item_component/add_reactions.html.erb +++ b/app/components/work_packages/activities_tab/journals/item_component/add_reactions.html.erb @@ -12,7 +12,7 @@ "aria-label": I18n.t("reactions.add_reaction"), title: I18n.t("reactions.add_reaction"), mr: 2, - test_selector: "add-reaction-button" + test_selector: "add-reactions-button" ) d.with_body(pt: 2) do diff --git a/app/components/work_packages/activities_tab/journals/item_component/add_reactions.rb b/app/components/work_packages/activities_tab/journals/item_component/add_reactions.rb index 071965999192..17bf05d501e0 100644 --- a/app/components/work_packages/activities_tab/journals/item_component/add_reactions.rb +++ b/app/components/work_packages/activities_tab/journals/item_component/add_reactions.rb @@ -40,13 +40,16 @@ def initialize(journal:) @journal = journal end + def render? + User.current.allowed_in_work_package?(:add_work_package_notes, work_package) + end + private attr_reader :journal - def wrapper_uniq_by - journal.id - end + def work_package = journal.journable + def wrapper_uniq_by = journal.id end end end diff --git a/app/components/work_packages/activities_tab/journals/item_component/reactions.html.erb b/app/components/work_packages/activities_tab/journals/item_component/reactions.html.erb index 289f6a65d013..187240412d09 100644 --- a/app/components/work_packages/activities_tab/journals/item_component/reactions.html.erb +++ b/app/components/work_packages/activities_tab/journals/item_component/reactions.html.erb @@ -8,10 +8,13 @@ scheme: button_scheme(data[:users]), color: :default, bg: counter_color(data[:users]), - id: "#{journal.id}-#{emoji}", + id: "#{journal.id}-#{emoji_alias(emoji)}", + test_selector: "reaction-#{emoji_alias(emoji)}", tag: :a, - href: toggle_reaction_work_package_activity_path(journal.journable.id, id: journal.id, emoji: emoji), - data: { "turbo-stream": true, "turbo-method": :put } + href: href(emoji:), + data: { turbo_stream: true, turbo_method: :put }, + disabled: current_user_cannot_react?, + classes: 'op-reactions-button' )) do |button| button.with_tooltip(text: tooltip_text(emoji, data[:users])) "#{emoji} #{data[:count]}".html_safe diff --git a/app/components/work_packages/activities_tab/journals/item_component/reactions.rb b/app/components/work_packages/activities_tab/journals/item_component/reactions.rb index 537c5d7f8878..d92d41617a2f 100644 --- a/app/components/work_packages/activities_tab/journals/item_component/reactions.rb +++ b/app/components/work_packages/activities_tab/journals/item_component/reactions.rb @@ -48,6 +48,8 @@ def wrapper_uniq_by journal.id end + def work_package = journal.journable + def reacted_by_current_user?(users) users.any? { |u| u[:id] == User.current.id } end @@ -80,6 +82,22 @@ def tooltip_text(emoji, users) result += I18n.t("reactions.reacted_with", emoji_shortcode: EmojiReaction.shortcode(emoji)) result end + + def href(emoji:) + return if current_user_cannot_react? + + toggle_reaction_work_package_activity_path(journal.journable.id, id: journal.id, emoji:) + end + + def emoji_alias(emoji) + EmojiReaction::EMOJI_MAP.invert[emoji].to_s + end + + def current_user_can_react? + User.current.allowed_in_work_package?(:add_work_package_notes, work_package) + end + + def current_user_cannot_react? = !current_user_can_react? end end end diff --git a/app/components/work_packages/activities_tab/journals/item_component/reactions.sass b/app/components/work_packages/activities_tab/journals/item_component/reactions.sass new file mode 100644 index 000000000000..f89b8bf53713 --- /dev/null +++ b/app/components/work_packages/activities_tab/journals/item_component/reactions.sass @@ -0,0 +1,2 @@ +.op-reactions-button:disabled + cursor: default diff --git a/spec/factories/emoji_reactions_factory.rb b/spec/factories/emoji_reactions_factory.rb new file mode 100644 index 000000000000..56800b9a8e45 --- /dev/null +++ b/spec/factories/emoji_reactions_factory.rb @@ -0,0 +1,39 @@ +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +FactoryBot.define do + factory :emoji_reaction do + transient do + emoji_alias { EmojiReaction::EMOJI_MAP.keys.sample } + end + + user + emoji { EmojiReaction::EMOJI_MAP[emoji_alias] } + reactable factory: :work_package_journal + end +end diff --git a/spec/features/activities/work_package/emoji_reactions_spec.rb b/spec/features/activities/work_package/emoji_reactions_spec.rb index 08003ebc33ff..d1f8a6204345 100644 --- a/spec/features/activities/work_package/emoji_reactions_spec.rb +++ b/spec/features/activities/work_package/emoji_reactions_spec.rb @@ -36,6 +36,7 @@ let(:viewer) { create_user_with_view_work_packages_permission } let(:viewer_with_commenting_permission) { create_user_with_view_and_commenting_permission } + let(:work_package) { create(:work_package, project:, author: admin) } let(:first_comment) do create(:work_package_journal, user: admin, notes: "First comment by admin", journable: work_package, version: 2) @@ -54,18 +55,87 @@ before do first_comment + create(:emoji_reaction, user: admin, reactable: first_comment, emoji_alias: :thumbs_down) + + wp_page.visit! + wp_page.wait_for_activity_tab + end + + it "can add emoji reactions and remove own reactions" do + activity_tab.add_first_emoji_reaction_for_journal(first_comment, "👍") + activity_tab.add_emoji_reaction_for_journal(first_comment, "👎") + activity_tab.expect_emoji_reactions_for_journal(first_comment, "👍" => 1, "👎" => 2) + + activity_tab.remove_emoji_reaction_for_journal(first_comment, "👎") + activity_tab.expect_emoji_reactions_for_journal(first_comment, "👍" => 1, "👎" => 1) + end + end + + context "when user only has `view_work_packages` permissions" do + current_user { viewer } + + before do + first_comment + + create(:emoji_reaction, user: admin, reactable: first_comment, emoji_alias: :thumbs_down) + + wp_page.visit! + wp_page.wait_for_activity_tab + end + + it "cannot add an emoji reactions but can view emoji reactions by other users" do + activity_tab.expect_no_add_reactions_button + + activity_tab.expect_emoji_reactions_for_journal(first_comment, "👎" => { count: 1, disabled: true }) + end + end + + context "when a user has `add_work_package_notes` and `edit_own_work_package_notes` permission" do + current_user { viewer_with_commenting_permission } + + before do + first_comment + + create(:emoji_reaction, user: admin, reactable: first_comment, emoji_alias: :rocket) + wp_page.visit! wp_page.wait_for_activity_tab end - it "can add an emoji reactions" do - activity_tab.can_add_emoji_reaction_for_journal(first_comment, "👍") + it "can add emoji reactions and remove own reactions" do + activity_tab.add_first_emoji_reaction_for_journal(first_comment, "😄") + activity_tab.add_emoji_reaction_for_journal(first_comment, "🚀") + activity_tab.expect_emoji_reactions_for_journal(first_comment, "😄" => 1, "🚀" => 2) + + activity_tab.remove_emoji_reaction_for_journal(first_comment, "🚀") + activity_tab.expect_emoji_reactions_for_journal(first_comment, "😄" => 1, "🚀" => 1) end end - context "when user only has `view_work_packages` permissions" - context "when a user has `add_work_package_notes` and `edit_own_work_package_notes` permission" - context "with anonymouse user" + context "when project is public", with_settings: { login_required: false } do + let(:project) { create(:project, public: true) } + let!(:anonymous_role) do + create(:anonymous_role, permissions: %i[view_project view_work_packages]) + end + + context "when visited by an anonymous visitor" do + before do + first_comment + create(:emoji_reaction, user: admin, reactable: first_comment, emoji_alias: :party_popper) + + login_as User.anonymous + + wp_page.visit! + wp_page.wait_for_activity_tab + end + + it "cannot add an emoji reactions but can view emoji reactions by other users" do + activity_tab.expect_no_add_reactions_button + + activity_tab.expect_emoji_reactions_for_journal(first_comment, "🎉" => { count: 1, disabled: true }) + end + end + end def create_user_as_project_member member_role = create(:project_role, diff --git a/spec/support/components/work_packages/emoji_reactions.rb b/spec/support/components/work_packages/emoji_reactions.rb index 5d4ab44134f6..75afde01a650 100644 --- a/spec/support/components/work_packages/emoji_reactions.rb +++ b/spec/support/components/work_packages/emoji_reactions.rb @@ -29,16 +29,22 @@ module Components module WorkPackages class EmojiReactions < Activities - def can_add_emoji_reaction_for_journal(journal, emoji, expected_emoji_count: 1) + def add_first_emoji_reaction_for_journal(journal, emoji) within_journal_entry(journal) do click_on "Add reaction" click_on emoji + end + end + def toggle_emoji_reaction_for_journal(journal, emoji) + within_journal_entry(journal) do page.within_test_selector("emoji-reactions") do - expect(page).to have_text("#{emoji} #{expected_emoji_count}") + click_on emoji end end end + alias add_emoji_reaction_for_journal toggle_emoji_reaction_for_journal + alias remove_emoji_reaction_for_journal toggle_emoji_reaction_for_journal def can_remove_emoji_reaction_for_journal(journal, emoji) within_journal_entry(journal) do @@ -49,6 +55,25 @@ def can_remove_emoji_reaction_for_journal(journal, emoji) end end + def expect_emoji_reactions_for_journal(journal, emojis_with_expected_options) + within_journal_entry(journal) do + page.within_test_selector("emoji-reactions") do + emojis_with_expected_options.each do |emoji, expected_emoji_options| + case expected_emoji_options + when Integer + expected_emoji_count = expected_emoji_options + capybara_options = {} + when Hash + expected_emoji_count = expected_emoji_options[:count] + capybara_options = expected_emoji_options.except(:count) + end + + expect(page).to have_selector(:link_or_button, text: "#{emoji} #{expected_emoji_count}", **capybara_options) + end + end + end + end + def expect_add_reactions_button expect(page).to have_test_selector("add-reactions-button") end From 31e75b3443f15aefa620b72c4c287ad721228002 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Fri, 11 Oct 2024 13:04:09 +0300 Subject: [PATCH 13/43] chore[Op#40437]: set view only reactions as invisible --- .../activities_tab/journals/item_component/reactions.sass | 1 + 1 file changed, 1 insertion(+) diff --git a/app/components/work_packages/activities_tab/journals/item_component/reactions.sass b/app/components/work_packages/activities_tab/journals/item_component/reactions.sass index f89b8bf53713..e0583044c2ad 100644 --- a/app/components/work_packages/activities_tab/journals/item_component/reactions.sass +++ b/app/components/work_packages/activities_tab/journals/item_component/reactions.sass @@ -1,2 +1,3 @@ .op-reactions-button:disabled cursor: default + background-color: transparent From 97518692a291944d80a51dd1c00a5c1726e314e3 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Mon, 14 Oct 2024 10:06:32 +0300 Subject: [PATCH 14/43] chore[Op#40437]: correct test example --- spec/features/activities/work_package/emoji_reactions_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/features/activities/work_package/emoji_reactions_spec.rb b/spec/features/activities/work_package/emoji_reactions_spec.rb index d1f8a6204345..bc53a2329b28 100644 --- a/spec/features/activities/work_package/emoji_reactions_spec.rb +++ b/spec/features/activities/work_package/emoji_reactions_spec.rb @@ -90,7 +90,7 @@ end end - context "when a user has `add_work_package_notes` and `edit_own_work_package_notes` permission" do + context "when a user has `add_work_package_notes` permission" do current_user { viewer_with_commenting_permission } before do From 6094509e0cfbb9520128d541d3f157379ab0ba64 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Mon, 14 Oct 2024 10:58:45 +0300 Subject: [PATCH 15/43] tests[Op#40437]: add emoji reaction contract specs --- .../emoji_reactions/base_contract.rb | 9 +- .../emoji_reactions/base_contract_spec.rb | 132 ++++++++++++++++++ .../emoji_reactions/delete_contract_spec.rb | 48 +++++++ 3 files changed, 181 insertions(+), 8 deletions(-) create mode 100644 spec/contracts/emoji_reactions/base_contract_spec.rb create mode 100644 spec/contracts/emoji_reactions/delete_contract_spec.rb diff --git a/app/contracts/emoji_reactions/base_contract.rb b/app/contracts/emoji_reactions/base_contract.rb index 8a0658291d79..cfb8943c86a1 100644 --- a/app/contracts/emoji_reactions/base_contract.rb +++ b/app/contracts/emoji_reactions/base_contract.rb @@ -37,11 +37,8 @@ class BaseContract < ::ModelContract validate :validate_user_exists validate :validate_acting_user validate :validate_reactable_exists - validate :validate_emoji_type - def self.model - EmojiReaction - end + def self.model = EmojiReaction private @@ -57,10 +54,6 @@ def validate_reactable_exists errors.add :reactable, :error_not_found if model.reactable.blank? end - def validate_emoji_type - errors.add :emoji, :inclusion unless EmojiReaction::AVAILABLE_EMOJIS.include?(model.emoji) - end - def manage_emoji_reactions_permission? unless manage_emoji_reactions? errors.add :base, :error_unauthorized diff --git a/spec/contracts/emoji_reactions/base_contract_spec.rb b/spec/contracts/emoji_reactions/base_contract_spec.rb new file mode 100644 index 000000000000..ad5a95bc49a6 --- /dev/null +++ b/spec/contracts/emoji_reactions/base_contract_spec.rb @@ -0,0 +1,132 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +require "spec_helper" +require "contracts/shared/model_contract_shared_context" + +RSpec.describe EmojiReactions::BaseContract do + include_context "ModelContract shared context" + + let(:contract) { described_class.new(emoji_reaction, user) } + let(:user) { build_stubbed(:admin) } + let(:emoji_reaction) { build_stubbed(:emoji_reaction, user:) } + + before do + User.current = user + allow(User).to receive(:exists?).with(user.id).and_return(true) + end + + describe "admin user" do + it_behaves_like "contract is valid" + end + + describe "non-admin user" do + context "with valid permissions" do + let(:user) { build_stubbed(:user) } + + before do + mock_permissions_for(user) do |mock| + mock.allow_in_project(:add_work_package_notes, project: emoji_reaction.reactable.project) + end + end + + it_behaves_like "contract is valid" + end + + context "without valid permissions" do + let(:user) { build_stubbed(:user) } + + it_behaves_like "contract is invalid", base: :error_unauthorized + end + end + + describe "validate user exists" do + context "when user does not exist" do + before { allow(User).to receive(:exists?).with(user.id).and_return(false) } + + it_behaves_like "contract is invalid", user: :error_not_found + end + end + + describe "validate acting user" do + context "when the current user is different from the reactable acting user" do + let(:different_user) { build_stubbed(:user) } + + before do + allow(User).to receive(:exists?).with(different_user.id).and_return(true) + emoji_reaction.user = different_user + end + + it_behaves_like "contract is invalid", user: :error_unauthorized + end + end + + describe "validate reactable object" do + context "when reactable is blank" do + before { emoji_reaction.reactable = nil } + + it_behaves_like "contract is invalid", reactable: :error_not_found + end + + context "when reactable is a work package" do + let(:work_package) { build_stubbed(:work_package) } + + before { emoji_reaction.reactable = work_package } + + it_behaves_like "contract is valid" + end + + context "when reactable is a journal" do + let(:journal) { build_stubbed(:work_package_journal) } + + before { emoji_reaction.reactable = journal } + + it_behaves_like "contract is valid" + end + + context "when reactable is neither a journal nor a work package" do + let(:unknown_object) { build_stubbed(:project) } + + before { emoji_reaction.reactable = unknown_object } + + it_behaves_like "contract is invalid", base: :error_unauthorized + end + end + + describe "validate emoji type" do + context "when emoji is not included in the available emojis" do + before { emoji_reaction.emoji = "not_an_emoji" } + + it_behaves_like "contract is invalid", emoji: :inclusion + end + end + + include_examples "contract reuses the model errors" +end diff --git a/spec/contracts/emoji_reactions/delete_contract_spec.rb b/spec/contracts/emoji_reactions/delete_contract_spec.rb new file mode 100644 index 000000000000..3e31870534fb --- /dev/null +++ b/spec/contracts/emoji_reactions/delete_contract_spec.rb @@ -0,0 +1,48 @@ +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +require "spec_helper" +require "contracts/shared/model_contract_shared_context" + +RSpec.describe EmojiReactions::DeleteContract do + include_context "ModelContract shared context" + + let(:contract) { described_class.new(emoji_reaction, current_user) } + let(:current_user) { build_stubbed(:admin) } + let(:emoji_reaction) { build_stubbed(:emoji_reaction, user: current_user) } + + context "when user is different from the one that created the reaction" do + let(:another_user) { build_stubbed(:admin) } + + before { emoji_reaction.user = another_user } + + it_behaves_like "contract user is unauthorized" + end + + include_examples "contract reuses the model errors" +end From 7392afa49c16ff96f61c6c86ed80db59b0999dc9 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Mon, 14 Oct 2024 11:35:28 +0300 Subject: [PATCH 16/43] tests[Op#40437]: add service specs --- .../emoji_reactions/create_service_spec.rb | 38 +++++++++++++++++++ .../emoji_reactions/delete_service_spec.rb | 38 +++++++++++++++++++ 2 files changed, 76 insertions(+) create mode 100644 spec/services/emoji_reactions/create_service_spec.rb create mode 100644 spec/services/emoji_reactions/delete_service_spec.rb diff --git a/spec/services/emoji_reactions/create_service_spec.rb b/spec/services/emoji_reactions/create_service_spec.rb new file mode 100644 index 000000000000..72af3755da34 --- /dev/null +++ b/spec/services/emoji_reactions/create_service_spec.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +require "spec_helper" +require "services/base_services/behaves_like_create_service" + +RSpec.describe EmojiReactions::CreateService, type: :model do + it_behaves_like "BaseServices create service" do + let(:factory) { :emoji_reaction } + end +end diff --git a/spec/services/emoji_reactions/delete_service_spec.rb b/spec/services/emoji_reactions/delete_service_spec.rb new file mode 100644 index 000000000000..3fc9dca49d9b --- /dev/null +++ b/spec/services/emoji_reactions/delete_service_spec.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +require "spec_helper" +require "services/base_services/behaves_like_delete_service" + +RSpec.describe EmojiReactions::DeleteService, type: :model do + it_behaves_like "BaseServices delete service" do + let(:factory) { :emoji_reaction } + end +end From f777c52fac7ebe6589595325100c1b57e0bf169d Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Mon, 14 Oct 2024 11:48:22 +0300 Subject: [PATCH 17/43] tests[Op#40437]: add model specs --- spec/models/emoji_reaction_spec.rb | 63 ++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) create mode 100644 spec/models/emoji_reaction_spec.rb diff --git a/spec/models/emoji_reaction_spec.rb b/spec/models/emoji_reaction_spec.rb new file mode 100644 index 000000000000..a2a50e896a90 --- /dev/null +++ b/spec/models/emoji_reaction_spec.rb @@ -0,0 +1,63 @@ +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +require "spec_helper" + +RSpec.describe EmojiReaction do + describe "Associations" do + it { is_expected.to belong_to(:user) } + it { is_expected.to belong_to(:reactable) } + end + + describe "Validations" do + it { is_expected.to validate_presence_of(:emoji) } + it { is_expected.to validate_inclusion_of(:emoji).in_array(EmojiReaction::AVAILABLE_EMOJIS) } + + it do + emoji_reaction = create(:emoji_reaction) + expect(emoji_reaction).to validate_uniqueness_of(:user_id).scoped_to(%i[reactable_type reactable_id emoji]) + end + end + + describe ".available_emojis" do + it "returns the available emojis as HTML codes" do + expect(described_class.available_emojis).to eq(["👍", "👎", "😄", "😕", "❤", + "🎉", "🚀", "👀"]) + end + end + + describe ".shortcode" do + it "returns the emoji shortcode for a given HTML code" do + expect(described_class.shortcode("👍")).to eq(":thumbs_up:") + end + + it "returns the HTML code if no shortcode is found" do + expect(described_class.shortcode("💩")).to eq("💩") + end + end +end From c09cffc5c70e807c0c034d36de14ec31c7b89ad7 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Mon, 14 Oct 2024 12:10:45 +0300 Subject: [PATCH 18/43] fix[Op#58142]: render error flash in error case --- .../activities_tab_controller.rb | 40 ++++++++++--------- config/locales/en.yml | 1 + 2 files changed, 22 insertions(+), 19 deletions(-) diff --git a/app/controllers/work_packages/activities_tab_controller.rb b/app/controllers/work_packages/activities_tab_controller.rb index 2864a27adf18..06f01a8bd132 100644 --- a/app/controllers/work_packages/activities_tab_controller.rb +++ b/app/controllers/work_packages/activities_tab_controller.rb @@ -30,6 +30,7 @@ class WorkPackages::ActivitiesTabController < ApplicationController include OpTurbo::ComponentStream + include FlashMessagesOutputSafetyHelper before_action :find_work_package before_action :find_project @@ -142,31 +143,32 @@ def update respond_with_turbo_streams end - def toggle_reaction - call = if @journal.emoji_reactions.exists?(user: User.current, emoji: params[:emoji]) - EmojiReactions::DeleteService.new( - user: User.current, - model: @journal.emoji_reactions.find_by(user: User.current, emoji: params[:emoji]) - ).call - else - EmojiReactions::CreateService.new( - user: User.current - ).call( - user: User.current, - reactable: @journal, - emoji: params[:emoji] - ) - end - - if call.success? + def toggle_reaction # rubocop:disable Metrics/AbcSize + emoji_reaction_service = + if @journal.emoji_reactions.exists?(user: User.current, emoji: params[:emoji]) + EmojiReactions::DeleteService + .new(user: User.current, + model: @journal.emoji_reactions.find_by(user: User.current, emoji: params[:emoji])) + .call + else + EmojiReactions::CreateService + .new(user: User.current) + .call(user: User.current, reactable: @journal, emoji: params[:emoji]) + end + + emoji_reaction_service.on_success do update_via_turbo_stream( component: WorkPackages::ActivitiesTab::Journals::ItemComponent::Show.new( journal: @journal, filter: params[:filter]&.to_sym || :all ) ) - else - # TODO: handle errors + end + + emoji_reaction_service.on_failure do + render_error_flash_message_via_turbo_stream( + message: join_flash_messages(emoji_reaction_service.errors.full_messages) + ) end respond_with_turbo_streams diff --git a/config/locales/en.yml b/config/locales/en.yml index faaf4f375993..5ae80738fb38 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -929,6 +929,7 @@ en: could_not_be_copied: "%{dependency} could not be (fully) copied." does_not_exist: "does not exist." error_enterprise_only: "%{action} is only available in the OpenProject Enterprise edition" + error_not_found: "not found." error_unauthorized: "may not be accessed." error_readonly: "was attempted to be written but is not writable." error_conflict: "Information has been updated by at least one other user in the meantime." From 994be6811ac4c742c22f76f68665ffa531191786 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Mon, 21 Oct 2024 12:11:53 +0300 Subject: [PATCH 19/43] chore[Op#40437]: store emoji in uncode format backed by string enum UTF-8 is widely adopted and the default format for Postgres; storing emoji's in unicode format is then possible and should be searchable as any other string- and leveragable via AR named enums. Whilst we could also store integer numbered enums, it would introduce additional overhead that seems unnecessary --- app/models/emoji_reaction.rb | 21 +++++++++++-------- spec/models/emoji_reaction_spec.rb | 33 ++++++++++++++++++++++-------- 2 files changed, 37 insertions(+), 17 deletions(-) diff --git a/app/models/emoji_reaction.rb b/app/models/emoji_reaction.rb index 15c8468f37da..da8b3344326d 100644 --- a/app/models/emoji_reaction.rb +++ b/app/models/emoji_reaction.rb @@ -27,15 +27,16 @@ #++ class EmojiReaction < ApplicationRecord + # See: https://unicode.org/Public/emoji/latest/emoji-test.txt EMOJI_MAP = { - thumbs_up: "👍", - thumbs_down: "👎", - grinning_face_with_smiling_eyes: "😄", - confused_face: "😕", - heart: "❤", - party_popper: "🎉", - rocket: "🚀", - eyes: "👀" + thumbs_up: "\u{1F44D}", + thumbs_down: "\u{1F44E}", + grinning_face_with_smiling_eyes: "\u{1F604}", + confused_face: "\u{1F615}", + heart: "\u{2764}", + party_popper: "\u{1F389}", + rocket: "\u{1F680}", + eyes: "\u{1F440}" }.freeze AVAILABLE_EMOJIS = EMOJI_MAP.values.freeze @@ -43,9 +44,11 @@ class EmojiReaction < ApplicationRecord belongs_to :user belongs_to :reactable, polymorphic: true - validates :emoji, presence: true, inclusion: { in: AVAILABLE_EMOJIS } + validates :emoji, presence: true validates :user_id, uniqueness: { scope: %i[reactable_type reactable_id emoji] } + enum :emoji, EMOJI_MAP + def self.available_emojis AVAILABLE_EMOJIS end diff --git a/spec/models/emoji_reaction_spec.rb b/spec/models/emoji_reaction_spec.rb index a2a50e896a90..035de1ba51bd 100644 --- a/spec/models/emoji_reaction_spec.rb +++ b/spec/models/emoji_reaction_spec.rb @@ -34,9 +34,25 @@ it { is_expected.to belong_to(:reactable) } end + describe "Enums" do + it do + expect(subject).to define_enum_for(:emoji) + .with_values( + thumbs_up: "\u{1F44D}", + thumbs_down: "\u{1F44E}", + grinning_face_with_smiling_eyes: "\u{1F604}", + confused_face: "\u{1F615}", + heart: "\u{2764}", + party_popper: "\u{1F389}", + rocket: "\u{1F680}", + eyes: "\u{1F440}" + ) + .backed_by_column_of_type(:string) + end + end + describe "Validations" do it { is_expected.to validate_presence_of(:emoji) } - it { is_expected.to validate_inclusion_of(:emoji).in_array(EmojiReaction::AVAILABLE_EMOJIS) } it do emoji_reaction = create(:emoji_reaction) @@ -46,18 +62,19 @@ describe ".available_emojis" do it "returns the available emojis as HTML codes" do - expect(described_class.available_emojis).to eq(["👍", "👎", "😄", "😕", "❤", - "🎉", "🚀", "👀"]) + expect(described_class.available_emojis).to eq(["👍", "👎", "😄", "😕", "❤", "🎉", "🚀", "👀"]) end end - describe ".shortcode" do - it "returns the emoji shortcode for a given HTML code" do - expect(described_class.shortcode("👍")).to eq(":thumbs_up:") + describe ".emoji_name" do + it "returns the emoji name for a given unicode", :aggregate_failures do + expect(described_class.emoji_name("\u{1F44D}")).to eq("thumbs up") + expect(described_class.emoji_name("😄")).to eq("grinning face with smiling eyes") end - it "returns the HTML code if no shortcode is found" do - expect(described_class.shortcode("💩")).to eq("💩") + it "returns nil if no emoji exists with given unicode" do + expect(described_class.emoji_name("\u{1F607}")).to be_nil + expect(described_class.emoji_name("🤗")).to be_nil end end end From b29c2770e436d1058e299fd353c4cfc9d3999d35 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Tue, 22 Oct 2024 13:09:33 +0300 Subject: [PATCH 20/43] feat[Op#58250]: Store reactions as indexed enums later mapped to unicode Avoid possible collation issues in uncode formats- storing reactions allows us to read the emojis from relevant stores at runtime --- .../item_component/add_reactions.html.erb | 14 +++---- .../item_component/reactions.html.erb | 10 ++--- .../journals/item_component/reactions.rb | 12 ++---- .../emoji_reactions/base_contract.rb | 2 +- .../activities_tab_controller.rb | 6 +-- app/models/concerns/reactable.rb | 39 +++++++------------ app/models/emoji_reaction.rb | 10 ++--- config/locales/en.yml | 4 +- .../20240624103354_create_emoji_reactions.rb | 8 ++-- .../emoji_reactions/base_contract_spec.rb | 8 ---- spec/factories/emoji_reactions_factory.rb | 6 +-- .../work_package/emoji_reactions_spec.rb | 8 ++-- spec/models/emoji_reaction_spec.rb | 39 ++++++++++--------- 13 files changed, 71 insertions(+), 95 deletions(-) diff --git a/app/components/work_packages/activities_tab/journals/item_component/add_reactions.html.erb b/app/components/work_packages/activities_tab/journals/item_component/add_reactions.html.erb index 0161d7998a14..e451fae6786f 100644 --- a/app/components/work_packages/activities_tab/journals/item_component/add_reactions.html.erb +++ b/app/components/work_packages/activities_tab/journals/item_component/add_reactions.html.erb @@ -1,6 +1,6 @@ <%= component_wrapper do - if journal.available_untaken_emojis.any? + if journal.available_emoji_reactions.any? render(Primer::Alpha::Overlay.new( title: I18n.t("reactions.action_title"), padding: :condensed, @@ -17,17 +17,17 @@ d.with_body(pt: 2) do flex_layout do |add_reactions_container| - journal.available_untaken_emojis.each do |emoji| + journal.available_emoji_reactions.each do |emoji, reaction| add_reactions_container.with_column(mr: 2) do render(Primer::Beta::Button.new( scheme: :invisible, - id: "#{journal.id}-#{emoji}", + id: "#{journal.id}-#{reaction}", tag: :a, - href: toggle_reaction_work_package_activity_path(journal.journable.id, id: journal.id, emoji: emoji), - data: { "turbo-stream": true, "turbo-method": :put}, - "aria-label": I18n.t("reactions.react_with", emoji_shortcode: EmojiReaction.shortcode(emoji)), + href: toggle_reaction_work_package_activity_path(journal.journable.id, id: journal.id, reaction:), + data: { "turbo-stream": true, "turbo-method": :put }, + "aria-label": I18n.t("reactions.react_with", reaction: reaction.tr("_", "")), )) do - emoji.html_safe + emoji end end end diff --git a/app/components/work_packages/activities_tab/journals/item_component/reactions.html.erb b/app/components/work_packages/activities_tab/journals/item_component/reactions.html.erb index 187240412d09..f2109ce4528f 100644 --- a/app/components/work_packages/activities_tab/journals/item_component/reactions.html.erb +++ b/app/components/work_packages/activities_tab/journals/item_component/reactions.html.erb @@ -8,16 +8,16 @@ scheme: button_scheme(data[:users]), color: :default, bg: counter_color(data[:users]), - id: "#{journal.id}-#{emoji_alias(emoji)}", - test_selector: "reaction-#{emoji_alias(emoji)}", + id: "#{journal.id}-#{data[:reaction]}", + test_selector: "reaction-#{data[:reaction]}", tag: :a, - href: href(emoji:), + href: href(reaction: data[:reaction]), data: { turbo_stream: true, turbo_method: :put }, disabled: current_user_cannot_react?, classes: 'op-reactions-button' )) do |button| - button.with_tooltip(text: tooltip_text(emoji, data[:users])) - "#{emoji} #{data[:count]}".html_safe + button.with_tooltip(text: tooltip_text(data[:reaction], data[:users])) + "#{emoji} #{data[:count]}" end end end diff --git a/app/components/work_packages/activities_tab/journals/item_component/reactions.rb b/app/components/work_packages/activities_tab/journals/item_component/reactions.rb index d92d41617a2f..e208f36ded61 100644 --- a/app/components/work_packages/activities_tab/journals/item_component/reactions.rb +++ b/app/components/work_packages/activities_tab/journals/item_component/reactions.rb @@ -62,7 +62,7 @@ def button_scheme(users) reacted_by_current_user?(users) ? :default : :invisible end - def tooltip_text(emoji, users) + def tooltip_text(reaction, users) max_displayed_users_count = 5 user_count = users.length @@ -79,18 +79,14 @@ def tooltip_text(emoji, users) result += " " - result += I18n.t("reactions.reacted_with", emoji_shortcode: EmojiReaction.shortcode(emoji)) + result += I18n.t("reactions.reacted_with", reaction: reaction.tr("_", " ")) result end - def href(emoji:) + def href(reaction:) return if current_user_cannot_react? - toggle_reaction_work_package_activity_path(journal.journable.id, id: journal.id, emoji:) - end - - def emoji_alias(emoji) - EmojiReaction::EMOJI_MAP.invert[emoji].to_s + toggle_reaction_work_package_activity_path(journal.journable.id, id: journal.id, reaction:) end def current_user_can_react? diff --git a/app/contracts/emoji_reactions/base_contract.rb b/app/contracts/emoji_reactions/base_contract.rb index cfb8943c86a1..6474099506a8 100644 --- a/app/contracts/emoji_reactions/base_contract.rb +++ b/app/contracts/emoji_reactions/base_contract.rb @@ -28,7 +28,7 @@ module EmojiReactions class BaseContract < ::ModelContract - attribute :emoji + attribute :reaction attribute :user_id attribute :reactable_id attribute :reactable_type diff --git a/app/controllers/work_packages/activities_tab_controller.rb b/app/controllers/work_packages/activities_tab_controller.rb index 06f01a8bd132..e99cb1efeff0 100644 --- a/app/controllers/work_packages/activities_tab_controller.rb +++ b/app/controllers/work_packages/activities_tab_controller.rb @@ -145,15 +145,15 @@ def update def toggle_reaction # rubocop:disable Metrics/AbcSize emoji_reaction_service = - if @journal.emoji_reactions.exists?(user: User.current, emoji: params[:emoji]) + if @journal.emoji_reactions.exists?(user: User.current, reaction: params[:reaction]) EmojiReactions::DeleteService .new(user: User.current, - model: @journal.emoji_reactions.find_by(user: User.current, emoji: params[:emoji])) + model: @journal.emoji_reactions.find_by(user: User.current, reaction: params[:reaction])) .call else EmojiReactions::CreateService .new(user: User.current) - .call(user: User.current, reactable: @journal, emoji: params[:emoji]) + .call(user: User.current, reactable: @journal, reaction: params[:reaction]) end emoji_reaction_service.on_success do diff --git a/app/models/concerns/reactable.rb b/app/models/concerns/reactable.rb index ef5eb26e433e..9ed36b07bcda 100644 --- a/app/models/concerns/reactable.rb +++ b/app/models/concerns/reactable.rb @@ -33,45 +33,32 @@ module Reactable has_many :emoji_reactions, as: :reactable, dependent: :destroy end - def add_reaction(user, emoji) - emoji_reactions.create(user:, emoji:) - end - - def remove_reaction(user, emoji) - emoji_reactions.find_by(user:, emoji:)&.destroy - end - - def grouped_emoji_reactions - emoji_reactions.group(:emoji).count + def available_emoji_reactions + (EmojiReaction.reactions.values - emoji_reactions.pluck(:reaction).uniq).index_by do |reaction| + EmojiReaction.emoji(reaction) + end.sort end def detailed_grouped_emoji_reactions # TODO: Refactor this to be database agnostic # fetch all emoji reactions and group them by emoji with their count and user ids - emoji_groups = emoji_reactions - .select("emoji, COUNT(*) as count, ARRAY_AGG(user_id) as user_ids") - .group(:emoji) - .order("emoji ASC") + reaction_groups = emoji_reactions + .select("reaction, COUNT(*) as count, ARRAY_AGG(user_id) as user_ids") + .group(:reaction) + .order("reaction ASC") # avoid N+1 queries by preloading all reacting users - user_ids = emoji_groups.flat_map(&:user_ids).uniq + user_ids = reaction_groups.flat_map(&:user_ids).uniq users = User.where(id: user_ids).select(:id, :firstname, :lastname).index_by(&:id) - # convert the result to a hash indexed by emoji suitable for rendering - emoji_groups.map do |result| + # convert the result to a hash indexed by reaction suitable for rendering + reaction_groups.map do |result| { - emoji: result.emoji, + reaction: result.reaction, + emoji: EmojiReaction.emoji(result.reaction), count: result.count, users: result.user_ids.map { |id| { id:, name: users[id].name } } } end.index_by { |r| r[:emoji] } end - - def available_emojis - EmojiReaction.available_emojis.sort - end - - def available_untaken_emojis - (EmojiReaction.available_emojis - detailed_grouped_emoji_reactions.keys).sort - end end diff --git a/app/models/emoji_reaction.rb b/app/models/emoji_reaction.rb index da8b3344326d..1c7e86515925 100644 --- a/app/models/emoji_reaction.rb +++ b/app/models/emoji_reaction.rb @@ -44,16 +44,16 @@ class EmojiReaction < ApplicationRecord belongs_to :user belongs_to :reactable, polymorphic: true - validates :emoji, presence: true - validates :user_id, uniqueness: { scope: %i[reactable_type reactable_id emoji] } + validates :reaction, presence: true + validates :user_id, uniqueness: { scope: %i[reactable_type reactable_id reaction] } - enum :emoji, EMOJI_MAP + enum :reaction, EMOJI_MAP.each_with_object({}) { |(k, _v), h| h[k] = k.to_s } def self.available_emojis AVAILABLE_EMOJIS end - def self.shortcode(html_code) - EMOJI_MAP.key(html_code)&.to_s&.prepend(":")&.concat(":") || html_code + def self.emoji(reaction) + EMOJI_MAP[reaction.to_sym] end end diff --git a/config/locales/en.yml b/config/locales/en.yml index 0cfa99f7de24..228a2e15417c 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -549,10 +549,10 @@ en: reactions: action_title: "React" add_reaction: "Add reaction" - react_with: "React with %{emoji_shortcode}" + react_with: "React with %{reaction}" and_n_others_singular: "and %{n} other" and_n_others_plural: "and %{n} others" - reacted_with: "reacted with %{emoji_shortcode}" + reacted_with: "reacted with %{reaction}" reportings: index: diff --git a/db/migrate/20240624103354_create_emoji_reactions.rb b/db/migrate/20240624103354_create_emoji_reactions.rb index d0f6ac11456f..0e3ab38496c6 100644 --- a/db/migrate/20240624103354_create_emoji_reactions.rb +++ b/db/migrate/20240624103354_create_emoji_reactions.rb @@ -3,11 +3,13 @@ def change create_table :emoji_reactions do |t| t.references :user, null: false, foreign_key: true t.references :reactable, polymorphic: true, null: false - t.string :emoji, null: false + t.string :reaction, null: false t.timestamps end - add_index :emoji_reactions, %i[user_id reactable_type reactable_id emoji], unique: true, - name: "index_emoji_reactions_uniqueness" + add_index :emoji_reactions, :reaction + add_index :emoji_reactions, %i[user_id reactable_type reactable_id reaction], + unique: true, + name: "index_emoji_reactions_uniqueness" end end diff --git a/spec/contracts/emoji_reactions/base_contract_spec.rb b/spec/contracts/emoji_reactions/base_contract_spec.rb index ad5a95bc49a6..435fd8919f63 100644 --- a/spec/contracts/emoji_reactions/base_contract_spec.rb +++ b/spec/contracts/emoji_reactions/base_contract_spec.rb @@ -120,13 +120,5 @@ end end - describe "validate emoji type" do - context "when emoji is not included in the available emojis" do - before { emoji_reaction.emoji = "not_an_emoji" } - - it_behaves_like "contract is invalid", emoji: :inclusion - end - end - include_examples "contract reuses the model errors" end diff --git a/spec/factories/emoji_reactions_factory.rb b/spec/factories/emoji_reactions_factory.rb index 56800b9a8e45..6979a7bb36fd 100644 --- a/spec/factories/emoji_reactions_factory.rb +++ b/spec/factories/emoji_reactions_factory.rb @@ -28,12 +28,8 @@ FactoryBot.define do factory :emoji_reaction do - transient do - emoji_alias { EmojiReaction::EMOJI_MAP.keys.sample } - end - user - emoji { EmojiReaction::EMOJI_MAP[emoji_alias] } + reaction { EmojiReaction.reactions.keys.sample } reactable factory: :work_package_journal end end diff --git a/spec/features/activities/work_package/emoji_reactions_spec.rb b/spec/features/activities/work_package/emoji_reactions_spec.rb index bc53a2329b28..7a97e3462731 100644 --- a/spec/features/activities/work_package/emoji_reactions_spec.rb +++ b/spec/features/activities/work_package/emoji_reactions_spec.rb @@ -55,7 +55,7 @@ before do first_comment - create(:emoji_reaction, user: admin, reactable: first_comment, emoji_alias: :thumbs_down) + create(:emoji_reaction, user: admin, reactable: first_comment, reaction: :thumbs_down) wp_page.visit! wp_page.wait_for_activity_tab @@ -77,7 +77,7 @@ before do first_comment - create(:emoji_reaction, user: admin, reactable: first_comment, emoji_alias: :thumbs_down) + create(:emoji_reaction, user: admin, reactable: first_comment, reaction: :thumbs_down) wp_page.visit! wp_page.wait_for_activity_tab @@ -96,7 +96,7 @@ before do first_comment - create(:emoji_reaction, user: admin, reactable: first_comment, emoji_alias: :rocket) + create(:emoji_reaction, user: admin, reactable: first_comment, reaction: :rocket) wp_page.visit! wp_page.wait_for_activity_tab @@ -121,7 +121,7 @@ context "when visited by an anonymous visitor" do before do first_comment - create(:emoji_reaction, user: admin, reactable: first_comment, emoji_alias: :party_popper) + create(:emoji_reaction, user: admin, reactable: first_comment, reaction: :party_popper) login_as User.anonymous diff --git a/spec/models/emoji_reaction_spec.rb b/spec/models/emoji_reaction_spec.rb index 035de1ba51bd..bb4d697bd1b7 100644 --- a/spec/models/emoji_reaction_spec.rb +++ b/spec/models/emoji_reaction_spec.rb @@ -36,27 +36,32 @@ describe "Enums" do it do - expect(subject).to define_enum_for(:emoji) + expect(subject).to define_enum_for(:reaction) .with_values( - thumbs_up: "\u{1F44D}", - thumbs_down: "\u{1F44E}", - grinning_face_with_smiling_eyes: "\u{1F604}", - confused_face: "\u{1F615}", - heart: "\u{2764}", - party_popper: "\u{1F389}", - rocket: "\u{1F680}", - eyes: "\u{1F440}" + thumbs_up: "thumbs_up", + thumbs_down: "thumbs_down", + grinning_face_with_smiling_eyes: "grinning_face_with_smiling_eyes", + confused_face: "confused_face", + heart: "heart", + party_popper: "party_popper", + rocket: "rocket", + eyes: "eyes" ) .backed_by_column_of_type(:string) end + + it "stores the reaction identifier" do + emoji_reaction = create(:emoji_reaction, reaction: :thumbs_up) + expect(emoji_reaction.reaction).to eq("thumbs_up") + end end describe "Validations" do - it { is_expected.to validate_presence_of(:emoji) } + it { is_expected.to validate_presence_of(:reaction) } it do emoji_reaction = create(:emoji_reaction) - expect(emoji_reaction).to validate_uniqueness_of(:user_id).scoped_to(%i[reactable_type reactable_id emoji]) + expect(emoji_reaction).to validate_uniqueness_of(:user_id).scoped_to(%i[reactable_type reactable_id reaction]) end end @@ -66,15 +71,13 @@ end end - describe ".emoji_name" do - it "returns the emoji name for a given unicode", :aggregate_failures do - expect(described_class.emoji_name("\u{1F44D}")).to eq("thumbs up") - expect(described_class.emoji_name("😄")).to eq("grinning face with smiling eyes") + describe ".emoji" do + it "returns the emoji for a given reaction" do + expect(described_class.emoji("thumbs_up")).to eq("👍") end - it "returns nil if no emoji exists with given unicode" do - expect(described_class.emoji_name("\u{1F607}")).to be_nil - expect(described_class.emoji_name("🤗")).to be_nil + it "returns nil if no reaction exists with given name" do + expect(described_class.emoji("rock_on")).to be_nil end end end From 5f57d6f75cc32aa5899068d770fd98ba9e9e5ff8 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Tue, 22 Oct 2024 14:13:42 +0300 Subject: [PATCH 21/43] fix[Op#40437]: Update reactions text * ARIA-label, show names and emoji type: "{Name of reaction} by {user A}, {user B} and {user C}". * Visually, show just names: "{user A}, {user B} and {user C}" --- .../item_component/reactions.html.erb | 38 ++++---- .../journals/item_component/reactions.rb | 48 +++++----- config/locales/en.yml | 3 +- .../journals/item_component/reactions_spec.rb | 96 +++++++++++++++++++ 4 files changed, 144 insertions(+), 41 deletions(-) create mode 100644 spec/components/work_packages/activities_tab/journals/item_component/reactions_spec.rb diff --git a/app/components/work_packages/activities_tab/journals/item_component/reactions.html.erb b/app/components/work_packages/activities_tab/journals/item_component/reactions.html.erb index f2109ce4528f..76966c3a6da4 100644 --- a/app/components/work_packages/activities_tab/journals/item_component/reactions.html.erb +++ b/app/components/work_packages/activities_tab/journals/item_component/reactions.html.erb @@ -1,24 +1,26 @@ <%= component_wrapper do - if journal.detailed_grouped_emoji_reactions.any? - flex_layout(test_selector: 'emoji-reactions') do |reactions_container| - journal.detailed_grouped_emoji_reactions.each do |emoji, data| - reactions_container.with_column(mr: 2) do - render(Primer::Beta::Button.new( - scheme: button_scheme(data[:users]), - color: :default, - bg: counter_color(data[:users]), - id: "#{journal.id}-#{data[:reaction]}", - test_selector: "reaction-#{data[:reaction]}", - tag: :a, - href: href(reaction: data[:reaction]), - data: { turbo_stream: true, turbo_method: :put }, - disabled: current_user_cannot_react?, - classes: 'op-reactions-button' - )) do |button| - button.with_tooltip(text: tooltip_text(data[:reaction], data[:users])) - "#{emoji} #{data[:count]}" + flex_layout(test_selector: "emoji-reactions") do |reactions_container| + journal.detailed_grouped_emoji_reactions.each do |emoji, data| + reactions_container.with_column(mr: 2) do + render(Primer::Beta::Button.new( + scheme: button_scheme(data[:users]), + color: :default, + bg: counter_color(data[:users]), + id: "#{journal.id}-#{data[:reaction]}", + test_selector: "reaction-#{data[:reaction]}", + tag: :a, + href: href(reaction: data[:reaction]), + data: { turbo_stream: true, turbo_method: :put }, + aria: { label: aria_label_text(data[:reaction], data[:users]) }, + disabled: current_user_cannot_react?, + classes: "op-reactions-button" + )) do |button| + button.with_tooltip(text: number_of_user_reactions_text(data[:users]), + test_selector: "reaction-tooltip-#{data[:reaction]}") do + button.with_icon(emoji, size: :small) end + "#{emoji} #{data[:count]}" end end end diff --git a/app/components/work_packages/activities_tab/journals/item_component/reactions.rb b/app/components/work_packages/activities_tab/journals/item_component/reactions.rb index e208f36ded61..6d4637c4dbfb 100644 --- a/app/components/work_packages/activities_tab/journals/item_component/reactions.rb +++ b/app/components/work_packages/activities_tab/journals/item_component/reactions.rb @@ -30,6 +30,8 @@ module WorkPackages module ActivitiesTab module Journals class ItemComponent::Reactions < ApplicationComponent + MAX_DISPLAYED_USERS_COUNT = 5 + include ApplicationHelper include OpPrimer::ComponentHelpers include OpTurbo::Streamable @@ -40,20 +42,17 @@ def initialize(journal:) @journal = journal end + def render? + journal.detailed_grouped_emoji_reactions.any? + end + private attr_reader :journal - def wrapper_uniq_by - journal.id - end - + def wrapper_uniq_by = journal.id def work_package = journal.journable - def reacted_by_current_user?(users) - users.any? { |u| u[:id] == User.current.id } - end - def counter_color(users) reacted_by_current_user?(users) ? :accent : nil end @@ -62,25 +61,30 @@ def button_scheme(users) reacted_by_current_user?(users) ? :default : :invisible end - def tooltip_text(reaction, users) - max_displayed_users_count = 5 + def reacted_by_current_user?(users) + users.any? { |u| u[:id] == User.current.id } + end + # ARIA-label, show names and emoji type: "{Name of reaction} by {user A}, {user B} and {user C}". + def aria_label_text(reaction, users) + "#{I18n.t('reactions.reaction_by', reaction: reaction.tr('_', ' '))} #{number_of_user_reactions_text(users)}" + end + + # Visually, show just names: "{user A}, {user B} and {user C}" + def number_of_user_reactions_text(users, max_displayed_users_count: 5) user_count = users.length displayed_users = users.take(max_displayed_users_count).pluck(:name) - result = if user_count <= max_displayed_users_count - displayed_users.join(", ") - elsif user_count == max_displayed_users_count + 1 - "#{displayed_users.join(', ')} #{I18n.t('reactions.and_n_others_singular', n: 1)}" - else - "#{displayed_users.join(', ')} #{I18n.t('reactions.and_n_others_plural', - n: user_count - max_displayed_users_count)}" - end - - result += " " + return displayed_users.first if user_count == 1 - result += I18n.t("reactions.reacted_with", reaction: reaction.tr("_", " ")) - result + if user_count <= max_displayed_users_count + "#{displayed_users[0..-2].join(', ')} #{I18n.t('reactions.and_user', user: displayed_users.last)}" + elsif user_count == max_displayed_users_count + 1 + "#{displayed_users.join(', ')} #{I18n.t('reactions.and_n_others_singular', n: 1)}" + else + "#{displayed_users.join(', ')} #{I18n.t('reactions.and_n_others_plural', + n: user_count - max_displayed_users_count)}" + end end def href(reaction:) diff --git a/config/locales/en.yml b/config/locales/en.yml index 228a2e15417c..33cd71880774 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -550,9 +550,10 @@ en: action_title: "React" add_reaction: "Add reaction" react_with: "React with %{reaction}" + and_user: "and %{user}" and_n_others_singular: "and %{n} other" and_n_others_plural: "and %{n} others" - reacted_with: "reacted with %{reaction}" + reaction_by: "%{reaction} by" reportings: index: diff --git a/spec/components/work_packages/activities_tab/journals/item_component/reactions_spec.rb b/spec/components/work_packages/activities_tab/journals/item_component/reactions_spec.rb new file mode 100644 index 000000000000..20ce3ea57126 --- /dev/null +++ b/spec/components/work_packages/activities_tab/journals/item_component/reactions_spec.rb @@ -0,0 +1,96 @@ +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ +# +require "spec_helper" + +RSpec.describe WorkPackages::ActivitiesTab::Journals::ItemComponent::Reactions, type: :component do + let(:journal) { build_stubbed(:work_package_journal) } + + context "with reactions" do + before do + allow(journal).to receive(:detailed_grouped_emoji_reactions).and_return(mock_detailed_grouped_emoji_reactions) + end + + it "renders the reactions" do + render_inline(described_class.new(journal:)) + + { + thumbs_up: { + count: 3, tooltip_text: "Bob Bobbit, Bob Bobbit and Bob Bobbit", + aria_label: "thumbs up by Bob Bobbit, Bob Bobbit and Bob Bobbit" + }, + thumbs_down: { + count: 1, tooltip_text: "Bob Bobbit", aria_label: "thumbs down by Bob Bobbit" + }, + eyes: { + count: 20, tooltip_text: "Bob Bobbit, Bob Bobbit, Bob Bobbit, Bob Bobbit, Bob Bobbit and 15 others", + aria_label: "eyes by Bob Bobbit, Bob Bobbit, Bob Bobbit, Bob Bobbit, Bob Bobbit and 15 others" + }, + rocket: { + count: 5, tooltip_text: "Bob Bobbit, Bob Bobbit, Bob Bobbit, Bob Bobbit and Bob Bobbit", + aria_label: "rocket by Bob Bobbit, Bob Bobbit, Bob Bobbit, Bob Bobbit and Bob Bobbit" + }, + confused_face: { + count: 6, + tooltip_text: "Bob Bobbit, Bob Bobbit, Bob Bobbit, Bob Bobbit, Bob Bobbit and 1 other", + aria_label: "confused face by Bob Bobbit, Bob Bobbit, Bob Bobbit, Bob Bobbit, Bob Bobbit and 1 other" + } + }.each { |reaction, details| expect_emoji_reaction(reaction:, **details) } + end + end + + context "with no reactions" do + before do + allow(journal).to receive(:detailed_grouped_emoji_reactions).and_return([]) + end + + it "does not render" do + render_inline(described_class.new(journal:)) + + expect(page.text).to be_empty + end + end + + def expect_emoji_reaction(reaction:, count:, tooltip_text:, aria_label:) + expect(page).to have_test_selector("reaction-#{reaction}", text: "#{EmojiReaction.emoji(reaction)} #{count}", + aria: { label: aria_label }) + expect(page).to have_test_selector("reaction-tooltip-#{reaction}", text: tooltip_text) + end + + def mock_detailed_grouped_emoji_reactions + users = build_stubbed_list(:user, 20).map { |user| { id: user.id, name: user.name } } + + { + EmojiReaction.emoji(:thumbs_up) => { reaction: "thumbs_up", users: users.sample(3), count: 3 }, + EmojiReaction.emoji(:thumbs_down) => { reaction: "thumbs_down", users: users.sample(1), count: 1 }, + EmojiReaction.emoji(:eyes) => { reaction: "eyes", users: users, count: 20 }, + EmojiReaction.emoji(:rocket) => { reaction: "rocket", users: users.sample(5), count: 5 }, + EmojiReaction.emoji(:confused_face) => { reaction: "confused_face", users: users.sample(6), count: 6 } + } + end +end From 7b632c319ca849cbca217d8591f995dc33fa73eb Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Tue, 22 Oct 2024 18:05:17 +0300 Subject: [PATCH 22/43] chore[Op#40437]: remove unused constant, touch up formatting --- .../journals/index_component.html.erb | 21 ++++++++++------ .../journals/index_component.rb | 6 ++++- .../item_component/add_reactions.html.erb | 24 +++++++++---------- .../journals/item_component/reactions.rb | 2 -- 4 files changed, 31 insertions(+), 22 deletions(-) diff --git a/app/components/work_packages/activities_tab/journals/index_component.html.erb b/app/components/work_packages/activities_tab/journals/index_component.html.erb index d6aa9ddc56c8..adc7832bac06 100644 --- a/app/components/work_packages/activities_tab/journals/index_component.html.erb +++ b/app/components/work_packages/activities_tab/journals/index_component.html.erb @@ -1,25 +1,32 @@ <%= component_wrapper(class: "work-packages-activities-tab-journals-index-component") do flex_layout do |journals_index_wrapper_container| - journals_index_wrapper_container.with_row(classes: "work-packages-activities-tab-journals-index-component--journals-inner-container", mb: inner_container_margin_bottom) do - flex_layout(id: insert_target_modifier_id, data: { "test_selector": "op-wp-journals-container" }) do |journals_index_container| + journals_index_wrapper_container.with_row( + classes: "work-packages-activities-tab-journals-index-component--journals-inner-container", + mb: inner_container_margin_bottom + ) do + flex_layout(id: insert_target_modifier_id, + data: { test_selector: "op-wp-journals-container" }) do |journals_index_container| if empty_state? journals_index_container.with_row(mt: 2, mb: 3) do render( - WorkPackages::ActivitiesTab::Journals::EmptyComponent.new() + WorkPackages::ActivitiesTab::Journals::EmptyComponent.new ) end end + journals.each do |journal| journals_index_container.with_row do - render( - WorkPackages::ActivitiesTab::Journals::ItemComponent.new(journal:, filter:) - ) + render(WorkPackages::ActivitiesTab::Journals::ItemComponent.new(journal:, filter:)) end end end end - journals_index_wrapper_container.with_row(classes: "work-packages-activities-tab-journals-index-component--stem-connection") unless empty_state? || journal_sorting == "desc" + + unless empty_state? || journal_sorting_desc? + journals_index_wrapper_container + .with_row(classes: "work-packages-activities-tab-journals-index-component--stem-connection") + end end end %> diff --git a/app/components/work_packages/activities_tab/journals/index_component.rb b/app/components/work_packages/activities_tab/journals/index_component.rb index 3e503df4334b..ecacf00f175c 100644 --- a/app/components/work_packages/activities_tab/journals/index_component.rb +++ b/app/components/work_packages/activities_tab/journals/index_component.rb @@ -59,6 +59,10 @@ def journal_sorting User.current.preference&.comments_sorting || "desc" end + def journal_sorting_desc? + journal_sorting == "desc" + end + def journals work_package.journals.includes(:user, :notifications).reorder(version: journal_sorting) end @@ -72,7 +76,7 @@ def empty_state? end def inner_container_margin_bottom - if journal_sorting == "desc" + if journal_sorting_desc? 3 else 0 diff --git a/app/components/work_packages/activities_tab/journals/item_component/add_reactions.html.erb b/app/components/work_packages/activities_tab/journals/item_component/add_reactions.html.erb index e451fae6786f..2860b93aaba9 100644 --- a/app/components/work_packages/activities_tab/journals/item_component/add_reactions.html.erb +++ b/app/components/work_packages/activities_tab/journals/item_component/add_reactions.html.erb @@ -2,11 +2,11 @@ component_wrapper do if journal.available_emoji_reactions.any? render(Primer::Alpha::Overlay.new( - title: I18n.t("reactions.action_title"), - padding: :condensed, - anchor_side: :outside_top, - visually_hide_title: true - )) do |d| + title: I18n.t("reactions.action_title"), + padding: :condensed, + anchor_side: :outside_top, + visually_hide_title: true + )) do |d| d.with_show_button( icon: "smiley", "aria-label": I18n.t("reactions.add_reaction"), @@ -20,13 +20,13 @@ journal.available_emoji_reactions.each do |emoji, reaction| add_reactions_container.with_column(mr: 2) do render(Primer::Beta::Button.new( - scheme: :invisible, - id: "#{journal.id}-#{reaction}", - tag: :a, - href: toggle_reaction_work_package_activity_path(journal.journable.id, id: journal.id, reaction:), - data: { "turbo-stream": true, "turbo-method": :put }, - "aria-label": I18n.t("reactions.react_with", reaction: reaction.tr("_", "")), - )) do + scheme: :invisible, + id: "#{journal.id}-#{reaction}", + tag: :a, + href: toggle_reaction_work_package_activity_path(journal.journable.id, id: journal.id, reaction:), + data: { "turbo-stream": true, "turbo-method": :put }, + "aria-label": I18n.t("reactions.react_with", reaction: reaction.tr("_", "")) + )) do emoji end end diff --git a/app/components/work_packages/activities_tab/journals/item_component/reactions.rb b/app/components/work_packages/activities_tab/journals/item_component/reactions.rb index 6d4637c4dbfb..c8e74f9c7b26 100644 --- a/app/components/work_packages/activities_tab/journals/item_component/reactions.rb +++ b/app/components/work_packages/activities_tab/journals/item_component/reactions.rb @@ -30,8 +30,6 @@ module WorkPackages module ActivitiesTab module Journals class ItemComponent::Reactions < ApplicationComponent - MAX_DISPLAYED_USERS_COUNT = 5 - include ApplicationHelper include OpPrimer::ComponentHelpers include OpTurbo::Streamable From 9f08b83e7cd49eccce079018cd9dffce40f90935 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Wed, 23 Oct 2024 20:12:47 +0300 Subject: [PATCH 23/43] feat[Op#40437]: Introduce single emoji reactions aggregation query to avoid n+1 calls on journals --- app/models/concerns/reactable.rb | 36 ++++++++ spec/models/concerns/reactable_spec.rb | 119 +++++++++++++++++++++++++ 2 files changed, 155 insertions(+) create mode 100644 spec/models/concerns/reactable_spec.rb diff --git a/app/models/concerns/reactable.rb b/app/models/concerns/reactable.rb index 9ed36b07bcda..10dde80ccf4e 100644 --- a/app/models/concerns/reactable.rb +++ b/app/models/concerns/reactable.rb @@ -33,6 +33,42 @@ module Reactable has_many :emoji_reactions, as: :reactable, dependent: :destroy end + class_methods do + def grouped_work_package_journals_emoji_reactions(work_package) + grouped_emoji_reactions(reactable_id: work_package.journal_ids, reactable_type: "Journal") + end + + def grouped_emoji_reactions(reactable_id:, reactable_type:) + EmojiReaction + .select("emoji_reactions.reactable_id, emoji_reactions.reaction, COUNT(emoji_reactions.id) as count, " \ + "json_agg(json_build_array(users.id, #{user_name_concat_format_sql}) ORDER BY emoji_reactions.created_at) as user_details") # rubocop:disable Layout/LineLength + .joins(:user) + .where(reactable_id:, reactable_type:) + .group("emoji_reactions.reactable_id, emoji_reactions.reaction") + end + + private + + def user_name_concat_format_sql + case Setting.user_format + when :firstname_lastname + "concat_ws(' ', users.firstname, users.lastname)" + when :firstname + "users.firstname" + when :lastname_firstname + "concat_ws(' ', users.lastname, users.firstname)" + when :lastname_coma_firstname + "concat_ws(', ', users.lastname, users.firstname)" + when :lastname_n_firstname + "concat_ws('', users.lastname, users.firstname)" + when :username + "users.login" + else + raise ArgumentError, "Unsupported user format: #{Setting.user_format}" + end + end + end + def available_emoji_reactions (EmojiReaction.reactions.values - emoji_reactions.pluck(:reaction).uniq).index_by do |reaction| EmojiReaction.emoji(reaction) diff --git a/spec/models/concerns/reactable_spec.rb b/spec/models/concerns/reactable_spec.rb new file mode 100644 index 000000000000..910bfdfb932d --- /dev/null +++ b/spec/models/concerns/reactable_spec.rb @@ -0,0 +1,119 @@ +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +require "spec_helper" + +RSpec.describe Reactable do + shared_let(:work_package) { create(:work_package) } + + let(:wp_journal1) { create(:work_package_journal, journable: work_package, version: 2) } + let(:wp_journal2) { create(:work_package_journal, journable: work_package, version: 3) } + + let(:user1) { create(:user) } + let(:user2) { create(:user) } + + let(:thumbs_up_reactions) do + [user1, user2].each do |user| + create(:emoji_reaction, reactable: wp_journal1, user: user, reaction: :thumbs_up) + end + end + + let(:thumbs_down_reactions) { create(:emoji_reaction, reactable: wp_journal2, user: user2, reaction: :thumbs_down) } + + describe "Associations" do + it { expect(wp_journal1).to have_many(:emoji_reactions) } + end + + describe ".grouped_work_package_journals_emoji_reactions" do + before do + thumbs_up_reactions + thumbs_down_reactions + end + + it "returns grouped emoji reactions" do + result = Journal.grouped_work_package_journals_emoji_reactions(work_package) + + expect(result.size).to eq({ "thumbs_up" => 2, "thumbs_down" => 1 }) + + expect(result[0].reaction).to eq("thumbs_up") + expect(result[0].count).to eq(2) + expect(result[0].user_details).to eq([[user1.id, user1.name], [user2.id, user2.name]]) + + expect(result[1].reaction).to eq("thumbs_down") + expect(result[1].count).to eq(1) + expect(result[1].user_details).to eq([[user2.id, user2.name]]) + end + + context "when user format is set to :username", with_settings: { user_format: :username } do + it "returns grouped emoji reactions with usernames" do + result = Journal.grouped_work_package_journals_emoji_reactions(work_package) + + expect(result[0].user_details).to eq([[user1.id, user1.login], [user2.id, user2.login]]) + end + end + + context "when user format is set to :firstname", with_settings: { user_format: :firstname } do + it "returns grouped emoji reactions with first and last names" do + result = Journal.grouped_work_package_journals_emoji_reactions(work_package) + + expect(result[0].user_details).to eq( + [ + [user1.id, user1.firstname], + [user2.id, user2.firstname] + ] + ) + end + end + + context "when user format is set to :lastname_coma_firstname", with_settings: { user_format: :lastname_coma_firstname } do + it "returns grouped emoji reactions with last coma firstname" do + result = Journal.grouped_work_package_journals_emoji_reactions(work_package) + + expect(result[0].user_details).to eq( + [ + [user1.id, "#{user1.lastname}, #{user1.firstname}"], + [user2.id, "#{user2.lastname}, #{user2.firstname}"] + ] + ) + end + end + + context "when user format is set to :lastname_n_firstname", with_settings: { user_format: :lastname_n_firstname } do + it "returns grouped emoji reactions with last firstname" do + result = Journal.grouped_work_package_journals_emoji_reactions(work_package) + + expect(result[0].user_details).to eq( + [ + [user1.id, "#{user1.lastname}#{user1.firstname}"], + [user2.id, "#{user2.lastname}#{user2.firstname}"] + ] + ) + end + end + end +end From 29f0155d98a7b8664ce12daf8d4df97f72097bbd Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Thu, 24 Oct 2024 13:52:06 +0300 Subject: [PATCH 24/43] feat[Op#40437]: define emoji reactions aggregation in index component Pass it down the component chain. TODO: reduce allocation bloat by extracting it to a lazy memoized helper --- .../journals/index_component.html.erb | 2 +- .../journals/index_component.rb | 4 + .../journals/item_component.html.erb | 74 ++++++++++--------- .../activities_tab/journals/item_component.rb | 7 +- .../item_component/reactions.html.erb | 16 ++-- .../journals/item_component/reactions.rb | 9 ++- .../journals/item_component/show.html.erb | 2 +- .../journals/item_component/show.rb | 9 ++- .../activities_tab_controller.rb | 3 +- app/models/concerns/reactable.rb | 39 ++++------ .../journals/item_component/reactions_spec.rb | 22 ++---- spec/models/concerns/reactable_spec.rb | 60 +++++++++++---- 12 files changed, 142 insertions(+), 105 deletions(-) diff --git a/app/components/work_packages/activities_tab/journals/index_component.html.erb b/app/components/work_packages/activities_tab/journals/index_component.html.erb index adc7832bac06..f9e0a1cad2ef 100644 --- a/app/components/work_packages/activities_tab/journals/index_component.html.erb +++ b/app/components/work_packages/activities_tab/journals/index_component.html.erb @@ -17,7 +17,7 @@ journals.each do |journal| journals_index_container.with_row do - render(WorkPackages::ActivitiesTab::Journals::ItemComponent.new(journal:, filter:)) + render(WorkPackages::ActivitiesTab::Journals::ItemComponent.new(journal:, filter:, index_component: self)) end end end diff --git a/app/components/work_packages/activities_tab/journals/index_component.rb b/app/components/work_packages/activities_tab/journals/index_component.rb index ecacf00f175c..dc6982d78f83 100644 --- a/app/components/work_packages/activities_tab/journals/index_component.rb +++ b/app/components/work_packages/activities_tab/journals/index_component.rb @@ -43,6 +43,10 @@ def initialize(work_package:, filter: :all) @filter = filter end + def grouped_emoji_reactions + @grouped_emoji_reactions ||= Journal.grouped_work_package_journals_emoji_reactions(work_package) + end + private attr_reader :work_package, :filter diff --git a/app/components/work_packages/activities_tab/journals/item_component.html.erb b/app/components/work_packages/activities_tab/journals/item_component.html.erb index 9419133d8029..0e87fad9783d 100644 --- a/app/components/work_packages/activities_tab/journals/item_component.html.erb +++ b/app/components/work_packages/activities_tab/journals/item_component.html.erb @@ -1,28 +1,31 @@ <%= component_wrapper(data: wrapper_data_attributes, class: "work-packages-activities-tab-journals-item-component") do - flex_layout(data: { "test_selector": "op-wp-journal-entry-#{journal.id}" }) do |journal_container| + flex_layout(data: { test_selector: "op-wp-journal-entry-#{journal.id}" }) do |journal_container| if show_comment_container? journal_container.with_row do render(border_box_container( - id: "activity-anchor-#{journal.version}", - padding: :condensed, - "aria-label": I18n.t("activities.work_packages.activity_tab.commented") - )) do |border_box_component| + id: "activity-anchor-#{journal.version}", + padding: :condensed, + "aria-label": I18n.t("activities.work_packages.activity_tab.commented") + )) do |border_box_component| border_box_component.with_header(px: 2, py: 1, data: { test_selector: "op-journal-notes-header" }) do flex_layout(align_items: :center, justify_content: :space_between) do |header_container| - header_container.with_column(flex_layout: true, classes: "work-packages-activities-tab-journals-item-component--header-start-container ellipsis") do |header_start_container| + header_container.with_column(flex_layout: true, + classes: "work-packages-activities-tab-journals-item-component--header-start-container ellipsis") do |header_start_container| header_start_container.with_column(mr: 2) do render Users::AvatarComponent.new(user: journal.user, show_name: false, size: :mini) end - header_start_container.with_column(mr: 1, flex_layout: true, classes: "work-packages-activities-tab-journals-item-component--user-name-container hidden-for-desktop") do |user_name_container| + header_start_container.with_column(mr: 1, flex_layout: true, + classes: "work-packages-activities-tab-journals-item-component--user-name-container hidden-for-desktop") do |user_name_container| user_name_container.with_row(classes: "work-packages-activities-tab-journals-item-component--user-name ellipsis") do truncated_user_name(journal.user) end user_name_container.with_row do render(Primer::Beta::Text.new(font_size: :small, color: :subtle, mt: 1)) { format_time(journal.created_at) } - end + end end - header_start_container.with_column(mr: 1, classes: "work-packages-activities-tab-journals-item-component--user-name ellipsis hidden-for-mobile") do + header_start_container.with_column(mr: 1, + classes: "work-packages-activities-tab-journals-item-component--user-name ellipsis hidden-for-mobile") do truncated_user_name(journal.user) end header_start_container.with_column(mr: 1, classes: "hidden-for-mobile") do @@ -33,32 +36,33 @@ if has_unread_notifications? header_end_container.with_column(mr: 2, pt: 1) do render(Primer::Beta::Octicon.new( - :"dot-fill", # color is set via CSS as requested by UI/UX Team - classes: "work-packages-activities-tab-journals-item-component--notification-dot-icon", - size: :medium, - data: { test_selector: "op-journal-unread-notification", "op-ian-center-update-immediate": true } - )) + :"dot-fill", # color is set via CSS as requested by UI/UX Team + classes: "work-packages-activities-tab-journals-item-component--notification-dot-icon", + size: :medium, + data: { test_selector: "op-journal-unread-notification", "op-ian-center-update-immediate": true } + )) end end header_end_container.with_column do render(Primer::Beta::Link.new( - href: "#", - scheme: :secondary, - underline: false, - font_size: :small, - data: { - turbo: false, - action: "click->work-packages--activities-tab--index#setAnchor:prevent", - "work-packages--activities-tab--index-id-param": journal.version - } - )) do + href: "#", + scheme: :secondary, + underline: false, + font_size: :small, + data: { + turbo: false, + action: "click->work-packages--activities-tab--index#setAnchor:prevent", + "work-packages--activities-tab--index-id-param": journal.version + } + )) do "##{journal.version}" end end - header_end_container.with_column(ml: 1, classes: "work-packages-activities-tab-journals-item-component--action-menu") do - render(Primer::Alpha::ActionMenu.new(data: { "test_selector": "op-wp-journal-#{journal.id}-action-menu" })) do |menu| + header_end_container.with_column(ml: 1, + classes: "work-packages-activities-tab-journals-item-component--action-menu") do + render(Primer::Alpha::ActionMenu.new(data: { test_selector: "op-wp-journal-#{journal.id}-action-menu" })) do |menu| menu.with_show_button(icon: "kebab-horizontal", - 'aria-label': I18n.t(:button_actions), + "aria-label": I18n.t(:button_actions), scheme: :invisible) copy_url_action_item(menu) edit_action_item(menu) if allowed_to_edit? @@ -70,24 +74,28 @@ end border_box_component.with_body( classes: "work-packages-activities-tab-journals-item-component--journal-notes-body", - data: { "test_selector": "op-journal-notes-body" } + data: { test_selector: "op-journal-notes-body" } ) do - unless noop? + if noop? + render(Primer::Beta::Text.new(font_style: :italic, color: :subtle, mt: 1)) do + I18n.t(:"journals.changes_retracted") + end + else case state when :show - render(WorkPackages::ActivitiesTab::Journals::ItemComponent::Show.new(journal:, filter:)) + render(WorkPackages::ActivitiesTab::Journals::ItemComponent::Show.new(journal:, filter:, + grouped_emoji_reactions:)) when :edit render(WorkPackages::ActivitiesTab::Journals::ItemComponent::Edit.new(journal:, filter:)) end - else - render(Primer::Beta::Text.new(font_style: :italic, color: :subtle, mt: 1)) { I18n.t(:"journals.changes_retracted") } end end end end end journal_container.with_row do - render(WorkPackages::ActivitiesTab::Journals::ItemComponent::Details.new(journal:, has_unread_notifications: notification_on_details?, filter:)) + render(WorkPackages::ActivitiesTab::Journals::ItemComponent::Details.new(journal:, + has_unread_notifications: notification_on_details?, filter:)) end end end diff --git a/app/components/work_packages/activities_tab/journals/item_component.rb b/app/components/work_packages/activities_tab/journals/item_component.rb index e79ecc2c7662..9bce8e57e328 100644 --- a/app/components/work_packages/activities_tab/journals/item_component.rb +++ b/app/components/work_packages/activities_tab/journals/item_component.rb @@ -35,16 +35,19 @@ class ItemComponent < ApplicationComponent include OpPrimer::ComponentHelpers include OpTurbo::Streamable - def initialize(journal:, filter:, state: :show) + def initialize(journal:, filter:, index_component:, state: :show) super @journal = journal - @state = state @filter = filter + @index_component = index_component + @state = state end private + delegate :grouped_emoji_reactions, to: :@index_component + attr_reader :journal, :state, :filter def wrapper_uniq_by diff --git a/app/components/work_packages/activities_tab/journals/item_component/reactions.html.erb b/app/components/work_packages/activities_tab/journals/item_component/reactions.html.erb index 76966c3a6da4..6e3307204476 100644 --- a/app/components/work_packages/activities_tab/journals/item_component/reactions.html.erb +++ b/app/components/work_packages/activities_tab/journals/item_component/reactions.html.erb @@ -1,26 +1,26 @@ <%= component_wrapper do flex_layout(test_selector: "emoji-reactions") do |reactions_container| - journal.detailed_grouped_emoji_reactions.each do |emoji, data| + emoji_reactions.each do |reaction, data| reactions_container.with_column(mr: 2) do render(Primer::Beta::Button.new( scheme: button_scheme(data[:users]), color: :default, bg: counter_color(data[:users]), - id: "#{journal.id}-#{data[:reaction]}", - test_selector: "reaction-#{data[:reaction]}", + id: "#{journal.id}-#{reaction}", + test_selector: "reaction-#{reaction}", tag: :a, - href: href(reaction: data[:reaction]), + href: href(reaction:), data: { turbo_stream: true, turbo_method: :put }, - aria: { label: aria_label_text(data[:reaction], data[:users]) }, + aria: { label: aria_label_text(reaction, data[:users]) }, disabled: current_user_cannot_react?, classes: "op-reactions-button" )) do |button| button.with_tooltip(text: number_of_user_reactions_text(data[:users]), - test_selector: "reaction-tooltip-#{data[:reaction]}") do - button.with_icon(emoji, size: :small) + test_selector: "reaction-tooltip-#{reaction}") do + button.with_icon(EmojiReactions.emoji(reaction), size: :small) end - "#{emoji} #{data[:count]}" + "#{EmojiReaction.emoji(reaction)} #{data[:count]}" end end end diff --git a/app/components/work_packages/activities_tab/journals/item_component/reactions.rb b/app/components/work_packages/activities_tab/journals/item_component/reactions.rb index c8e74f9c7b26..d4e561555eb5 100644 --- a/app/components/work_packages/activities_tab/journals/item_component/reactions.rb +++ b/app/components/work_packages/activities_tab/journals/item_component/reactions.rb @@ -34,19 +34,20 @@ class ItemComponent::Reactions < ApplicationComponent include OpPrimer::ComponentHelpers include OpTurbo::Streamable - def initialize(journal:) + def initialize(journal:, emoji_reactions:) super @journal = journal + @emoji_reactions = emoji_reactions end def render? - journal.detailed_grouped_emoji_reactions.any? + emoji_reactions.any? end private - attr_reader :journal + attr_reader :journal, :emoji_reactions def wrapper_uniq_by = journal.id def work_package = journal.journable @@ -65,7 +66,7 @@ def reacted_by_current_user?(users) # ARIA-label, show names and emoji type: "{Name of reaction} by {user A}, {user B} and {user C}". def aria_label_text(reaction, users) - "#{I18n.t('reactions.reaction_by', reaction: reaction.tr('_', ' '))} #{number_of_user_reactions_text(users)}" + "#{I18n.t('reactions.reaction_by', reaction: reaction.to_s.tr('_', ' '))} #{number_of_user_reactions_text(users)}" end # Visually, show just names: "{user A}, {user B} and {user C}" diff --git a/app/components/work_packages/activities_tab/journals/item_component/show.html.erb b/app/components/work_packages/activities_tab/journals/item_component/show.html.erb index ca5bae75d1de..65051e6b6c69 100644 --- a/app/components/work_packages/activities_tab/journals/item_component/show.html.erb +++ b/app/components/work_packages/activities_tab/journals/item_component/show.html.erb @@ -12,7 +12,7 @@ render(WorkPackages::ActivitiesTab::Journals::ItemComponent::AddReactions.new(journal:)) end reactions_container.with_column do - render(WorkPackages::ActivitiesTab::Journals::ItemComponent::Reactions.new(journal:)) + render(WorkPackages::ActivitiesTab::Journals::ItemComponent::Reactions.new(journal:, emoji_reactions:)) end end end diff --git a/app/components/work_packages/activities_tab/journals/item_component/show.rb b/app/components/work_packages/activities_tab/journals/item_component/show.rb index e3abc2222065..91edf0e77fdf 100644 --- a/app/components/work_packages/activities_tab/journals/item_component/show.rb +++ b/app/components/work_packages/activities_tab/journals/item_component/show.rb @@ -36,20 +36,25 @@ class ItemComponent::Show < ApplicationComponent include OpPrimer::ComponentHelpers include OpTurbo::Streamable - def initialize(journal:, filter:) + def initialize(journal:, filter:, grouped_emoji_reactions:) super @journal = journal @filter = filter + @grouped_emoji_reactions = grouped_emoji_reactions end private - attr_reader :journal, :filter + attr_reader :journal, :filter, :grouped_emoji_reactions def wrapper_uniq_by journal.id end + + def emoji_reactions + grouped_emoji_reactions[journal.id] + end end end end diff --git a/app/controllers/work_packages/activities_tab_controller.rb b/app/controllers/work_packages/activities_tab_controller.rb index e99cb1efeff0..ca1718b87d7d 100644 --- a/app/controllers/work_packages/activities_tab_controller.rb +++ b/app/controllers/work_packages/activities_tab_controller.rb @@ -160,7 +160,8 @@ def toggle_reaction # rubocop:disable Metrics/AbcSize update_via_turbo_stream( component: WorkPackages::ActivitiesTab::Journals::ItemComponent::Show.new( journal: @journal, - filter: params[:filter]&.to_sym || :all + filter: params[:filter]&.to_sym || :all, + grouped_emoji_reactions: Journal.grouped_journal_emoji_reactions(@journal) ) ) end diff --git a/app/models/concerns/reactable.rb b/app/models/concerns/reactable.rb index 10dde80ccf4e..bb379a5d0d57 100644 --- a/app/models/concerns/reactable.rb +++ b/app/models/concerns/reactable.rb @@ -34,8 +34,22 @@ module Reactable end class_methods do + def grouped_journal_emoji_reactions(journal) + grouped_emoji_reactions_by_reactable(reactable_id: journal.id, reactable_type: "Journal") + end + def grouped_work_package_journals_emoji_reactions(work_package) - grouped_emoji_reactions(reactable_id: work_package.journal_ids, reactable_type: "Journal") + grouped_emoji_reactions_by_reactable(reactable_id: work_package.journal_ids, reactable_type: "Journal") + end + + def grouped_emoji_reactions_by_reactable(reactable_id:, reactable_type:) + grouped_emoji_reactions(reactable_id:, reactable_type:).each_with_object({}) do |row, hash| + hash[row.reactable_id] ||= {} + hash[row.reactable_id][row.reaction.to_sym] = { + count: row.count, + users: row.user_details.map { |(id, name)| { id:, name: } } + } + end end def grouped_emoji_reactions(reactable_id:, reactable_type:) @@ -74,27 +88,4 @@ def available_emoji_reactions EmojiReaction.emoji(reaction) end.sort end - - def detailed_grouped_emoji_reactions - # TODO: Refactor this to be database agnostic - # fetch all emoji reactions and group them by emoji with their count and user ids - reaction_groups = emoji_reactions - .select("reaction, COUNT(*) as count, ARRAY_AGG(user_id) as user_ids") - .group(:reaction) - .order("reaction ASC") - - # avoid N+1 queries by preloading all reacting users - user_ids = reaction_groups.flat_map(&:user_ids).uniq - users = User.where(id: user_ids).select(:id, :firstname, :lastname).index_by(&:id) - - # convert the result to a hash indexed by reaction suitable for rendering - reaction_groups.map do |result| - { - reaction: result.reaction, - emoji: EmojiReaction.emoji(result.reaction), - count: result.count, - users: result.user_ids.map { |id| { id:, name: users[id].name } } - } - end.index_by { |r| r[:emoji] } - end end diff --git a/spec/components/work_packages/activities_tab/journals/item_component/reactions_spec.rb b/spec/components/work_packages/activities_tab/journals/item_component/reactions_spec.rb index 20ce3ea57126..e47c67434285 100644 --- a/spec/components/work_packages/activities_tab/journals/item_component/reactions_spec.rb +++ b/spec/components/work_packages/activities_tab/journals/item_component/reactions_spec.rb @@ -32,12 +32,8 @@ let(:journal) { build_stubbed(:work_package_journal) } context "with reactions" do - before do - allow(journal).to receive(:detailed_grouped_emoji_reactions).and_return(mock_detailed_grouped_emoji_reactions) - end - it "renders the reactions" do - render_inline(described_class.new(journal:)) + render_inline(described_class.new(journal:, emoji_reactions: mock_detailed_grouped_emoji_reactions)) { thumbs_up: { @@ -65,12 +61,8 @@ end context "with no reactions" do - before do - allow(journal).to receive(:detailed_grouped_emoji_reactions).and_return([]) - end - it "does not render" do - render_inline(described_class.new(journal:)) + render_inline(described_class.new(journal:, emoji_reactions: {})) expect(page.text).to be_empty end @@ -86,11 +78,11 @@ def mock_detailed_grouped_emoji_reactions users = build_stubbed_list(:user, 20).map { |user| { id: user.id, name: user.name } } { - EmojiReaction.emoji(:thumbs_up) => { reaction: "thumbs_up", users: users.sample(3), count: 3 }, - EmojiReaction.emoji(:thumbs_down) => { reaction: "thumbs_down", users: users.sample(1), count: 1 }, - EmojiReaction.emoji(:eyes) => { reaction: "eyes", users: users, count: 20 }, - EmojiReaction.emoji(:rocket) => { reaction: "rocket", users: users.sample(5), count: 5 }, - EmojiReaction.emoji(:confused_face) => { reaction: "confused_face", users: users.sample(6), count: 6 } + thumbs_up: { users: users.sample(3), count: 3 }, + thumbs_down: { users: users.sample(1), count: 1 }, + eyes: { users: users, count: 20 }, + rocket: { users: users.sample(5), count: 5 }, + confused_face: { users: users.sample(6), count: 6 } } end end diff --git a/spec/models/concerns/reactable_spec.rb b/spec/models/concerns/reactable_spec.rb index 910bfdfb932d..a3b155b0d1a7 100644 --- a/spec/models/concerns/reactable_spec.rb +++ b/spec/models/concerns/reactable_spec.rb @@ -45,18 +45,55 @@ let(:thumbs_down_reactions) { create(:emoji_reaction, reactable: wp_journal2, user: user2, reaction: :thumbs_down) } + before do + thumbs_up_reactions + thumbs_down_reactions + end + describe "Associations" do it { expect(wp_journal1).to have_many(:emoji_reactions) } end describe ".grouped_work_package_journals_emoji_reactions" do - before do - thumbs_up_reactions - thumbs_down_reactions + it "returns grouped emoji reactions for work package journals" do + result = Journal.grouped_work_package_journals_emoji_reactions(work_package) + + expect(result.size).to eq(2) + + expect(result[wp_journal1.id].size).to eq(1) + expect(result[wp_journal1.id][:thumbs_up]).to eq( + count: 2, + users: [{ id: user1.id, name: user1.name }, { id: user2.id, name: user2.name }] + ) + + expect(result[wp_journal2.id].size).to eq(1) + expect(result[wp_journal2.id][:thumbs_down]).to eq( + count: 1, + users: [{ id: user2.id, name: user2.name }] + ) end + end + describe ".grouped_journal_emoji_reactions" do + context "with a single reactable" do + it "returns grouped emoji reactions for that journal" do + result = Journal.grouped_journal_emoji_reactions(wp_journal1) + + expect(result).to eq( + wp_journal1.id => { + thumbs_up: { + count: 2, + users: [{ id: user1.id, name: user1.name }, { id: user2.id, name: user2.name }] + } + } + ) + end + end + end + + describe ".grouped_emoji_reactions" do it "returns grouped emoji reactions" do - result = Journal.grouped_work_package_journals_emoji_reactions(work_package) + result = Journal.grouped_emoji_reactions(reactable_id: work_package.journal_ids, reactable_type: "Journal") expect(result.size).to eq({ "thumbs_up" => 2, "thumbs_down" => 1 }) @@ -71,7 +108,7 @@ context "when user format is set to :username", with_settings: { user_format: :username } do it "returns grouped emoji reactions with usernames" do - result = Journal.grouped_work_package_journals_emoji_reactions(work_package) + result = Journal.grouped_emoji_reactions(reactable_id: work_package.journal_ids, reactable_type: "Journal") expect(result[0].user_details).to eq([[user1.id, user1.login], [user2.id, user2.login]]) end @@ -79,20 +116,15 @@ context "when user format is set to :firstname", with_settings: { user_format: :firstname } do it "returns grouped emoji reactions with first and last names" do - result = Journal.grouped_work_package_journals_emoji_reactions(work_package) + result = Journal.grouped_emoji_reactions(reactable_id: wp_journal2.id, reactable_type: "Journal") - expect(result[0].user_details).to eq( - [ - [user1.id, user1.firstname], - [user2.id, user2.firstname] - ] - ) + expect(result[0].user_details).to eq([[user2.id, user2.firstname]]) end end context "when user format is set to :lastname_coma_firstname", with_settings: { user_format: :lastname_coma_firstname } do it "returns grouped emoji reactions with last coma firstname" do - result = Journal.grouped_work_package_journals_emoji_reactions(work_package) + result = Journal.grouped_emoji_reactions(reactable_id: wp_journal1.id, reactable_type: "Journal") expect(result[0].user_details).to eq( [ @@ -105,7 +137,7 @@ context "when user format is set to :lastname_n_firstname", with_settings: { user_format: :lastname_n_firstname } do it "returns grouped emoji reactions with last firstname" do - result = Journal.grouped_work_package_journals_emoji_reactions(work_package) + result = Journal.grouped_emoji_reactions(reactable_id: wp_journal1.id, reactable_type: "Journal") expect(result[0].user_details).to eq( [ From 3856149b28efe87a1e76ad3d1f831bce644323e3 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Thu, 24 Oct 2024 14:53:46 +0300 Subject: [PATCH 25/43] fix[Op#40437]: Update streams to use grouped emoji reactions query --- .../journals/index_component.html.erb | 2 +- .../activities_tab/journals/index_component.rb | 8 ++++---- .../activities_tab/journals/item_component.rb | 8 +++----- .../journals/item_component/reactions.rb | 2 +- .../work_packages/activities_tab_controller.rb | 11 ++++++----- app/models/concerns/reactable.rb | 17 ++++++++++------- 6 files changed, 25 insertions(+), 23 deletions(-) diff --git a/app/components/work_packages/activities_tab/journals/index_component.html.erb b/app/components/work_packages/activities_tab/journals/index_component.html.erb index f9e0a1cad2ef..cf1e4264bd2d 100644 --- a/app/components/work_packages/activities_tab/journals/index_component.html.erb +++ b/app/components/work_packages/activities_tab/journals/index_component.html.erb @@ -17,7 +17,7 @@ journals.each do |journal| journals_index_container.with_row do - render(WorkPackages::ActivitiesTab::Journals::ItemComponent.new(journal:, filter:, index_component: self)) + render(WorkPackages::ActivitiesTab::Journals::ItemComponent.new(journal:, filter:, grouped_emoji_reactions:)) end end end diff --git a/app/components/work_packages/activities_tab/journals/index_component.rb b/app/components/work_packages/activities_tab/journals/index_component.rb index dc6982d78f83..69b23958fc9b 100644 --- a/app/components/work_packages/activities_tab/journals/index_component.rb +++ b/app/components/work_packages/activities_tab/journals/index_component.rb @@ -43,10 +43,6 @@ def initialize(work_package:, filter: :all) @filter = filter end - def grouped_emoji_reactions - @grouped_emoji_reactions ||= Journal.grouped_work_package_journals_emoji_reactions(work_package) - end - private attr_reader :work_package, :filter @@ -75,6 +71,10 @@ def journal_with_notes journals.where.not(notes: "") end + def grouped_emoji_reactions + @grouped_emoji_reactions ||= Journal.grouped_work_package_journals_emoji_reactions(work_package) + end + def empty_state? filter == :only_comments && journal_with_notes.empty? end diff --git a/app/components/work_packages/activities_tab/journals/item_component.rb b/app/components/work_packages/activities_tab/journals/item_component.rb index 9bce8e57e328..1d20546545bc 100644 --- a/app/components/work_packages/activities_tab/journals/item_component.rb +++ b/app/components/work_packages/activities_tab/journals/item_component.rb @@ -35,20 +35,18 @@ class ItemComponent < ApplicationComponent include OpPrimer::ComponentHelpers include OpTurbo::Streamable - def initialize(journal:, filter:, index_component:, state: :show) + def initialize(journal:, filter:, grouped_emoji_reactions:, state: :show) super @journal = journal @filter = filter - @index_component = index_component + @grouped_emoji_reactions = grouped_emoji_reactions @state = state end private - delegate :grouped_emoji_reactions, to: :@index_component - - attr_reader :journal, :state, :filter + attr_reader :journal, :state, :filter, :grouped_emoji_reactions def wrapper_uniq_by journal.id diff --git a/app/components/work_packages/activities_tab/journals/item_component/reactions.rb b/app/components/work_packages/activities_tab/journals/item_component/reactions.rb index d4e561555eb5..ffb2de6af4a5 100644 --- a/app/components/work_packages/activities_tab/journals/item_component/reactions.rb +++ b/app/components/work_packages/activities_tab/journals/item_component/reactions.rb @@ -42,7 +42,7 @@ def initialize(journal:, emoji_reactions:) end def render? - emoji_reactions.any? + emoji_reactions.present? end private diff --git a/app/controllers/work_packages/activities_tab_controller.rb b/app/controllers/work_packages/activities_tab_controller.rb index ca1718b87d7d..0865eb8f3f5f 100644 --- a/app/controllers/work_packages/activities_tab_controller.rb +++ b/app/controllers/work_packages/activities_tab_controller.rb @@ -334,14 +334,15 @@ def generate_time_based_update_streams(last_update_timestamp) end end - def generate_time_based_reaction_update_streams(last_update_timestamp) + def generate_time_based_reaction_update_streams(last_updated_at) # Current limitation: Only shows added reactions, not removed ones - EmojiReaction - .where(reactable_id: @work_package.journal_ids) - .where("updated_at > ?", last_update_timestamp).find_each do |reaction| + Journal.grouped_work_package_journals_emoji_reactions( + @work_package, last_updated_at: + ).each do |journal_id, emoji_reactions| update_via_turbo_stream( component: WorkPackages::ActivitiesTab::Journals::ItemComponent::Reactions.new( - journal: reaction.reactable + journal: @work_package.journals.find(journal_id), + emoji_reactions: ) ) end diff --git a/app/models/concerns/reactable.rb b/app/models/concerns/reactable.rb index bb379a5d0d57..855634d9cfed 100644 --- a/app/models/concerns/reactable.rb +++ b/app/models/concerns/reactable.rb @@ -38,12 +38,12 @@ def grouped_journal_emoji_reactions(journal) grouped_emoji_reactions_by_reactable(reactable_id: journal.id, reactable_type: "Journal") end - def grouped_work_package_journals_emoji_reactions(work_package) - grouped_emoji_reactions_by_reactable(reactable_id: work_package.journal_ids, reactable_type: "Journal") + def grouped_work_package_journals_emoji_reactions(work_package, last_updated_at: nil) + grouped_emoji_reactions_by_reactable(reactable_id: work_package.journal_ids, reactable_type: "Journal", last_updated_at:) end - def grouped_emoji_reactions_by_reactable(reactable_id:, reactable_type:) - grouped_emoji_reactions(reactable_id:, reactable_type:).each_with_object({}) do |row, hash| + def grouped_emoji_reactions_by_reactable(reactable_id:, reactable_type:, last_updated_at: nil) + grouped_emoji_reactions(reactable_id:, reactable_type:, last_updated_at:).each_with_object({}) do |row, hash| hash[row.reactable_id] ||= {} hash[row.reactable_id][row.reaction.to_sym] = { count: row.count, @@ -52,13 +52,16 @@ def grouped_emoji_reactions_by_reactable(reactable_id:, reactable_type:) end end - def grouped_emoji_reactions(reactable_id:, reactable_type:) - EmojiReaction + def grouped_emoji_reactions(reactable_id:, reactable_type:, last_updated_at: nil) + query = EmojiReaction .select("emoji_reactions.reactable_id, emoji_reactions.reaction, COUNT(emoji_reactions.id) as count, " \ "json_agg(json_build_array(users.id, #{user_name_concat_format_sql}) ORDER BY emoji_reactions.created_at) as user_details") # rubocop:disable Layout/LineLength .joins(:user) .where(reactable_id:, reactable_type:) - .group("emoji_reactions.reactable_id, emoji_reactions.reaction") + + query = query.where("emoji_reactions.updated_at > ?", last_updated_at) if last_updated_at + + query.group("emoji_reactions.reactable_id, emoji_reactions.reaction") end private From 65c2aa2a210ee775f769b84cf0a8a74c728c7bee Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Thu, 24 Oct 2024 18:19:52 +0300 Subject: [PATCH 26/43] chore[Op#40437]: add timestamp spec --- spec/models/concerns/reactable_spec.rb | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/spec/models/concerns/reactable_spec.rb b/spec/models/concerns/reactable_spec.rb index a3b155b0d1a7..edb119e63cb5 100644 --- a/spec/models/concerns/reactable_spec.rb +++ b/spec/models/concerns/reactable_spec.rb @@ -72,6 +72,22 @@ users: [{ id: user2.id, name: user2.name }] ) end + + context "when last_updated_at is set" do + it "returns grouped emoji reactions for work package journals that were updated after last_updated_at" do + wp_journal1.update!(updated_at: 1.day.ago) + result = Journal.grouped_work_package_journals_emoji_reactions(work_package, last_updated_at: wp_journal2.updated_at) + + expect(result).to eq( + wp_journal2.id => { + thumbs_down: { + count: 1, + users: [{ id: user2.id, name: user2.name }] + } + } + ) + end + end end describe ".grouped_journal_emoji_reactions" do From 2a0b25f9bb0b59e5d25e5ed3bafe9ceeba841e0a Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Thu, 24 Oct 2024 18:37:19 +0300 Subject: [PATCH 27/43] fix[Op#40437]: Safely parse last updated at timestamp --- .../activities_tab_controller.rb | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/app/controllers/work_packages/activities_tab_controller.rb b/app/controllers/work_packages/activities_tab_controller.rb index 0865eb8f3f5f..16f3a55b25a6 100644 --- a/app/controllers/work_packages/activities_tab_controller.rb +++ b/app/controllers/work_packages/activities_tab_controller.rb @@ -49,12 +49,7 @@ def index end def update_streams - if params[:last_update_timestamp].present? - generate_time_based_update_streams(params[:last_update_timestamp]) - generate_time_based_reaction_update_streams(params[:last_update_timestamp]) - else - @turbo_status = :bad_request - end + perform_update_streams_from_last_update_timestamp respond_with_turbo_streams end @@ -254,8 +249,16 @@ def handle_other_filters_on_create(call) if call.result.initial? update_index_component # update the whole index component to reset empty state else - generate_time_based_update_streams(params[:last_update_timestamp]) - generate_time_based_reaction_update_streams(params[:last_update_timestamp]) + perform_update_streams_from_last_update_timestamp + end + end + + def perform_update_streams_from_last_update_timestamp + if params[:last_update_timestamp].present? && (last_updated_at = Time.zone.parse(params[:last_update_timestamp])) + generate_time_based_update_streams(last_updated_at) + generate_time_based_reaction_update_streams(last_updated_at) + else + @turbo_status = :bad_request end end From 340895aeb10c21efb3da198455a0b9048d965062 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Thu, 24 Oct 2024 20:05:42 +0300 Subject: [PATCH 28/43] feat[Op#40437]: update tab controller to pick out emoji reactions --- .../journals/index_component.html.erb | 5 +- .../journals/index_component.rb | 4 +- .../item_component/reactions.html.erb | 2 +- .../journals/item_component/reactions.rb | 8 +- .../journals/item_component/show.html.erb | 2 +- .../journals/item_component/show.rb | 4 - .../activities_tab_controller.rb | 79 ++++++++++++------- .../journals/item_component/reactions_spec.rb | 4 +- 8 files changed, 65 insertions(+), 43 deletions(-) diff --git a/app/components/work_packages/activities_tab/journals/index_component.html.erb b/app/components/work_packages/activities_tab/journals/index_component.html.erb index cf1e4264bd2d..df3573e39ee3 100644 --- a/app/components/work_packages/activities_tab/journals/index_component.html.erb +++ b/app/components/work_packages/activities_tab/journals/index_component.html.erb @@ -17,7 +17,10 @@ journals.each do |journal| journals_index_container.with_row do - render(WorkPackages::ActivitiesTab::Journals::ItemComponent.new(journal:, filter:, grouped_emoji_reactions:)) + render(WorkPackages::ActivitiesTab::Journals::ItemComponent.new( + journal:, filter:, + grouped_emoji_reactions: wp_journals_grouped_emoji_reactions[journal.id] + )) end end end diff --git a/app/components/work_packages/activities_tab/journals/index_component.rb b/app/components/work_packages/activities_tab/journals/index_component.rb index 69b23958fc9b..582432c9440d 100644 --- a/app/components/work_packages/activities_tab/journals/index_component.rb +++ b/app/components/work_packages/activities_tab/journals/index_component.rb @@ -71,8 +71,8 @@ def journal_with_notes journals.where.not(notes: "") end - def grouped_emoji_reactions - @grouped_emoji_reactions ||= Journal.grouped_work_package_journals_emoji_reactions(work_package) + def wp_journals_grouped_emoji_reactions + @wp_journals_grouped_emoji_reactions ||= Journal.grouped_work_package_journals_emoji_reactions(work_package) end def empty_state? diff --git a/app/components/work_packages/activities_tab/journals/item_component/reactions.html.erb b/app/components/work_packages/activities_tab/journals/item_component/reactions.html.erb index 6e3307204476..cafca01cc3b0 100644 --- a/app/components/work_packages/activities_tab/journals/item_component/reactions.html.erb +++ b/app/components/work_packages/activities_tab/journals/item_component/reactions.html.erb @@ -1,7 +1,7 @@ <%= component_wrapper do flex_layout(test_selector: "emoji-reactions") do |reactions_container| - emoji_reactions.each do |reaction, data| + grouped_emoji_reactions.each do |reaction, data| reactions_container.with_column(mr: 2) do render(Primer::Beta::Button.new( scheme: button_scheme(data[:users]), diff --git a/app/components/work_packages/activities_tab/journals/item_component/reactions.rb b/app/components/work_packages/activities_tab/journals/item_component/reactions.rb index ffb2de6af4a5..125c610d07df 100644 --- a/app/components/work_packages/activities_tab/journals/item_component/reactions.rb +++ b/app/components/work_packages/activities_tab/journals/item_component/reactions.rb @@ -34,20 +34,20 @@ class ItemComponent::Reactions < ApplicationComponent include OpPrimer::ComponentHelpers include OpTurbo::Streamable - def initialize(journal:, emoji_reactions:) + def initialize(journal:, grouped_emoji_reactions:) super @journal = journal - @emoji_reactions = emoji_reactions + @grouped_emoji_reactions = grouped_emoji_reactions end def render? - emoji_reactions.present? + grouped_emoji_reactions.present? end private - attr_reader :journal, :emoji_reactions + attr_reader :journal, :grouped_emoji_reactions def wrapper_uniq_by = journal.id def work_package = journal.journable diff --git a/app/components/work_packages/activities_tab/journals/item_component/show.html.erb b/app/components/work_packages/activities_tab/journals/item_component/show.html.erb index 65051e6b6c69..0e7832c5ed88 100644 --- a/app/components/work_packages/activities_tab/journals/item_component/show.html.erb +++ b/app/components/work_packages/activities_tab/journals/item_component/show.html.erb @@ -12,7 +12,7 @@ render(WorkPackages::ActivitiesTab::Journals::ItemComponent::AddReactions.new(journal:)) end reactions_container.with_column do - render(WorkPackages::ActivitiesTab::Journals::ItemComponent::Reactions.new(journal:, emoji_reactions:)) + render(WorkPackages::ActivitiesTab::Journals::ItemComponent::Reactions.new(journal:, grouped_emoji_reactions:)) end end end diff --git a/app/components/work_packages/activities_tab/journals/item_component/show.rb b/app/components/work_packages/activities_tab/journals/item_component/show.rb index 91edf0e77fdf..038ae2576256 100644 --- a/app/components/work_packages/activities_tab/journals/item_component/show.rb +++ b/app/components/work_packages/activities_tab/journals/item_component/show.rb @@ -51,10 +51,6 @@ def initialize(journal:, filter:, grouped_emoji_reactions:) def wrapper_uniq_by journal.id end - - def emoji_reactions - grouped_emoji_reactions[journal.id] - end end end end diff --git a/app/controllers/work_packages/activities_tab_controller.rb b/app/controllers/work_packages/activities_tab_controller.rb index 16f3a55b25a6..8e2cad79a20c 100644 --- a/app/controllers/work_packages/activities_tab_controller.rb +++ b/app/controllers/work_packages/activities_tab_controller.rb @@ -93,7 +93,7 @@ def update_sorting def edit if allowed_to_edit?(@journal) - update_item_component(journal: @journal, state: :edit) + update_item_edit_component(journal: @journal) else @turbo_status = :forbidden end @@ -103,7 +103,7 @@ def edit def cancel_edit if allowed_to_edit?(@journal) - update_item_component(journal: @journal, state: :show) + update_item_show_component(journal: @journal, grouped_emoji_reactions: grouped_emoji_reactions_for_journal) else @turbo_status = :forbidden end @@ -130,7 +130,7 @@ def update ) if call.success? && call.result - update_item_component(journal: call.result, state: :show) + update_item_show_component(journal: call.result, grouped_emoji_reactions: grouped_emoji_reactions_for_journal) else handle_failed_update_call(call) end @@ -156,7 +156,7 @@ def toggle_reaction # rubocop:disable Metrics/AbcSize component: WorkPackages::ActivitiesTab::Journals::ItemComponent::Show.new( journal: @journal, filter: params[:filter]&.to_sym || :all, - grouped_emoji_reactions: Journal.grouped_journal_emoji_reactions(@journal) + grouped_emoji_reactions: grouped_emoji_reactions_for_journal ) ) end @@ -308,16 +308,6 @@ def create_journal_service_call ### end - def update_item_component(journal:, filter: @filter, state: :show) - update_via_turbo_stream( - component: WorkPackages::ActivitiesTab::Journals::ItemComponent.new( - journal:, - state:, - filter: - ) - ) - end - def generate_time_based_update_streams(last_update_timestamp) journals = @work_package.journals @@ -325,13 +315,21 @@ def generate_time_based_update_streams(last_update_timestamp) journals = journals.where.not(notes: "") end - rerender_updated_journals(journals, last_update_timestamp) - - rerender_journals_with_updated_notification(journals, last_update_timestamp) + grouped_emoji_reactions = + if journals.present? + Journal.grouped_emoji_reactions_by_reactable( + reactable_id: journals.pluck(:id), + reactable_type: "Journal", last_updated_at: last_update_timestamp + ) + else + {} + end - append_or_prepend_journals(journals, last_update_timestamp) + rerender_updated_journals(journals, last_update_timestamp, grouped_emoji_reactions) + rerender_journals_with_updated_notification(journals, last_update_timestamp, grouped_emoji_reactions) + append_or_prepend_journals(journals, last_update_timestamp, grouped_emoji_reactions) - if journals.any? + if journals.present? remove_potential_empty_state update_activity_counter end @@ -345,41 +343,43 @@ def generate_time_based_reaction_update_streams(last_updated_at) update_via_turbo_stream( component: WorkPackages::ActivitiesTab::Journals::ItemComponent::Reactions.new( journal: @work_package.journals.find(journal_id), - emoji_reactions: + grouped_emoji_reactions: ) ) end end - def rerender_updated_journals(journals, last_update_timestamp) + def rerender_updated_journals(journals, last_update_timestamp, grouped_emoji_reactions) journals.where("updated_at > ?", last_update_timestamp).find_each do |journal| - update_item_component(journal:) + update_item_show_component(journal:, grouped_emoji_reactions: grouped_emoji_reactions.fetch(journal.id, {})) end end - def rerender_journals_with_updated_notification(journals, last_update_timestamp) + def rerender_journals_with_updated_notification(journals, last_update_timestamp, grouped_emoji_reactions) # Case: the user marked the journal as read somewhere else and expects the bubble to disappear journals .joins(:notifications) .where("notifications.updated_at > ?", last_update_timestamp) .find_each do |journal| - update_item_component(journal:) + update_item_show_component(journal:, grouped_emoji_reactions: grouped_emoji_reactions.fetch(journal.id, {})) end end - def append_or_prepend_journals(journals, last_update_timestamp) + def append_or_prepend_journals(journals, last_update_timestamp, grouped_emoji_reactions) journals.where("created_at > ?", last_update_timestamp).find_each do |journal| - append_or_prepend_latest_journal_via_turbo_stream(journal) + append_or_prepend_latest_journal_via_turbo_stream(journal, grouped_emoji_reactions.fetch(journal.id, {})) end end - def append_or_prepend_latest_journal_via_turbo_stream(journal) + def append_or_prepend_latest_journal_via_turbo_stream(journal, grouped_emoji_reactions) target_component = WorkPackages::ActivitiesTab::Journals::IndexComponent.new( work_package: @work_package, filter: @filter ) - component = WorkPackages::ActivitiesTab::Journals::ItemComponent.new(journal:, filter: @filter) + component = WorkPackages::ActivitiesTab::Journals::ItemComponent.new( + journal:, filter: @filter, grouped_emoji_reactions: + ) stream_config = { target_component:, @@ -401,6 +401,25 @@ def remove_potential_empty_state ) end + def update_item_edit_component(journal:, grouped_emoji_reactions: {}) + update_item_component(journal:, state: :edit, grouped_emoji_reactions:) + end + + def update_item_show_component(journal:, grouped_emoji_reactions:) + update_item_component(journal:, state: :show, grouped_emoji_reactions:) + end + + def update_item_component(journal:, grouped_emoji_reactions:, state:, filter: @filter) + update_via_turbo_stream( + component: WorkPackages::ActivitiesTab::Journals::ItemComponent.new( + journal:, + state:, + filter:, + grouped_emoji_reactions: + ) + ) + end + def update_activity_counter # update the activity counter in the primerized tabs # not targeting the legacy tab! @@ -409,6 +428,10 @@ def update_activity_counter ) end + def grouped_emoji_reactions_for_journal + Journal.grouped_journal_emoji_reactions(@journal).fetch(@journal.id, {}) + end + def allowed_to_edit?(journal) journal.editable_by?(User.current) end diff --git a/spec/components/work_packages/activities_tab/journals/item_component/reactions_spec.rb b/spec/components/work_packages/activities_tab/journals/item_component/reactions_spec.rb index e47c67434285..6c39bc5cdb08 100644 --- a/spec/components/work_packages/activities_tab/journals/item_component/reactions_spec.rb +++ b/spec/components/work_packages/activities_tab/journals/item_component/reactions_spec.rb @@ -33,7 +33,7 @@ context "with reactions" do it "renders the reactions" do - render_inline(described_class.new(journal:, emoji_reactions: mock_detailed_grouped_emoji_reactions)) + render_inline(described_class.new(journal:, grouped_emoji_reactions: mock_detailed_grouped_emoji_reactions)) { thumbs_up: { @@ -62,7 +62,7 @@ context "with no reactions" do it "does not render" do - render_inline(described_class.new(journal:, emoji_reactions: {})) + render_inline(described_class.new(journal:, grouped_emoji_reactions: {})) expect(page.text).to be_empty end From b08ed5bf66890f0625f76dbe80214c7c7ce5d457 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Thu, 24 Oct 2024 21:29:34 +0300 Subject: [PATCH 29/43] fix[Op#40437]: set emoji reactions correctly --- app/controllers/work_packages/activities_tab_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/work_packages/activities_tab_controller.rb b/app/controllers/work_packages/activities_tab_controller.rb index 8e2cad79a20c..6752478e0ce4 100644 --- a/app/controllers/work_packages/activities_tab_controller.rb +++ b/app/controllers/work_packages/activities_tab_controller.rb @@ -339,7 +339,7 @@ def generate_time_based_reaction_update_streams(last_updated_at) # Current limitation: Only shows added reactions, not removed ones Journal.grouped_work_package_journals_emoji_reactions( @work_package, last_updated_at: - ).each do |journal_id, emoji_reactions| + ).each do |journal_id, grouped_emoji_reactions| update_via_turbo_stream( component: WorkPackages::ActivitiesTab::Journals::ItemComponent::Reactions.new( journal: @work_package.journals.find(journal_id), From 30b831faa86452fbb532a2c9da6a17c1e681cf21 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Fri, 25 Oct 2024 10:55:08 +0300 Subject: [PATCH 30/43] chore[Op#40437]: ensure emoji reactions returns empty hash when no reactions --- .../work_packages/activities_tab_controller.rb | 13 ++++--------- spec/models/concerns/reactable_spec.rb | 9 +++++++++ 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/app/controllers/work_packages/activities_tab_controller.rb b/app/controllers/work_packages/activities_tab_controller.rb index 6752478e0ce4..8bae3e898a0b 100644 --- a/app/controllers/work_packages/activities_tab_controller.rb +++ b/app/controllers/work_packages/activities_tab_controller.rb @@ -315,15 +315,10 @@ def generate_time_based_update_streams(last_update_timestamp) journals = journals.where.not(notes: "") end - grouped_emoji_reactions = - if journals.present? - Journal.grouped_emoji_reactions_by_reactable( - reactable_id: journals.pluck(:id), - reactable_type: "Journal", last_updated_at: last_update_timestamp - ) - else - {} - end + grouped_emoji_reactions = Journal.grouped_emoji_reactions_by_reactable( + reactable_id: journals.pluck(:id), + reactable_type: "Journal", last_updated_at: last_update_timestamp + ) rerender_updated_journals(journals, last_update_timestamp, grouped_emoji_reactions) rerender_journals_with_updated_notification(journals, last_update_timestamp, grouped_emoji_reactions) diff --git a/spec/models/concerns/reactable_spec.rb b/spec/models/concerns/reactable_spec.rb index edb119e63cb5..7f47ff64eaab 100644 --- a/spec/models/concerns/reactable_spec.rb +++ b/spec/models/concerns/reactable_spec.rb @@ -88,6 +88,15 @@ ) end end + + context "when no reactions exist" do + it "returns an empty hash" do + work_package = build_stubbed(:work_package) + result = Journal.grouped_work_package_journals_emoji_reactions(work_package) + + expect(result).to eq({}) + end + end end describe ".grouped_journal_emoji_reactions" do From e6aed59bbe3926d1a1c13b813386f2d82444f81c Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Fri, 25 Oct 2024 11:48:20 +0300 Subject: [PATCH 31/43] fix[Op#40437]: ensure emojis and user reactions are ordered in asc order --- app/models/concerns/reactable.rb | 18 ++++++++++++--- spec/models/concerns/reactable_spec.rb | 32 ++++++++++++++++++++++++-- 2 files changed, 45 insertions(+), 5 deletions(-) diff --git a/app/models/concerns/reactable.rb b/app/models/concerns/reactable.rb index 855634d9cfed..73de0b65b92e 100644 --- a/app/models/concerns/reactable.rb +++ b/app/models/concerns/reactable.rb @@ -54,18 +54,30 @@ def grouped_emoji_reactions_by_reactable(reactable_id:, reactable_type:, last_up def grouped_emoji_reactions(reactable_id:, reactable_type:, last_updated_at: nil) query = EmojiReaction - .select("emoji_reactions.reactable_id, emoji_reactions.reaction, COUNT(emoji_reactions.id) as count, " \ - "json_agg(json_build_array(users.id, #{user_name_concat_format_sql}) ORDER BY emoji_reactions.created_at) as user_details") # rubocop:disable Layout/LineLength + .select(emoji_reactions_group_selection_sql) .joins(:user) .where(reactable_id:, reactable_type:) query = query.where("emoji_reactions.updated_at > ?", last_updated_at) if last_updated_at - query.group("emoji_reactions.reactable_id, emoji_reactions.reaction") + query + .group("emoji_reactions.reactable_id, emoji_reactions.reaction") + .order("first_created_at ASC") end private + def emoji_reactions_group_selection_sql + <<~SQL.squish + emoji_reactions.reactable_id, emoji_reactions.reaction, COUNT(emoji_reactions.id) as count, + json_agg( + json_build_array(users.id, #{user_name_concat_format_sql}) + ORDER BY emoji_reactions.created_at + ) as user_details, + MIN(emoji_reactions.created_at) as first_created_at + SQL + end + def user_name_concat_format_sql case Setting.user_format when :firstname_lastname diff --git a/spec/models/concerns/reactable_spec.rb b/spec/models/concerns/reactable_spec.rb index 7f47ff64eaab..b8f11724c8b6 100644 --- a/spec/models/concerns/reactable_spec.rb +++ b/spec/models/concerns/reactable_spec.rb @@ -114,14 +114,42 @@ ) end end + + context "with multiple reactions from different users at different times" do + let(:user3) { create(:user) } + + before do + create(:emoji_reaction, reactable: wp_journal1, user: user3, reaction: :thumbs_up, created_at: 1.day.ago) + create(:emoji_reaction, reactable: wp_journal1, user: user3, reaction: :thumbs_down, created_at: 2.days.ago) + end + + it "groups emoji reactions and users in ascending order" do + result = Journal.grouped_journal_emoji_reactions(wp_journal1) + + expect(result).to eq( + wp_journal1.id => { + thumbs_down: { + count: 1, + users: [{ id: user3.id, name: user3.name }] + }, + thumbs_up: { + count: 3, + users: [ + { id: user3.id, name: user3.name }, + { id: user1.id, name: user1.name }, + { id: user2.id, name: user2.name } + ] + } + } + ) + end + end end describe ".grouped_emoji_reactions" do it "returns grouped emoji reactions" do result = Journal.grouped_emoji_reactions(reactable_id: work_package.journal_ids, reactable_type: "Journal") - expect(result.size).to eq({ "thumbs_up" => 2, "thumbs_down" => 1 }) - expect(result[0].reaction).to eq("thumbs_up") expect(result[0].count).to eq(2) expect(result[0].user_details).to eq([[user1.id, user1.name], [user2.id, user2.name]]) From fb419dd23be7aae2a4dc9e5e4fa6c08ccd23889e Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Fri, 25 Oct 2024 11:49:04 +0300 Subject: [PATCH 32/43] chore[Op#40437]: remove redundant reactions single update, rely on full component refresh --- .../work_packages/activities_tab_controller.rb | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/app/controllers/work_packages/activities_tab_controller.rb b/app/controllers/work_packages/activities_tab_controller.rb index 8bae3e898a0b..2539b48aa15a 100644 --- a/app/controllers/work_packages/activities_tab_controller.rb +++ b/app/controllers/work_packages/activities_tab_controller.rb @@ -256,7 +256,6 @@ def handle_other_filters_on_create(call) def perform_update_streams_from_last_update_timestamp if params[:last_update_timestamp].present? && (last_updated_at = Time.zone.parse(params[:last_update_timestamp])) generate_time_based_update_streams(last_updated_at) - generate_time_based_reaction_update_streams(last_updated_at) else @turbo_status = :bad_request end @@ -330,20 +329,6 @@ def generate_time_based_update_streams(last_update_timestamp) end end - def generate_time_based_reaction_update_streams(last_updated_at) - # Current limitation: Only shows added reactions, not removed ones - Journal.grouped_work_package_journals_emoji_reactions( - @work_package, last_updated_at: - ).each do |journal_id, grouped_emoji_reactions| - update_via_turbo_stream( - component: WorkPackages::ActivitiesTab::Journals::ItemComponent::Reactions.new( - journal: @work_package.journals.find(journal_id), - grouped_emoji_reactions: - ) - ) - end - end - def rerender_updated_journals(journals, last_update_timestamp, grouped_emoji_reactions) journals.where("updated_at > ?", last_update_timestamp).find_each do |journal| update_item_show_component(journal:, grouped_emoji_reactions: grouped_emoji_reactions.fetch(journal.id, {})) From 232df803f505c51d1e5ccc1ed04fc3610771dbc7 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Fri, 25 Oct 2024 11:55:22 +0300 Subject: [PATCH 33/43] fix[Op#40437]: remove support for emoji reactions timeslicing Emoji Reactions should always be returned in their current state, timeslicing will yield incorrect results. At any given time, we ONLY need to know what reactions a Journal has. Further, EmojiReactions are never updated, rather created and destroyed. Retain timeslicing only for the journal render- where that is relevant to find updated journals --- .../work_packages/activities_tab_controller.rb | 3 +-- app/models/concerns/reactable.rb | 16 ++++++---------- spec/models/concerns/reactable_spec.rb | 16 ---------------- 3 files changed, 7 insertions(+), 28 deletions(-) diff --git a/app/controllers/work_packages/activities_tab_controller.rb b/app/controllers/work_packages/activities_tab_controller.rb index 2539b48aa15a..52adf8283626 100644 --- a/app/controllers/work_packages/activities_tab_controller.rb +++ b/app/controllers/work_packages/activities_tab_controller.rb @@ -315,8 +315,7 @@ def generate_time_based_update_streams(last_update_timestamp) end grouped_emoji_reactions = Journal.grouped_emoji_reactions_by_reactable( - reactable_id: journals.pluck(:id), - reactable_type: "Journal", last_updated_at: last_update_timestamp + reactable_id: journals.pluck(:id), reactable_type: "Journal" ) rerender_updated_journals(journals, last_update_timestamp, grouped_emoji_reactions) diff --git a/app/models/concerns/reactable.rb b/app/models/concerns/reactable.rb index 73de0b65b92e..72f4600b1b46 100644 --- a/app/models/concerns/reactable.rb +++ b/app/models/concerns/reactable.rb @@ -38,12 +38,12 @@ def grouped_journal_emoji_reactions(journal) grouped_emoji_reactions_by_reactable(reactable_id: journal.id, reactable_type: "Journal") end - def grouped_work_package_journals_emoji_reactions(work_package, last_updated_at: nil) - grouped_emoji_reactions_by_reactable(reactable_id: work_package.journal_ids, reactable_type: "Journal", last_updated_at:) + def grouped_work_package_journals_emoji_reactions(work_package) + grouped_emoji_reactions_by_reactable(reactable_id: work_package.journal_ids, reactable_type: "Journal") end - def grouped_emoji_reactions_by_reactable(reactable_id:, reactable_type:, last_updated_at: nil) - grouped_emoji_reactions(reactable_id:, reactable_type:, last_updated_at:).each_with_object({}) do |row, hash| + def grouped_emoji_reactions_by_reactable(reactable_id:, reactable_type:) + grouped_emoji_reactions(reactable_id:, reactable_type:).each_with_object({}) do |row, hash| hash[row.reactable_id] ||= {} hash[row.reactable_id][row.reaction.to_sym] = { count: row.count, @@ -52,15 +52,11 @@ def grouped_emoji_reactions_by_reactable(reactable_id:, reactable_type:, last_up end end - def grouped_emoji_reactions(reactable_id:, reactable_type:, last_updated_at: nil) - query = EmojiReaction + def grouped_emoji_reactions(reactable_id:, reactable_type:) + EmojiReaction .select(emoji_reactions_group_selection_sql) .joins(:user) .where(reactable_id:, reactable_type:) - - query = query.where("emoji_reactions.updated_at > ?", last_updated_at) if last_updated_at - - query .group("emoji_reactions.reactable_id, emoji_reactions.reaction") .order("first_created_at ASC") end diff --git a/spec/models/concerns/reactable_spec.rb b/spec/models/concerns/reactable_spec.rb index b8f11724c8b6..5ac0e6067f71 100644 --- a/spec/models/concerns/reactable_spec.rb +++ b/spec/models/concerns/reactable_spec.rb @@ -73,22 +73,6 @@ ) end - context "when last_updated_at is set" do - it "returns grouped emoji reactions for work package journals that were updated after last_updated_at" do - wp_journal1.update!(updated_at: 1.day.ago) - result = Journal.grouped_work_package_journals_emoji_reactions(work_package, last_updated_at: wp_journal2.updated_at) - - expect(result).to eq( - wp_journal2.id => { - thumbs_down: { - count: 1, - users: [{ id: user2.id, name: user2.name }] - } - } - ) - end - end - context "when no reactions exist" do it "returns an empty hash" do work_package = build_stubbed(:work_package) From a302f3f9957094e2b622ee9fce41a6d602e9aa26 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Fri, 25 Oct 2024 14:04:05 +0300 Subject: [PATCH 34/43] fix[Op#40437]: Reinstate emoji reactions streams Journalsa are updated on last updated_at; reactions cannot rely on this as they do not "touch" the journal, so a full journal refresh would not be viable. We avoid "touching" the journal on emoji reactions update to retain concurrent user actions. E.g. If a user has a journal in edit mode, we do not want to overrite it on the next polling update- this is why it's safer to stream emoji reactions separately This reverts commit fb419dd23be7aae2a4dc9e5e4fa6c08ccd23889e. --- .../item_component/reactions.html.erb | 42 ++++++++++--------- .../journals/item_component/reactions.rb | 4 -- .../activities_tab_controller.rb | 13 ++++++ .../journals/item_component/reactions_spec.rb | 6 ++- .../work_package/emoji_reactions_spec.rb | 42 +++++++++++++++++++ 5 files changed, 81 insertions(+), 26 deletions(-) diff --git a/app/components/work_packages/activities_tab/journals/item_component/reactions.html.erb b/app/components/work_packages/activities_tab/journals/item_component/reactions.html.erb index cafca01cc3b0..1d29d4e57fb9 100644 --- a/app/components/work_packages/activities_tab/journals/item_component/reactions.html.erb +++ b/app/components/work_packages/activities_tab/journals/item_component/reactions.html.erb @@ -1,26 +1,28 @@ <%= component_wrapper do - flex_layout(test_selector: "emoji-reactions") do |reactions_container| - grouped_emoji_reactions.each do |reaction, data| - reactions_container.with_column(mr: 2) do - render(Primer::Beta::Button.new( - scheme: button_scheme(data[:users]), - color: :default, - bg: counter_color(data[:users]), - id: "#{journal.id}-#{reaction}", - test_selector: "reaction-#{reaction}", - tag: :a, - href: href(reaction:), - data: { turbo_stream: true, turbo_method: :put }, - aria: { label: aria_label_text(reaction, data[:users]) }, - disabled: current_user_cannot_react?, - classes: "op-reactions-button" - )) do |button| - button.with_tooltip(text: number_of_user_reactions_text(data[:users]), - test_selector: "reaction-tooltip-#{reaction}") do - button.with_icon(EmojiReactions.emoji(reaction), size: :small) + if grouped_emoji_reactions.present? + flex_layout(test_selector: "emoji-reactions") do |reactions_container| + grouped_emoji_reactions.each do |reaction, data| + reactions_container.with_column(mr: 2) do + render(Primer::Beta::Button.new( + scheme: button_scheme(data[:users]), + color: :default, + bg: counter_color(data[:users]), + id: "#{journal.id}-#{reaction}", + test_selector: "reaction-#{reaction}", + tag: :a, + href: href(reaction:), + data: { turbo_stream: true, turbo_method: :put }, + aria: { label: aria_label_text(reaction, data[:users]) }, + disabled: current_user_cannot_react?, + classes: "op-reactions-button" + )) do |button| + button.with_tooltip(text: number_of_user_reactions_text(data[:users]), + test_selector: "reaction-tooltip-#{reaction}") do + button.with_icon(EmojiReactions.emoji(reaction), size: :small) + end + "#{EmojiReaction.emoji(reaction)} #{data[:count]}" end - "#{EmojiReaction.emoji(reaction)} #{data[:count]}" end end end diff --git a/app/components/work_packages/activities_tab/journals/item_component/reactions.rb b/app/components/work_packages/activities_tab/journals/item_component/reactions.rb index 125c610d07df..c4e387d324d5 100644 --- a/app/components/work_packages/activities_tab/journals/item_component/reactions.rb +++ b/app/components/work_packages/activities_tab/journals/item_component/reactions.rb @@ -41,10 +41,6 @@ def initialize(journal:, grouped_emoji_reactions:) @grouped_emoji_reactions = grouped_emoji_reactions end - def render? - grouped_emoji_reactions.present? - end - private attr_reader :journal, :grouped_emoji_reactions diff --git a/app/controllers/work_packages/activities_tab_controller.rb b/app/controllers/work_packages/activities_tab_controller.rb index 52adf8283626..e3c3c50a86ad 100644 --- a/app/controllers/work_packages/activities_tab_controller.rb +++ b/app/controllers/work_packages/activities_tab_controller.rb @@ -256,6 +256,7 @@ def handle_other_filters_on_create(call) def perform_update_streams_from_last_update_timestamp if params[:last_update_timestamp].present? && (last_updated_at = Time.zone.parse(params[:last_update_timestamp])) generate_time_based_update_streams(last_updated_at) + generate_work_package_journals_emoji_reactions_update_streams else @turbo_status = :bad_request end @@ -328,6 +329,18 @@ def generate_time_based_update_streams(last_update_timestamp) end end + def generate_work_package_journals_emoji_reactions_update_streams + # Current limitation: Only shows added reactions, not removed ones + Journal.grouped_work_package_journals_emoji_reactions(@work_package).each do |journal_id, grouped_emoji_reactions| + update_via_turbo_stream( + component: WorkPackages::ActivitiesTab::Journals::ItemComponent::Reactions.new( + journal: @work_package.journals.find(journal_id), + grouped_emoji_reactions: + ) + ) + end + end + def rerender_updated_journals(journals, last_update_timestamp, grouped_emoji_reactions) journals.where("updated_at > ?", last_update_timestamp).find_each do |journal| update_item_show_component(journal:, grouped_emoji_reactions: grouped_emoji_reactions.fetch(journal.id, {})) diff --git a/spec/components/work_packages/activities_tab/journals/item_component/reactions_spec.rb b/spec/components/work_packages/activities_tab/journals/item_component/reactions_spec.rb index 6c39bc5cdb08..762e3fc0bb17 100644 --- a/spec/components/work_packages/activities_tab/journals/item_component/reactions_spec.rb +++ b/spec/components/work_packages/activities_tab/journals/item_component/reactions_spec.rb @@ -61,10 +61,12 @@ end context "with no reactions" do - it "does not render" do + it "renders just the component node" do + # NB: This is so that there is always a node to update during WorkPackages::ActivitiesTabController#update_streams polling render_inline(described_class.new(journal:, grouped_emoji_reactions: {})) - expect(page.text).to be_empty + component = page.find("#work-packages-activities-tab-journals-item-component-reactions-#{journal.id}") + expect(component.text).to be_empty end end diff --git a/spec/features/activities/work_package/emoji_reactions_spec.rb b/spec/features/activities/work_package/emoji_reactions_spec.rb index 7a97e3462731..587ca692cd3f 100644 --- a/spec/features/activities/work_package/emoji_reactions_spec.rb +++ b/spec/features/activities/work_package/emoji_reactions_spec.rb @@ -137,6 +137,48 @@ end end + describe "reactions updates" do + let(:work_package) { create(:work_package, project:, author: admin) } + let(:first_comment_by_member) do + create(:work_package_journal, user: member, notes: "Second comment by member", journable: work_package, + version: 2) + end + + current_user { member } + + before do + # set WORK_PACKAGES_ACTIVITIES_TAB_POLLING_INTERVAL_IN_MS to 1000 + # to speed up the polling interval for test duration + ENV["WORK_PACKAGES_ACTIVITIES_TAB_POLLING_INTERVAL_IN_MS"] = "1000" + + wp_page.visit! + wp_page.wait_for_activity_tab + end + + after do + ENV.delete("WORK_PACKAGES_ACTIVITIES_TAB_POLLING_INTERVAL_IN_MS") + end + + it "shows the updated reactions without reload", :aggregate_failures do + activity_tab.expect_journal_notes(text: first_comment_by_member.notes) + + # Simulate another user adding a reaction + EmojiReactions::CreateService + .new(user: admin) + .call(user: admin, reactable: first_comment_by_member, reaction: :confused_face) + + wait_for do + activity_tab.expect_emoji_reactions_for_journal(first_comment_by_member, "😕" => 1) + end + + # Current user adds several reactions + activity_tab.add_first_emoji_reaction_for_journal(first_comment_by_member, "👍") + activity_tab.add_emoji_reaction_for_journal(first_comment_by_member, "😕") + + activity_tab.expect_emoji_reactions_for_journal(first_comment_by_member, "👍" => 1, "😕" => 2) + end + end + def create_user_as_project_member member_role = create(:project_role, permissions: %i[view_work_packages edit_work_packages add_work_packages work_package_assigned From 80cea6e41ed5b10f8f70b829a754d0462a59f9e3 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Fri, 25 Oct 2024 15:08:15 +0300 Subject: [PATCH 35/43] chore[Op#40437]: improve var naming; make less generic --- app/models/concerns/reactable.rb | 9 +++++---- spec/models/concerns/reactable_spec.rb | 16 ++++++++-------- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/app/models/concerns/reactable.rb b/app/models/concerns/reactable.rb index 72f4600b1b46..10392482fc2b 100644 --- a/app/models/concerns/reactable.rb +++ b/app/models/concerns/reactable.rb @@ -46,8 +46,8 @@ def grouped_emoji_reactions_by_reactable(reactable_id:, reactable_type:) grouped_emoji_reactions(reactable_id:, reactable_type:).each_with_object({}) do |row, hash| hash[row.reactable_id] ||= {} hash[row.reactable_id][row.reaction.to_sym] = { - count: row.count, - users: row.user_details.map { |(id, name)| { id:, name: } } + count: row.reactions_count, + users: row.reacting_users.map { |(id, name)| { id:, name: } } } end end @@ -65,11 +65,12 @@ def grouped_emoji_reactions(reactable_id:, reactable_type:) def emoji_reactions_group_selection_sql <<~SQL.squish - emoji_reactions.reactable_id, emoji_reactions.reaction, COUNT(emoji_reactions.id) as count, + emoji_reactions.reactable_id, emoji_reactions.reaction, + COUNT(emoji_reactions.id) as reactions_count, json_agg( json_build_array(users.id, #{user_name_concat_format_sql}) ORDER BY emoji_reactions.created_at - ) as user_details, + ) as reacting_users, MIN(emoji_reactions.created_at) as first_created_at SQL end diff --git a/spec/models/concerns/reactable_spec.rb b/spec/models/concerns/reactable_spec.rb index 5ac0e6067f71..34530f4e2f64 100644 --- a/spec/models/concerns/reactable_spec.rb +++ b/spec/models/concerns/reactable_spec.rb @@ -135,19 +135,19 @@ result = Journal.grouped_emoji_reactions(reactable_id: work_package.journal_ids, reactable_type: "Journal") expect(result[0].reaction).to eq("thumbs_up") - expect(result[0].count).to eq(2) - expect(result[0].user_details).to eq([[user1.id, user1.name], [user2.id, user2.name]]) + expect(result[0].reactions_count).to eq(2) + expect(result[0].reacting_users).to eq([[user1.id, user1.name], [user2.id, user2.name]]) expect(result[1].reaction).to eq("thumbs_down") - expect(result[1].count).to eq(1) - expect(result[1].user_details).to eq([[user2.id, user2.name]]) + expect(result[1].reactions_count).to eq(1) + expect(result[1].reacting_users).to eq([[user2.id, user2.name]]) end context "when user format is set to :username", with_settings: { user_format: :username } do it "returns grouped emoji reactions with usernames" do result = Journal.grouped_emoji_reactions(reactable_id: work_package.journal_ids, reactable_type: "Journal") - expect(result[0].user_details).to eq([[user1.id, user1.login], [user2.id, user2.login]]) + expect(result[0].reacting_users).to eq([[user1.id, user1.login], [user2.id, user2.login]]) end end @@ -155,7 +155,7 @@ it "returns grouped emoji reactions with first and last names" do result = Journal.grouped_emoji_reactions(reactable_id: wp_journal2.id, reactable_type: "Journal") - expect(result[0].user_details).to eq([[user2.id, user2.firstname]]) + expect(result[0].reacting_users).to eq([[user2.id, user2.firstname]]) end end @@ -163,7 +163,7 @@ it "returns grouped emoji reactions with last coma firstname" do result = Journal.grouped_emoji_reactions(reactable_id: wp_journal1.id, reactable_type: "Journal") - expect(result[0].user_details).to eq( + expect(result[0].reacting_users).to eq( [ [user1.id, "#{user1.lastname}, #{user1.firstname}"], [user2.id, "#{user2.lastname}, #{user2.firstname}"] @@ -176,7 +176,7 @@ it "returns grouped emoji reactions with last firstname" do result = Journal.grouped_emoji_reactions(reactable_id: wp_journal1.id, reactable_type: "Journal") - expect(result[0].user_details).to eq( + expect(result[0].reacting_users).to eq( [ [user1.id, "#{user1.lastname}#{user1.firstname}"], [user2.id, "#{user2.lastname}#{user2.firstname}"] From d57d69f8ea5ae02662d91cda082db3925b650e70 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Fri, 25 Oct 2024 15:51:09 +0300 Subject: [PATCH 36/43] fix[Op#40437]: ensure all wp journals are updated including removals --- .../work_packages/activities_tab_controller.rb | 8 ++++---- .../work_package/emoji_reactions_spec.rb | 15 ++++++++++++--- .../components/work_packages/emoji_reactions.rb | 8 ++++++++ 3 files changed, 24 insertions(+), 7 deletions(-) diff --git a/app/controllers/work_packages/activities_tab_controller.rb b/app/controllers/work_packages/activities_tab_controller.rb index e3c3c50a86ad..ab1b16aafb71 100644 --- a/app/controllers/work_packages/activities_tab_controller.rb +++ b/app/controllers/work_packages/activities_tab_controller.rb @@ -330,12 +330,12 @@ def generate_time_based_update_streams(last_update_timestamp) end def generate_work_package_journals_emoji_reactions_update_streams - # Current limitation: Only shows added reactions, not removed ones - Journal.grouped_work_package_journals_emoji_reactions(@work_package).each do |journal_id, grouped_emoji_reactions| + wp_journal_emoji_reactions = Journal.grouped_work_package_journals_emoji_reactions(@work_package) + @work_package.journals.each do |journal| update_via_turbo_stream( component: WorkPackages::ActivitiesTab::Journals::ItemComponent::Reactions.new( - journal: @work_package.journals.find(journal_id), - grouped_emoji_reactions: + journal:, + grouped_emoji_reactions: wp_journal_emoji_reactions[journal.id] || {} ) ) end diff --git a/spec/features/activities/work_package/emoji_reactions_spec.rb b/spec/features/activities/work_package/emoji_reactions_spec.rb index 587ca692cd3f..0340bf8b29b0 100644 --- a/spec/features/activities/work_package/emoji_reactions_spec.rb +++ b/spec/features/activities/work_package/emoji_reactions_spec.rb @@ -167,15 +167,24 @@ .new(user: admin) .call(user: admin, reactable: first_comment_by_member, reaction: :confused_face) - wait_for do - activity_tab.expect_emoji_reactions_for_journal(first_comment_by_member, "😕" => 1) - end + activity_tab.expect_emoji_reactions_for_journal(first_comment_by_member, "😕" => { count: 1, wait: 3 }) # Current user adds several reactions activity_tab.add_first_emoji_reaction_for_journal(first_comment_by_member, "👍") activity_tab.add_emoji_reaction_for_journal(first_comment_by_member, "😕") activity_tab.expect_emoji_reactions_for_journal(first_comment_by_member, "👍" => 1, "😕" => 2) + + # Current user removes reaction and other user removes as well + activity_tab.remove_emoji_reaction_for_journal(first_comment_by_member, "👍") + activity_tab.remove_emoji_reaction_for_journal(first_comment_by_member, "😕") + + EmojiReactions::DeleteService + .new(user: admin, + model: first_comment_by_member.emoji_reactions.find_by(user: admin, reaction: :confused_face)) + .call + + activity_tab.expect_no_emoji_reactions_for_journal(first_comment_by_member) end end diff --git a/spec/support/components/work_packages/emoji_reactions.rb b/spec/support/components/work_packages/emoji_reactions.rb index 75afde01a650..ce4e53197cfe 100644 --- a/spec/support/components/work_packages/emoji_reactions.rb +++ b/spec/support/components/work_packages/emoji_reactions.rb @@ -29,6 +29,8 @@ module Components module WorkPackages class EmojiReactions < Activities + include RSpec::Wait + def add_first_emoji_reaction_for_journal(journal, emoji) within_journal_entry(journal) do click_on "Add reaction" @@ -74,6 +76,12 @@ def expect_emoji_reactions_for_journal(journal, emojis_with_expected_options) end end + def expect_no_emoji_reactions_for_journal(journal) + within_journal_entry(journal) do + wait(3.seconds).for { page }.not_to have_test_selector("emoji-reactions") + end + end + def expect_add_reactions_button expect(page).to have_test_selector("add-reactions-button") end From 2a22e8b8d897f1405dff85ccfb1c457370ea96f9 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Fri, 25 Oct 2024 17:47:23 +0300 Subject: [PATCH 37/43] chore[Op#40437]: use meaningful var names --- .../journals/item_component/add_reactions.html.erb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/components/work_packages/activities_tab/journals/item_component/add_reactions.html.erb b/app/components/work_packages/activities_tab/journals/item_component/add_reactions.html.erb index 2860b93aaba9..2a062625fc6f 100644 --- a/app/components/work_packages/activities_tab/journals/item_component/add_reactions.html.erb +++ b/app/components/work_packages/activities_tab/journals/item_component/add_reactions.html.erb @@ -6,8 +6,8 @@ padding: :condensed, anchor_side: :outside_top, visually_hide_title: true - )) do |d| - d.with_show_button( + )) do |overlay| + overlay.with_show_button( icon: "smiley", "aria-label": I18n.t("reactions.add_reaction"), title: I18n.t("reactions.add_reaction"), @@ -15,7 +15,7 @@ test_selector: "add-reactions-button" ) - d.with_body(pt: 2) do + overlay.with_body(pt: 2) do flex_layout do |add_reactions_container| journal.available_emoji_reactions.each do |emoji, reaction| add_reactions_container.with_column(mr: 2) do From e58b9bd18c3694a6eb898d6f4e654d1cd75e6c02 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Fri, 25 Oct 2024 17:49:03 +0300 Subject: [PATCH 38/43] chore[Op#40437]: favor contract validations as opposed to AR validations --- app/models/emoji_reaction.rb | 1 - spec/models/emoji_reaction_spec.rb | 2 -- 2 files changed, 3 deletions(-) diff --git a/app/models/emoji_reaction.rb b/app/models/emoji_reaction.rb index 1c7e86515925..d6b01952fed6 100644 --- a/app/models/emoji_reaction.rb +++ b/app/models/emoji_reaction.rb @@ -44,7 +44,6 @@ class EmojiReaction < ApplicationRecord belongs_to :user belongs_to :reactable, polymorphic: true - validates :reaction, presence: true validates :user_id, uniqueness: { scope: %i[reactable_type reactable_id reaction] } enum :reaction, EMOJI_MAP.each_with_object({}) { |(k, _v), h| h[k] = k.to_s } diff --git a/spec/models/emoji_reaction_spec.rb b/spec/models/emoji_reaction_spec.rb index bb4d697bd1b7..8e1a7220b7a1 100644 --- a/spec/models/emoji_reaction_spec.rb +++ b/spec/models/emoji_reaction_spec.rb @@ -57,8 +57,6 @@ end describe "Validations" do - it { is_expected.to validate_presence_of(:reaction) } - it do emoji_reaction = create(:emoji_reaction) expect(emoji_reaction).to validate_uniqueness_of(:user_id).scoped_to(%i[reactable_type reactable_id reaction]) From 54c0ff631d937ac1e2acde42acfc1a0a94117396 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Fri, 25 Oct 2024 18:01:19 +0300 Subject: [PATCH 39/43] chore[Op#40437]: remap model validations names to be more suitable --- app/contracts/emoji_reactions/base_contract.rb | 6 +++--- config/locales/en.yml | 2 +- .../emoji_reactions/base_contract_spec.rb | 14 +++----------- 3 files changed, 7 insertions(+), 15 deletions(-) diff --git a/app/contracts/emoji_reactions/base_contract.rb b/app/contracts/emoji_reactions/base_contract.rb index 6474099506a8..4de709e0d1da 100644 --- a/app/contracts/emoji_reactions/base_contract.rb +++ b/app/contracts/emoji_reactions/base_contract.rb @@ -43,15 +43,15 @@ def self.model = EmojiReaction private def validate_user_exists - errors.add :user, :error_not_found unless User.exists?(model.user_id) + errors.add :user, :not_found unless User.exists?(model.user_id) end def validate_acting_user - errors.add :user, :error_unauthorized unless model.user_id == user.id + errors.add :user, :invalid unless model.user_id == user.id end def validate_reactable_exists - errors.add :reactable, :error_not_found if model.reactable.blank? + errors.add :reactable, :not_found if model.reactable.blank? end def manage_emoji_reactions_permission? diff --git a/config/locales/en.yml b/config/locales/en.yml index 2f63b8ef6a02..5de5d229a935 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -943,7 +943,6 @@ en: could_not_be_copied: "%{dependency} could not be (fully) copied." does_not_exist: "does not exist." error_enterprise_only: "%{action} is only available in the OpenProject Enterprise edition" - error_not_found: "not found." error_unauthorized: "may not be accessed." error_readonly: "was attempted to be written but is not writable." error_conflict: "Information has been updated by at least one other user in the meantime." @@ -968,6 +967,7 @@ en: not_available: "is not available due to a system configuration." not_deletable: "cannot be deleted." not_current_user: "is not the current user." + not_found: "not found." not_a_date: "is not a valid date." not_a_datetime: "is not a valid date time." not_a_number: "is not a number." diff --git a/spec/contracts/emoji_reactions/base_contract_spec.rb b/spec/contracts/emoji_reactions/base_contract_spec.rb index 435fd8919f63..332f85a9e749 100644 --- a/spec/contracts/emoji_reactions/base_contract_spec.rb +++ b/spec/contracts/emoji_reactions/base_contract_spec.rb @@ -71,7 +71,7 @@ context "when user does not exist" do before { allow(User).to receive(:exists?).with(user.id).and_return(false) } - it_behaves_like "contract is invalid", user: :error_not_found + it_behaves_like "contract is invalid", user: :not_found end end @@ -84,7 +84,7 @@ emoji_reaction.user = different_user end - it_behaves_like "contract is invalid", user: :error_unauthorized + it_behaves_like "contract is invalid", user: :invalid end end @@ -92,7 +92,7 @@ context "when reactable is blank" do before { emoji_reaction.reactable = nil } - it_behaves_like "contract is invalid", reactable: :error_not_found + it_behaves_like "contract is invalid", reactable: :not_found end context "when reactable is a work package" do @@ -110,14 +110,6 @@ it_behaves_like "contract is valid" end - - context "when reactable is neither a journal nor a work package" do - let(:unknown_object) { build_stubbed(:project) } - - before { emoji_reaction.reactable = unknown_object } - - it_behaves_like "contract is invalid", base: :error_unauthorized - end end include_examples "contract reuses the model errors" From 2627f2ce5739f99f5c0c4877dcd96b1f9e592781 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Fri, 25 Oct 2024 18:16:24 +0300 Subject: [PATCH 40/43] fix[Op#40437]: switch to i18n pluralization See: https://guides.rubyonrails.org/i18n.html#pluralization Co-authored-by: Eric Schubert --- .../activities_tab/journals/item_component/reactions.rb | 6 ++---- config/locales/en.yml | 5 +++-- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/app/components/work_packages/activities_tab/journals/item_component/reactions.rb b/app/components/work_packages/activities_tab/journals/item_component/reactions.rb index c4e387d324d5..6c651f7e8f9f 100644 --- a/app/components/work_packages/activities_tab/journals/item_component/reactions.rb +++ b/app/components/work_packages/activities_tab/journals/item_component/reactions.rb @@ -74,11 +74,9 @@ def number_of_user_reactions_text(users, max_displayed_users_count: 5) if user_count <= max_displayed_users_count "#{displayed_users[0..-2].join(', ')} #{I18n.t('reactions.and_user', user: displayed_users.last)}" - elsif user_count == max_displayed_users_count + 1 - "#{displayed_users.join(', ')} #{I18n.t('reactions.and_n_others_singular', n: 1)}" else - "#{displayed_users.join(', ')} #{I18n.t('reactions.and_n_others_plural', - n: user_count - max_displayed_users_count)}" + "#{displayed_users.join(', ')} #{I18n.t('reactions.and_others', + count: user_count - max_displayed_users_count)}" end end diff --git a/config/locales/en.yml b/config/locales/en.yml index 5de5d229a935..81234865011a 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -551,8 +551,9 @@ en: add_reaction: "Add reaction" react_with: "React with %{reaction}" and_user: "and %{user}" - and_n_others_singular: "and %{n} other" - and_n_others_plural: "and %{n} others" + and_others: + one: and 1 other + other: and %{count} others reaction_by: "%{reaction} by" reportings: From 88047f1d7f15418a20571e7bfe6f4788cb5ae6ca Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Fri, 25 Oct 2024 18:21:46 +0300 Subject: [PATCH 41/43] chore[Op#40437]: humanize reaction instead --- .../journals/item_component/add_reactions.html.erb | 2 +- .../activities_tab/journals/item_component/reactions.rb | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/app/components/work_packages/activities_tab/journals/item_component/add_reactions.html.erb b/app/components/work_packages/activities_tab/journals/item_component/add_reactions.html.erb index 2a062625fc6f..c19ccd4bb1aa 100644 --- a/app/components/work_packages/activities_tab/journals/item_component/add_reactions.html.erb +++ b/app/components/work_packages/activities_tab/journals/item_component/add_reactions.html.erb @@ -25,7 +25,7 @@ tag: :a, href: toggle_reaction_work_package_activity_path(journal.journable.id, id: journal.id, reaction:), data: { "turbo-stream": true, "turbo-method": :put }, - "aria-label": I18n.t("reactions.react_with", reaction: reaction.tr("_", "")) + "aria-label": I18n.t("reactions.react_with", reaction: reaction.to_s.humanize(capitalize: false)) )) do emoji end diff --git a/app/components/work_packages/activities_tab/journals/item_component/reactions.rb b/app/components/work_packages/activities_tab/journals/item_component/reactions.rb index 6c651f7e8f9f..584fd23afe96 100644 --- a/app/components/work_packages/activities_tab/journals/item_component/reactions.rb +++ b/app/components/work_packages/activities_tab/journals/item_component/reactions.rb @@ -62,7 +62,8 @@ def reacted_by_current_user?(users) # ARIA-label, show names and emoji type: "{Name of reaction} by {user A}, {user B} and {user C}". def aria_label_text(reaction, users) - "#{I18n.t('reactions.reaction_by', reaction: reaction.to_s.tr('_', ' '))} #{number_of_user_reactions_text(users)}" + "#{I18n.t('reactions.reaction_by', + reaction: reaction.to_s.humanize(capitalize: false))} #{number_of_user_reactions_text(users)}" end # Visually, show just names: "{user A}, {user B} and {user C}" From a6427d331804fa7849213fe18e7778f0bac9a463 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Fri, 25 Oct 2024 18:37:28 +0300 Subject: [PATCH 42/43] =?UTF-8?q?fix[Op#40437]:=20Use=20fully=20qualified?= =?UTF-8?q?=20=E2=9D=A4=EF=B8=8F?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use both hex codes to ensure red color! Tested on Safari --- app/models/emoji_reaction.rb | 2 +- spec/models/emoji_reaction_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/emoji_reaction.rb b/app/models/emoji_reaction.rb index d6b01952fed6..dc73fa68fc40 100644 --- a/app/models/emoji_reaction.rb +++ b/app/models/emoji_reaction.rb @@ -33,7 +33,7 @@ class EmojiReaction < ApplicationRecord thumbs_down: "\u{1F44E}", grinning_face_with_smiling_eyes: "\u{1F604}", confused_face: "\u{1F615}", - heart: "\u{2764}", + heart: "\u{2764 FE0F}", party_popper: "\u{1F389}", rocket: "\u{1F680}", eyes: "\u{1F440}" diff --git a/spec/models/emoji_reaction_spec.rb b/spec/models/emoji_reaction_spec.rb index 8e1a7220b7a1..00cbb7a960f7 100644 --- a/spec/models/emoji_reaction_spec.rb +++ b/spec/models/emoji_reaction_spec.rb @@ -65,7 +65,7 @@ describe ".available_emojis" do it "returns the available emojis as HTML codes" do - expect(described_class.available_emojis).to eq(["👍", "👎", "😄", "😕", "❤", "🎉", "🚀", "👀"]) + expect(described_class.available_emojis).to eq(["👍", "👎", "😄", "😕", "❤️", "🎉", "🚀", "👀"]) end end From 840deff6612265ed8a0c594afd29f5dd7b1ab17d Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Fri, 25 Oct 2024 19:23:35 +0300 Subject: [PATCH 43/43] chore[Op#40437]: extend wait time for journal change Flaky spec: https://github.com/opf/openproject/actions/runs/11521381821/job/32074903613 --- spec/support/components/work_packages/activities.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/support/components/work_packages/activities.rb b/spec/support/components/work_packages/activities.rb index 1dd2f9ea3173..b6848487046b 100644 --- a/spec/support/components/work_packages/activities.rb +++ b/spec/support/components/work_packages/activities.rb @@ -68,7 +68,7 @@ def within_journal_entry(journal, &) end def expect_journal_changed_attribute(text:) - expect(page).to have_test_selector("op-journal-detail-description", text:) + expect(page).to have_test_selector("op-journal-detail-description", text:, wait: 10) end def expect_no_journal_changed_attribute(text: nil)