diff --git a/app/components/admin/custom_fields/custom_field_projects/row_component.rb b/app/components/admin/custom_fields/custom_field_projects/row_component.rb index 51ce950bbac0..d7b6ac1d62ee 100644 --- a/app/components/admin/custom_fields/custom_field_projects/row_component.rb +++ b/app/components/admin/custom_fields/custom_field_projects/row_component.rb @@ -57,11 +57,10 @@ def more_menu_detach_project def detach_from_project_url url_helpers.custom_field_project_path( custom_field_id: @table.params[:custom_field].id, - custom_fields_project: { project_id: project.id } + custom_fields_project: { project_id: project.id }, + page: current_page ) end - - def project = model.first end end end diff --git a/app/components/admin/custom_fields/custom_field_projects/table_component.rb b/app/components/admin/custom_fields/custom_field_projects/table_component.rb index 9ef7650dab1a..5eb0f19a4cd5 100644 --- a/app/components/admin/custom_fields/custom_field_projects/table_component.rb +++ b/app/components/admin/custom_fields/custom_field_projects/table_component.rb @@ -30,7 +30,7 @@ module Admin module CustomFields module CustomFieldProjects class TableComponent < Projects::TableComponent - include OpTurbo::Streamable + include ::Projects::Concerns::TableComponent::StreamablePaginationLinksConstraints def columns @columns ||= query.selects.reject { |select| select.is_a?(Queries::Selects::NotExistingSelect) } @@ -39,23 +39,6 @@ def columns def sortable? false end - - # @override optional_pagination_options are passed to the pagination_options - # which are passed to #pagination_links_full in pagination_helper.rb - # - # In Turbo streamable components, we need to be able to specify the url_for(action:) so that links are - # generated in the context of the component index action, instead of any turbo stream actions performing - # partial updates on the page. - # - # params[:url_for_action] is passed to the pagination_options making it's way down to any pagination links - # that are generated via link_to which calls url_for which uses the params[:url_for_action] to specify - # the controller action that link_to should use. - # - def optional_pagination_options - return super unless params[:url_for_action] - - super.merge(params: { action: params[:url_for_action] }) - end end end end diff --git a/app/components/projects/concerns/table_component/streamable_pagination_links_constraints.rb b/app/components/projects/concerns/table_component/streamable_pagination_links_constraints.rb new file mode 100644 index 000000000000..ddaedeaa11a7 --- /dev/null +++ b/app/components/projects/concerns/table_component/streamable_pagination_links_constraints.rb @@ -0,0 +1,55 @@ +#-- 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 Projects + module Concerns + module TableComponent + module StreamablePaginationLinksConstraints + # @override optional_pagination_options are passed to the pagination_options + # which are passed to #pagination_links_full in pagination_helper.rb + # + # In Turbo streamable components, we need to be able to specify the url_for(action:) so that links are + # generated in the context of the component index action, instead of any turbo stream actions performing + # partial updates on the page. + # + # params[:url_for_action] is passed to the pagination_options making it's way down to any pagination links + # that are generated via link_to which calls url_for which uses the params[:url_for_action] to specify + # the controller action that link_to should use. + # + # data-turbo-action="advance" is added to the pagination links ensure links pushState to the browser + # + def optional_pagination_options + super.tap do |options| + options[:params] = { action: params[:url_for_action] } if params[:url_for_action] + options[:turbo_action] = "advance" + end + end + end + end + end +end diff --git a/app/components/projects/row_component.rb b/app/components/projects/row_component.rb index 70adc2566d7d..3f540eac0ecd 100644 --- a/app/components/projects/row_component.rb +++ b/app/components/projects/row_component.rb @@ -373,5 +373,9 @@ def user_can_view_project? def custom_field_column?(column) column.is_a?(::Queries::Projects::Selects::CustomField) end + + def current_page + table.model.current_page.to_s + end end end diff --git a/app/components/settings/project_custom_fields/project_custom_field_mapping/row_component.rb b/app/components/settings/project_custom_fields/project_custom_field_mapping/row_component.rb index f4645d2a23ff..5ee09b6baf3e 100644 --- a/app/components/settings/project_custom_fields/project_custom_field_mapping/row_component.rb +++ b/app/components/settings/project_custom_fields/project_custom_field_mapping/row_component.rb @@ -35,7 +35,8 @@ class RowComponent < Admin::CustomFields::CustomFieldProjects::RowComponent def detach_from_project_url url_helpers.unlink_admin_settings_project_custom_field_path( id: @table.params[:custom_field].id, - project_custom_field_project_mapping: { project_id: project.id } + project_custom_field_project_mapping: { project_id: project.id }, + page: current_page ) end end diff --git a/app/controllers/admin/custom_fields/custom_field_projects_controller.rb b/app/controllers/admin/custom_fields/custom_field_projects_controller.rb index 8f69e323e74a..e21966dd67b9 100644 --- a/app/controllers/admin/custom_fields/custom_field_projects_controller.rb +++ b/app/controllers/admin/custom_fields/custom_field_projects_controller.rb @@ -103,7 +103,7 @@ def render_project_list(url_for_action: action_name) update_via_turbo_stream( component: Admin::CustomFields::CustomFieldProjects::TableComponent.new( query: available_custom_fields_projects_query, - params: { custom_field: @custom_field, url_for_action: } + params: params.merge({ custom_field: @custom_field, url_for_action: }) ) ) end diff --git a/app/controllers/admin/settings/project_custom_fields_controller.rb b/app/controllers/admin/settings/project_custom_fields_controller.rb index b532962b12f5..5bccb8501fc7 100644 --- a/app/controllers/admin/settings/project_custom_fields_controller.rb +++ b/app/controllers/admin/settings/project_custom_fields_controller.rb @@ -158,7 +158,7 @@ def render_project_list(url_for_action: action_name) update_via_turbo_stream( component: Settings::ProjectCustomFields::ProjectCustomFieldMapping::TableComponent.new( query: project_custom_field_mappings_query, - params: { custom_field: @custom_field, url_for_action: } + params: params.merge({ custom_field: @custom_field, url_for_action: }) ) ) end diff --git a/app/helpers/pagination_helper.rb b/app/helpers/pagination_helper.rb index c8a53aaadc42..28e9f79f3c80 100644 --- a/app/helpers/pagination_helper.rb +++ b/app/helpers/pagination_helper.rb @@ -202,6 +202,7 @@ def previous_or_next_page(page, text, class_suffix) def link(text, target, attributes) new_attributes = attributes.dup new_attributes["data-turbo-stream"] = true if turbo? + new_attributes["data-turbo-action"] = turbo_action if turbo_action.present? super(text, target, new_attributes) end @@ -210,6 +211,20 @@ def allowed_params @options[:allowed_params] # rubocop:disable Rails/HelperInstanceVariable end + # Customize the Turbo visit action for pagination links. Can be set to "advance" or "replace". + # "advance" - push a new entry onto the history stack. + # "replace" - replace the current history entry. + # See: https://turbo.hotwired.dev/reference/attributes + # + # Example: Promoting a Frame Navigation to a Page Visit + # By default navigation within a turbo frame does not change the rest of the browser's state, + # but you can promote a frame navigation a "Visit" by setting the turbo-action attribute to "advance". + # See: https://turbo.hotwired.dev/handbook/frames#promoting-a-frame-navigation-to-a-page-visit + # + def turbo_action + @options[:turbo_action] # rubocop:disable Rails/HelperInstanceVariable + end + def turbo? @options[:turbo] # rubocop:disable Rails/HelperInstanceVariable end diff --git a/modules/storages/app/components/storages/project_storages/destroy_confirmation_dialog_component.html.erb b/modules/storages/app/components/storages/project_storages/destroy_confirmation_dialog_component.html.erb index 956475512f2a..cef571b12e07 100644 --- a/modules/storages/app/components/storages/project_storages/destroy_confirmation_dialog_component.html.erb +++ b/modules/storages/app/components/storages/project_storages/destroy_confirmation_dialog_component.html.erb @@ -31,7 +31,7 @@ See COPYRIGHT and LICENSE files for more details. component_wrapper do primer_form_with( model: @project_storage, - url: admin_settings_storage_project_storage_path(id: @project_storage), + url: admin_settings_storage_project_storage_path(id: @project_storage, page: current_page), data: { turbo: true, controller: 'disable-when-checked', diff --git a/modules/storages/app/components/storages/project_storages/destroy_confirmation_dialog_component.rb b/modules/storages/app/components/storages/project_storages/destroy_confirmation_dialog_component.rb index 228e6125fb47..ecd4b986e0d4 100644 --- a/modules/storages/app/components/storages/project_storages/destroy_confirmation_dialog_component.rb +++ b/modules/storages/app/components/storages/project_storages/destroy_confirmation_dialog_component.rb @@ -32,11 +32,12 @@ class DestroyConfirmationDialogComponent < ApplicationComponent include OpTurbo::Streamable include OpPrimer::ComponentHelpers - def initialize(storage:, project_storage:) + def initialize(storage:, project_storage:, params: {}) super @storage = storage @project_storage = project_storage + @params = params end def id @@ -56,6 +57,10 @@ def text text end + def current_page + @params[:page] + end + def confirmation_text I18n.t("project_storages.remove_project.dialog.confirmation_text") end diff --git a/modules/storages/app/components/storages/project_storages/projects/row_component.rb b/modules/storages/app/components/storages/project_storages/projects/row_component.rb index b144b519edd1..e1995faaeec4 100644 --- a/modules/storages/app/components/storages/project_storages/projects/row_component.rb +++ b/modules/storages/app/components/storages/project_storages/projects/row_component.rb @@ -68,7 +68,8 @@ def more_menu_detach_project icon: :trash, label: I18n.t("project_storages.remove_project.label"), href: destroy_confirmation_dialog_admin_settings_storage_project_storage_path( - id: project_storage.id + id: project_storage.id, + page: current_page ), data: { controller: "async-dialog" diff --git a/modules/storages/app/components/storages/project_storages/projects/table_component.rb b/modules/storages/app/components/storages/project_storages/projects/table_component.rb index 243ccbe46ab1..ea1db72e0516 100644 --- a/modules/storages/app/components/storages/project_storages/projects/table_component.rb +++ b/modules/storages/app/components/storages/project_storages/projects/table_component.rb @@ -32,7 +32,7 @@ # for every "column" defined below. module Storages::ProjectStorages::Projects class TableComponent < Projects::TableComponent - include OpTurbo::Streamable + include ::Projects::Concerns::TableComponent::StreamablePaginationLinksConstraints options :storage diff --git a/modules/storages/app/controllers/storages/admin/storages/project_storages_controller.rb b/modules/storages/app/controllers/storages/admin/storages/project_storages_controller.rb index 3e842e26be0f..f83ffe11d00b 100644 --- a/modules/storages/app/controllers/storages/admin/storages/project_storages_controller.rb +++ b/modules/storages/app/controllers/storages/admin/storages/project_storages_controller.rb @@ -117,7 +117,8 @@ def update def destroy_confirmation_dialog respond_with_dialog Storages::ProjectStorages::DestroyConfirmationDialogComponent.new( storage: @storage, - project_storage: @project_storage + project_storage: @project_storage, + params: ) end @@ -186,7 +187,7 @@ def update_project_list_via_turbo_stream(url_for_action: action_name) component: Storages::ProjectStorages::Projects::TableComponent.new( query: storage_projects_query, storage: @storage, - params: { url_for_action: } + params: params.merge({ url_for_action: }) ) ) end diff --git a/modules/storages/spec/features/storages/admin/project_storages_spec.rb b/modules/storages/spec/features/storages/admin/project_storages_spec.rb index f4ae5ee03f8c..9057b15def07 100644 --- a/modules/storages/spec/features/storages/admin/project_storages_spec.rb +++ b/modules/storages/spec/features/storages/admin/project_storages_spec.rb @@ -228,6 +228,10 @@ expect(page).to have_text(project.name) expect(page).to have_text(subproject.name) + + aggregate_failures "pagination links maintain the correct url" do + project_storages_index_page.expect_page_sizes(model: storage) + end end context "when the user does not select a folder" do @@ -297,6 +301,10 @@ project_storages_index_page.within_the_table_row_containing(project.name) do expect(page).to have_text("No specific folder") end + + aggregate_failures "pagination links maintain the correct url" do + project_storages_index_page.expect_page_sizes(model: storage) + end end context "when oauth access has not been granted and manual selection" do @@ -363,8 +371,15 @@ def call expect(page).to have_text(project.name) end - it "is possible to remove the project after checking the confirmation checkbox in the dialog" do - expect(page).to have_text(project.name) + it "is possible to remove the project after checking the confirmation checkbox in the dialog", + with_settings: { per_page_options: "2,5" } do + projects = create_list(:project, 4) + projects.each { |project| create(:project_storage, storage:, project:) } + + current_page = 3 + visit admin_settings_storage_project_storages_path(storage, page: current_page) + + project = project_storages_index_page.project_in_first_row(column_text_separator: "\t") project_storages_index_page.click_menu_item_of("Remove project", project) # The original DeleteService would try to remove actual files from actual storages, @@ -388,6 +403,10 @@ def call expect(page).to have_no_selector("dialog") expect(page).to have_text("Successful deletion.") expect(page).to have_no_text(project.name) + + aggregate_failures "pagination links maintain the correct url" do + project_storages_index_page.expect_page_links(model: storage, current_page:) + end end end end diff --git a/modules/storages/spec/support/pages/admin/storages/project_storages/project_storages_index.rb b/modules/storages/spec/support/pages/admin/storages/project_storages/project_storages_index.rb index b1008cb411ab..ebd99db68b2d 100644 --- a/modules/storages/spec/support/pages/admin/storages/project_storages/project_storages_index.rb +++ b/modules/storages/spec/support/pages/admin/storages/project_storages/project_storages_index.rb @@ -33,7 +33,7 @@ module Admin module Storages module ProjectStorages class Index < ::Pages::Projects::Index - def path + def path(storage) "/admin/settings/storages/#{storage.id}/project_storages" end diff --git a/spec/features/admin/custom_fields/custom_fields_project_spec.rb b/spec/features/admin/custom_fields/custom_fields_project_spec.rb index 8081ae4c1534..7726caf0d324 100644 --- a/spec/features/admin/custom_fields/custom_fields_project_spec.rb +++ b/spec/features/admin/custom_fields/custom_fields_project_spec.rb @@ -115,38 +115,25 @@ expect(page).to have_text(subproject.name) aggregate_failures "pagination links maintain the correct url" do - within ".op-pagination" do - pagination_links = page.all(".op-pagination--item-link") - expect(pagination_links.size).to be_positive - - pagination_links.each do |pagination_link| - uri = URI.parse(pagination_link["href"]) - expect(uri.path).to eq(custom_field_projects_path(custom_field)) - end - end + custom_field_projects_page.expect_page_sizes(model: custom_field) end end - it "allows unlinking a project from a custom field" do - project = create(:project) - create(:custom_fields_project, custom_field:, project:) + it "allows unlinking a project from a custom field", with_settings: { per_page_options: "2,5" } do + projects = create_list(:project, 4) + projects.each { |project| create(:custom_fields_project, custom_field:, project:) } - visit custom_field_projects_path(custom_field) + current_page = 3 + visit custom_field_projects_path(custom_field, page: current_page) + project = custom_field_projects_page.project_in_first_row custom_field_projects_page.click_menu_item_of("Remove from project", project) expect(page).to have_no_text(project.name) aggregate_failures "pagination links maintain the correct url after unlinking is done" do - within ".op-pagination" do - pagination_links = page.all(".op-pagination--item-link") - expect(pagination_links.size).to be_positive - - pagination_links.each do |pagination_link| - uri = URI.parse(pagination_link["href"]) - expect(uri.path).to eq(custom_field_projects_path(custom_field)) - end - end + custom_field_projects_page.expect_page_links(model: custom_field, current_page:) + custom_field_projects_page.expect_current_page_number(current_page) end end diff --git a/spec/features/admin/project_custom_fields/project_mappings_spec.rb b/spec/features/admin/project_custom_fields/project_mappings_spec.rb index ea0eee6e21e6..97769b0e80a9 100644 --- a/spec/features/admin/project_custom_fields/project_mappings_spec.rb +++ b/spec/features/admin/project_custom_fields/project_mappings_spec.rb @@ -119,38 +119,25 @@ expect(page).to have_text(subproject.name) aggregate_failures "pagination links maintain the correct url" do - within ".op-pagination" do - pagination_links = page.all(".op-pagination--item-link") - expect(pagination_links.size).to be_positive - - pagination_links.each do |pagination_link| - uri = URI.parse(pagination_link["href"]) - expect(uri.path).to eq(project_mappings_admin_settings_project_custom_field_path(project_custom_field)) - end - end + project_custom_field_mappings_page.expect_page_sizes(model: project_custom_field) end end - it "allows unlinking a project from a custom field" do - project = create(:project) - create(:project_custom_field_project_mapping, project_custom_field:, project:) + it "allows unlinking a project from a custom field", with_settings: { per_page_options: "2,5" } do + projects = create_list(:project, 4) + projects.each { |project| create(:project_custom_field_project_mapping, project_custom_field:, project:) } - visit project_mappings_admin_settings_project_custom_field_path(project_custom_field) + current_page = 3 + visit project_mappings_admin_settings_project_custom_field_path(project_custom_field, page: current_page) + project = project_custom_field_mappings_page.project_in_first_row project_custom_field_mappings_page.click_menu_item_of("Remove from project", project) expect(page).to have_no_text(project.name) aggregate_failures "pagination links maintain the correct url after unlinking is done" do - within ".op-pagination" do - pagination_links = page.all(".op-pagination--item-link") - expect(pagination_links.size).to be_positive - - pagination_links.each do |pagination_link| - uri = URI.parse(pagination_link["href"]) - expect(uri.path).to eq(project_mappings_admin_settings_project_custom_field_path(project_custom_field)) - end - end + project_custom_field_mappings_page.expect_page_links(model: project_custom_field, current_page:) + project_custom_field_mappings_page.expect_current_page_number(current_page) end end diff --git a/spec/support/pages/admin/custom_fields/custom_fields_projects/custom_field_projects_index.rb b/spec/support/pages/admin/custom_fields/custom_fields_projects/custom_field_projects_index.rb index a170b17e58ef..78fb409497e0 100644 --- a/spec/support/pages/admin/custom_fields/custom_fields_projects/custom_field_projects_index.rb +++ b/spec/support/pages/admin/custom_fields/custom_fields_projects/custom_field_projects_index.rb @@ -33,17 +33,19 @@ module Admin module CustomFields module CustomFieldsProjects class CustomFieldProjectsIndex < ::Pages::Projects::Index - def path + def path(custom_field) "/custom_fields/#{custom_field.id}/projects" end def within_row(project) - row = page.find("#admin-custom-fields-custom-field-projects-row-component-project-#{project.id}") + row = page.find("#{row_id_prefix}-#{project.id}") row.hover within row do yield row end end + + def row_id_prefix = "#admin-custom-fields-custom-field-projects-row-component-project" end end end diff --git a/spec/support/pages/admin/settings/project_custom_fields/project_custom_field_mappings_index.rb b/spec/support/pages/admin/settings/project_custom_fields/project_custom_field_mappings_index.rb index ab41fabafb8f..136762591a4c 100644 --- a/spec/support/pages/admin/settings/project_custom_fields/project_custom_field_mappings_index.rb +++ b/spec/support/pages/admin/settings/project_custom_fields/project_custom_field_mappings_index.rb @@ -32,18 +32,12 @@ module Pages module Admin module Settings module ProjectCustomFields - class ProjectCustomFieldMappingsIndex < ::Pages::Projects::Index - def path + class ProjectCustomFieldMappingsIndex < ::Pages::Admin::CustomFields::CustomFieldsProjects::CustomFieldProjectsIndex + def path(project_custom_field) "/admin/settings/project_custom_fields/#{project_custom_field.id}/project_mappings" end - def within_row(project) - row = page.find("#settings-project-custom-fields-project-custom-field-mapping-row-component-project-#{project.id}") - row.hover - within row do - yield row - end - end + def row_id_prefix = "#settings-project-custom-fields-project-custom-field-mapping-row-component-project" end end end diff --git a/spec/support/pages/projects/index.rb b/spec/support/pages/projects/index.rb index 66810eb38d40..d83f6f04ac43 100644 --- a/spec/support/pages/projects/index.rb +++ b/spec/support/pages/projects/index.rb @@ -33,7 +33,7 @@ module Projects class Index < ::Pages::Page include ::Components::Autocompleter::NgSelectAutocompleteHelpers - def path + def path(*) "/projects" end @@ -127,6 +127,39 @@ def expect_page_link(text) end end + def expect_page_links(model:, current_page: 1) + within ".op-pagination--pages" do + pagination_links = page.all(".op-pagination--item-link") + expect(pagination_links.size).to be_positive + + page_number_links = pagination_links.reject { |link| link.text =~ /previous|next/i } + page_number_links.each.with_index(1) do |pagination_link, page_number| + uri = URI.parse(pagination_link["href"]) + expect(uri.path).to eq(path(model)) + expect(uri.query).to include("page=#{page_number}") + end + + if current_page > 1 + expect(page).to have_link("Previous", href: "#{path(model)}?#{{ page: current_page - 1 }.to_query}") + else + expect(page).to have_link("Next", href: "#{path(model)}?#{{ page: current_page + 1 }.to_query}") + end + end + end + + def expect_page_sizes(model:) + within ".op-pagination--options" do + pagination_links = page.all(".op-pagination--item-link") + expect(pagination_links.size).to be_positive + expect(page).to have_css(".op-pagination--item_current") + + pagination_links.each do |pagination_link| + uri = URI.parse(pagination_link["href"]) + expect(uri.path).to eq(path(model)) + end + end + end + def expect_filters_container_toggled expect(page).to have_css(".op-filters-form") end @@ -531,6 +564,11 @@ def within_table(&) within "#project-table", & end + def project_in_first_row(column_text_separator: "\n") + first_row = within("#projects-table") { find(".op-project-row-component", match: :first) } + Project.find_by!(name: first_row.text.split(column_text_separator).first) + end + def within_row(project) row = page.find("#project-#{project.id}") row.hover