From bc0ed1abd5f4492f4cabe8767189bf8c98f00ea2 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Fri, 25 Oct 2024 14:04:05 +0300 Subject: [PATCH] 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 ++- 4 files changed, 39 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