From dbf7bd6845a6c16929e1a8166912d5a760b2c433 Mon Sep 17 00:00:00 2001 From: Jonas Jabari Date: Wed, 8 Jan 2025 14:20:08 +0100 Subject: [PATCH 1/4] [#59374] [Regression] Missing "Commit Message" Comments in Work Packages after upgrade to 15.0.1 https://community.openproject.org/work_packages/59374 From 5e762ea891881ed5141babfa637b171a5530a577 Mon Sep 17 00:00:00 2001 From: Jonas Jabari Date: Mon, 13 Jan 2025 16:04:57 +0100 Subject: [PATCH 2/4] render revisions in new activity tab --- app/components/_index.sass | 1 + .../journals/index_component.html.erb | 30 ++++--- .../journals/index_component.rb | 81 +++++++++++------- .../journals/revision_component.html.erb | 81 ++++++++++++++++++ .../journals/revision_component.rb | 84 +++++++++++++++++++ .../journals/revision_component.sass | 5 ++ 6 files changed, 242 insertions(+), 40 deletions(-) create mode 100644 app/components/work_packages/activities_tab/journals/revision_component.html.erb create mode 100644 app/components/work_packages/activities_tab/journals/revision_component.rb create mode 100644 app/components/work_packages/activities_tab/journals/revision_component.sass diff --git a/app/components/_index.sass b/app/components/_index.sass index 59f6c3cdfa43..993e12daeb75 100644 --- a/app/components/_index.sass +++ b/app/components/_index.sass @@ -2,6 +2,7 @@ @import "work_packages/activities_tab/journals/new_component" @import "work_packages/activities_tab/journals/index_component" @import "work_packages/activities_tab/journals/item_component" +@import "work_packages/activities_tab/journals/revision_component" @import "work_packages/activities_tab/journals/item_component/details" @import "work_packages/activities_tab/journals/item_component/add_reactions" @import "work_packages/activities_tab/journals/item_component/reactions" 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 c541372a20a6..9ef8ded4e52b 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 @@ -7,7 +7,7 @@ mb: inner_container_margin_bottom ) do flex_layout(id: insert_target_modifier_id, - data: { test_selector: "op-wp-journals-container" }) do |journals_index_container| + 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( @@ -22,12 +22,16 @@ end end - recent_journals.each do |journal| + recent_journals.each do |record| journals_index_container.with_row do - render(WorkPackages::ActivitiesTab::Journals::ItemComponent.new( - journal:, filter:, - grouped_emoji_reactions: wp_journals_grouped_emoji_reactions[journal.id] - )) + if record.is_a?(Changeset) + render(WorkPackages::ActivitiesTab::Journals::RevisionComponent.new(changeset: record, filter:)) + else + render(WorkPackages::ActivitiesTab::Journals::ItemComponent.new( + journal: record, filter:, + grouped_emoji_reactions: wp_journals_grouped_emoji_reactions[record.id] + )) + end end end @@ -48,12 +52,16 @@ else helpers.turbo_frame_tag("work-package-activities-tab-content-older-journals") do flex_layout do |older_journals_container| - older_journals.each do |journal| + older_journals.each do |record| older_journals_container.with_row do - render(WorkPackages::ActivitiesTab::Journals::ItemComponent.new( - journal:, filter:, - grouped_emoji_reactions: wp_journals_grouped_emoji_reactions[journal.id] - )) + if record.is_a?(Changeset) + render(WorkPackages::ActivitiesTab::Journals::RevisionComponent.new(changeset: record, filter:)) + else + render(WorkPackages::ActivitiesTab::Journals::ItemComponent.new( + journal: record, filter:, + grouped_emoji_reactions: wp_journals_grouped_emoji_reactions[record.id] + )) + end 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 469f0e3d4e11..67a199e280a0 100644 --- a/app/components/work_packages/activities_tab/journals/index_component.rb +++ b/app/components/work_packages/activities_tab/journals/index_component.rb @@ -64,47 +64,70 @@ def journal_sorting_desc? end def base_journals - work_package - .journals - .includes( - :user, - :customizable_journals, - :attachable_journals, - :storable_journals, - :notifications - ) - .reorder(version: journal_sorting) - .with_sequence_version + # Get journals with eager loading + journals = API::V3::Activities::ActivityEagerLoadingWrapper.wrap( + work_package + .journals + .includes( + :user, + :customizable_journals, + :attachable_journals, + :storable_journals, + :notifications + ) + .reorder(version: journal_sorting) + .with_sequence_version + ) + + # Get associated revisions + revisions = work_package.changesets.includes(:user, :repository) + + # Combine and sort them by date + if journal_sorting_desc? + (journals + revisions).sort_by do |record| + timestamp = if record.is_a?(API::V3::Activities::ActivityEagerLoadingWrapper) + record.created_at&.to_i + elsif record.is_a?(Changeset) + record.committed_on.to_i + end + [-timestamp, -record.id] + end + else + (journals + revisions).sort_by do |record| + timestamp = if record.is_a?(API::V3::Activities::ActivityEagerLoadingWrapper) + record.created_at&.to_i + elsif record.is_a?(Changeset) + record.committed_on.to_i + end + [timestamp, record.id] + end + end end def journals - API::V3::Activities::ActivityEagerLoadingWrapper.wrap(base_journals) + base_journals end def recent_journals - recent_ones = if journal_sorting_desc? - base_journals.first(MAX_RECENT_JOURNALS) - else - base_journals.last(MAX_RECENT_JOURNALS) - end - - API::V3::Activities::ActivityEagerLoadingWrapper.wrap(recent_ones) + if journal_sorting_desc? + base_journals.first(MAX_RECENT_JOURNALS) + else + base_journals.last(MAX_RECENT_JOURNALS) + end end def older_journals - older_ones = if journal_sorting_desc? - base_journals.offset(MAX_RECENT_JOURNALS) - else - total = base_journals.count - limit = [total - MAX_RECENT_JOURNALS, 0].max - base_journals.limit(limit) - end - - API::V3::Activities::ActivityEagerLoadingWrapper.wrap(older_ones) + if journal_sorting_desc? + base_journals.drop(MAX_RECENT_JOURNALS) + else + base_journals.take(base_journals.size - MAX_RECENT_JOURNALS) + end end def journal_with_notes - base_journals.where.not(notes: "") + work_package + .journals + .where.not(notes: "") end def wp_journals_grouped_emoji_reactions diff --git a/app/components/work_packages/activities_tab/journals/revision_component.html.erb b/app/components/work_packages/activities_tab/journals/revision_component.html.erb new file mode 100644 index 000000000000..4aac173b7895 --- /dev/null +++ b/app/components/work_packages/activities_tab/journals/revision_component.html.erb @@ -0,0 +1,81 @@ +<%= + component_wrapper(class: "work-packages-activities-tab-journals-item-component") do + flex_layout(data: { + test_selector: "op-wp-revision-entry-#{changeset.id}" + }) do |revision_container| + revision_container.with_row do + render(border_box_container( + id: "activity-anchor-r#{changeset.revision}", + 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-revision-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_start_container.with_column(mr: 2) do + if changeset.user + render(Users::AvatarComponent.new(user: changeset.user, show_name: false, size: :mini)) + end + 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| + user_name_container.with_row(classes: "work-packages-activities-tab-journals-item-component--user-name ellipsis") do + render_user_name + end + user_name_container.with_row do + render(Primer::Beta::Text.new(font_size: :small, color: :subtle, mr: 1)) do + committed_text = render(Primer::Beta::Link.new( + href: revision_url, + scheme: :secondary, + underline: false, + font_size: :small, + target: "_blank" + )) do + I18n.t("js.label_committed_link", revision_identifier: short_revision) + end + I18n.t("js.label_committed_at", + committed_revision_link: committed_text.html_safe, + date: format_time(changeset.committed_on)).html_safe + 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 + render_user_name + end + header_start_container.with_column(mr: 1, classes: "hidden-for-mobile") do + render(Primer::Beta::Text.new(font_size: :small, color: :subtle, mr: 1)) do + committed_text = render(Primer::Beta::Link.new( + href: revision_url, + scheme: :secondary, + underline: false, + font_size: :small, + target: "_blank" + )) do + I18n.t("js.label_committed_link", revision_identifier: short_revision) + end + I18n.t("js.label_committed_at", + committed_revision_link: committed_text.html_safe, + date: format_time(changeset.committed_on)).html_safe + end + end + end + end + end + border_box_component.with_body( + classes: "work-packages-activities-tab-journals-item-component--journal-notes-body", + data: { test_selector: "op-revision-notes-body" } + ) do + render(Primer::Box.new(mt: 1, classes: "op-uc-container")) do + format_text(changeset, :comments) + end + end + end + end + revision_container.with_row(flex_layout: true, classes: "work-packages-activities-tab-revision-component--stem-line-container") do |stem_line_container| + stem_line_container.with_column(border: :left, classes: "work-packages-activities-tab-revision-component--stem-line") + end + end + end +%> \ No newline at end of file diff --git a/app/components/work_packages/activities_tab/journals/revision_component.rb b/app/components/work_packages/activities_tab/journals/revision_component.rb new file mode 100644 index 000000000000..e401584bfcdd --- /dev/null +++ b/app/components/work_packages/activities_tab/journals/revision_component.rb @@ -0,0 +1,84 @@ +module WorkPackages + module ActivitiesTab + module Journals + class RevisionComponent < ApplicationComponent + include ApplicationHelper + include OpPrimer::ComponentHelpers + include OpTurbo::Streamable + + def initialize(changeset:, filter:) + super + + @changeset = changeset + @filter = filter + end + + private + + attr_reader :changeset, :filter + + def render? + filter != :only_comments + end + + def user_name + if changeset.user + changeset.user.name + else + # Extract name from committer string (format: "name ") + changeset.committer.split("<").first.strip + end + end + + def revision_url + repository = changeset.repository + project = repository.project + + show_revision_project_repository_path(project_id: project.id, rev: changeset.revision) + end + + def short_revision + changeset.revision[0..7] + end + + def copy_url_action_item(menu) + menu.with_item(label: t("button_copy_link_to_clipboard"), + tag: :button, + content_arguments: { + data: { + action: "click->work-packages--activities-tab--item#copyActivityUrlToClipboard" + } + }) do |item| + item.with_leading_visual_icon(icon: :copy) + end + end + + def render_user_name + if changeset.user + render_user_link(changeset.user) + else + render_committer_name(changeset.committer) + end + end + + def render_user_link(user) + render(Primer::Beta::Link.new( + href: user_url(user), + target: "_blank", + scheme: :primary, + underline: false, + font_weight: :bold + )) do + changeset.user.name + end + end + + def render_committer_name(committer) + render(Primer::Beta::Text.new(font_weight: :bold, mr: 1)) do + committer.split("<").first.strip + end + end + end + end + end +end diff --git a/app/components/work_packages/activities_tab/journals/revision_component.sass b/app/components/work_packages/activities_tab/journals/revision_component.sass new file mode 100644 index 000000000000..5d875d4cab43 --- /dev/null +++ b/app/components/work_packages/activities_tab/journals/revision_component.sass @@ -0,0 +1,5 @@ +.work-packages-activities-tab-revision-component + &--stem-line-container + min-height: 20px + &--stem-line + margin-left: 19px \ No newline at end of file From e0c0604732cbcdf8655529e95d5a704ea9d3e336 Mon Sep 17 00:00:00 2001 From: Jonas Jabari Date: Mon, 13 Jan 2025 17:25:31 +0100 Subject: [PATCH 3/4] refactoring and added specs --- .../journals/revision_component.rb | 2 +- .../work_package/revision_component_spec.rb | 118 ++++++++++++++++++ 2 files changed, 119 insertions(+), 1 deletion(-) create mode 100644 spec/features/activities/work_package/revision_component_spec.rb diff --git a/app/components/work_packages/activities_tab/journals/revision_component.rb b/app/components/work_packages/activities_tab/journals/revision_component.rb index e401584bfcdd..abcc30323072 100644 --- a/app/components/work_packages/activities_tab/journals/revision_component.rb +++ b/app/components/work_packages/activities_tab/journals/revision_component.rb @@ -75,7 +75,7 @@ def render_user_link(user) def render_committer_name(committer) render(Primer::Beta::Text.new(font_weight: :bold, mr: 1)) do - committer.split("<").first.strip + committer.gsub(%r{<.+@.+>}, "").strip end end end diff --git a/spec/features/activities/work_package/revision_component_spec.rb b/spec/features/activities/work_package/revision_component_spec.rb new file mode 100644 index 000000000000..64e98590b488 --- /dev/null +++ b/spec/features/activities/work_package/revision_component_spec.rb @@ -0,0 +1,118 @@ +#-- 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 "Work package revision component", :js, :with_cuprite, with_flag: { primerized_work_package_activities: true } do + let(:project) { create(:project) } + let(:user) { create(:admin) } + let(:work_package) { create(:work_package, project:) } + let(:wp_page) { Pages::FullWorkPackage.new(work_package, project) } + let(:activity_tab) { Components::WorkPackages::Activities.new(work_package) } + + let(:repository) { create(:repository_subversion, project:) } + let(:revision_time) { 2.days.ago } + let(:revision_message) { "A commit message for a revision" } + let(:revision_identifier) { "123" } + + current_user { user } + + before do + # Enable subversion in settings + Setting.enabled_scm = Setting.enabled_scm << repository.vendor + + # Associate changeset with work package + work_package.changesets << changeset + + wp_page.visit! + wp_page.wait_for_activity_tab + end + + shared_examples "shows revision details" do + it "displays the revision details correctly" do + # Verify revision message is displayed + expect(page).to have_test_selector("op-revision-notes-body", text: revision_message) + + # Verify revision link is displayed with correct identifier + expect(page).to have_link(revision_identifier, + href: show_revision_project_repository_path(project_id: project.id, rev: revision_identifier)) + + # Verify revision is shown when filter is set to all + activity_tab.filter_journals(:all) + expect(page).to have_test_selector("op-revision-notes-body", text: revision_message) + + # Verify revision is shown when filter is set to only changes + activity_tab.filter_journals(:only_changes) + expect(page).to have_test_selector("op-revision-notes-body", text: revision_message) + + # Verify revision is not shown when filter is set to only comments + activity_tab.filter_journals(:only_comments) + expect(page).not_to have_test_selector("op-revision-notes-body", text: revision_message) + end + end + + context "with unmapped repository user" do + let!(:changeset) do + create(:changeset, + comments: revision_message, + committed_on: revision_time, + repository:, + committer: "a_person", + revision: revision_identifier) + end + + it "displays the committer name" do + expect(page).to have_text("a_person") + end + + include_examples "shows revision details" + end + + context "with mapped repository user" do + let(:repository_user) { create(:user, firstname: "Repository", lastname: "User") } + let!(:changeset) do + create(:changeset, + comments: revision_message, + committed_on: revision_time, + repository:, + committer: repository_user.login, + user: repository_user, + revision: revision_identifier) + end + + it "displays the mapped user with avatar" do + expect(page).to have_test_selector("op-revision-header") + within(page.find_test_selector("op-revision-header")) do + expect(page).to have_test_selector("op-principal") + expect(page).to have_text("Repository User") + end + end + + include_examples "shows revision details" + end +end From b5796c3b2770e07f210b13ee0c100708308b014e Mon Sep 17 00:00:00 2001 From: Jonas Jabari Date: Tue, 14 Jan 2025 16:00:27 +0100 Subject: [PATCH 4/4] fixed header height --- .../activities_tab/journals/revision_component.html.erb | 2 +- .../activities_tab/journals/revision_component.sass | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/app/components/work_packages/activities_tab/journals/revision_component.html.erb b/app/components/work_packages/activities_tab/journals/revision_component.html.erb index 4aac173b7895..be28bcec0dd7 100644 --- a/app/components/work_packages/activities_tab/journals/revision_component.html.erb +++ b/app/components/work_packages/activities_tab/journals/revision_component.html.erb @@ -10,7 +10,7 @@ "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-revision-header" }) do - flex_layout(align_items: :center, justify_content: :space_between) do |header_container| + flex_layout(align_items: :center, justify_content: :space_between, classes: "work-packages-activities-tab-revision-component--header") 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_start_container.with_column(mr: 2) do diff --git a/app/components/work_packages/activities_tab/journals/revision_component.sass b/app/components/work_packages/activities_tab/journals/revision_component.sass index 5d875d4cab43..cb7dd8bd956a 100644 --- a/app/components/work_packages/activities_tab/journals/revision_component.sass +++ b/app/components/work_packages/activities_tab/journals/revision_component.sass @@ -1,4 +1,6 @@ .work-packages-activities-tab-revision-component + &--header + min-height: 32px &--stem-line-container min-height: 20px &--stem-line