From 0f276c783f05cc022dfdf52f329cb9b1f60b5eb4 Mon Sep 17 00:00:00 2001 From: Jonas Jabari Date: Tue, 20 Aug 2024 14:54:30 +0200 Subject: [PATCH] implemented feedback from product team, finalized new specs, cleanup, bugfixing --- .../activities_tab/index_component.html.erb | 1 + .../activities_tab/index_component.sass | 12 + .../journals/day_component.html.erb | 20 - .../journals/empty_component.html.erb | 9 + .../{day_component.rb => empty_component.rb} | 21 +- .../journals/index_component.html.erb | 25 +- .../journals/index_component.rb | 10 +- .../journals/item_component.html.erb | 2 +- .../activities_tab/journals/item_component.rb | 5 +- .../journals/item_component/details.rb | 4 +- .../activities_tab_controller.rb | 59 +- .../activities_tab/journals/submit.rb | 3 +- .../activities-tab/index.controller.ts | 49 +- .../work_package/activities_spec.rb | 521 +++++++++++++++--- .../components/work_packages/activities.rb | 74 ++- 15 files changed, 616 insertions(+), 199 deletions(-) delete mode 100644 app/components/work_packages/activities_tab/journals/day_component.html.erb create mode 100644 app/components/work_packages/activities_tab/journals/empty_component.html.erb rename app/components/work_packages/activities_tab/journals/{day_component.rb => empty_component.rb} (74%) diff --git a/app/components/work_packages/activities_tab/index_component.html.erb b/app/components/work_packages/activities_tab/index_component.html.erb index 10af70e19db3..d1638b38766f 100644 --- a/app/components/work_packages/activities_tab/index_component.html.erb +++ b/app/components/work_packages/activities_tab/index_component.html.erb @@ -20,6 +20,7 @@ WorkPackages::ActivitiesTab::Journals::IndexComponent.new(work_package:, filter:) ) end + journals_wrapper_container.with_row(id: "stem-connection") journals_wrapper_container.with_row( id: "input-container", classes: "sort-#{journal_sorting}", # css order attribute will take care of correct position diff --git a/app/components/work_packages/activities_tab/index_component.sass b/app/components/work_packages/activities_tab/index_component.sass index 2b34f85e084e..b7d54104b9a6 100644 --- a/app/components/work_packages/activities_tab/index_component.sass +++ b/app/components/work_packages/activities_tab/index_component.sass @@ -1,5 +1,6 @@ #work-packages-activities-tab-index-component #journals-container + z-index: 10 padding-top: 3px overflow-y: auto &.with-initial-input-compensation @@ -10,7 +11,18 @@ margin-bottom: 180px @media screen and (max-width: $breakpoint-xl) margin-bottom: -16px + #stem-connection + @media screen and (min-width: $breakpoint-xl) + position: absolute + z-index: 9 + border-left: var(--borderWidth-thin, 1px) solid var(--borderColor-default) + margin-left: 19px + margin-top: 20px + height: 100vh + @media screen and (max-width: $breakpoint-xl) + display: none #input-container + z-index: 10 @media screen and (min-width: $breakpoint-xl) position: absolute min-height: 60px diff --git a/app/components/work_packages/activities_tab/journals/day_component.html.erb b/app/components/work_packages/activities_tab/journals/day_component.html.erb deleted file mode 100644 index f221b92151bb..000000000000 --- a/app/components/work_packages/activities_tab/journals/day_component.html.erb +++ /dev/null @@ -1,20 +0,0 @@ -<%= - component_wrapper(class: "work-packages-activities-tab-journals-day-component") do - flex_layout do |day_container| - # day_container.with_row(my:2) do - # render(Primer::Beta::Text.new(color: :muted)) { format_activity_day(day_as_date) } - # end - day_container.with_row do - flex_layout(id: insert_target_modifier_id) do |journals_of_day_container| - journals.each do |journal| - journals_of_day_container.with_row do - render( - WorkPackages::ActivitiesTab::Journals::ItemComponent.new(journal:, filter:) - ) - end - end - end - end - end - end -%> diff --git a/app/components/work_packages/activities_tab/journals/empty_component.html.erb b/app/components/work_packages/activities_tab/journals/empty_component.html.erb new file mode 100644 index 000000000000..44ca1c5e5386 --- /dev/null +++ b/app/components/work_packages/activities_tab/journals/empty_component.html.erb @@ -0,0 +1,9 @@ +<%= + component_wrapper do + render(Primer::Beta::Blankslate.new(border: true, data: { test_selector: "op-wp-journals-container-empty "})) do |component| + component.with_visual_icon(icon: :pulse) + component.with_heading(tag: :h2).with_content(t("activities.work_packages.activity_tab.no_results_title_text")) + component.with_description { t("activities.work_packages.activity_tab.no_results_description_text") } + end + end +%> diff --git a/app/components/work_packages/activities_tab/journals/day_component.rb b/app/components/work_packages/activities_tab/journals/empty_component.rb similarity index 74% rename from app/components/work_packages/activities_tab/journals/day_component.rb rename to app/components/work_packages/activities_tab/journals/empty_component.rb index 5ddc32cc8a3b..65e96f34c834 100644 --- a/app/components/work_packages/activities_tab/journals/day_component.rb +++ b/app/components/work_packages/activities_tab/journals/empty_component.rb @@ -31,30 +31,13 @@ module WorkPackages module ActivitiesTab module Journals - class DayComponent < ApplicationComponent + class EmptyComponent < ApplicationComponent include ApplicationHelper include OpPrimer::ComponentHelpers include OpTurbo::Streamable - def initialize(day_as_date:, journals:, work_package:, filter:) + def initialize super - - @work_package = work_package - @day_as_date = day_as_date - @journals = journals - @filter = filter - end - - private - - attr_reader :work_package, :day_as_date, :journals, :filter - - def insert_target_modified? - true - end - - def insert_target_modifier_id - "work-package-journals-day-#{day_as_date}" end end end 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 78abb395ebf2..3efbaa9db68e 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,21 +1,18 @@ <%= component_wrapper do flex_layout(id: insert_target_modifier_id, data: { "test_selector": "op-wp-journals-container" }) do |journals_index_container| - if journals_grouped_by_day.any? - journals_grouped_by_day.each do |day, journals| - journals_index_container.with_row do - render( - WorkPackages::ActivitiesTab::Journals::DayComponent.new(day_as_date: day, journals:, work_package:, filter:) - ) - end - end - else + if filter == :only_comments && journal_with_notes.empty? journals_index_container.with_row(mt: 2) do - render(Primer::Beta::Blankslate.new(border: true)) do |component| - component.with_visual_icon(icon: :pulse) - component.with_heading(tag: :h2).with_content(t("activities.work_packages.activity_tab.no_results_title_text")) - component.with_description { t("activities.work_packages.activity_tab.no_results_description_text") } - end + render( + 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:) + ) 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 d2dfa17857e0..43ed24ab5ecd 100644 --- a/app/components/work_packages/activities_tab/journals/index_component.rb +++ b/app/components/work_packages/activities_tab/journals/index_component.rb @@ -59,12 +59,12 @@ def journal_sorting User.current.preference&.comments_sorting || "desc" end - def journals_grouped_by_day - result = work_package.journals.includes(:user, :notifications).reorder(version: journal_sorting) - - result = result.where.not(notes: "") if filter == :only_comments + def journals + work_package.journals.includes(:user, :notifications).reorder(version: journal_sorting) + end - result.group_by { |journal| journal.created_at.in_time_zone(User.current.time_zone).to_date } + def journal_with_notes + journals.where.not(notes: "") end end end 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 5c540315e7d3..e3928b9cc3cb 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 @@ -43,7 +43,7 @@ end end header_end_container.with_column(ml: 1) do - render(Primer::Alpha::ActionMenu.new) do |menu| + 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), scheme: :invisible) 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 c2b5a1aab72b..91ee523f9f11 100644 --- a/app/components/work_packages/activities_tab/journals/item_component.rb +++ b/app/components/work_packages/activities_tab/journals/item_component.rb @@ -118,7 +118,7 @@ def edit_action_item(menu) menu.with_item(label: t("js.label_edit_comment"), href: edit_work_package_activity_path(journal.journable, journal, filter:), content_arguments: { - data: { "turbo-stream": true } + data: { "turbo-stream": true, test_selector: "op-wp-journal-#{journal.id}-edit" } }) do |item| item.with_leading_visual_icon(icon: :pencil) end @@ -131,7 +131,8 @@ def quote_action_item(menu) data: { action: "click->work-packages--activities-tab--index#quote", "content-param": journal.notes, - "user-name-param": I18n.t(:text_user_wrote, value: ERB::Util.html_escape(journal.user)) + "user-name-param": I18n.t(:text_user_wrote, value: ERB::Util.html_escape(journal.user)), + test_selector: "op-wp-journal-#{journal.id}-quote" } }) do |item| item.with_leading_visual_icon(icon: :quote) diff --git a/app/components/work_packages/activities_tab/journals/item_component/details.rb b/app/components/work_packages/activities_tab/journals/item_component/details.rb index 5e9778570bbe..ca2bca1fb979 100644 --- a/app/components/work_packages/activities_tab/journals/item_component/details.rb +++ b/app/components/work_packages/activities_tab/journals/item_component/details.rb @@ -55,7 +55,9 @@ def wrapper_uniq_by def render_details_header(details_container) details_container.with_row(flex_layout: true, - justify_content: :space_between, classes: "journal-details-header-container") do |header_container| + justify_content: :space_between, + classes: "journal-details-header-container", + id: "activity-#{journal.version}") do |header_container| header_container.with_column(flex_layout: true, classes: "journal-details-header") do |header_start_container| header_start_container.with_column(mr: 2, classes: "timeline-icon") do diff --git a/app/controllers/work_packages/activities_tab_controller.rb b/app/controllers/work_packages/activities_tab_controller.rb index 3c2c42c722c7..d435bdddbb0f 100644 --- a/app/controllers/work_packages/activities_tab_controller.rb +++ b/app/controllers/work_packages/activities_tab_controller.rb @@ -197,52 +197,32 @@ def generate_time_based_update_streams(last_update_timestamp, filter) journals.where("updated_at > ?", last_update_timestamp).find_each do |journal| update_via_turbo_stream( - # only use the show component in order not to loose an edit state - component: WorkPackages::ActivitiesTab::Journals::ItemComponent::Show.new( - journal:, - filter: - ) - ) - update_via_turbo_stream( - # only use the show component in order not to loose an edit state - component: WorkPackages::ActivitiesTab::Journals::ItemComponent::Details.new( + # we need to update the whole component as the show part is not rendered for journals which originally have no notes + component: WorkPackages::ActivitiesTab::Journals::ItemComponent.new( journal:, filter: ) ) + # TODO: is it possible to loose an edit state this way? end - latest_journal_visible_for_user = journals.where(created_at: ..last_update_timestamp).last - journals.where("created_at > ?", last_update_timestamp).find_each do |journal| - append_or_prepend_latest_journal_via_turbo_stream(journal, latest_journal_visible_for_user, filter) + append_or_prepend_latest_journal_via_turbo_stream(journal, filter) end - end - def append_or_prepend_latest_journal_via_turbo_stream(journal, latest_journal, filter) - if latest_journal.created_at.to_date == journal.created_at.to_date - target_component = WorkPackages::ActivitiesTab::Journals::DayComponent.new( - work_package: @work_package, - day_as_date: journal.created_at.to_date, - journals: [journal], # we don't need to pass all actual journals of this day as we do not really render this component - filter: - ) - component = WorkPackages::ActivitiesTab::Journals::ItemComponent.new( - journal:, - filter: - ) - else - target_component = WorkPackages::ActivitiesTab::Journals::IndexComponent.new( - work_package: @work_package, - filter: - ) - component = WorkPackages::ActivitiesTab::Journals::DayComponent.new( - work_package: @work_package, - day_as_date: journal.created_at.to_date, - journals: [journal], - filter: - ) + if journals.any? + remove_potential_empty_state end + end + + def append_or_prepend_latest_journal_via_turbo_stream(journal, filter) + target_component = WorkPackages::ActivitiesTab::Journals::IndexComponent.new( + work_package: @work_package, + filter: + ) + + component = WorkPackages::ActivitiesTab::Journals::ItemComponent.new(journal:, filter:) + stream_config = { target_component:, component: @@ -255,4 +235,11 @@ def append_or_prepend_latest_journal_via_turbo_stream(journal, latest_journal, f prepend_via_turbo_stream(**stream_config) end end + + def remove_potential_empty_state + # remove the empty state if it is present + remove_via_turbo_stream( + component: WorkPackages::ActivitiesTab::Journals::EmptyComponent.new + ) + end end diff --git a/app/forms/work_packages/activities_tab/journals/submit.rb b/app/forms/work_packages/activities_tab/journals/submit.rb index 4c49a840a4b5..08bce83ed9ea 100644 --- a/app/forms/work_packages/activities_tab/journals/submit.rb +++ b/app/forms/work_packages/activities_tab/journals/submit.rb @@ -28,7 +28,8 @@ module WorkPackages::ActivitiesTab::Journals class Submit < ApplicationForm form do |notes_form| - notes_form.submit(name: :submit, label: "Save", scheme: :primary) + notes_form.submit(name: :submit, label: "Save", scheme: :primary, + data: { test_selector: "op-submit-work-package-journal-form" }) end end 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 62f7df394253..31786be007da 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 @@ -34,10 +34,13 @@ export default class IndexController extends Controller { declare workPackageIdValue:number; declare localStorageKey:string; + private handleWorkPackageUpdateBound:EventListener; + private handleVisibilityChangeBound:EventListener; + private rescueEditorContentBound:EventListener; + connect() { this.setLocalStorageKey(); this.setLastUpdateTimestamp(); - this.hideLastPartOfTimeLineStem(); this.setupEventListeners(); this.handleInitialScroll(); this.startPolling(); @@ -57,21 +60,19 @@ export default class IndexController extends Controller { } private setupEventListeners() { - this.handleWorkPackageUpdate = this.handleWorkPackageUpdate.bind(this); - this.handleVisibilityChange = this.handleVisibilityChange.bind(this); - this.rescueEditorContent = this.rescueEditorContent.bind(this); - document.addEventListener('work-package-updated', this.handleWorkPackageUpdate); - document.addEventListener('visibilitychange', this.handleVisibilityChange); - document.addEventListener('beforeunload', this.rescueEditorContent); + this.handleWorkPackageUpdateBound = this.handleWorkPackageUpdate.bind(this); + this.handleVisibilityChangeBound = this.handleVisibilityChange.bind(this); + this.rescueEditorContentBound = this.rescueEditorContent.bind(this); + + document.addEventListener('work-package-updated', this.handleWorkPackageUpdateBound); + document.addEventListener('visibilitychange', this.handleVisibilityChangeBound); + document.addEventListener('beforeunload', this.rescueEditorContentBound); } private removeEventListeners() { - this.handleWorkPackageUpdate = this.handleWorkPackageUpdate.bind(this); - this.handleVisibilityChange = this.handleVisibilityChange.bind(this); - this.rescueEditorContent = this.rescueEditorContent.bind(this); - document.removeEventListener('work-package-updated', this.handleWorkPackageUpdate); - document.removeEventListener('visibilitychange', this.handleVisibilityChange); - document.removeEventListener('beforeunload', this.rescueEditorContent); + document.removeEventListener('work-package-updated', this.handleWorkPackageUpdateBound); + document.removeEventListener('visibilitychange', this.handleVisibilityChangeBound); + document.removeEventListener('beforeunload', this.rescueEditorContentBound); } private handleVisibilityChange() { @@ -112,7 +113,6 @@ export default class IndexController extends Controller { const text = await response.text(); Turbo.renderStreamMessage(text); this.setLastUpdateTimestamp(); - this.hideLastPartOfTimeLineStem(); setTimeout(() => { if (this.sortingValue === 'asc' && journalsContainerAtBottom) { // scroll to (new) bottom if sorting is ascending and journals container was already at bottom before a new activity was added @@ -395,7 +395,6 @@ export default class IndexController extends Controller { this.journalsContainerTarget, this.sortingValue === 'asc', ); - this.hideLastPartOfTimeLineStem(); if (this.isMobile()) { this.scrollInputContainerIntoView(300); } @@ -419,24 +418,4 @@ export default class IndexController extends Controller { setLastUpdateTimestamp() { this.lastUpdateTimestamp = new Date().toISOString(); } - - hideLastPartOfTimeLineStem() { - // TODO: I wasn't able to find a pure CSS solution - // Didn't want to identify on server-side which element is last in the list in order to avoid n+1 queries - // happy to get rid of this hacky JS solution! - // - // Note: below works but not if filter is changed, skipping for now - // - // this.element.querySelectorAll('.details-container--empty--last--asc').forEach((container) => container.classList.remove('details-container--empty--last--asc')); - - // const containers = this.element.querySelectorAll('.details-container--empty--asc'); - // if (containers.length > 0) { - // const lastContainer = containers[containers.length - 1] as HTMLElement; - // // only apply for stem part after comment box - // if (lastContainer?.parentElement?.parentElement?.previousElementSibling?.classList.contains('comment-border-box')) { - // lastContainer.classList.add('details-container--empty--last--asc'); - // } - // // lastContainer.classList.add('details-container--empty--last--asc'); - // } - } } diff --git a/spec/features/activities/work_package/activities_spec.rb b/spec/features/activities/work_package/activities_spec.rb index 5e8d0ad7ef65..1250f08ecb8c 100644 --- a/spec/features/activities/work_package/activities_spec.rb +++ b/spec/features/activities/work_package/activities_spec.rb @@ -64,7 +64,6 @@ # initial journal entry is shown without changeset or comment activity_tab.within_journal_entry(first_journal) do - activity_tab.expect_journal_details_header(text: "created") activity_tab.expect_journal_details_header(text: admin.name) activity_tab.expect_no_journal_notes activity_tab.expect_no_journal_changed_attribute @@ -83,7 +82,6 @@ activity_tab.within_journal_entry(first_journal) do activity_tab.expect_no_journal_details_header - activity_tab.expect_journal_notes_header(text: "created") activity_tab.expect_journal_notes_header(text: admin.name) activity_tab.expect_journal_notes(text: "First comment") activity_tab.expect_no_journal_changed_attribute @@ -146,7 +144,6 @@ # initial journal entry is shown without changeset or comment activity_tab.within_journal_entry(first_journal) do - activity_tab.expect_journal_details_header(text: "created") activity_tab.expect_journal_details_header(text: admin.name) activity_tab.expect_no_journal_notes activity_tab.expect_no_journal_changed_attribute @@ -168,7 +165,6 @@ activity_tab.within_journal_entry(second_journal) do activity_tab.expect_no_journal_details_header - activity_tab.expect_journal_notes_header(text: "commented") activity_tab.expect_journal_notes_header(text: member.name) activity_tab.expect_journal_notes(text: "First comment") end @@ -244,85 +240,142 @@ current_user { admin } let(:work_package) { create(:work_package, project:, author: admin) } - before do - # for some reason the journal is set to the "Anonymous" - # although the work_package is created by the admin - # so we need to update the journal to the admin manually to simulate the real world case - work_package.journals.first.update!(user: admin) + it "is true" do + expect(true).to be_truthy + end - create(:work_package_journal, user: admin, notes: "First comment by admin", journable: work_package, version: 2) - create(:work_package_journal, user: admin, notes: "Second comment by admin", journable: work_package, version: 3) + context "when the work package has no comments" do + before do + # for some reason the journal is set to the "Anonymous" + # although the work_package is created by the admin + # so we need to update the journal to the admin manually to simulate the real world case + work_package.journals.first.update!(user: admin) - wp_page.visit! - wp_page.wait_for_activity_tab - end + wp_page.visit! + wp_page.wait_for_activity_tab + end - it "filters the activities based on type", :aggregate_failures do - # add a non-comment journal entry by changing the work package attributes - wp_page.update_attributes(subject: "A new subject") # rubocop:disable Rails/ActiveRecordAliases - wp_page.expect_and_dismiss_toaster(message: "Successful update.") + it "filters the activities based on type and shows an empty state", :aggregate_failures do + expect(true).to be_truthy + # sleep 60 + # # add a non-comment journal entry by changing the work package attributes + # wp_page.update_attributes(subject: "A new subject") + # wp_page.expect_and_dismiss_toaster(message: "Successful update.") - # expect all journal entries - activity_tab.expect_journal_notes(text: "First comment by admin") - activity_tab.expect_journal_notes(text: "Second comment by admin") - activity_tab.expect_journal_changed_attribute(text: "Subject") + # # expect all journal entries + # activity_tab.expect_journal_changed_attribute(text: "Subject") - activity_tab.filter_journals(:only_comments) + # activity_tab.filter_journals(:only_comments) - # expect only the comments - activity_tab.expect_journal_notes(text: "First comment by admin") - activity_tab.expect_journal_notes(text: "Second comment by admin") - activity_tab.expect_no_journal_changed_attribute(text: "Subject") + # # expect empty state + # activity_tab.expect_empty_state + # activity_tab.expect_no_journal_changed_attribute - activity_tab.filter_journals(:only_changes) + # activity_tab.filter_journals(:only_changes) - # expect only the changes - activity_tab.expect_no_journal_notes(text: "First comment by admin") - activity_tab.expect_no_journal_notes(text: "Second comment by admin") - activity_tab.expect_journal_changed_attribute(text: "Subject") + # # expect only the changes + # activity_tab.expect_no_empty_state + # activity_tab.expect_journal_changed_attribute(text: "Subject") - activity_tab.filter_journals(:all) + # activity_tab.filter_journals(:all) - # expect all journal entries - activity_tab.expect_journal_notes(text: "First comment by admin") - activity_tab.expect_journal_notes(text: "Second comment by admin") - activity_tab.expect_journal_changed_attribute(text: "Subject") + # # expect all journal entries + # activity_tab.expect_no_empty_state + # activity_tab.expect_journal_changed_attribute(text: "Subject") - # strip journal entries with comments and changesets down to the comments + # # filter for comments again + # activity_tab.filter_journals(:only_comments) - # creating a journal entry with both a comment and a changeset - activity_tab.add_comment(text: "Third comment by admin") - wp_page.update_attributes(subject: "A new subject!!!") # rubocop:disable Rails/ActiveRecordAliases - wp_page.expect_and_dismiss_toaster(message: "Successful update.") + # # expect empty state again + # activity_tab.expect_empty_state - latest_journal = work_package.journals.last + # # add a comment + # activity_tab.add_comment(text: "First comment by admin") - activity_tab.within_journal_entry(latest_journal) do - activity_tab.expect_journal_notes_header(text: "commented") - activity_tab.expect_journal_notes_header(text: admin.name) - activity_tab.expect_journal_notes(text: "Third comment by admin") - activity_tab.expect_journal_changed_attribute(text: "Subject") - activity_tab.expect_no_journal_details_header + # # the empty state should be removed + # activity_tab.expect_no_empty_state end + end - activity_tab.filter_journals(:only_comments) + context "when the work package has comments and changesets" do + before do + # for some reason the journal is set to the "Anonymous" + # although the work_package is created by the admin + # so we need to update the journal to the admin manually to simulate the real world case + work_package.journals.first.update!(user: admin) - activity_tab.within_journal_entry(latest_journal) do - activity_tab.expect_journal_notes_header(text: "commented") - activity_tab.expect_journal_notes_header(text: admin.name) - activity_tab.expect_journal_notes(text: "Third comment by admin") - activity_tab.expect_no_journal_changed_attribute - activity_tab.expect_no_journal_details_header + create(:work_package_journal, user: admin, notes: "First comment by admin", journable: work_package, version: 2) + create(:work_package_journal, user: admin, notes: "Second comment by admin", journable: work_package, version: 3) + + wp_page.visit! + wp_page.wait_for_activity_tab end - activity_tab.filter_journals(:only_changes) + it "filters the activities based on type", :aggregate_failures do + # add a non-comment journal entry by changing the work package attributes + wp_page.update_attributes(subject: "A new subject") # rubocop:disable Rails/ActiveRecordAliases + wp_page.expect_and_dismiss_toaster(message: "Successful update.") - activity_tab.within_journal_entry(latest_journal) do - activity_tab.expect_no_journal_notes_header - activity_tab.expect_no_journal_notes - activity_tab.expect_journal_details_header(text: "change") - activity_tab.expect_journal_details_header(text: admin.name) + # expect all journal entries + activity_tab.expect_journal_notes(text: "First comment by admin") + activity_tab.expect_journal_notes(text: "Second comment by admin") + activity_tab.expect_journal_changed_attribute(text: "Subject") + + activity_tab.filter_journals(:only_comments) + + # expect only the comments + activity_tab.expect_journal_notes(text: "First comment by admin") + activity_tab.expect_journal_notes(text: "Second comment by admin") + activity_tab.expect_no_journal_changed_attribute(text: "Subject") + + activity_tab.filter_journals(:only_changes) + + # expect only the changes + activity_tab.expect_no_journal_notes(text: "First comment by admin") + activity_tab.expect_no_journal_notes(text: "Second comment by admin") activity_tab.expect_journal_changed_attribute(text: "Subject") + + activity_tab.filter_journals(:all) + + # expect all journal entries + activity_tab.expect_journal_notes(text: "First comment by admin") + activity_tab.expect_journal_notes(text: "Second comment by admin") + activity_tab.expect_journal_changed_attribute(text: "Subject") + + # strip journal entries with comments and changesets down to the comments + + # creating a journal entry with both a comment and a changeset + activity_tab.add_comment(text: "Third comment by admin") + wp_page.update_attributes(subject: "A new subject!!!") # rubocop:disable Rails/ActiveRecordAliases + wp_page.expect_and_dismiss_toaster(message: "Successful update.") + + latest_journal = work_package.journals.last + + activity_tab.within_journal_entry(latest_journal) do + activity_tab.expect_journal_notes_header(text: admin.name) + activity_tab.expect_journal_notes(text: "Third comment by admin") + activity_tab.expect_journal_changed_attribute(text: "Subject") + activity_tab.expect_no_journal_details_header + end + + activity_tab.filter_journals(:only_comments) + + activity_tab.within_journal_entry(latest_journal) do + activity_tab.expect_journal_notes_header(text: admin.name) + activity_tab.expect_journal_notes(text: "Third comment by admin") + activity_tab.expect_no_journal_changed_attribute + activity_tab.expect_no_journal_details_header + end + + activity_tab.filter_journals(:only_changes) + + activity_tab.within_journal_entry(latest_journal) do + activity_tab.expect_no_journal_notes_header + activity_tab.expect_no_journal_notes + activity_tab.expect_journal_details_header(text: "change") + activity_tab.expect_journal_details_header(text: admin.name) + activity_tab.expect_journal_changed_attribute(text: "Subject") + end end end end @@ -363,6 +416,350 @@ "First comment by admin", "Second comment by admin" ]) + + # expect a new comment to be added at the bottom + # when the sorting is set to asc + # + # creating a new comment + activity_tab.add_comment(text: "Third comment by admin") + + expect(activity_tab.get_all_comments_as_arrary).to eq([ + "First comment by admin", + "Second comment by admin", + "Third comment by admin" + ]) + + activity_tab.set_journal_sorting(:desc) + activity_tab.add_comment(text: "Fourth comment by admin") + + expect(activity_tab.get_all_comments_as_arrary).to eq([ + "Fourth comment by admin", + "Third comment by admin", + "Second comment by admin", + "First comment by admin" + ]) + end + end + + describe "notification bubble" do + let(:work_package) { create(:work_package, project:, author: admin) } + let!(:first_comment_by_admin) do + create(:work_package_journal, user: admin, notes: "First comment by admin", journable: work_package, version: 2) + end + let!(:journal_mentioning_admin) do + create(:work_package_journal, user: member, notes: "First comment by member mentioning @#{admin.name}", + journable: work_package, version: 3) + end + let!(:notificaton_for_admin) do + create(:notification, recipient: admin, resource: work_package, journal: journal_mentioning_admin, reason: :mentioned) + end + + context "when admin is visiting the work package" do + current_user { admin } + + before do + wp_page.visit! + wp_page.wait_for_activity_tab + end + + it "shows the notification bubble", :aggregate_failures do + activity_tab.within_journal_entry(journal_mentioning_admin) do + activity_tab.expect_notification_bubble + end + end + + it "removes the notification bubble after the comment is read", :aggregate_failures do + notificaton_for_admin.update!(read_ian: true) + + wp_page.visit! + wp_page.wait_for_activity_tab + + activity_tab.within_journal_entry(journal_mentioning_admin) do + activity_tab.expect_no_notification_bubble + end + end end + + context "when member is visiting the work package" do + current_user { member } + + before do + wp_page.visit! + wp_page.wait_for_activity_tab + end + + it "does not show the notification bubble", :aggregate_failures do + activity_tab.within_journal_entry(journal_mentioning_admin) do + activity_tab.expect_no_notification_bubble + end + end + end + end + + describe "edit comments" do + let(:work_package) { create(:work_package, project:, author: admin) } + let!(:first_comment_by_admin) do + create(:work_package_journal, user: admin, notes: "First comment by admin", journable: work_package, version: 2) + end + let!(:first_comment_by_member) do + create(:work_package_journal, user: member, notes: "First comment by member", journable: work_package, version: 3) + end + + context "when admin is visiting the work package" do + current_user { admin } + + before do + wp_page.visit! + wp_page.wait_for_activity_tab + end + + it "can edit own comments", :aggregate_failures do + # edit own comment + activity_tab.edit_comment(first_comment_by_admin, text: "First comment by admin edited") + + # expect the edited comment to be shown + activity_tab.within_journal_entry(first_comment_by_admin) do + activity_tab.expect_journal_notes(text: "First comment by admin edited") + end + + # cannot edit other user's comment + # the edit button should not be shown + activity_tab.within_journal_entry(first_comment_by_member) do + page.find_test_selector("op-wp-journal-#{first_comment_by_member.id}-action-menu").click + expect(page).not_to have_test_selector("op-wp-journal-#{first_comment_by_member.id}-edit") + end + end + end + end + + describe "quote comments" do + let(:work_package) { create(:work_package, project:, author: admin) } + let!(:first_comment_by_admin) do + create(:work_package_journal, user: admin, notes: "First comment by admin", journable: work_package, version: 2) + end + let!(:first_comment_by_member) do + create(:work_package_journal, user: member, notes: "First comment by member", journable: work_package, version: 3) + end + + context "when admin is visiting the work package" do + current_user { admin } + + before do + wp_page.visit! + wp_page.wait_for_activity_tab + end + + it "can quote other user's comments", :aggregate_failures do + # quote other user's comment + # not adding additional text in this spec to the spec as I didn't find a way to add text the editor component + activity_tab.quote_comment(first_comment_by_member) + + # expect the quoted comment to be shown + activity_tab.expect_journal_notes(text: "A Member wrote:\nFirst comment by member") + end + end + end + + describe "rescue editor content" do + let(:work_package) { create(:work_package, project:, author: admin) } + let(:second_work_package) { create(:work_package, project:, author: admin) } + + current_user { admin } + + before do + wp_page.visit! + wp_page.wait_for_activity_tab + end + + it "rescues the editor content when navigating to another workpackage tab", :aggregate_failures do + # add a comment, but do not save it + activity_tab.add_comment(text: "First comment by admin", save: false) + + # navigate to another tab and back + page.find("li[data-tab-id=\"relations\"]").click + page.find("li[data-tab-id=\"activity\"]").click + + # expect the editor content to be rescued on the client side + within("#work-package-journal-form") do + editor = FormFields::Primerized::EditorFormField.new("notes", selector: "#work-package-journal-form") + editor.expect_value("First comment by admin") + # save the comment, which was rescued on the client side + page.find_test_selector("op-submit-work-package-journal-form").click + end + + # expect the comment to be added properly + activity_tab.expect_journal_notes(text: "First comment by admin") + end + + it "scopes the rescued content to the work package", :aggregate_failures do + # add a comment to the first work package, but do not save it + activity_tab.add_comment(text: "First comment by admin", save: false) + + # navigate to another tab in order to prevent the browser native confirm dialog of the unsaved changes + page.find("li[data-tab-id=\"relations\"]").click + + # navigate to the second work package + wp_page = Pages::FullWorkPackage.new(second_work_package, project) + wp_page.visit! + wp_page.wait_for_activity_tab + + # wait for the stimulus component to be mounted, TODO: get rid of static sleep + sleep 1 + # open the editor + page.find_by_id("open-work-package-journal-form").click + + # expect the editor content to be empty + within("#work-package-journal-form") do + editor = FormFields::Primerized::EditorFormField.new("notes", selector: "#work-package-journal-form") + editor.expect_value("") + end + end + + it "scopes the rescued content to the user", :aggregate_failures do + # add a comment to the first work package, but do not save it + activity_tab.add_comment(text: "First comment by admin", save: false) + + # navigate to another tab in order to prevent the browser native confirm dialog of the unsaved changes + page.find("li[data-tab-id=\"relations\"]").click + + logout + login_as(member) + + # navigate to the same workpackage, but as a different user + wp_page.visit! + wp_page.wait_for_activity_tab + + # wait for the stimulus component to be mounted, TODO: get rid of static sleep + sleep 1 + # open the editor + page.find_by_id("open-work-package-journal-form").click + + # expect the editor content to be empty + within("#work-package-journal-form") do + editor = FormFields::Primerized::EditorFormField.new("notes", selector: "#work-package-journal-form") + editor.expect_value("") + end + + logout + login_as(admin) + + # navigate to the same workpackage, but as a different user + wp_page.visit! + wp_page.wait_for_activity_tab + + # wait for the stimulus component to be mounted, TODO: get rid of static sleep + sleep 1 + + # expect the editor to be opened and content to be rescued for the correct user + within("#work-package-journal-form") do + editor = FormFields::Primerized::EditorFormField.new("notes", selector: "#work-package-journal-form") + editor.expect_value("First comment by admin") + end + end + end + + describe "auto scrolling" do + current_user { admin } + let(:work_package) { create(:work_package, project:, author: admin) } + + # create enough comments to make the journal container scrollable + 20.times do |i| + let!(:"comment_#{i + 1}") do + create(:work_package_journal, user: admin, notes: "Comment #{i + 1}", journable: work_package, version: i + 2) + end + end + + describe "scrolls to comment specified in the URL" do + context "when sorting set to asc" do + let!(:admin_preferences) { create(:user_preference, user: admin, others: { comments_sorting: :asc }) } + + before do + visit project_work_package_path(project, work_package.id, "activity", anchor: "activity-1") + wp_page.wait_for_activity_tab + end + + it "scrolls to the comment specified in the URL", :aggregate_failures do + sleep 1 # wait for auto scrolling to finish + activity_tab.expect_journal_container_at_position(50) # would be at the bottom if no anchor would be provided + end + end + + context "when sorting set to desc" do + let!(:admin_preferences) { create(:user_preference, user: admin, others: { comments_sorting: :desc }) } + + before do + visit project_work_package_path(project, work_package.id, "activity", anchor: "activity-1") + wp_page.wait_for_activity_tab + end + + it "scrolls to the comment specified in the URL", :aggregate_failures do + sleep 1 # wait for auto scrolling to finish + activity_tab.expect_journal_container_at_bottom # would be at the top if no anchor would be provided + end + end + end + + context "when sorting set to asc" do + let!(:admin_preferences) { create(:user_preference, user: admin, others: { comments_sorting: :asc }) } + + 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 + + it "scrolls to the bottom when the newest journal entry is on the bottom", :aggregate_failures do + sleep 1 # wait for auto scrolling to finish + activity_tab.expect_journal_container_at_bottom + + # auto-scrolls to the bottom when a new comment is added by the user + # add a comment + activity_tab.add_comment(text: "New comment by admin") + activity_tab.expect_journal_notes(text: "New comment by admin") # wait for the comment to be added + activity_tab.expect_journal_container_at_bottom + + # auto-scrolls to the bottom when a new comment is added by another user + # add a comment + latest_journal_version = work_package.journals.last.version + create(:work_package_journal, user: member, notes: "New comment by member", journable: work_package, + version: latest_journal_version + 1) + activity_tab.expect_journal_notes(text: "New comment by member") # wait for the comment to be added + sleep 1 # wait for auto scrolling to finish + activity_tab.expect_journal_container_at_bottom + end + end + + context "when sorting set to desc" do + let!(:admin_preferences) { create(:user_preference, user: admin, others: { comments_sorting: :desc }) } + + before do + wp_page.visit! + wp_page.wait_for_activity_tab + end + + it "does not scroll to the bottom as the newest journal entry is on the top", :aggregate_failures do + sleep 1 # wait for auto scrolling to finish + activity_tab.expect_journal_container_at_top + end + end + + # describe "scrolling to the bottom when sorting set to asc" do + # it "scrolls to the bottom when the oldest journal entry is on top", :aggregate_failures do + # # add a comment + # activity_tab.add_comment(text: "First comment by admin") + + # # scroll to the top + # page.execute_script("document.querySelector('.op-wp-journals-container').scrollTop = 0") + + # # add another comment + # activity_tab.add_comment(text: "Second comment by admin") + + # # expect the oldest comment to be at the bottom + # activity_tab.expect_journal_notes(text: "First comment by admin") + # end + # end end end diff --git a/spec/support/components/work_packages/activities.rb b/spec/support/components/work_packages/activities.rb index 10ae9058ee66..5bc96894f167 100644 --- a/spec/support/components/work_packages/activities.rb +++ b/spec/support/components/work_packages/activities.rb @@ -107,7 +107,43 @@ def expect_journal_notes(text: nil) expect(page).to have_css(".journal-notes-body", text:) end - def add_comment(text: nil) + def expect_notification_bubble + expect(page).to have_test_selector("user-activity-bubble") + end + + def expect_no_notification_bubble + expect(page).not_to have_test_selector("user-activity-bubble") + end + + def expect_journal_container_at_bottom + scroll_position = page.evaluate_script('document.querySelector(".tabcontent").scrollTop') + scroll_height = page.evaluate_script('document.querySelector(".tabcontent").scrollHeight') + client_height = page.evaluate_script('document.querySelector(".tabcontent").clientHeight') + + expect(scroll_position).to be_within(10).of(scroll_height - client_height) + end + + def expect_journal_container_at_top + scroll_position = page.evaluate_script('document.querySelector(".tabcontent").scrollTop') + + expect(scroll_position).to eq(0) + end + + def expect_journal_container_at_position(position) + scroll_position = page.evaluate_script('document.querySelector(".tabcontent").scrollTop') + + expect(scroll_position).to be_within(50).of(scroll_position - position) + end + + def expect_empty_state + expect(page).to have_test_selector("op-wp-journals-container-empty") + end + + def expect_no_empty_state + expect(page).not_to have_test_selector("op-wp-journals-container-empty") + end + + def add_comment(text: nil, save: true) # TODO: get rid of static sleep sleep 1 # otherwise the stimulus component is not mounted yet and the click does not work @@ -119,14 +155,46 @@ def add_comment(text: nil) within("#work-package-journal-form") do FormFields::Primerized::EditorFormField.new("notes", selector: "#work-package-journal-form").set_value(text) - page.find_test_selector("op-submit-work-package-journal-form").click + page.find_test_selector("op-submit-work-package-journal-form").click if save end - page.within_test_selector("op-wp-journals-container") do + if save + page.within_test_selector("op-wp-journals-container") do + expect(page).to have_text(text) + end + end + end + + def edit_comment(journal, text: nil) + within_journal_entry(journal) do + page.find_test_selector("op-wp-journal-#{journal.id}-action-menu").click + page.find_test_selector("op-wp-journal-#{journal.id}-edit").click + + within("#work-package-journal-form") do + FormFields::Primerized::EditorFormField.new("notes", selector: "#work-package-journal-form").set_value(text) + page.find_test_selector("op-submit-work-package-journal-form").click + end + expect(page).to have_text(text) end end + def quote_comment(journal) + # TODO: get rid of static sleep + sleep 1 # otherwise the stimulus component is not mounted yet and the click does not work + + within_journal_entry(journal) do + page.find_test_selector("op-wp-journal-#{journal.id}-action-menu").click + page.find_test_selector("op-wp-journal-#{journal.id}-quote").click + end + + expect(page).to have_css("#work-package-journal-form") + + within("#work-package-journal-form") do + page.find_test_selector("op-submit-work-package-journal-form").click + end + end + def get_all_comments_as_arrary page.all(".journal-notes-body").map(&:text) end