diff --git a/.github/workflows/pullpreview.yml b/.github/workflows/pullpreview.yml index 04ec243c0a67..d0a565f99a39 100644 --- a/.github/workflows/pullpreview.yml +++ b/.github/workflows/pullpreview.yml @@ -37,6 +37,7 @@ jobs: echo "OPENPROJECT_ENTERPRISE__CHARGEBEE__SITE=openproject-enterprise-test" >> .env.pullpreview echo "OPENPROJECT_ENTERPRISE__TRIAL__CREATION__HOST=https://start.openproject-edge.com" >> .env.pullpreview echo "OPENPROJECT_FEATURE_BLOCK_NOTE_EDITOR=true" >> .env.pullpreview + echo "OPENPROJECT_FEATURE_WP_ACTIVITY_TAB_LAZY_PAGINATION_ACTIVE=true" >> .env.pullpreview - name: Boot as BIM edition if: contains(github.ref, 'bim/') || contains(github.head_ref, 'bim/') run: | diff --git a/Gemfile b/Gemfile index ee24063f9eeb..76a0c4ac1be1 100644 --- a/Gemfile +++ b/Gemfile @@ -55,6 +55,7 @@ gem "request_store", "~> 1.7.0" gem "warden", "~> 1.2" gem "warden-basic_auth", "~> 0.2.1" +gem "pagy" gem "will_paginate", "~> 4.0.0" gem "friendly_id", "~> 5.5.0" diff --git a/Gemfile.lock b/Gemfile.lock index 9fc37b0829b9..1d58120531c5 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1119,6 +1119,7 @@ GEM ostruct (0.6.3) ox (2.14.23) bigdecimal (>= 3.0) + pagy (9.4.0) paper_trail (16.0.0) activerecord (>= 6.1) request_store (~> 1.4) @@ -1710,6 +1711,7 @@ DEPENDENCIES opentelemetry-sdk (~> 1.9) overviews! ox + pagy paper_trail (~> 16.0.0) parallel_tests (~> 4.0) pdf-inspector (~> 1.2) @@ -2147,6 +2149,7 @@ CHECKSUMS ostruct (0.6.3) sha256=95a2ed4a4bd1d190784e666b47b2d3f078e4a9efda2fccf18f84ddc6538ed912 overviews (1.0.0) ox (2.14.23) sha256=4a9aedb4d6c78c5ebac1d7287dc7cc6808e14a8831d7adb727438f6a1b461b66 + pagy (9.4.0) sha256=db3f2e043f684155f18f78be62a81e8d033e39b9f97b1e1a8d12ad38d7bce738 paper_trail (16.0.0) sha256=e9b9f0fb1b8b590c8231cfa931b282ba92f90e066e393930a5e1c61ae4c5019d parallel (1.27.0) sha256=4ac151e1806b755fb4e2dc2332cbf0e54f2e24ba821ff2d3dcf86bf6dc4ae130 parallel_tests (4.10.1) sha256=df05458c691462b210f7a41fc2651d4e4e8a881e8190e6d1e122c92c07735d70 diff --git a/app/components/concerns/work_packages/activities_tab/journal_sorting_inquirable.rb b/app/components/concerns/work_packages/activities_tab/journal_sorting_inquirable.rb new file mode 100644 index 000000000000..9d5f90ae3bab --- /dev/null +++ b/app/components/concerns/work_packages/activities_tab/journal_sorting_inquirable.rb @@ -0,0 +1,42 @@ +# 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. +# ++ + +module WorkPackages + module ActivitiesTab + module JournalSortingInquirable + extend ActiveSupport::Concern + + def journal_sorting + ActiveSupport::StringInquirer + .new(User.current.preference&.comments_sorting || OpenProject::Configuration.default_comment_sort_order) + end + end + end +end diff --git a/app/components/concerns/work_packages/activities_tab/stimulus_controllers.rb b/app/components/concerns/work_packages/activities_tab/stimulus_controllers.rb index 6b6c2c000b97..71c087766435 100644 --- a/app/components/concerns/work_packages/activities_tab/stimulus_controllers.rb +++ b/app/components/concerns/work_packages/activities_tab/stimulus_controllers.rb @@ -37,6 +37,7 @@ def auto_scrolling_stimulus_controller(suffix = nil) = "#{stimulus_controller_na def editor_stimulus_controller(suffix = nil) = "#{stimulus_controller_namespace}--editor#{suffix}" def index_stimulus_controller(suffix = nil) = "#{stimulus_controller_namespace}--index#{suffix}" def internal_comment_stimulus_controller(suffix = nil) = "#{stimulus_controller_namespace}--internal-comment#{suffix}" + def lazy_page_stimulus_controller(suffix = nil) = "#{stimulus_controller_namespace}--lazy-page#{suffix}" def polling_stimulus_controller(suffix = nil) = "#{stimulus_controller_namespace}--polling#{suffix}" def stems_stimulus_controller(suffix = nil) = "#{stimulus_controller_namespace}--stems#{suffix}" def quote_comments_stimulus_controller(suffix = nil) = "#{stimulus_controller_namespace}--quote-comment#{suffix}" 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 fcacece9d01f..eb5a69537fb5 100644 --- a/app/components/work_packages/activities_tab/index_component.html.erb +++ b/app/components/work_packages/activities_tab/index_component.html.erb @@ -28,9 +28,7 @@ classes: "work-packages-activities-tab-index-component--journals-container work-packages-activities-tab-index-component--journals-container_with-initial-input-compensation", data: { "work-packages--activities-tab--index-target": "journalsContainer" } ) do - render( - WorkPackages::ActivitiesTab::Journals::IndexComponent.new(work_package:, filter:) - ) + render(list_journals_component) end if adding_comment_allowed? diff --git a/app/components/work_packages/activities_tab/index_component.rb b/app/components/work_packages/activities_tab/index_component.rb index c1bd41061ef1..c07a0ff50c06 100644 --- a/app/components/work_packages/activities_tab/index_component.rb +++ b/app/components/work_packages/activities_tab/index_component.rb @@ -51,6 +51,10 @@ def self.index_content_wrapper_key = WorkPackages::ActivitiesTab::StimulusContro def self.add_comment_wrapper_key = "work-packages-activities-tab-add-comment-component" delegate :index_content_wrapper_key, :add_comment_wrapper_key, to: :class + def list_journals_component + WorkPackages::ActivitiesTab::Journals::IndexComponent.new(work_package:, filter:) + end + private attr_reader :work_package, :filter, :last_server_timestamp, :deferred diff --git a/app/components/work_packages/activities_tab/journals/lazy_index_component.html.erb b/app/components/work_packages/activities_tab/journals/lazy_index_component.html.erb new file mode 100644 index 000000000000..763773a9fd8f --- /dev/null +++ b/app/components/work_packages/activities_tab/journals/lazy_index_component.html.erb @@ -0,0 +1,29 @@ +<%= + component_wrapper(class: "work-packages-activities-tab-journals-index-component") do + flex_layout(data: { test_selector: "op-wp-journals-#{filter}-#{journal_sorting}" }) 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( + 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) + end + else + journals_index_container.with_row(id: insert_target_modifier_id) do + pages.map { concat render(it) } + end + end + end + end + + unless empty_state? + 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/lazy_index_component.rb b/app/components/work_packages/activities_tab/journals/lazy_index_component.rb new file mode 100644 index 000000000000..71bc21ca435b --- /dev/null +++ b/app/components/work_packages/activities_tab/journals/lazy_index_component.rb @@ -0,0 +1,78 @@ +# 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. +# ++ + +module WorkPackages + module ActivitiesTab + module Journals + class LazyIndexComponent < IndexComponent + def initialize(work_package:, journals:, paginator:, filter: :all) + super(work_package:, filter:, deferred: false) + + @journals = journals + @paginator = paginator + end + + def pages + current_page = paginator.page + + result = (1..paginator.pages).map do |page| + if page == current_page + page_component(page) + else + lazy_page_component(page) + end + end + + result.tap { it.reverse! if journal_sorting.asc? && paginator.pages > 1 } + end + + def page_component(page) + WorkPackages::ActivitiesTab::Journals::PageComponent + .new(journals:, emoji_reactions: wp_journals_grouped_emoji_reactions, page:, filter:) + end + + def lazy_page_component(page) + WorkPackages::ActivitiesTab::Journals::LazyPageComponent.new(work_package:, page:) + end + + def self.insert_target_modifier_id = "#{wrapper_key}-pages" + delegate :insert_target_modifier_id, to: :class + + private + + attr_reader :journals, :paginator + + def insert_target_modified? + true + end + end + end + end +end diff --git a/app/components/work_packages/activities_tab/journals/lazy_page_component.html.erb b/app/components/work_packages/activities_tab/journals/lazy_page_component.html.erb new file mode 100644 index 000000000000..cd0c1b0e348d --- /dev/null +++ b/app/components/work_packages/activities_tab/journals/lazy_page_component.html.erb @@ -0,0 +1,47 @@ +<%# -- 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. + +++# %> + +<%= + component_wrapper(data: wrapper_data_attributes) do + flex_layout( + class: "work-packages-activities-tab-journals-item-component", + bg: :default + ) do |skeleton| + 5.times do + skeleton.with_row(mb: 2) do + render(Primer::Alpha::SkeletonBox.new(width: "25%", height: "40px")) + end + + skeleton.with_row(mb: 3) do + render(Primer::Alpha::SkeletonBox.new(width: "100%", height: "150px")) + end + end + end + end +%> diff --git a/app/components/work_packages/activities_tab/journals/lazy_page_component.rb b/app/components/work_packages/activities_tab/journals/lazy_page_component.rb new file mode 100644 index 000000000000..de80f189dc1c --- /dev/null +++ b/app/components/work_packages/activities_tab/journals/lazy_page_component.rb @@ -0,0 +1,73 @@ +# 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. +#++ + +module WorkPackages + module ActivitiesTab + module Journals + class LazyPageComponent < ApplicationComponent + include OpPrimer::ComponentHelpers + include OpTurbo::Streamable + include WorkPackages::ActivitiesTab::StimulusControllers + + def initialize(work_package:, page:) + super + @work_package = work_package + @page = page + end + + def self.wrapper_key + WorkPackages::ActivitiesTab::Journals::PageComponent.wrapper_key + end + + def wrapper_uniq_by + page + end + + private + + attr_reader :work_package, :page + + def wrapper_data_attributes + { + controller: lazy_page_stimulus_controller, + lazy_page_stimulus_controller("-insert-target-id-value") => wrapper_key, + lazy_page_stimulus_controller("-page-value") => page, + lazy_page_stimulus_controller("-url-value") => page_streams_url, + lazy_page_stimulus_controller("-is-loaded-value") => false + } + end + + def page_streams_url + page_streams_work_package_activities_path(work_package, format: :turbo_stream) + end + end + end + end +end diff --git a/app/components/work_packages/activities_tab/journals/page_component.html.erb b/app/components/work_packages/activities_tab/journals/page_component.html.erb new file mode 100644 index 000000000000..cc02170f5fc8 --- /dev/null +++ b/app/components/work_packages/activities_tab/journals/page_component.html.erb @@ -0,0 +1,20 @@ +<%= + component_wrapper do + flex_layout do |flex_container| + journals.each do |record| + flex_container.with_row do + 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: emoji_reactions.fetch(record.id, {}) + ) + ) + end + end + end + end + end +%> diff --git a/app/components/work_packages/activities_tab/journals/page_component.rb b/app/components/work_packages/activities_tab/journals/page_component.rb new file mode 100644 index 000000000000..86a4a6122608 --- /dev/null +++ b/app/components/work_packages/activities_tab/journals/page_component.rb @@ -0,0 +1,62 @@ +# 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. +#++ + +module WorkPackages + module ActivitiesTab + module Journals + class PageComponent < ApplicationComponent + include OpPrimer::ComponentHelpers + include OpTurbo::Streamable + include WorkPackages::ActivitiesTab::SharedHelpers + + def initialize(journals:, emoji_reactions:, page:, filter: :all) + super + + @journals = journals + @emoji_reactions = emoji_reactions + @filter = filter + @page = page + end + + def render? + journals.any? + end + + def wrapper_uniq_by + page + end + + private + + attr_reader :journals, :emoji_reactions, :page, :filter + end + end + end +end diff --git a/app/components/work_packages/activities_tab/lazy_index_component.rb b/app/components/work_packages/activities_tab/lazy_index_component.rb new file mode 100644 index 000000000000..fc46e47a3692 --- /dev/null +++ b/app/components/work_packages/activities_tab/lazy_index_component.rb @@ -0,0 +1,51 @@ +# 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. +# ++ + +module WorkPackages + module ActivitiesTab + class LazyIndexComponent < IndexComponent + def initialize(work_package:, journals:, paginator:, last_server_timestamp:, filter: :all) + super(work_package:, last_server_timestamp:, filter:, deferred: false) + + @journals = journals + @paginator = paginator + end + + def list_journals_component + WorkPackages::ActivitiesTab::Journals::LazyIndexComponent + .new(work_package:, journals:, filter:, paginator:) + end + + private + + attr_reader :journals, :paginator + end + end +end diff --git a/app/components/work_packages/activities_tab/shared_helpers.rb b/app/components/work_packages/activities_tab/shared_helpers.rb index b9a03043d47d..0a9350045601 100644 --- a/app/components/work_packages/activities_tab/shared_helpers.rb +++ b/app/components/work_packages/activities_tab/shared_helpers.rb @@ -31,6 +31,12 @@ module WorkPackages module ActivitiesTab module SharedHelpers + extend ActiveSupport::Concern + + included do + include WorkPackages::ActivitiesTab::JournalSortingInquirable + end + def truncated_user_name(user, hover_card: false) helpers.primer_link_to_user(user, scheme: :primary, font_weight: :bold, hover_card:) end @@ -61,10 +67,6 @@ def journal_updated_at_formatted_time(journal) end end - def journal_sorting - User.current.preference&.comments_sorting || OpenProject::Configuration.default_comment_sort_order - end - def activity_url(journal) "#{project_work_package_url(journal.journable.project, journal.journable)}/activity#{activity_anchor(journal)}" end diff --git a/app/controllers/concerns/op_turbo/component_stream.rb b/app/controllers/concerns/op_turbo/component_stream.rb index be727ca7ecbc..54d26aa95a0e 100644 --- a/app/controllers/concerns/op_turbo/component_stream.rb +++ b/app/controllers/concerns/op_turbo/component_stream.rb @@ -71,6 +71,15 @@ def modify_via_turbo_stream(component:, action:, status:, method: nil) ) end + def insert_via_turbo_stream(action:, component:, target_component:) + case action + when :append + append_via_turbo_stream(component:, target_component:) + when :prepend + prepend_via_turbo_stream(component:, target_component:) + end + end + def append_via_turbo_stream(component:, target_component:) turbo_streams << target_component.insert_as_turbo_stream(component:, view_context:, action: :append) end diff --git a/app/controllers/work_packages/activities_tab_controller.rb b/app/controllers/work_packages/activities_tab_controller.rb index 7a538028f363..d8348e2ae0e6 100644 --- a/app/controllers/work_packages/activities_tab_controller.rb +++ b/app/controllers/work_packages/activities_tab_controller.rb @@ -31,23 +31,50 @@ class WorkPackages::ActivitiesTabController < ApplicationController include OpTurbo::ComponentStream include FlashMessagesOutputSafetyHelper + include WorkPackages::ActivitiesTab::JournalSortingInquirable + include WorkPackages::ActivitiesTab::StimulusControllers before_action :find_work_package before_action :find_project before_action :find_journal, only: %i[edit cancel_edit update toggle_reaction] before_action :set_filter before_action :authorize + before_action :initialize_pagination, only: %i[page_streams] def index - render( - WorkPackages::ActivitiesTab::IndexComponent.new( - work_package: @work_package, - filter: @filter, - last_server_timestamp: get_current_server_timestamp, - deferred: ActiveRecord::Type::Boolean.new.cast(params[:deferred]) - ), - layout: false + index_component = + if OpenProject::FeatureDecisions.wp_activity_tab_lazy_pagination_active? + initialize_pagination + WorkPackages::ActivitiesTab::LazyIndexComponent.new( + work_package: @work_package, + journals: @paginated_journals, + paginator: @paginator, + filter: @filter, + last_server_timestamp: get_current_server_timestamp + ) + else + WorkPackages::ActivitiesTab::IndexComponent.new( + work_package: @work_package, + filter: @filter, + last_server_timestamp: get_current_server_timestamp, + deferred: ActiveRecord::Type::Boolean.new.cast(params[:deferred]) + ) + end + + render(index_component, layout: false) + end + + def page_streams + replace_via_turbo_stream( + component: WorkPackages::ActivitiesTab::Journals::PageComponent.new( + journals: @paginated_journals, + emoji_reactions: wp_journals_emoji_reactions, + page: @paginator.page, + filter: @filter + ) ) + + respond_with_turbo_streams end def update_streams @@ -173,6 +200,10 @@ def toggle_reaction # rubocop:disable Metrics/AbcSize private + def initialize_pagination + @paginator, @paginated_journals = WorkPackages::ActivitiesTab::Paginator.paginate(@work_package, params) + end + def respond_with_error(error_message) respond_to do |format| # turbo_frame requests (tab is initially rendered and an error occured) are handled below @@ -217,10 +248,6 @@ def set_filter @filter = (params[:filter] || params.dig(:journal, :filter))&.to_sym || :all end - def journal_sorting - User.current.preference&.comments_sorting || OpenProject::Configuration.default_comment_sort_order - end - def sanitized_journal_notes WorkPackages::ActivitiesTab::InternalCommentMentionsSanitizer.sanitize(@work_package, journal_params[:notes]) end @@ -284,22 +311,37 @@ def handle_internal_server_error(error) end def replace_whole_tab - replace_via_turbo_stream( - component: WorkPackages::ActivitiesTab::IndexComponent.new( - work_package: @work_package, - filter: @filter, - last_server_timestamp: get_current_server_timestamp - ) - ) + component = + if OpenProject::FeatureDecisions.wp_activity_tab_lazy_pagination_active? + initialize_pagination # re-initialize pagination to pick up changes to sorting/filtering + WorkPackages::ActivitiesTab::LazyIndexComponent.new( + work_package: @work_package, + journals: @paginated_journals, + paginator: @paginator, + filter: @filter, + last_server_timestamp: get_current_server_timestamp + ) + else + WorkPackages::ActivitiesTab::IndexComponent.new( + work_package: @work_package, + filter: @filter, + last_server_timestamp: get_current_server_timestamp + ) + end + replace_via_turbo_stream(component:) end def update_index_component - update_via_turbo_stream( - component: WorkPackages::ActivitiesTab::Journals::IndexComponent.new( - work_package: @work_package, - filter: @filter - ) - ) + component = + if OpenProject::FeatureDecisions.wp_activity_tab_lazy_pagination_active? + initialize_pagination # re-initialize pagination to pick up changes to sorting/filtering + WorkPackages::ActivitiesTab::Journals::LazyIndexComponent + .new(work_package: @work_package, journals: @paginated_journals, paginator: @paginator, filter: @filter) + else + WorkPackages::ActivitiesTab::Journals::IndexComponent + .new(work_package: @work_package, filter: @filter) + end + update_via_turbo_stream(component:) end def create_journal_service_call @@ -339,7 +381,7 @@ def generate_time_based_update_streams(last_update_timestamp, editing_journals) rerender_updated_journals(journals, last_update_timestamp, grouped_emoji_reactions, editing_journals) rerender_journals_with_updated_notification(journals, last_update_timestamp, grouped_emoji_reactions, editing_journals) - append_or_prepend_journals(journals, last_update_timestamp, grouped_emoji_reactions) + insert_latest_journals_via_turbo_stream(journals, last_update_timestamp, grouped_emoji_reactions) if journals.present? remove_potential_empty_state @@ -348,13 +390,11 @@ def generate_time_based_update_streams(last_update_timestamp, editing_journals) end def generate_work_package_journals_emoji_reactions_update_streams - wp_journal_emoji_reactions = - EmojiReactions::GroupedQueries.grouped_work_package_journals_emoji_reactions_by_reactable(@work_package) @work_package.journals.each do |journal| update_via_turbo_stream( component: WorkPackages::ActivitiesTab::Journals::ItemComponent::Reactions.new( journal:, - grouped_emoji_reactions: wp_journal_emoji_reactions[journal.id] || {} + grouped_emoji_reactions: wp_journals_emoji_reactions[journal.id] || {} ) ) end @@ -404,32 +444,30 @@ def rerender_journals_with_updated_notification(journals, last_update_timestamp, end end - 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, grouped_emoji_reactions.fetch(journal.id, {})) - end - end - - 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, grouped_emoji_reactions: - ) - - stream_config = { - target_component:, - component: - } + def insert_latest_journals_via_turbo_stream(journals, last_update_timestamp, emoji_reactions) + target_component = + if OpenProject::FeatureDecisions.wp_activity_tab_lazy_pagination_active? + WorkPackages::ActivitiesTab::Journals::LazyIndexComponent.new( + work_package: @work_package, + journals: Journal.none, # we do not need to pass any journals here since we just want the component key + paginator: nil, + filter: @filter + ) + else + WorkPackages::ActivitiesTab::Journals::IndexComponent.new( + work_package: @work_package, + filter: @filter + ) + end - # Append or prepend the new journal depending on the sorting - if journal_sorting == "asc" - append_via_turbo_stream(**stream_config) - else - prepend_via_turbo_stream(**stream_config) + journals.where("created_at > ?", last_update_timestamp).find_each do |journal| + insert_via_turbo_stream( + target_component:, + component: WorkPackages::ActivitiesTab::Journals::ItemComponent.new( + journal:, filter: @filter, grouped_emoji_reactions: emoji_reactions.fetch(journal.id, {}) + ), + action: journal_sorting.asc? ? :append : :prepend + ) end end @@ -467,6 +505,11 @@ def update_activity_counter ) end + def wp_journals_emoji_reactions + @wp_journals_emoji_reactions ||= EmojiReactions::GroupedQueries + .grouped_work_package_journals_emoji_reactions_by_reactable(@work_package) + end + def grouped_emoji_reactions_for_journal EmojiReactions::GroupedQueries .grouped_emoji_reactions_by_reactable(reactable: @journal)[@journal.id] diff --git a/app/services/work_packages/activities_tab/paginator.rb b/app/services/work_packages/activities_tab/paginator.rb new file mode 100644 index 000000000000..21d1ee9ca5db --- /dev/null +++ b/app/services/work_packages/activities_tab/paginator.rb @@ -0,0 +1,134 @@ +# 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. +#++ + +class WorkPackages::ActivitiesTab::Paginator + include Pagy::Backend + include WorkPackages::ActivitiesTab::JournalSortingInquirable + + def self.paginate(work_package, params = {}) + new(work_package, params).call + end + + def initialize(work_package, params = {}) + @work_package = work_package + @params = params + end + + def call + anchor_type, target_record_id = extract_target_record_id + + pagy, records = + if anchor_type && target_record_id + pagy_array_for_target_journal(anchor_type, target_record_id) + else + pagy_array(base_journals, **pagy_options) + end + + # For UI display: if user wants "oldest first" UI, reverse the array + records = records.reverse if journal_sorting.asc? + + [pagy, records] + end + + private + + attr_reader :work_package, :params + + def pagy_options + { page: params[:page] || 1, limit: params[:limit] || Pagy::DEFAULT[:limit], max_pages: 100 }.compact + end + + def extract_target_record_id + anchor = params[:anchor] # e.g., "comment-78758" (without #) + return nil unless anchor + + match = anchor.match(/^(comment|activity)-(\d+)$/) + match && match.length == 3 ? [match[1].inquiry, match[2].to_i] : [] + end + + def pagy_array_for_target_journal(anchor_type, target_record_id) + journals = base_journals + + target_index = journals.find_index do |record| + if anchor_type.comment? + record.id == target_record_id + elsif anchor_type.activity? + record.sequence_version == target_record_id + else + false + end + end + + if target_index + limit = pagy_options[:limit] + target_page = (target_index / limit) + 1 + pagy_array(journals, **pagy_options, page: target_page) + else + # Journal might be filtered out or deleted - fallback to page 1 + pagy_array(journals, **pagy_options, page: 1) + end + end + + def base_journals + combine_and_sort_records(fetch_journals, fetch_revisions) + end + + def fetch_journals + API::V3::Activities::ActivityEagerLoadingWrapper.wrap(fetch_ar_journals) + end + + def fetch_ar_journals + work_package + .journals + .internal_visible + .includes(:user, :customizable_journals, :attachable_journals, :storable_journals, :notifications) + .reorder(version: :desc) # Always fetch newest first for pagination + .with_sequence_version + end + + def fetch_revisions + work_package.changesets.includes(:user, :repository) + end + + def combine_and_sort_records(journals, revisions) + (journals + revisions).sort_by do |record| + timestamp = record_timestamp(record) + [-timestamp, -record.id] # Always sort DESC (newest first) + end + end + + def record_timestamp(record) + if record.is_a?(API::V3::Activities::ActivityEagerLoadingWrapper) + record.created_at&.to_i + elsif record.is_a?(Changeset) + record.committed_on.to_i + end + end +end diff --git a/config/initializers/feature_decisions.rb b/config/initializers/feature_decisions.rb index 9f21912e6169..e5f187ff2cdd 100644 --- a/config/initializers/feature_decisions.rb +++ b/config/initializers/feature_decisions.rb @@ -75,3 +75,6 @@ OpenProject::FeatureDecisions.add :change_hierarchy_item_parent, description: "Enables a functionality to change the parent of a hierarchy item of " \ "custom fields of type hierarchy and scored list." + +OpenProject::FeatureDecisions.add :wp_activity_tab_lazy_pagination, + description: "Enables lazy pagination for the activity tab." diff --git a/config/initializers/pagy.rb b/config/initializers/pagy.rb new file mode 100644 index 000000000000..b9901321dbda --- /dev/null +++ b/config/initializers/pagy.rb @@ -0,0 +1,240 @@ +# 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. +#++ + +# Pagy initializer file (9.3.3) +# Customize only what you really need and notice that the core Pagy works also without any of the following lines. +# Should you just cherry pick part of this file, please maintain the require-order of the extras + +# Pagy Variables +# See https://ddnexus.github.io/pagy/docs/api/pagy#variables +# You can set any pagy variable as a Pagy::DEFAULT. They can also be overridden per instance by just passing them to +# Pagy.new|Pagy::Countless.new|Pagy::Calendar::*.new or any of the #pagy* controller methods +# Here are the few that make more sense as DEFAULTs: +# Pagy::DEFAULT[:limit] = 20 # default +# Pagy::DEFAULT[:size] = 7 # default +# Pagy::DEFAULT[:ends] = true # default +# Pagy::DEFAULT[:page_param] = :page # default +# Pagy::DEFAULT[:count_args] = [] # example for non AR ORMs +# Pagy::DEFAULT[:max_pages] = 3000 # example + +# Extras +# See https://ddnexus.github.io/pagy/categories/extra + +# Legacy Compatibility Extras + +# Size extra: Enable the Array type for the `:size` variable (e.g. `size: [1,4,4,1]`) +# See https://ddnexus.github.io/pagy/docs/extras/size +# require 'pagy/extras/size' # must be required before the other extras + +# Backend Extras + +# Arel extra: For better performance utilizing grouped ActiveRecord collections: +# See: https://ddnexus.github.io/pagy/docs/extras/arel +# require 'pagy/extras/arel' + +# Array extra: Paginate arrays efficiently, avoiding expensive array-wrapping and without overriding +# See https://ddnexus.github.io/pagy/docs/extras/array +require "pagy/extras/array" + +# Calendar extra: Add pagination filtering by calendar time unit (year, quarter, month, week, day) +# See https://ddnexus.github.io/pagy/docs/extras/calendar +# require 'pagy/extras/calendar' +# Default for each calendar unit class in IRB: +# >> Pagy::Calendar::Year::DEFAULT +# >> Pagy::Calendar::Quarter::DEFAULT +# >> Pagy::Calendar::Month::DEFAULT +# >> Pagy::Calendar::Week::DEFAULT +# >> Pagy::Calendar::Day::DEFAULT +# +# Uncomment the following lines, if you need calendar localization without using the I18n extra +# module LocalizePagyCalendar +# def localize(time, opts) +# ::I18n.l(time, **opts) +# end +# end +# Pagy::Calendar.prepend LocalizePagyCalendar + +# Countless extra: Paginate without any count, saving one query per rendering +# See https://ddnexus.github.io/pagy/docs/extras/countless +# require 'pagy/extras/countless' +# Pagy::DEFAULT[:countless_minimal] = false # default (eager loading) + +# Elasticsearch Rails extra: Paginate `ElasticsearchRails::Results` objects +# See https://ddnexus.github.io/pagy/docs/extras/elasticsearch_rails +# Default :pagy_search method: change only if you use also +# the searchkick or meilisearch extra that defines the same +# Pagy::DEFAULT[:elasticsearch_rails_pagy_search] = :pagy_search +# Default original :search method called internally to do the actual search +# Pagy::DEFAULT[:elasticsearch_rails_search] = :search +# require 'pagy/extras/elasticsearch_rails' + +# Headers extra: http response headers (and other helpers) useful for API pagination +# See https://ddnexus.github.io/pagy/docs/extras/headers +# require 'pagy/extras/headers' +# Pagy::DEFAULT[:headers] = { page: 'Current-Page', +# limit: 'Page-Items', +# count: 'Total-Count', +# pages: 'Total-Pages' } # default + +# Keyset extra: Paginate with the Pagy keyset pagination technique +# See https://ddnexus.github.io/pagy/docs/extras/keyset +# require "pagy/extras/keyset" + +# Meilisearch extra: Paginate `Meilisearch` result objects +# See https://ddnexus.github.io/pagy/docs/extras/meilisearch +# Default :pagy_search method: change only if you use also +# the elasticsearch_rails or searchkick extra that define the same method +# Pagy::DEFAULT[:meilisearch_pagy_search] = :pagy_search +# Default original :search method called internally to do the actual search +# Pagy::DEFAULT[:meilisearch_search] = :ms_search +# require 'pagy/extras/meilisearch' + +# Metadata extra: Provides the pagination metadata to Javascript frameworks like Vue.js, react.js, etc. +# See https://ddnexus.github.io/pagy/docs/extras/metadata +# you must require the JS Tools internal extra (BEFORE the metadata extra) ONLY if you need also the :sequels +# require 'pagy/extras/js_tools' +# require 'pagy/extras/metadata' +# For performance reasons, you should explicitly set ONLY the metadata you use in the frontend +# Pagy::DEFAULT[:metadata] = %i[scaffold_url page prev next last] # example + +# Searchkick extra: Paginate `Searchkick::Results` objects +# See https://ddnexus.github.io/pagy/docs/extras/searchkick +# Default :pagy_search method: change only if you use also +# the elasticsearch_rails or meilisearch extra that defines the same +# Pagy::DEFAULT[:searchkick_pagy_search] = :pagy_search +# Default original :search method called internally to do the actual search +# Pagy::DEFAULT[:searchkick_search] = :search +# require 'pagy/extras/searchkick' +# uncomment if you are going to use Searchkick.pagy_search +# Searchkick.extend Pagy::Searchkick + +# Frontend Extras + +# Bootstrap extra: Add nav, nav_js and combo_nav_js helpers and templates for Bootstrap pagination +# See https://ddnexus.github.io/pagy/docs/extras/bootstrap +# require 'pagy/extras/bootstrap' + +# Bulma extra: Add nav, nav_js and combo_nav_js helpers and templates for Bulma pagination +# See https://ddnexus.github.io/pagy/docs/extras/bulma +# require 'pagy/extras/bulma' + +# Pagy extra: Add the pagy styled versions of the javascript-powered navs +# and a few other components to the Pagy::Frontend module. +# See https://ddnexus.github.io/pagy/docs/extras/pagy +# require 'pagy/extras/pagy' + +# Multi size var used by the *_nav_js helpers +# See https://ddnexus.github.io/pagy/docs/extras/pagy#steps +# Pagy::DEFAULT[:steps] = { 0 => 5, 540 => 7, 720 => 9 } # example + +# Feature Extras + +# Gearbox extra: Automatically change the limit per page depending on the page number +# See https://ddnexus.github.io/pagy/docs/extras/gearbox +# require 'pagy/extras/gearbox' +# set to false only if you want to make :gearbox_extra an opt-in variable +# Pagy::DEFAULT[:gearbox_extra] = false # default true +# Pagy::DEFAULT[:gearbox_limit] = [15, 30, 60, 100] # default + +# Limit extra: Allow the client to request a custom limit per page with an optional selector UI +# See https://ddnexus.github.io/pagy/docs/extras/limit +# require 'pagy/extras/limit' +# set to false only if you want to make :limit_extra an opt-in variable +# Pagy::DEFAULT[:limit_extra] = false # default true +# Pagy::DEFAULT[:limit_param] = :limit # default +# Pagy::DEFAULT[:limit_max] = 100 # default + +# Overflow extra: Allow for easy handling of overflowing pages +# See https://ddnexus.github.io/pagy/docs/extras/overflow +# require 'pagy/extras/overflow' +# Pagy::DEFAULT[:overflow] = :empty_page # default (other options: :last_page and :exception) + +# Trim extra: Remove the page=1 param from links +# See https://ddnexus.github.io/pagy/docs/extras/trim +# require 'pagy/extras/trim' +# set to false only if you want to make :trim_extra an opt-in variable +# Pagy::DEFAULT[:trim_extra] = false # default true + +# Standalone extra: Use pagy in non Rack environment/gem +# See https://ddnexus.github.io/pagy/docs/extras/standalone +# require 'pagy/extras/standalone' +# Pagy::DEFAULT[:url] = 'http://www.example.com/subdir' # optional default + +# Jsonapi extra: Implements JSON:API specifications +# See https://ddnexus.github.io/pagy/docs/extras/jsonapi +# require 'pagy/extras/jsonapi' # must be required after the other extras +# set to false only if you want to make :jsonapi an opt-in variable +# Pagy::DEFAULT[:jsonapi] = false # default true + +# Rails +# Enable the .js file required by the helpers that use javascript +# (pagy*_nav_js, pagy*_combo_nav_js, and pagy_limit_selector_js) +# See https://ddnexus.github.io/pagy/docs/api/javascript + +# With the asset pipeline +# Sprockets need to look into the pagy javascripts dir, so add it to the assets paths +# Rails.application.config.assets.paths << Pagy.root.join('javascripts') + +# I18n + +# Pagy internal I18n: ~18x faster using ~10x less memory than the i18n gem +# See https://ddnexus.github.io/pagy/docs/api/i18n +# Notice: No need to configure anything in this section if your app uses only "en" +# or if you use the i18n extra below +# +# Examples: +# load the "de" built-in locale: +# Pagy::I18n.load(locale: 'de') +# +# load the "de" locale defined in the custom file at :filepath: +# Pagy::I18n.load(locale: 'de', filepath: 'path/to/pagy-de.yml') +# +# load the "de", "en" and "es" built-in locales: +# (the first passed :locale will be used also as the default_locale) +# Pagy::I18n.load({ locale: 'de' }, +# { locale: 'en' }, +# { locale: 'es' }) +# +# load the "en" built-in locale, a custom "es" locale, +# and a totally custom locale complete with a custom :pluralize proc: +# (the first passed :locale will be used also as the default_locale) +# Pagy::I18n.load({ locale: 'en' }, +# { locale: 'es', filepath: 'path/to/pagy-es.yml' }, +# { locale: 'xyz', # not built-in +# filepath: 'path/to/pagy-xyz.yml', +# pluralize: lambda{ |count| ... } ) + +# I18n extra: uses the standard i18n gem which is ~18x slower using ~10x more memory +# than the default pagy internal i18n (see above) +# See https://ddnexus.github.io/pagy/docs/extras/i18n +# require 'pagy/extras/i18n' + +# When you are done setting your own default freeze it, so it will not get changed accidentally +Pagy::DEFAULT.freeze diff --git a/config/initializers/permissions.rb b/config/initializers/permissions.rb index abd50c324bda..8b9fe2fc7ba3 100644 --- a/config/initializers/permissions.rb +++ b/config/initializers/permissions.rb @@ -261,7 +261,7 @@ work_packages: %i[show index show_conflict_flash_message share_upsell], work_packages_api: [:get], "work_packages/reports": %i[report report_details], - "work_packages/activities_tab": %i[index update_streams update_sorting update_filter], + "work_packages/activities_tab": %i[index page_streams update_streams update_sorting update_filter], "work_packages/menus": %i[show], "work_packages/hover_card": %i[show], work_package_relations_tab: %i[index], diff --git a/config/routes.rb b/config/routes.rb index 7a7665d1432e..7549787d8e2d 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -735,6 +735,7 @@ end collection do + get :page_streams get :update_streams get :update_filter # filter not persisted put :update_sorting # sorting is persisted diff --git a/frontend/src/app/features/work-packages/components/wp-single-view-tabs/activity-panel/activity-base.controller.ts b/frontend/src/app/features/work-packages/components/wp-single-view-tabs/activity-panel/activity-base.controller.ts index 76711600c944..01cb30fbf20f 100644 --- a/frontend/src/app/features/work-packages/components/wp-single-view-tabs/activity-panel/activity-base.controller.ts +++ b/frontend/src/app/features/work-packages/components/wp-single-view-tabs/activity-panel/activity-base.controller.ts @@ -36,6 +36,7 @@ import { WpSingleViewService } from 'core-app/features/work-packages/routing/wp- import { BrowserDetector } from 'core-app/core/browser/browser-detector.service'; import { DeviceService } from 'core-app/core/browser/device.service'; import { PathHelperService } from 'core-app/core/path-helper/path-helper.service'; +import { UrlHelpers } from 'core-stimulus/controllers/dynamic/work-packages/activities-tab/services/url-helpers'; @Directive() export class ActivityPanelBaseController extends UntilDestroyedMixin implements OnInit { @@ -59,6 +60,18 @@ export class ActivityPanelBaseController extends UntilDestroyedMixin implements } ngOnInit():void { - this.turboFrameSrc = `${this.pathHelper.staticBase}/work_packages/${this.workPackageId}/activities`; + this.turboFrameSrc = this.buildTurboFrameSrc(); + } + + protected buildTurboFrameSrc():string { + const baseUrl = window.location.origin; + const url = new URL(`${this.pathHelper.staticBase}/work_packages/${this.workPackageId}/activities`, baseUrl); + const anchorInfo = UrlHelpers.extractActivityAnchor(window.location.hash); + + if (anchorInfo) { + url.searchParams.set('anchor', `${anchorInfo.type}-${anchorInfo.id}`); + } + + return url.toString(); } } diff --git a/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/auto-scrolling.controller.ts b/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/auto-scrolling.controller.ts index f78d76fec1c5..e1d6e4f8d127 100644 --- a/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/auto-scrolling.controller.ts +++ b/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/auto-scrolling.controller.ts @@ -29,16 +29,12 @@ */ import BaseController from './base.controller'; - -enum AnchorType { - Comment = 'comment', - Activity = 'activity', -} +import { UrlHelpers, ActivityAnchorType, ActivityAnchor } from './services/url-helpers'; interface CustomEventWithIdParam extends Event { params:{ id:string; - anchorName:AnchorType; + anchorName:ActivityAnchorType; }; } @@ -59,7 +55,7 @@ export default class AutoScrollingController extends BaseController { // not using the scrollToActivity method here as it is causing flickering issues // in case of a setAnchor click, we can go for a direct scroll approach const scrollableContainer = this.scrollableContainer; - const activityElement = this.getActivityAnchorElement(anchorName, activityId); + const activityElement = this.getActivityAnchorElement({ type: anchorName, id: activityId }); const locationHash = `#${anchorName}-${activityId}`; if (scrollableContainer && activityElement) { @@ -120,12 +116,12 @@ export default class AutoScrollingController extends BaseController { } private handleInitialScroll() { - const anchorTypeRegex = new RegExp(`#(${AnchorType.Comment}|${AnchorType.Activity})-(\\d+)`, 'i'); - const activityIdMatch = window.location.hash.match(anchorTypeRegex); // Ex. [ "#comment-80", "comment", "80" ] + const hash = window.location.hash; + const anchorInfo = UrlHelpers.extractActivityAnchor(hash); - if (activityIdMatch && activityIdMatch.length === 3) { - const activityElement = this.getActivityAnchorElement(activityIdMatch[1] as AnchorType, activityIdMatch[2]); - this.brieflyHighlightAndResetUrl(activityElement, window.location.hash); + if (anchorInfo) { + const activityElement = this.getActivityAnchorElement(anchorInfo); + this.brieflyHighlightAndResetUrl(activityElement, hash); this.scrollToActivity(activityElement); } else if (this.indexOutlet.sortingAscending && (!this.isMobile() || this.isWithinNotificationCenter())) { this.scrollToBottom(); @@ -179,7 +175,6 @@ export default class AutoScrollingController extends BaseController { let timeoutId:ReturnType; const observer = new MutationObserver(() => { - // eslint-disable-next-line @typescript-eslint/no-unsafe-argument clearTimeout(timeoutId); timeoutId = setTimeout(() => { @@ -211,8 +206,8 @@ export default class AutoScrollingController extends BaseController { } } - private getActivityAnchorElement(activityAnchorName:AnchorType, activityId:string):HTMLElement | null { - return document.querySelector(`[data-anchor-${activityAnchorName}-id="${activityId}"]`); + private getActivityAnchorElement(activityAnchor:ActivityAnchor):HTMLElement | null { + return document.querySelector(`[data-anchor-${activityAnchor.type}-id="${activityAnchor.id}"]`); } private get inputContainer():HTMLElement | null { diff --git a/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/lazy-page.controller.ts b/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/lazy-page.controller.ts new file mode 100644 index 000000000000..33e7c1dc3cbd --- /dev/null +++ b/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/lazy-page.controller.ts @@ -0,0 +1,161 @@ +/* + * -- 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. + * ++ + */ + +import { TurboRequestsService } from 'core-app/core/turbo/turbo-requests.service'; +import type { TurboBeforeStreamRenderEvent } from 'core-typings/turbo'; +import { useIntersection } from 'stimulus-use'; +import BaseController from './base.controller'; +import { DomHelpers } from './services/dom_helpers'; + +export default class LazyPageController extends BaseController { + static values = { + url: String, + insertTargetId: String, + page: { type: Number, default: 1 }, + isLoaded: { type: Boolean, default: false }, + loadDelayMs: { type: Number, default: 75 }, + }; + + declare urlValue:string; + declare insertTargetIdValue:string; + declare pageValue:number; + declare isLoadedValue:boolean; + declare loadDelayMsValue:number; + + private turboRequests:TurboRequestsService; + private abortController = new AbortController(); + private pageStreamHandler?:(_event:TurboBeforeStreamRenderEvent) => void; + private stopObserving?:() => void; + private loadTimeout?:number; + + connect() { + if (this.isLoadedValue) return; + + super.connect(); + void this.initializeTurboRequestService(); + this.startObserving(); + this.setupScrollPreservation(); + } + + disconnect() { + super.disconnect(); + this.tearDownScrollPreservation(); + this.cancelPendingLoad(); + this.stopObserving?.(); + } + + appear() { + if (this.isLoadedValue) return; + + // Delay loading to allow rapid scrolling without triggering loads + this.loadTimeout = window.setTimeout(() => { + void this.fetchPageStream() + .catch((error) => { + console.error('Error fetching next page:', error); + }).finally(() => { + this.isLoadedValue = true; + }); + }, this.loadDelayMsValue); + } + + disappear() { + this.cancelPendingLoad(); // Cancel load if element leaves viewport before delay expires + } + + private setupScrollPreservation() { + if (this.pageStreamHandler) return; + + const { signal } = this.abortController; + const scrollContainer = this.scrollableContainer; + + this.pageStreamHandler = (event:TurboBeforeStreamRenderEvent) => { + event.preventDefault(); + + const stream = event.detail.newStream; + const insertTargetId = this.insertTargetIdValue; + + if (scrollContainer && insertTargetId && stream.target.includes(insertTargetId)) { + const isPrepending = this.indexOutlet.sortingAscending; // Newest at the bottom sorting order + void DomHelpers.keepScroll(scrollContainer, isPrepending, () => { + event.detail.render(stream); + return Promise.resolve(); + }); + } else { + event.detail.render(stream); + } + }; + + document.addEventListener('turbo:before-stream-render', this.pageStreamHandler as EventListener, { signal }); + } + + private startObserving(root = this.scrollableContainer) { + if (!root) return; + + const [_observe, unobserve] = useIntersection(this, { + root, + threshold: 0.25, + dispatchEvent: false + }); + + this.stopObserving = unobserve; + } + + private tearDownScrollPreservation() { + this.abortController.abort(); + if (this.pageStreamHandler) this.pageStreamHandler = undefined; + } + + private fetchPageStream():Promise<{ html:string, headers:Headers }> { + const url = this.preparePageStreamsUrl(); + return this.turboRequests.requestStream(url); + } + + private preparePageStreamsUrl():string { + const baseUrl = window.location.origin; + const url = new URL(this.urlValue, baseUrl); + + url.searchParams.set('page', this.pageValue.toString()); + url.searchParams.set('filter', this.indexOutlet.filterValue); + + return url.toString(); + } + + private async initializeTurboRequestService() { + const context = await window.OpenProject.getPluginContext(); + this.turboRequests = context.services.turboRequests; + } + + private cancelPendingLoad() { + if (this.loadTimeout) { + window.clearTimeout(this.loadTimeout); + this.loadTimeout = undefined; + } + } +} diff --git a/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/services/dom_helpers.ts b/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/services/dom_helpers.ts new file mode 100644 index 000000000000..485ed60ee746 --- /dev/null +++ b/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/services/dom_helpers.ts @@ -0,0 +1,120 @@ +/* +* -- copyright +* OpenProject is an open source project management software. +* Copyright (C) 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. +* ++ +* +* Copyright (c) 37signals, LLC +* +* Permission is hereby granted, free of charge, to any person obtaining +* a copy of this software and associated documentation files (the +* "Software"), to deal in the Software without restriction, including +* without limitation the rights to use, copy, modify, merge, publish, +* distribute, sublicense, and/or sell copies of the Software, and to +* permit persons to whom the Software is furnished to do so, subject to +* the following conditions: +* +* The above copyright notice and this permission notice shall be +* included in all copies or substantial portions of the Software. +* +* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, +* EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +* MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND +* NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE +* LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION +* OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION +* WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. +* +* See also the original source: +* * https://github.com/basecamp/once-campfire/blob/5c0526eaf7f83d129472a520fa673bfcb9c2f397/MIT-LICENSE +* * https://github.com/basecamp/once-campfire/blob/5c0526eaf7f83d129472a520fa673bfcb9c2f397/app/javascript/helpers/dom_helpers.js#L44-L65 +*/ + +export namespace DomHelpers { + /** + * Preserves scroll position during DOM manipulations that change content height. + * + * This is crucial for infinite scroll UX - when older content is prepended above + * the current view, we need to adjust scroll position so the user doesn't suddenly + * jump to a different part of the content. + * + * @param {HTMLElement} container - The scrollable container + * @param {boolean} isPrepending - Whether content is being added at the top (prepend) + * true: adjust scroll for prepended content + * false: maintain position for appended content + * @param {Function} renderFn - Async function that performs the DOM manipulation + * + * Algorithm: + * - Capture current scroll position and total height BEFORE DOM changes + * - Execute the DOM manipulation (adding/removing content) + * - Calculate height difference and adjust scroll position accordingly + * + * For prepend: scrollTop + heightDifference (push view down by added content height) + * For append: maintain original scrollTop (new content below doesn't affect view) + */ + export async function keepScroll(container:HTMLElement, isPrepending:boolean, renderFn:() => Promise) { + pauseInertiaScroll(container); + + const scrollTop = container.scrollTop; + const scrollHeight = container.scrollHeight; + + await renderFn(); + + if (isPrepending) { + container.scrollTop = scrollTop + (container.scrollHeight - scrollHeight); + } else { + container.scrollTop = scrollTop; + } + } + + /** + * Temporarily pauses inertial/momentum scrolling to prevent scroll position drift. + * + * CRITICAL for iOS Safari and other mobile browsers with momentum scrolling! + * + * The Problem: + * When programmatically setting scrollTop during an active momentum scroll, + * the browser continues the inertial scroll AFTER our position adjustment, + * causing the scroll position to drift away from our intended target. + * + * The Solution: + * - Set overflow: hidden to immediately stop any momentum scrolling + * - Use requestAnimationFrame to restore overflow on next frame + * - This gives us a clean slate to set the exact scroll position + * + * Without this, scroll preservation would be unreliable on mobile devices, + * especially during infinite scroll when users are actively scrolling up. + * + * @param {Element} container - The scrollable container to pause momentum on + */ + function pauseInertiaScroll(container:HTMLElement) { + container.style.overflow = 'hidden'; + + requestAnimationFrame(() => { + container.style.overflow = ''; + }); + } +} diff --git a/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/services/url-helpers.ts b/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/services/url-helpers.ts new file mode 100644 index 000000000000..ab9761be18ce --- /dev/null +++ b/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/services/url-helpers.ts @@ -0,0 +1,69 @@ +/* + * -- copyright + * OpenProject is an open source project management software. + * Copyright (C) 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. + * ++ + */ + +/* eslint-disable no-unused-vars */ +export enum ActivityAnchorType { + Comment = 'comment', + Activity = 'activity', +} +/* eslint-enable no-unused-vars */ + +export interface ActivityAnchor { + type:ActivityAnchorType; + id:string; +} + +const anchorTypeRegex = new RegExp(`#(${ActivityAnchorType.Comment}|${ActivityAnchorType.Activity})-(\\d+)`, 'i'); + +export namespace UrlHelpers { + /** + * Extracts activity anchor information from a URL anchor string. + * + * @param anchor - The anchor string to parse (e.g., "#comment-80", "#activity-45") + * @returns An ActivityAnchor object containing the type and id, or null if parsing fails + * + * @example + * ```typescript + * const result = extractActivityAnchor("#comment-80"); + * // Returns: { type: "comment", id: "80" } + * ``` + */ + export function extractActivityAnchor(anchor:string):ActivityAnchor | null { + const activityIdMatch = anchor.match(anchorTypeRegex); // Ex. [ "#comment-80", "comment", "80" ] + + if (activityIdMatch?.[1] && activityIdMatch?.[2]) { + const type = activityIdMatch[1]; + if (Object.values(ActivityAnchorType).includes(type as ActivityAnchorType)) { + return { type: type as ActivityAnchorType, id: activityIdMatch[2] }; + } + } + return null; + } +} diff --git a/frontend/src/stimulus/setup.ts b/frontend/src/stimulus/setup.ts index 174b3260c37d..f2d125a389f4 100644 --- a/frontend/src/stimulus/setup.ts +++ b/frontend/src/stimulus/setup.ts @@ -24,6 +24,7 @@ import AutoScrollingController from './controllers/dynamic/work-packages/activit import PollingController from './controllers/dynamic/work-packages/activities-tab/polling.controller'; import StemsController from './controllers/dynamic/work-packages/activities-tab/stems.controller'; import EditorController from './controllers/dynamic/work-packages/activities-tab/editor.controller'; +import LazyPageController from './controllers/dynamic/work-packages/activities-tab/lazy-page.controller'; import AutoSubmit from '@stimulus-components/auto-submit'; import AutoThemeSwitcher from './controllers/auto-theme-switcher.controller'; @@ -64,6 +65,7 @@ OpenProjectStimulusApplication.preregister('work-packages--activities-tab--auto- OpenProjectStimulusApplication.preregister('work-packages--activities-tab--polling', PollingController); OpenProjectStimulusApplication.preregister('work-packages--activities-tab--stems', StemsController); OpenProjectStimulusApplication.preregister('work-packages--activities-tab--editor', EditorController); +OpenProjectStimulusApplication.preregister('work-packages--activities-tab--lazy-page', LazyPageController); OpenProjectStimulusApplication.preregister('beforeunload', BeforeunloadController); OpenProjectStimulusApplication.preregister('auto-theme-switcher', AutoThemeSwitcher); OpenProjectStimulusApplication.preregister('external-links', ExternalLinksController); diff --git a/frontend/src/typings/turbo.d.ts b/frontend/src/typings/turbo.d.ts index e7a6389d8e42..ef9bcfed1462 100644 --- a/frontend/src/typings/turbo.d.ts +++ b/frontend/src/typings/turbo.d.ts @@ -6,3 +6,10 @@ export interface TurboStreamElement extends HTMLElement { action:string; target:string; } + +export interface TurboBeforeStreamRenderEvent extends CustomEvent { + detail:{ + newStream:TurboStreamElement; + render:(stream:TurboStreamElement) => void; + }; +} diff --git a/spec/factories/work_package_factory.rb b/spec/factories/work_package_factory.rb index 0eb609fabd73..2dcafac0ce96 100644 --- a/spec/factories/work_package_factory.rb +++ b/spec/factories/work_package_factory.rb @@ -146,7 +146,6 @@ end end - # force done_ratio in status-based mode if given done_ratio is different from status default callback(:after_create) do |work_package, evaluator| next unless WorkPackage.status_based_mode? diff --git a/spec/features/activities/work_package/activities_spec.rb b/spec/features/activities/work_package/activities_spec.rb index 378e165e87b1..f4e471d8b1ea 100644 --- a/spec/features/activities/work_package/activities_spec.rb +++ b/spec/features/activities/work_package/activities_spec.rb @@ -1035,7 +1035,7 @@ let(:work_package) { create(:work_package, project:, author: admin) } # create enough comments to make the journal container scrollable - 20.times do |i| + 25.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 diff --git a/spec/services/work_packages/activities_tab/paginator_spec.rb b/spec/services/work_packages/activities_tab/paginator_spec.rb new file mode 100644 index 000000000000..cb2c72403896 --- /dev/null +++ b/spec/services/work_packages/activities_tab/paginator_spec.rb @@ -0,0 +1,259 @@ +# 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" + +RSpec.describe WorkPackages::ActivitiesTab::Paginator do + shared_let(:user) { create(:admin) } + shared_let(:project) { create(:project) } + shared_let(:work_package) { create(:work_package, :created_in_past, project:, author: user, created_at: 4.days.ago) } + + let(:params) { {} } + let(:paginator) { described_class.new(work_package, params) } + + before do + allow(User).to receive(:current).and_return(user) + end + + describe "#call" do + context "with no additional journals" do + let(:work_package) { create(:work_package, project:, author: user) } + + it "returns paginated results with the initial journal" do + pagy, records = paginator.call + + expect(pagy).to be_a(Pagy) + expect(pagy.page).to eq(1) + expect(pagy.count).to eq(1) + expect(records).to have_attributes(size: 1) + expect(records.first).to be_a(API::V3::Activities::ActivityEagerLoadingWrapper) + end + end + + context "with multiple journals" do + 3.times do |i| + let!(:"journal_#{i + 1}") do + create(:work_package_journal, user:, notes: "Comment #{i + 1}", journable: work_package, version: i + 2) + end + end + + context "when user preference is set to asc sorting" do + before do + user.pref.update!(comments_sorting: :asc) + end + + it "returns journals reversed (oldest first)" do + _pagy, records = paginator.call + + expect(records.map(&:notes)).to eq(["", journal_1.notes, journal_2.notes, journal_3.notes]) + end + end + + context "when user preference is set to desc sorting" do + before do + user.pref.update!(comments_sorting: :desc) + end + + it "returns journals in DESC order (newest first)" do + _pagy, records = paginator.call + + expect(records.map(&:notes)).to eq([journal_3.notes, journal_2.notes, journal_1.notes, ""]) + end + end + end + + context "with changesets" do + let(:repository) { create(:repository_subversion, project:) } + + 2.times do |i| + let!(:"changeset_#{i + 1}") do + create(:changeset, + repository:, + committed_on: (2 - i).days.ago, # yesterday and today + revision: "rev#{i + 1}") + end + end + + before do + work_package.changesets << [changeset_1, changeset_2] + end + + it "includes changesets in the sorted results" do + _pagy, records = paginator.call + + expect(records.size).to eq(3) # 1 initial journal + 2 changesets + expect(records).to include(changeset_1, changeset_2) + end + + it "sorts changesets along with journals by timestamp" do + user.pref.update!(comments_sorting: :desc) + journal = create(:work_package_journal, + user:, + notes: "Comment between changesets", + journable: work_package, + version: work_package.journals.last.version + 1, + created_at: 1.5.days.ago) + + _pagy, records = paginator.call + expect(records).to eq([changeset_2, journal, changeset_1, work_package.journals.first]) + end + end + + context "with pagination" do + # Create enough journals to span multiple pages (using limit of 5 for testing) + let(:test_limit) { 5 } + + 10.times do |i| + let!(:"journal_#{i + 1}") do + create(:work_package_journal, user:, notes: "Comment #{i + 1}", journable: work_package, version: i + 2) + end + end + + before do + params[:limit] = test_limit + end + + it "returns the first page with specified limit" do + pagy, records = paginator.call + + expect(pagy.page).to eq(1) + expect(pagy.count).to eq(11) # 10 journals + 1 initial + expect(pagy.pages).to eq(3) + expect(records.size).to eq(test_limit) + end + + it "returns the second page when requested" do + params[:page] = 2 + pagy, records = paginator.call + + expect(pagy.page).to eq(2) + expect(records.size).to eq(test_limit) + end + + context "with anchor to target journal" do + context "with comment anchor" do + it "returns the page containing the target journal" do + params[:anchor] = "comment-#{journal_1.id}" + pagy, records = paginator.call + + # journal_1 is old, so it should be on page 2 + expect(pagy.page).to eq(2) + expect(records.map(&:id)).to include(journal_1.id) + end + + it "handles invalid anchor format gracefully" do + params[:anchor] = "invalid-anchor" + pagy, _records = paginator.call + + expect(pagy.page).to eq(1) + end + + it "falls back to page 1 if journal not found" do + params[:anchor] = "comment-999999" + pagy, _records = paginator.call + + expect(pagy.page).to eq(1) + end + end + + context "with activity anchor" do + it "returns the page containing the target activity by sequence_version" do + params[:anchor] = "activity-2" + pagy, records = paginator.call + + # activity-2 corresponds to journal with sequence_version 2 + # which should be journal_1 + expect(pagy.page).to eq(2) + wrapped_journal = records.find { it.is_a?(API::V3::Activities::ActivityEagerLoadingWrapper) && it.id == journal_1.id } + expect(wrapped_journal.sequence_version).to eq(2) + end + + it "handles activity anchor for initial journal" do + params[:anchor] = "activity-1" + _pagy, records = paginator.call + + # activity-1 should be on the last page (oldest) + expect(records.any? { it.respond_to?(:sequence_version) && it.sequence_version == 1 }).to be(true) + end + end + end + end + + context "with internal comments filtering" do + let!(:internal_journal) do + create(:work_package_journal, + user:, + notes: "Internal comment", + journable: work_package, + internal: true, + version: 2) + end + let!(:public_journal) do + create(:work_package_journal, + user:, + notes: "Public comment", + journable: work_package, + internal: false, + version: 3) + end + + before do + work_package.project.enabled_internal_comments = true + work_package.project.save! + end + + context "when user can see internal comments" do + it "includes internal journals" do + _pagy, records = paginator.call + + journal_notes = records.map(&:notes) + expect(journal_notes).to include("Internal comment", "Public comment") + end + end + + context "when user cannot see internal comments" do + let(:member_role) { create(:project_role, permissions: %i[view_work_packages]) } + let(:member_user) { create(:user, member_with_roles: { project => member_role }) } + + before do + allow(User).to receive(:current).and_return(member_user) + end + + it "excludes internal journals" do + _pagy, records = described_class.new(work_package, params).call + + journal_notes = records.map(&:notes) + expect(journal_notes).not_to include("Internal comment") + expect(journal_notes).to include("Public comment") + end + end + end + end +end diff --git a/spec/support/components/work_packages/activities.rb b/spec/support/components/work_packages/activities.rb index ab6e48675669..c088f9ec6510 100644 --- a/spec/support/components/work_packages/activities.rb +++ b/spec/support/components/work_packages/activities.rb @@ -234,6 +234,9 @@ def expect_unsaved_content(text) end def type_comment(text) + # Wait for any pending Turbo Stream updates to complete + wait_for_network_idle + begin open_new_comment_editor if page.find_test_selector("op-open-work-package-journal-form-trigger") rescue Capybara::ElementNotFound