diff --git a/app/components/admin/enumerations/index_component.html.erb b/app/components/admin/enumerations/index_component.html.erb index b5ab6e2be94f..ce47865d18b2 100644 --- a/app/components/admin/enumerations/index_component.html.erb +++ b/app/components/admin/enumerations/index_component.html.erb @@ -1,6 +1,6 @@ <%= -component_wrapper do - flex_layout(data: wrapper_data_attributes) do |flex| +component_wrapper(data: wrapper_data_attributes) do + flex_layout do |flex| flex.with_row do render(Primer::OpenProject::SubHeader.new) do |subheader| subheader.with_action_button( @@ -21,12 +21,12 @@ component_wrapper do OpenProject::Common::BorderBoxListComponent.new( container: "#{wrapper_key}-box", position: :relative, - data: drop_target_config + data: list_data ) ) do |list| list.with_header( title_tag: :h3, - title: enumeration_class.model_name.human(count: :other) + title: enumeration_title ) list.with_empty_state( @@ -36,8 +36,8 @@ component_wrapper do ) enumerations.each do |enumeration| - list.with_item(test_selector: "enumeration-row-#{enumeration.id}", data: draggable_item_config(enumeration)) do - render(item_component_class.new(enumeration: enumeration, max_position: max_position)) + list.with_item(test_selector: "enumeration-row-#{enumeration.id}", data: item_data(enumeration)) do + render(item_component_class.new(enumeration:)) end end end diff --git a/app/components/admin/enumerations/index_component.rb b/app/components/admin/enumerations/index_component.rb index 2c597ea931a2..85885cee4bf4 100644 --- a/app/components/admin/enumerations/index_component.rb +++ b/app/components/admin/enumerations/index_component.rb @@ -35,38 +35,55 @@ class IndexComponent < ApplicationComponent include OpPrimer::ComponentHelpers include OpTurbo::Streamable - options :enumerations + def initialize(enumerations:) + super() + @enumerations = enumerations + end private - def max_position - enumerations.map(&:position).max - end + attr_reader :enumerations def wrapper_data_attributes { - controller: "generic-drag-and-drop" + controller: "sortable-lists", + sortable_lists_move_url_template_value: move_url_template, + sortable_lists_sortable_lists__list_outlet: "##{wrapper_key} [data-controller~='sortable-lists--list']", + sortable_lists_sortable_lists__item_outlet: "##{wrapper_key} [data-controller~='sortable-lists--item']" } end - def drop_target_config + # Built from the route helper with a sentinel so relative-URL-root + # installations keep working; {id} is expanded client-side. + def move_url_template + id_placeholder = "__id__" + helpers.url_for(action: :move, id: id_placeholder).sub(id_placeholder, "{id}") + end + + def list_data { - generic_drag_and_drop_target: "container", - "target-container-accessor": ":scope > ul", - "target-allowed-drag-type": "enumeration" + controller: "sortable-lists--list", + sortable_lists__list_type_value: sortable_list_type, + sortable_lists__list_accepted_type_value: sortable_list_type, + sortable_lists__list_name_value: enumeration_title } end - def draggable_item_config(enumeration) + def item_data(enumeration) { - "draggable-id": enumeration.id, - "draggable-type": "enumeration", - "drop-url": helpers.url_for(action: :move, id: enumeration.id) + controller: "sortable-lists--item", + sortable_lists__item_id_value: enumeration.id, + sortable_lists__item_type_value: sortable_list_type, + sortable_lists__item_label_value: enumeration.name } end - def enumeration_class - enumerations.klass + def sortable_list_type + enumerations.model_name.param_key + end + + def enumeration_title + enumerations.model_name.human(count: :other) end def item_component_class diff --git a/app/components/admin/enumerations/item_component.html.erb b/app/components/admin/enumerations/item_component.html.erb index e3179351c7fc..a14c5dd9aac3 100644 --- a/app/components/admin/enumerations/item_component.html.erb +++ b/app/components/admin/enumerations/item_component.html.erb @@ -3,7 +3,7 @@ flex_layout(align_items: :center, justify_content: :space_between) do |enumeration_container| enumeration_container.with_column(flex_layout: true) do |enumeration_info| enumeration_info.with_column(mr: 2) do - render(Primer::OpenProject::DragHandle.new) + render(Primer::OpenProject::DragHandle.new(data: { sortable_lists__item_target: "handle" })) end if colored? diff --git a/app/components/admin/enumerations/item_component.rb b/app/components/admin/enumerations/item_component.rb index 6bd39f3279a6..2f44dcbd622e 100644 --- a/app/components/admin/enumerations/item_component.rb +++ b/app/components/admin/enumerations/item_component.rb @@ -34,37 +34,27 @@ class ItemComponent < ApplicationComponent include ApplicationHelper include OpPrimer::ComponentHelpers include OpTurbo::Streamable - - options :enumeration - options :max_position + include SortableLists::MoveMenu delegate :colored?, to: :enumeration + def initialize(enumeration:) + super() + @enumeration = enumeration + end + private + attr_reader :enumeration + def wrapper_uniq_by enumeration.id end - def first_item? - enumeration.position == 1 - end - - def last_item? - enumeration.position == max_position - end - def build_enumeration_menu(menu) - with_item_group(menu) { edit_enumeration(menu) } with_item_group(menu) do - unless first_item? - move_to_top_enumeration(menu) - move_up_enumeration(menu) - end - unless last_item? - move_down_enumeration(menu) - move_to_bottom_enumeration(menu) - end + edit_enumeration(menu) + move_enumeration(menu) end with_item_group(menu) { deletion_enumeration(menu) } end @@ -77,51 +67,17 @@ def edit_enumeration(menu) end end - def move_to_top_enumeration(menu) - form_inputs = [{ name: "move_to", value: "highest" }] - - menu.with_item(label: I18n.t(:label_sort_highest), - tag: :button, - href: helpers.url_for(action: :move, id: enumeration), - # content_arguments: { data: { turbo_frame: ItemsComponent.wrapper_key } }, - form_arguments: { method: :put, inputs: form_inputs }) do |item| - item.with_leading_visual_icon(icon: "move-to-top") - end - end - - def move_up_enumeration(menu) - form_inputs = [{ name: "move_to", value: "higher" }] - - menu.with_item(label: I18n.t(:label_sort_higher), - tag: :button, - href: helpers.url_for(action: :move, id: enumeration), - # content_arguments: { data: { turbo_frame: ItemsComponent.wrapper_key } }, - form_arguments: { method: :put, inputs: form_inputs }) do |item| - item.with_leading_visual_icon(icon: "chevron-up") - end - end - - def move_down_enumeration(menu) - form_inputs = [{ name: "move_to", value: "lower" }] - - menu.with_item(label: I18n.t(:label_sort_lower), - tag: :button, - href: helpers.url_for(action: :move, id: enumeration), - # content_arguments: { data: { turbo_frame: ItemsComponent.wrapper_key } }, - form_arguments: { method: :put, inputs: form_inputs }) do |item| - item.with_leading_visual_icon(icon: "chevron-down") - end - end - - def move_to_bottom_enumeration(menu) - form_inputs = [{ name: "move_to", value: "lowest" }] - - menu.with_item(label: I18n.t(:label_sort_lowest), - tag: :button, - href: helpers.url_for(action: :move, id: enumeration), - # content_arguments: { data: { turbo_frame: ItemsComponent.wrapper_key } }, - form_arguments: { method: :put, inputs: form_inputs }) do |item| - item.with_leading_visual_icon(icon: "move-to-bottom") + def move_enumeration(menu) + menu.with_item( + component_klass: Primer::Alpha::ActionMenu::SubMenuItem, + label: I18n.t(:button_move), + select_variant: :none, + form_arguments: {}, + data: { sortable_lists__item_target: "moveMenu" } + ) do |submenu| + submenu.with_leading_visual_icon(icon: :"op-arrow-in") + + with_move_items(submenu) end end diff --git a/app/controllers/admin/settings/enumerations_controller_base.rb b/app/controllers/admin/settings/enumerations_controller_base.rb index 4b9e2853877d..c2bb059f8232 100644 --- a/app/controllers/admin/settings/enumerations_controller_base.rb +++ b/app/controllers/admin/settings/enumerations_controller_base.rb @@ -82,21 +82,17 @@ def destroy end def move - if @enumeration.update(move_params) - render_success_flash_message_via_turbo_stream( - message: I18n.t(:enumeration_caption_order_changed) - ) + moved = move_after_anchor + + if moved + render_move_success else render_error_flash_message_via_turbo_stream( - message: I18n.t(:enumeration_could_not_be_moved) + message: I18n.t(:error_invalid_list_move_anchor) ) end - replace_via_turbo_stream( - component: index_component_class.new(enumerations: enumeration_class.all) - ) - - respond_with_turbo_streams + respond_with_turbo_streams(status: moved ? :ok : :unprocessable_entity) end def reassign @@ -105,17 +101,43 @@ def reassign private - def move_params - move_to = params[:move_to] - position = Integer(params[:position], exception: false) + # Morph first: Turbo applies streams in order, so the flash only shows + # once the list has been reconciled. + def render_move_success + update_via_turbo_stream(component: index_component, method: :morph) + render_success_flash_message_via_turbo_stream( + message: I18n.t(:enumeration_caption_order_changed) + ) + end + + def index_component + index_component_class.new(enumerations: enumeration_class.all) + end - if move_to.in? %w(highest higher lower lowest) - { move_to: move_to } - elsif position - { position: position } - else - {} - end + def move_after_anchor + return false unless valid_drop_request? + + @enumeration.move_after_anchor(drop_params[:prev_id], scope: enumeration_class.all) + end + + def valid_drop_request? + drop_params[:list_type] == sortable_list_type && + unscoped_list_id? && + drop_params.key?(:prev_id) + end + + # Enumeration lists carry no list id. The raw param is checked because + # permit cannot tell an absent value from a filtered-out array or hash. + def unscoped_list_id? + params[:list_id].nil? || params[:list_id] == "" + end + + def drop_params + @drop_params ||= params.permit(:list_type, :list_id, :prev_id) + end + + def sortable_list_type + enumeration_class.model_name.param_key end def handle_reassignment_on_deletion diff --git a/app/models/concerns/lists/move_after_anchor.rb b/app/models/concerns/lists/move_after_anchor.rb index c4834d884fd2..8b5877b19bf7 100644 --- a/app/models/concerns/lists/move_after_anchor.rb +++ b/app/models/concerns/lists/move_after_anchor.rb @@ -33,18 +33,22 @@ module Lists # directly below another record of the same list, addressed by id. module MoveAfterAnchor # Moves the record below the record identified by `prev_id` within - # `scope` (a relation over the same acts_as_list list). A blank - # `prev_id` moves the record to the top. + # `scope` (a relation over the same acts_as_list list). `nil` or an + # empty string moves the record to the top; otherwise `prev_id` must + # be a positive Integer or its canonical decimal String. # - # Returns false without mutating when the anchor is unknown, outside - # the scope, or the record itself. + # Returns false without mutating for any other `prev_id`, or when the + # anchor is unknown, outside the scope, or the record itself. def move_after_anchor(prev_id, scope:) # rubocop:disable Naming/PredicateMethod -- verb command, not a query - if prev_id.blank? + if prev_id.nil? || prev_id == "" move_to_top return true end - anchor = scope.find_by(id: prev_id) + anchor_id = anchor_id_from(prev_id) + return false if anchor_id.nil? + + anchor = scope.find_by(id: anchor_id) return false if anchor.nil? || anchor.id == id # Removing the record first shifts the anchor up by one when the @@ -53,5 +57,14 @@ def move_after_anchor(prev_id, scope:) # rubocop:disable Naming/PredicateMethod insert_at(position > anchor.position ? anchor.position + 1 : anchor.position) true end + + private + + def anchor_id_from(prev_id) + case prev_id + when Integer then prev_id if prev_id.positive? + when /\A[1-9]\d*\z/ then prev_id.to_i + end + end end end diff --git a/app/models/enumeration.rb b/app/models/enumeration.rb index a7d652a7b88d..e8cfbd947bb9 100644 --- a/app/models/enumeration.rb +++ b/app/models/enumeration.rb @@ -29,6 +29,8 @@ #++ class Enumeration < ApplicationRecord + include Lists::MoveAfterAnchor + default_scope { order("#{Enumeration.table_name}.position ASC") } belongs_to :project, optional: true diff --git a/modules/costs/app/controllers/admin/settings/time_entry_activities_controller.rb b/modules/costs/app/controllers/admin/settings/time_entry_activities_controller.rb index 224248dbe6a6..d66bde5f567a 100644 --- a/modules/costs/app/controllers/admin/settings/time_entry_activities_controller.rb +++ b/modules/costs/app/controllers/admin/settings/time_entry_activities_controller.rb @@ -38,10 +38,6 @@ class TimeEntryActivitiesController < EnumerationsControllerBase def enumeration_class TimeEntryActivity end - - def enumeration_param_key - enumeration_class.model_name.param_key - end end end end diff --git a/modules/costs/spec/features/admin/settings/time_entry_activities_spec.rb b/modules/costs/spec/features/admin/settings/time_entry_activities_spec.rb new file mode 100644 index 000000000000..dd9059e74012 --- /dev/null +++ b/modules/costs/spec/features/admin/settings/time_entry_activities_spec.rb @@ -0,0 +1,65 @@ +# 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 "Time entry activities admin", :js do + include Flash::Expectations + include EnumerationAdminHelpers + + current_user { create(:admin) } + + let!(:alpha) { create(:time_entry_activity, name: "Alpha") } + let!(:beta) { create(:time_entry_activity, name: "Beta") } + let!(:gamma) { create(:time_entry_activity, name: "Gamma") } + + before do + gamma.move_to_top + beta.move_to_top + alpha.move_to_top + end + + def enumeration_list_selector = "#admin-enumerations-index-component" + def enumeration_actions_label = "Actions" + + it "reorders through the move menu" do + visit admin_settings_time_entry_activities_path + + expect_enumeration_order("Alpha", "Beta", "Gamma") + + move_enumeration(gamma, I18n.t(:label_sort_highest)) + + expect_enumeration_move_settled("Gamma", "Alpha", "Beta") + + refresh + + expect_enumeration_order("Gamma", "Alpha", "Beta") + end +end diff --git a/modules/costs/spec/requests/admin/settings/time_entry_activities_spec.rb b/modules/costs/spec/requests/admin/settings/time_entry_activities_spec.rb new file mode 100644 index 000000000000..b033558d9446 --- /dev/null +++ b/modules/costs/spec/requests/admin/settings/time_entry_activities_spec.rb @@ -0,0 +1,64 @@ +# 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 "Time entry activities", :skip_csrf, type: :rails_request do + shared_let(:admin) { create(:admin) } + + current_user { admin } + + describe "PUT /admin/settings/time_entry_activities/:id/move" do + let!(:first_record) { create(:time_entry_activity, name: "Alpha") } + let!(:second_record) { create(:time_entry_activity, name: "Beta") } + let!(:third_record) { create(:time_entry_activity, name: "Gamma") } + let!(:sibling_record) { create(:issue_priority) } + let(:list_type) { "time_entry_activity" } + let(:morph_target) { "admin-enumerations-index-component" } + + before do + third_record.move_to_top + second_record.move_to_top + first_record.move_to_top + end + + def move_path(record) + move_admin_settings_time_entry_activity_path(record) + end + + def ordered_names + TimeEntryActivity.reorder(:position).where(name: %w[Alpha Beta Gamma]).pluck(:name) + end + + it_behaves_like "an anchor-only enumeration move endpoint" + # IssuePriority and TimeEntryActivity are STI siblings on the enumerations table. + it_behaves_like "an enumeration move endpoint refusing sibling-class anchors" + end +end diff --git a/modules/documents/app/components/documents/admin/document_types/index_component.html.erb b/modules/documents/app/components/documents/admin/document_types/index_component.html.erb index 485fdadf611d..a75de8b87310 100644 --- a/modules/documents/app/components/documents/admin/document_types/index_component.html.erb +++ b/modules/documents/app/components/documents/admin/document_types/index_component.html.erb @@ -30,8 +30,8 @@ %> <%= - component_wrapper do - flex_layout(data: wrapper_data_attributes) do |flex| + component_wrapper(data: wrapper_data_attributes) do + flex_layout do |flex| flex.with_row do render(Primer::OpenProject::SubHeader.new(test_selector: "admin-document-types-subheader")) do |subheader| subheader.with_action_button( @@ -48,7 +48,7 @@ end flex.with_row do - render(border_box_container(data: drop_target_config)) do |component| + render(border_box_container(data: list_data)) do |component| component.with_header(font_weight: :bold) do grid_layout("op-documents-types-list--header", tag: :div, align_items: :center) do |grid| grid.with_area(:name, tag: :div, mr: 3) do @@ -67,8 +67,8 @@ end else document_types.each do |document_type| - component.with_row(test_selector: "document-type-row-#{document_type.id}", data: draggable_item_config(document_type)) do - render(item_component_class.new(enumeration: document_type, max_position: max_position)) + component.with_row(test_selector: "document-type-row-#{document_type.id}", data: item_data(document_type)) do + render(item_component_class.new(enumeration: document_type)) end end end diff --git a/modules/documents/app/components/documents/admin/document_types/item_component.html.erb b/modules/documents/app/components/documents/admin/document_types/item_component.html.erb index 83528c10016e..346b7a40202e 100644 --- a/modules/documents/app/components/documents/admin/document_types/item_component.html.erb +++ b/modules/documents/app/components/documents/admin/document_types/item_component.html.erb @@ -32,7 +32,7 @@ <%= component_wrapper do grid_layout("op-documents-types-list--item", tag: :div, align_items: :center) do |grid| grid.with_area(:"drag-handle", tag: :div, classes: "hide-when-print") do - render(Primer::OpenProject::DragHandle.new) + render(Primer::OpenProject::DragHandle.new(data: { sortable_lists__item_target: "handle" })) end grid.with_area(:name, tag: :div, classes: "ellipsis") do diff --git a/modules/documents/app/models/document_type.rb b/modules/documents/app/models/document_type.rb index ac4859210373..0bb6111e2260 100644 --- a/modules/documents/app/models/document_type.rb +++ b/modules/documents/app/models/document_type.rb @@ -30,6 +30,7 @@ class DocumentType < ApplicationRecord include ::Documents::EnumerationModel + include Lists::MoveAfterAnchor default_scope { order(:position) } acts_as_list diff --git a/modules/documents/spec/components/documents/admin/document_types/index_component_spec.rb b/modules/documents/spec/components/documents/admin/document_types/index_component_spec.rb new file mode 100644 index 000000000000..ca25aedaa117 --- /dev/null +++ b/modules/documents/spec/components/documents/admin/document_types/index_component_spec.rb @@ -0,0 +1,59 @@ +# 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 Documents::Admin::DocumentTypes::IndexComponent, type: :component do + let!(:first_type) { create(:document_type, name: "Note") } + let!(:second_type) { create(:document_type, name: "Report") } + + subject(:rendered_component) do + with_request_url("/admin/settings/document_types") do + render_inline(described_class.new(enumerations: DocumentType.reorder(:position))) + end + end + + it_behaves_like "a sortable-lists root", + wrapper_id: "documents-admin-document-types-index-component", + move_url_template: "/admin/settings/document_types/{id}/move" + it_behaves_like "a sortable-lists list", + list_type: "document_type", + name: DocumentType.model_name.human(count: :other) + it_behaves_like "a Border Box sortable list", row_count: 2 + it_behaves_like "sortable-lists items", list_type: "document_type" do + let(:sortable_records) { [first_type, second_type] } + end + it_behaves_like "no legacy drag-and-drop wiring" + + it "keeps the two-column grid" do + expect(rendered_component).to have_css(".op-documents-types-list--header", visible: :all) + expect(rendered_component).to have_css(".op-documents-types-list--item", count: 2, visible: :all) + end +end diff --git a/modules/documents/spec/features/documents/admin/settings/document_types_spec.rb b/modules/documents/spec/features/documents/admin/settings/document_types_spec.rb index 0e1e6608974a..30d76cec819f 100644 --- a/modules/documents/spec/features/documents/admin/settings/document_types_spec.rb +++ b/modules/documents/spec/features/documents/admin/settings/document_types_spec.rb @@ -32,9 +32,13 @@ RSpec.describe "Document types admin", :js do include Flash::Expectations + include EnumerationAdminHelpers current_user { create(:admin) } + def enumeration_list_selector = "#documents-admin-document-types-index-component" + def enumeration_actions_label = I18n.t("documents.document_type_actions") + def within_enumeration_item(type, &) page.within("#documents-admin-document-types-item-component-#{type.id}", &) end @@ -187,23 +191,44 @@ def within_enumeration_item(type, &) end end + context "with three document types" do + let!(:alpha) { create(:document_type, name: "Alpha") } + let!(:beta) { create(:document_type, name: "Beta") } + let!(:gamma) { create(:document_type, name: "Gamma") } + + before do + gamma.move_to_top + beta.move_to_top + alpha.move_to_top + end + + it "reorders through the move menu" do + visit admin_settings_document_types_path + + expect_enumeration_order("Alpha", "Beta", "Gamma") + + move_enumeration(gamma, I18n.t(:label_sort_highest)) + + expect_enumeration_move_settled("Gamma", "Alpha", "Beta") + + refresh + + expect_enumeration_order("Gamma", "Alpha", "Beta") + end + end + context "with a single document type" do let!(:only_type) { create(:document_type, name: "Only type") } it "shows a single separator (no duplicate) in the more menu" do visit admin_settings_document_types_path - within_enumeration_item(only_type) do - click_on accessible_name: "Document type actions" + within_enumeration_menu(only_type) do |menu| + expect(menu).to have_selector(:menuitem, "Edit") + expect(menu).to have_selector(:menuitem, "Delete") + expect(menu).to have_no_selector(:menuitem, I18n.t(:button_move)) + expect(menu).to have_css("li.ActionList-sectionDivider", count: 1) end - - expect(page).to have_link("Edit") - expect(page).to have_link("Delete") - expect(page).to have_no_button(I18n.t(:label_sort_highest)) - expect(page).to have_no_button(I18n.t(:label_sort_higher)) - expect(page).to have_no_button(I18n.t(:label_sort_lower)) - expect(page).to have_no_button(I18n.t(:label_sort_lowest)) - expect(page).to have_css("li.ActionList-sectionDivider", count: 1) end end diff --git a/modules/documents/spec/requests/documents/admin/settings/document_types_spec.rb b/modules/documents/spec/requests/documents/admin/settings/document_types_spec.rb new file mode 100644 index 000000000000..a8600c92dac1 --- /dev/null +++ b/modules/documents/spec/requests/documents/admin/settings/document_types_spec.rb @@ -0,0 +1,61 @@ +# 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 "Document types", :skip_csrf, type: :rails_request do + shared_let(:admin) { create(:admin) } + + current_user { admin } + + describe "PUT /admin/settings/document_types/:id/move" do + let!(:first_record) { create(:document_type, name: "Alpha") } + let!(:second_record) { create(:document_type, name: "Beta") } + let!(:third_record) { create(:document_type, name: "Gamma") } + let(:list_type) { "document_type" } + let(:morph_target) { "documents-admin-document-types-index-component" } + + before do + third_record.move_to_top + second_record.move_to_top + first_record.move_to_top + end + + def move_path(record) + move_admin_settings_document_type_path(record) + end + + def ordered_names + DocumentType.reorder(:position).where(name: %w[Alpha Beta Gamma]).pluck(:name) + end + + it_behaves_like "an anchor-only enumeration move endpoint" + end +end diff --git a/spec/components/admin/enumerations/index_component_spec.rb b/spec/components/admin/enumerations/index_component_spec.rb index 21a9633504e8..c90155897c13 100644 --- a/spec/components/admin/enumerations/index_component_spec.rb +++ b/spec/components/admin/enumerations/index_component_spec.rb @@ -52,13 +52,17 @@ expect(rendered_component).to have_css(".Box-row", text: "Trivial") end - it_behaves_like "a reorderable Border Box List", drag_type: "enumeration" do - let(:draggable_records) { [priority_a, priority_b] } - - def drop_url_for(record) - "/work_package_priorities/#{record.id}/move" - end + it_behaves_like "a sortable-lists root", + wrapper_id: "admin-enumerations-index-component", + move_url_template: "/admin/settings/work_package_priorities/{id}/move" + it_behaves_like "a sortable-lists list", + list_type: "issue_priority", + name: IssuePriority.model_name.human(count: :other) + it_behaves_like "a Border Box sortable list", row_count: 2 + it_behaves_like "sortable-lists items", list_type: "issue_priority" do + let(:sortable_records) { [priority_a, priority_b] } end + it_behaves_like "no legacy drag-and-drop wiring" end context "without enumerations" do diff --git a/spec/components/admin/enumerations/item_component_spec.rb b/spec/components/admin/enumerations/item_component_spec.rb new file mode 100644 index 000000000000..e6ec7c308f64 --- /dev/null +++ b/spec/components/admin/enumerations/item_component_spec.rb @@ -0,0 +1,70 @@ +# 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 "rails_helper" + +RSpec.describe Admin::Enumerations::ItemComponent, type: :component do + let!(:priority) { create(:priority, name: "Urgent") } + + subject(:rendered_component) do + with_request_url("/admin/settings/work_package_priorities") do + render_inline(described_class.new(enumeration: priority)) + end + end + + it "targets the drag handle for the item controller" do + expect(rendered_component) + .to have_css(".DragHandle[data-sortable-lists--item-target~='handle']", visible: :all) + end + + # The four directions themselves are covered by the SortableLists::MoveMenu spec. + it "renders the Move submenu carrying the moveMenu target with the shared move items" do + expect(rendered_component).to have_css("li[data-sortable-lists--item-target~='moveMenu']", visible: :all) do |item| + expect(item).to have_css("[role='menuitem']", text: I18n.t(:button_move), visible: :all) do |trigger| + expect(rendered_component) + .to have_css("##{trigger['aria-controls']} li[data-sortable-lists--item-target~='moveItem']", count: 4, visible: :all) + end + end + end + + it "posts no move_to form", :aggregate_failures do + expect(rendered_component).to have_no_field("move_to", type: :hidden) + expect(rendered_component).to have_no_field("position", type: :hidden) + end + + it "renders exactly one divider, so hiding the Move submenu leaves a single separator" do + expect(rendered_component).to have_css("li.ActionList-sectionDivider", count: 1, visible: :all) + end + + it "keeps Edit and Delete in the menu", :aggregate_failures do + expect(rendered_component).to have_link(I18n.t(:button_edit), visible: :all) + expect(rendered_component).to have_button(I18n.t(:button_delete), visible: :all) + end +end diff --git a/spec/features/admin/settings/work_package_priorities_spec.rb b/spec/features/admin/settings/work_package_priorities_spec.rb index 9db4cff1bdfe..1ddf58888687 100644 --- a/spec/features/admin/settings/work_package_priorities_spec.rb +++ b/spec/features/admin/settings/work_package_priorities_spec.rb @@ -32,14 +32,30 @@ RSpec.describe "Work package priorities", :js do include Flash::Expectations + include EnumerationAdminHelpers current_user { create(:admin) } let!(:default_priority) { create(:issue_priority, is_default: true, name: "Normal") } + def enumeration_list_selector = "#admin-enumerations-index-component" + def enumeration_actions_label = "Actions" + def within_enumeration_item(priority, &) page.within("#admin-enumerations-item-component-#{priority.id}", &) end + def drag_priority(priority, after:) + handle = enumeration_drag_handle(priority) + target = enumeration_row(after) + offset_y = (target.native.rect.height / 2) - [6, target.native.rect.height / 4].min + + perform_native_drag(source: handle, target:, offset_y: offset_y.round) + + # Assert Pragmatic DnD tore down its own honey-pot overlay, so a regression + # leaving it stuck is caught here rather than as an unrelated click failure. + expect(page).to have_no_css("[data-pdnd-honey-pot]", wait: 2, visible: :all) + end + it "can be managed (created, updated, deleted)" do visit admin_settings_work_package_priorities_path @@ -105,4 +121,89 @@ def within_enumeration_item(priority, &) expect(page).to have_no_content("Default") end end + + context "with three priorities" do + let!(:alpha) { create(:issue_priority, name: "Alpha") } + let!(:beta) { create(:issue_priority, name: "Beta") } + let!(:gamma) { create(:issue_priority, name: "Gamma") } + + before do + gamma.move_to_top + beta.move_to_top + alpha.move_to_top + end + + # The second drag runs without a reload on purpose: the sortable root + # re-registers Pragmatic's drop targets after a morph, and only a drag that + # follows a completed morph exercises that repair. + it "reorders by dragging twice across a morph", :selenium do + visit admin_settings_work_package_priorities_path + + expect_enumeration_order("Alpha", "Beta", "Gamma", "Normal") + + drag_priority(alpha, after: beta) + + expect_enumeration_move_settled("Beta", "Alpha", "Gamma", "Normal") + + drag_priority(beta, after: gamma) + + expect_enumeration_move_settled("Alpha", "Gamma", "Beta", "Normal") + + refresh + + expect_enumeration_order("Alpha", "Gamma", "Beta", "Normal") + end + + # The moved item's menu is reopened after the morph: only a refreshed menu + # hides the directions that stopped being available. + it "reorders through the move menu twice across a morph" do + visit admin_settings_work_package_priorities_path + + expect_enumeration_order("Alpha", "Beta", "Gamma", "Normal") + + move_enumeration(gamma, I18n.t(:label_sort_highest)) + + expect_enumeration_move_settled("Gamma", "Alpha", "Beta", "Normal") + + within_enumeration_move_submenu(gamma) do |submenu| + expect(submenu).to have_no_selector(:menuitem, I18n.t(:label_sort_highest)) + expect(submenu).to have_no_selector(:menuitem, I18n.t(:label_sort_higher)) + expect(submenu).to have_selector(:menuitem, I18n.t(:label_sort_lower)) + submenu.find(:menuitem, I18n.t(:label_sort_lowest)).click + end + + expect_enumeration_move_settled("Alpha", "Beta", "Normal", "Gamma") + + refresh + + expect_enumeration_order("Alpha", "Beta", "Normal", "Gamma") + end + + it "rolls back and reports a move whose anchor no longer exists" do + visit admin_settings_work_package_priorities_path + + expect_enumeration_order("Alpha", "Beta", "Gamma", "Normal") + + beta.destroy + + move_enumeration(alpha, I18n.t(:label_sort_lower)) + + expect_flash(type: :error, message: I18n.t(:error_invalid_list_move_anchor)) + expect(page).to have_no_css("[data-sortable-lists-busy]") + expect_enumeration_order("Alpha", "Beta", "Gamma", "Normal") + end + end + + context "with a single priority" do + it "hides the Move submenu and renders one separator" do + visit admin_settings_work_package_priorities_path + + within_enumeration_menu(default_priority) do |menu| + expect(menu).to have_selector(:menuitem, I18n.t(:button_edit)) + expect(menu).to have_selector(:menuitem, I18n.t(:button_delete)) + expect(menu).to have_no_selector(:menuitem, I18n.t(:button_move)) + expect(menu).to have_css("li.ActionList-sectionDivider", count: 1) + end + end + end end diff --git a/spec/models/concerns/lists/move_after_anchor_spec.rb b/spec/models/concerns/lists/move_after_anchor_spec.rb index ce6a5530bf2f..dc3915627e85 100644 --- a/spec/models/concerns/lists/move_after_anchor_spec.rb +++ b/spec/models/concerns/lists/move_after_anchor_spec.rb @@ -39,16 +39,26 @@ def order = scope.reload.order(:position).pluck(:name) - it "moves to the top for a blank anchor" do + it "moves to the top for an empty anchor" do expect(section_c.move_after_anchor("", scope:)).to be(true) expect(order).to eq(%w[C A B]) end + it "moves to the top for a nil anchor" do + expect(section_c.move_after_anchor(nil, scope:)).to be(true) + expect(order).to eq(%w[C A B]) + end + it "moves downward directly below the anchor" do expect(section_a.move_after_anchor(section_b.id.to_s, scope:)).to be(true) expect(order).to eq(%w[B A C]) end + it "accepts an Integer anchor" do + expect(section_a.move_after_anchor(section_b.id, scope:)).to be(true) + expect(order).to eq(%w[B A C]) + end + it "moves upward directly below the anchor" do expect(section_c.move_after_anchor(section_a.id.to_s, scope:)).to be(true) expect(order).to eq(%w[A C B]) @@ -69,4 +79,28 @@ def order = scope.reload.order(:position).pluck(:name) expect(section_a.move_after_anchor(foreign.id.to_s, scope:)).to be(false) expect(order).to eq(%w[A B C]) end + + describe "malformed anchors" do + { + "false" => -> { false }, + "true" => -> { true }, + "a Float" => -> { section_b.id + 0.5 }, + "zero" => -> { 0 }, + "a zero string" => -> { "0" }, + "a negative Integer" => -> { -1 }, + "a negative string" => -> { "-1" }, + "a signed id" => -> { "+#{section_b.id}" }, + "a zero-padded id" => -> { "0#{section_b.id}" }, + "a decimal id string" => -> { "#{section_b.id}.0" }, + "a suffixed id" => -> { "#{section_b.id}junk" }, + "a padded id" => -> { " #{section_b.id}" }, + "an Array" => -> { [section_b.id] }, + "a Hash" => -> { { id: section_b.id } } + }.each do |description, anchor_builder| + it "rejects #{description} without mutating" do + expect(section_c.move_after_anchor(instance_exec(&anchor_builder), scope:)).to be(false) + expect(order).to eq(%w[A B C]) + end + end + end end diff --git a/spec/models/enumeration/enumeration_anchor_move_spec.rb b/spec/models/enumeration/enumeration_anchor_move_spec.rb new file mode 100644 index 000000000000..e6d104aefe88 --- /dev/null +++ b/spec/models/enumeration/enumeration_anchor_move_spec.rb @@ -0,0 +1,59 @@ +# 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 Enumeration, "anchor moves" do + it "moves a priority below its anchor" do + first = create(:issue_priority) + second = create(:issue_priority) + + expect(first.move_after_anchor(second.id, scope: IssuePriority.all)).to be(true) + expect(IssuePriority.reorder(:position).ids).to eq([second.id, first.id]) + end + + it "moves a document type to the top for a blank anchor" do + first = create(:document_type) + second = create(:document_type) + + expect(second.move_after_anchor("", scope: DocumentType.all)).to be(true) + expect(DocumentType.reorder(:position).ids).to eq([second.id, first.id]) + end + + it "refuses an anchor from another enumeration class" do + priority = create(:issue_priority) + create(:issue_priority) + activity = create(:time_entry_activity) + original = IssuePriority.reorder(:position).ids + + expect(priority.move_after_anchor(activity.id, scope: IssuePriority.all)).to be(false) + expect(IssuePriority.reorder(:position).ids).to eq(original) + end +end diff --git a/spec/requests/admin/settings/work_package_priorities_spec.rb b/spec/requests/admin/settings/work_package_priorities_spec.rb index 7b7ea93ae091..de3a7451e9a6 100644 --- a/spec/requests/admin/settings/work_package_priorities_spec.rb +++ b/spec/requests/admin/settings/work_package_priorities_spec.rb @@ -91,11 +91,29 @@ end describe "PUT /admin/settings/work_package_priorities/:id/move" do - it "moves the category to the bottom" do - put move_admin_settings_work_package_priority_path(priority), params: { move_to: "lowest" }, as: :turbo_stream + let!(:first_record) { create(:issue_priority, name: "Alpha") } + let!(:second_record) { create(:issue_priority, name: "Beta") } + let!(:third_record) { create(:issue_priority, name: "Gamma") } + let!(:sibling_record) { create(:time_entry_activity) } + let(:list_type) { "issue_priority" } + let(:morph_target) { "admin-enumerations-index-component" } - expect(response).to have_http_status(:ok) - expect(priority.reload.position).to be > other_priority.reload.position + def move_path(record) + move_admin_settings_work_package_priority_path(record) + end + + def ordered_names + IssuePriority.reorder(:position).where(name: %w[Alpha Beta Gamma]).pluck(:name) end + + before do + third_record.move_to_top + second_record.move_to_top + first_record.move_to_top + end + + it_behaves_like "an anchor-only enumeration move endpoint" + # IssuePriority and TimeEntryActivity are STI siblings on the enumerations table. + it_behaves_like "an enumeration move endpoint refusing sibling-class anchors" end end diff --git a/spec/support/shared/components/sortable_lists.rb b/spec/support/shared/components/sortable_lists.rb new file mode 100644 index 000000000000..d9934a944401 --- /dev/null +++ b/spec/support/shared/components/sortable_lists.rb @@ -0,0 +1,85 @@ +# 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. +#++ + +RSpec.shared_examples "a sortable-lists root" do |wrapper_id:, move_url_template:| + it "wires ##{wrapper_id} as the sortable-lists root" do + expect(rendered_component).to have_css("##{wrapper_id}") do |root| + expect(root["data-controller"]).to eq("sortable-lists") + expect(root["data-sortable-lists-move-url-template-value"]).to eq(move_url_template) + expect(root["data-sortable-lists-sortable-lists--list-outlet"]) + .to eq("##{wrapper_id} [data-controller~='sortable-lists--list']") + expect(root["data-sortable-lists-sortable-lists--item-outlet"]) + .to eq("##{wrapper_id} [data-controller~='sortable-lists--item']") + end + end +end + +RSpec.shared_examples "a sortable-lists list" do |list_type:, name:| + it "wires a single list of type #{list_type}" do + expect(rendered_component).to have_css("[data-controller~='sortable-lists--list']", count: 1) do |list| + expect(list["data-sortable-lists--list-type-value"]).to eq(list_type) + expect(list["data-sortable-lists--list-accepted-type-value"]).to eq(list_type) + expect(list["data-sortable-lists--list-name-value"]).to eq(name) + expect(list["data-sortable-lists--list-rows-container-element"]).to be_nil + end + end +end + +# Primer's BorderBox renders its rows into a direct `ul` child, which is the +# list controller's default rows container. +RSpec.shared_examples "a Border Box sortable list" do |row_count:| + it "keeps #{row_count} rows in the list controller's default rows container" do + expect(rendered_component) + .to have_css("[data-controller~='sortable-lists--list'] > ul.Box-list > li.Box-row", count: row_count) + end +end + +# Consumers define `sortable_records` with a `let` in the inclusion block. +RSpec.shared_examples "sortable-lists items" do |list_type:| + it "wires every row as a sortable item of type #{list_type}" do + sortable_records.each do |record| + expect(rendered_component) + .to have_css("li.Box-row[data-sortable-lists--item-id-value='#{record.id}']") do |row| + expect(row["data-controller"]).to eq("sortable-lists--item") + expect(row["data-sortable-lists--item-type-value"]).to eq(list_type) + expect(row["data-sortable-lists--item-label-value"]).to eq(record.name) + expect(row).to have_css(".DragHandle[data-sortable-lists--item-target~='handle']", visible: :all) + end + end + end +end + +RSpec.shared_examples "no legacy drag-and-drop wiring" do + it "no longer wires the generic drag-and-drop controller", :aggregate_failures do + expect(rendered_component).to have_no_css("[data-controller~='generic-drag-and-drop']") + expect(rendered_component).to have_no_css("[data-generic-drag-and-drop-target]") + expect(rendered_component).to have_no_css("[data-drop-url]") + end +end diff --git a/spec/support/shared/enumeration_admin_helpers.rb b/spec/support/shared/enumeration_admin_helpers.rb new file mode 100644 index 000000000000..73904fd1caa0 --- /dev/null +++ b/spec/support/shared/enumeration_admin_helpers.rb @@ -0,0 +1,91 @@ +# 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. +#++ + +# Menu and ordering helpers for the enumeration admin lists (priorities, +# time entry activities, document types). Included per feature spec. +module EnumerationAdminHelpers + def within_enumeration_list(&) + page.within(enumeration_list_selector, &) + end + + def expect_enumeration_order(*names) + within_enumeration_list do + expect(page).to have_list_item(count: names.size) + names.each_with_index do |name, index| + expect(page).to have_list_item(name, position: index + 1) + end + end + end + + def expect_enumeration_move_settled(*names) + expect_and_dismiss_flash(message: I18n.t(:enumeration_caption_order_changed)) + expect_enumeration_order(*names) + expect(page).to have_no_css("[data-sortable-lists-busy]") + end + + def within_enumeration_menu(record, &) + within_enumeration_list do + within(:list_item, record.name) do + button = find(:button, accessible_name: enumeration_actions_label) + within(open_controlled_menu(button), &) + end + end + end + + def within_enumeration_move_submenu(record, &) + within_enumeration_menu(record) do |menu| + within(open_controlled_menu(menu.find(:menuitem, I18n.t(:button_move))), &) + end + end + + def move_enumeration(record, direction_label) + within_enumeration_move_submenu(record) do |submenu| + submenu.find(:menuitem, direction_label).click + end + end + + def enumeration_drag_handle(record) + within_enumeration_list do + find(:list_item, record.name) + .find(:button, accessible_name: I18n.t("drag_handle.button_drag")) + end + end + + def enumeration_row(record) + within_enumeration_list { find(:list_item, record.name) } + end + + private + + def open_controlled_menu(button) + button.click + page.find(:menu, id: button["aria-controls"]) + end +end diff --git a/spec/support/shared/enumeration_anchor_move.rb b/spec/support/shared/enumeration_anchor_move.rb new file mode 100644 index 000000000000..ba1e66979912 --- /dev/null +++ b/spec/support/shared/enumeration_anchor_move.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. +#++ + +RSpec.shared_examples "an anchor-only enumeration move endpoint" do + let(:drop_at_top_params) { { list_type:, list_id: "", prev_id: "" } } + + def drop_below_params(anchor) + drop_at_top_params.merge(prev_id: anchor.id) + end + + def move(record, params) + put move_path(record), params:, as: :turbo_stream + end + + it "moves the record to the top for a blank anchor" do + move(third_record, drop_at_top_params) + + expect(response).to have_http_status(:ok) + expect(ordered_names).to eq([third_record.name, first_record.name, second_record.name]) + end + + it "moves the record down, below a later anchor" do + move(first_record, drop_below_params(third_record)) + + expect(response).to have_http_status(:ok) + expect(ordered_names).to eq([second_record.name, third_record.name, first_record.name]) + end + + it "moves the record up, below an earlier anchor" do + move(third_record, drop_below_params(first_record)) + + expect(response).to have_http_status(:ok) + expect(ordered_names).to eq([first_record.name, third_record.name, second_record.name]) + end + + it "morphs the existing list boundary and reports success", :aggregate_failures do + move(third_record, drop_at_top_params) + + expect(response.media_type).to eq("text/vnd.turbo-stream.html") + expect(response.body) + .to have_css("turbo-stream[action='update'][method='morph'][target='#{morph_target}']", visible: :all) + expect(response.body).to include(I18n.t(:enumeration_caption_order_changed)) + expect(response.body.index(%(target="#{morph_target}"))) + .to be < response.body.index(I18n.t(:enumeration_caption_order_changed)) + end + + { + "a missing anchor" => -> { { list_type:, list_id: "" } }, + "an absent anchor ID" => lambda { + { list_type:, list_id: "", prev_id: (first_record.class.maximum(:id) + 1_000).to_s } + }, + "a self anchor" => -> { { list_type:, list_id: "", prev_id: first_record.id.to_s } }, + "a wrong list type with a valid anchor" => lambda { + { list_type: "section", list_id: "", prev_id: second_record.id.to_s } + }, + "a missing list type" => -> { { list_id: "", prev_id: "" } }, + "a nonblank list ID" => -> { { list_type:, list_id: second_record.id.to_s, prev_id: "" } }, + "an array anchor" => -> { { list_type:, list_id: "", prev_id: [""] } }, + "a hash anchor" => -> { { list_type:, list_id: "", prev_id: { id: "1" } } }, + "an array list ID" => -> { { list_type:, list_id: [""], prev_id: "" } }, + "a hash list ID" => -> { { list_type:, list_id: { id: "" }, prev_id: "" } }, + "a suffixed anchor ID" => -> { { list_type:, list_id: "", prev_id: "#{second_record.id}junk" } }, + "a legacy move_to request" => -> { { move_to: "lowest" } }, + "a retired position request" => -> { { position: 1 } } + }.each do |description, params_builder| + it "refuses #{description} without changing the order", :aggregate_failures do + original_order = ordered_names + + move(first_record, instance_exec(¶ms_builder)) + + expect(response).to have_http_status(:unprocessable_entity) + expect(ordered_names).to eq(original_order) + expect(response.body).to include(I18n.t(:error_invalid_list_move_anchor)) + end + end + + { + "an empty array list ID" => -> { { list_type:, list_id: [], prev_id: "" } }, + "an empty hash list ID" => -> { { list_type:, list_id: {}, prev_id: "" } }, + "a false list ID with a valid anchor" => -> { { list_type:, list_id: false, prev_id: second_record.id } }, + "a false anchor" => -> { { list_type:, list_id: "", prev_id: false } }, + "a fractional anchor ID" => -> { { list_type:, list_id: "", prev_id: second_record.id + 0.5 } } + }.each do |description, params_builder| + it "refuses #{description} in a JSON body without changing the order", :aggregate_failures do + original_order = ordered_names + + put move_path(first_record), + params: instance_exec(¶ms_builder).to_json, + headers: { "CONTENT_TYPE" => "application/json", "ACCEPT" => "text/vnd.turbo-stream.html" } + + expect(response).to have_http_status(:unprocessable_entity) + expect(ordered_names).to eq(original_order) + expect(response.body).to include(I18n.t(:error_invalid_list_move_anchor)) + end + end +end + +RSpec.shared_examples "an enumeration move endpoint refusing sibling-class anchors" do + it "refuses an anchor of a sibling enumeration class without changing the order", :aggregate_failures do + original_order = ordered_names + + put move_path(first_record), params: { list_type:, list_id: "", prev_id: sibling_record.id.to_s }, as: :turbo_stream + + expect(response).to have_http_status(:unprocessable_entity) + expect(ordered_names).to eq(original_order) + end +end