diff --git a/modules/backlogs/app/components/backlogs/backlog_component.html.erb b/modules/backlogs/app/components/backlogs/backlog_component.html.erb index 868b91db6b8c..dd18db687a75 100644 --- a/modules/backlogs/app/components/backlogs/backlog_component.html.erb +++ b/modules/backlogs/app/components/backlogs/backlog_component.html.erb @@ -56,7 +56,7 @@ See COPYRIGHT and LICENSE files for more details. ), tabindex: 0 ) do %> - <%= render(Backlogs::StoryComponent.new(story:, sprint:, max_position:)) %> + <%= render(Backlogs::StoryComponent.new(story:, project:, sprint:, max_position:)) %> <% end %> <% end %> <% end %> diff --git a/modules/backlogs/app/components/backlogs/sprint_component.html.erb b/modules/backlogs/app/components/backlogs/sprint_component.html.erb index fc6fe3ac72bc..5c867c09cfdc 100644 --- a/modules/backlogs/app/components/backlogs/sprint_component.html.erb +++ b/modules/backlogs/app/components/backlogs/sprint_component.html.erb @@ -30,7 +30,7 @@ See COPYRIGHT and LICENSE files for more details. <%= component_wrapper(tag: :section) do %> <%= render(Primer::Beta::BorderBox.new(**@system_arguments)) do |border_box| %> <% border_box.with_header(id: dom_target(sprint, :header)) do %> - <%= render(Backlogs::SprintHeaderComponent.new(sprint:, folded: folded?)) %> + <%= render(Backlogs::SprintHeaderComponent.new(sprint:, project:, folded: folded?)) %> <% end %> <% if stories.empty? %> <% border_box.with_row(data: { empty_list_item: true }) do %> @@ -45,18 +45,11 @@ See COPYRIGHT and LICENSE files for more details. <% stories.each do |story| %> <% border_box.with_row( id: dom_id(story), - classes: "Box-row--hover-blue Box-row--focus-gray Box-row--clickable Box-row--draggable", - data: draggable_item_config(story).merge( - story: true, - controller: "backlogs--story", - backlogs__story_id_value: story.id, - backlogs__story_split_url_value: details_backlogs_project_backlogs_path(project, story), - backlogs__story_full_url_value: work_package_path(story), - backlogs__story_selected_class: "Box-row--blue" - ), + classes: story_classes_attribute, + data: story_data_attribute(story), tabindex: 0 ) do %> - <%= render(Backlogs::StoryComponent.new(story:, sprint:, max_position:)) %> + <%= render(Backlogs::StoryComponent.new(story:, sprint:, project:, max_position:)) %> <% end %> <% end %> <% end %> diff --git a/modules/backlogs/app/components/backlogs/sprint_component.rb b/modules/backlogs/app/components/backlogs/sprint_component.rb index 19758de4839e..e7768517bf25 100644 --- a/modules/backlogs/app/components/backlogs/sprint_component.rb +++ b/modules/backlogs/app/components/backlogs/sprint_component.rb @@ -34,14 +34,13 @@ class SprintComponent < ApplicationComponent include OpTurbo::Streamable include RbCommonHelper - attr_reader :sprint, :current_user + attr_reader :sprint, :current_user, :project - delegate :project, to: :sprint - - def initialize(sprint:, current_user: User.current, **system_arguments) + def initialize(sprint:, project:, current_user: User.current, **system_arguments) super() @sprint = sprint + @project = project @current_user = current_user @system_arguments = system_arguments @@ -50,12 +49,13 @@ def initialize(sprint:, current_user: User.current, **system_arguments) @system_arguments[:padding] = :condensed @system_arguments[:data] = merge_data( @system_arguments, - { data: drop_target_config } + { data: drop_target_config }, + { data: { test_selector: "sprint-#{sprint.id}" } } ) end def stories - sprint.work_packages + sprint.work_packages.where(project:).order(:position) end def wrapper_uniq_by @@ -81,12 +81,44 @@ def drop_target_config } end + def story_classes_attribute + classes = "Box-row--hover-blue Box-row--focus-gray Box-row--clickable" + + if work_package_draggable? + classes += " Box-row--draggable" + end + + classes + end + + def story_data_attribute(story) + draggable_item_config(story).merge( + story: true, + controller: "backlogs--story", + backlogs__story_id_value: story.id, + backlogs__story_split_url_value: details_backlogs_project_backlogs_path(project, story), + backlogs__story_full_url_value: work_package_path(story), + backlogs__story_selected_class: "Box-row--blue", + test_selector: card_test_selector(story) + ) + end + def draggable_item_config(story) + return {} unless work_package_draggable? + { draggable_id: story.id, draggable_type: "story", drop_url: move_project_sprint_story_path(project, sprint, story) } end + + def card_test_selector(story) + "work-package-#{story.id}" + end + + def work_package_draggable? + current_user.allowed_in_project?(:manage_sprint_items, project) + end end end diff --git a/modules/backlogs/app/components/backlogs/sprint_header_component.rb b/modules/backlogs/app/components/backlogs/sprint_header_component.rb index 968276e1d509..ca5a8c69c323 100644 --- a/modules/backlogs/app/components/backlogs/sprint_header_component.rb +++ b/modules/backlogs/app/components/backlogs/sprint_header_component.rb @@ -36,19 +36,20 @@ class SprintHeaderComponent < ApplicationComponent include Redmine::I18n include RbCommonHelper - attr_reader :sprint, :collapsed, :current_user + attr_reader :sprint, :project, :collapsed, :current_user - delegate :project, to: :sprint delegate :name, to: :sprint, prefix: :sprint def initialize( sprint:, + project:, folded: false, current_user: User.current ) super() @sprint = sprint + @project = project @collapsed = folded @current_user = current_user end diff --git a/modules/backlogs/app/components/backlogs/story_component.html.erb b/modules/backlogs/app/components/backlogs/story_component.html.erb index 05381aa1c48f..37dd1cc7bc00 100644 --- a/modules/backlogs/app/components/backlogs/story_component.html.erb +++ b/modules/backlogs/app/components/backlogs/story_component.html.erb @@ -29,14 +29,14 @@ See COPYRIGHT and LICENSE files for more details. <%= grid_layout("op-backlogs-story", tag: :article) do |grid| %> <% grid.with_area(:drag_handle, classes: "hide-when-print") do %> - <%= - render( - Primer::OpenProject::DragHandle.new( - classes: "op-backlogs-story--drag_handle_button", - label: t(".label_drag_story", name: story.subject) - ) - ) - %> + <%= if draggable? + render( + Primer::OpenProject::DragHandle.new( + classes: "op-backlogs-story--drag_handle_button", + label: t(".label_drag_story", name: story.subject) + ) + ) + end %> <% end %> <% grid.with_area(:info_line) do %> @@ -51,7 +51,7 @@ See COPYRIGHT and LICENSE files for more details. <% end %> <% grid.with_area(:menu) do %> - <%= render(Backlogs::StoryMenuComponent.new(story:, sprint:, max_position:)) %> + <%= render(Backlogs::StoryMenuComponent.new(story:, sprint:, project:, max_position:)) %> <% end %> <% grid.with_area(:subject) do %> diff --git a/modules/backlogs/app/components/backlogs/story_component.rb b/modules/backlogs/app/components/backlogs/story_component.rb index 39c37f48a7f9..50f2841b6d12 100644 --- a/modules/backlogs/app/components/backlogs/story_component.rb +++ b/modules/backlogs/app/components/backlogs/story_component.rb @@ -32,13 +32,14 @@ module Backlogs class StoryComponent < ApplicationComponent include OpPrimer::ComponentHelpers - attr_reader :story, :sprint, :max_position, :current_user + attr_reader :story, :sprint, :project, :max_position, :current_user - def initialize(story:, sprint:, max_position:, current_user: User.current) + def initialize(story:, sprint:, project:, max_position:, current_user: User.current) super() @story = story @sprint = sprint + @project = project @max_position = max_position @current_user = current_user end @@ -48,5 +49,9 @@ def initialize(story:, sprint:, max_position:, current_user: User.current) def story_points story.story_points || 0 end + + def draggable? + current_user.allowed_in_project?(:manage_sprint_items, project) + end end end diff --git a/modules/backlogs/app/components/backlogs/story_menu_component.rb b/modules/backlogs/app/components/backlogs/story_menu_component.rb index bd643286c7bb..ef760fcc04fd 100644 --- a/modules/backlogs/app/components/backlogs/story_menu_component.rb +++ b/modules/backlogs/app/components/backlogs/story_menu_component.rb @@ -32,12 +32,12 @@ module Backlogs class StoryMenuComponent < ApplicationComponent attr_reader :story, :sprint, :project, :max_position, :current_user - def initialize(story:, sprint:, max_position:, current_user: User.current, **system_arguments) + def initialize(story:, sprint:, project:, max_position:, current_user: User.current, **system_arguments) super() @story = story @sprint = sprint - @project = sprint.project + @project = project @max_position = max_position @current_user = current_user diff --git a/modules/backlogs/app/controllers/projects/settings/backlogs_controller.rb b/modules/backlogs/app/controllers/projects/settings/backlogs_controller.rb index a59b335f0557..695adaf79801 100644 --- a/modules/backlogs/app/controllers/projects/settings/backlogs_controller.rb +++ b/modules/backlogs/app/controllers/projects/settings/backlogs_controller.rb @@ -42,7 +42,11 @@ def update end def rebuild_positions - @project.rebuild_positions + if OpenProject::FeatureDecisions.scrum_projects_active? + WorkPackages::RebuildPositionsService.new(project: @project).call + else + @project.rebuild_positions + end flash[:notice] = I18n.t("backlogs.positions_rebuilt_successfully") redirect_to_backlogs_settings diff --git a/modules/backlogs/app/controllers/rb_application_controller.rb b/modules/backlogs/app/controllers/rb_application_controller.rb index e46be1676c1f..1b37ec0444b2 100644 --- a/modules/backlogs/app/controllers/rb_application_controller.rb +++ b/modules/backlogs/app/controllers/rb_application_controller.rb @@ -43,17 +43,24 @@ class RbApplicationController < ApplicationController def load_sprint_and_project load_project - # because of strong params, we want to pluck this variable out right now, - # otherwise it causes issues where we are doing `attributes=`. - if (@sprint_id = params.delete(:sprint_id)) - @sprint = Sprint.visible.apply_to(@project).find(@sprint_id) - end + load_sprint end def load_project @project = Project.visible.find(params[:project_id]) end + def load_sprint + @sprint_id = params.delete(:sprint_id) + return unless @sprint_id + + @sprint = if OpenProject::FeatureDecisions.scrum_projects_active? + Agile::Sprint.for_project(@project).visible.find(@sprint_id) + else + Sprint.visible.apply_to(@project).find(@sprint_id) + end + end + def check_if_plugin_is_configured return if OpenProject::FeatureDecisions.scrum_projects_active? diff --git a/modules/backlogs/app/controllers/rb_sprints_controller.rb b/modules/backlogs/app/controllers/rb_sprints_controller.rb index e4fc3b88a9bc..65cdfe5371a5 100644 --- a/modules/backlogs/app/controllers/rb_sprints_controller.rb +++ b/modules/backlogs/app/controllers/rb_sprints_controller.rb @@ -155,7 +155,8 @@ def update_header_component_via_turbo_stream(state: :show) def update_sprint_header_component_via_turbo_stream(sprint:) update_via_turbo_stream( - component: Backlogs::SprintHeaderComponent.new(sprint:), + component: Backlogs::SprintHeaderComponent.new(sprint:, + project: @project), method: :morph ) end diff --git a/modules/backlogs/app/controllers/rb_stories_controller.rb b/modules/backlogs/app/controllers/rb_stories_controller.rb index c9ab6c288b24..3cc1d4aebf0a 100644 --- a/modules/backlogs/app/controllers/rb_stories_controller.rb +++ b/modules/backlogs/app/controllers/rb_stories_controller.rb @@ -31,12 +31,7 @@ class RbStoriesController < RbApplicationController include OpTurbo::ComponentStream - NEW_SPRINT_ACTIONS = %i[move].freeze - - skip_before_action :load_sprint_and_project, only: NEW_SPRINT_ACTIONS - - before_action :legacy_load_story, except: NEW_SPRINT_ACTIONS - prepend_before_action :load_sprint, :load_project, :load_story, only: NEW_SPRINT_ACTIONS + before_action :load_story # Move a story from a Sprint to another Sprint or an Agile::Sprint. def move_legacy @@ -90,7 +85,7 @@ def reorder return respond_with_turbo_streams(status: :unprocessable_entity) end - replace_backlog_component_via_turbo_stream(sprint: @sprint) + replace_typed_component_via_turbo_stream(sprint: @sprint) respond_with_turbo_streams end @@ -184,19 +179,16 @@ def replace_backlog_component_via_turbo_stream(sprint:) end def replace_sprint_component_via_turbo_stream(sprint:) - replace_via_turbo_stream(component: Backlogs::SprintComponent.new(sprint: sprint)) - end - - def legacy_load_story - @story = Story.visible.find(params[:id]) + replace_via_turbo_stream(component: Backlogs::SprintComponent.new(sprint: sprint, project: @project), + method: :morph) end def load_story - @story = WorkPackage.visible.find(params[:id]) - end - - def load_sprint - @sprint = Agile::Sprint.for_project(@project).visible.find(params[:sprint_id]) + @story = if OpenProject::FeatureDecisions.scrum_projects_active? + WorkPackage.visible.find(params[:id]) + else + Story.visible.find(params[:id]) + end end def move_params diff --git a/modules/backlogs/app/services/work_packages/rebuild_positions_service.rb b/modules/backlogs/app/services/work_packages/rebuild_positions_service.rb new file mode 100644 index 000000000000..0cf950f6968f --- /dev/null +++ b/modules/backlogs/app/services/work_packages/rebuild_positions_service.rb @@ -0,0 +1,58 @@ +# 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::RebuildPositionsService + def initialize(project: nil) + @project = project + end + + def call + condition = if @project + ::OpenProject::SqlSanitization.sanitize " AND work_packages.project_id = :project_id", + project_id: @project.id + end + + WorkPackage.connection.execute <<~SQL.squish + UPDATE work_packages + SET position = mapping.new_position + FROM ( + SELECT + id, + ROW_NUMBER() OVER ( + PARTITION BY project_id, sprint_id + ORDER BY position, created_at + ) AS new_position + FROM work_packages + ) AS mapping + WHERE work_packages.id = mapping.id + #{condition} + SQL + end +end diff --git a/modules/backlogs/app/views/rb_master_backlogs/_agile_list.html.erb b/modules/backlogs/app/views/rb_master_backlogs/_agile_list.html.erb index 412df5cf8ffd..4e83ad1090a5 100644 --- a/modules/backlogs/app/views/rb_master_backlogs/_agile_list.html.erb +++ b/modules/backlogs/app/views/rb_master_backlogs/_agile_list.html.erb @@ -42,7 +42,7 @@ See COPYRIGHT and LICENSE files for more details. <% else %>
- <%= render(Backlogs::SprintComponent.with_collection(@sprints)) %> + <%= render(Backlogs::SprintComponent.with_collection(@sprints, project: @project)) %>
<%= render(Backlogs::BacklogComponent.with_collection(@owner_backlogs, project: @project)) %> diff --git a/modules/backlogs/app/views/rb_master_backlogs/_sprint_planning_list.html.erb b/modules/backlogs/app/views/rb_master_backlogs/_sprint_planning_list.html.erb index c221805349e2..8c83ad8e49e2 100644 --- a/modules/backlogs/app/views/rb_master_backlogs/_sprint_planning_list.html.erb +++ b/modules/backlogs/app/views/rb_master_backlogs/_sprint_planning_list.html.erb @@ -100,7 +100,7 @@ See COPYRIGHT and LICENSE files for more details. end end %> - <%= render(Backlogs::SprintComponent.with_collection(@sprints)) %> + <%= render(Backlogs::SprintComponent.with_collection(@sprints, project: @project)) %>
<% end %> diff --git a/modules/backlogs/lib/open_project/backlogs/list.rb b/modules/backlogs/lib/open_project/backlogs/list.rb index e884a450b5e5..3d7029dd6340 100644 --- a/modules/backlogs/lib/open_project/backlogs/list.rb +++ b/modules/backlogs/lib/open_project/backlogs/list.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + #-- copyright # OpenProject is an open source project management software. # Copyright (C) the OpenProject GmbH @@ -30,31 +32,71 @@ module OpenProject::Backlogs::List extend ActiveSupport::Concern included do + # Once the scrum_projects_active feature flag is removed, + # add + # scope [:project_id, :sprint_id] acts_as_list touch_on_update: false + # acts as list adds a before destroy hook which messes # with the parent_id_was value skip_callback(:destroy, :before, :reload) # Reorder list, if work_package is removed from sprint + # To be removed once the scrum_projects_active feature flag is removed before_update :fix_other_work_package_positions + # To be removed once the scrum_projects_active feature flag is removed before_update :fix_own_work_package_position + private + # Used by acts_list to limit the list to a certain subset within # the table. # # Also sanitize_sql seems to be unavailable in a sensible way. Therefore # we're using send to circumvent visibility work_packages. def scope_condition - self.class.send(:sanitize_sql, ["project_id = ? AND version_id = ? AND type_id IN (?)", - project_id, version_id, types]) + if OpenProject::FeatureDecisions.scrum_projects_active? + { project_id:, sprint_id: } + else + self.class.send(:sanitize_sql, ["project_id = ? AND version_id = ? AND type_id IN (?)", + project_id, version_id, types]) + end + end + + # rubocop:disable Style/ArrayIntersect + # rubocop:disable Performance/InefficientHashSearch + # Copied from acts_as_list. + # To be removed once the scrum_projects_active feature flag is removed + def scope_changed? + return false unless OpenProject::FeatureDecisions.scrum_projects_active? + + (scope_condition.keys & changed.map(&:to_sym)).any? end + # Copied from acts_as_list + # To be removed once the scrum_projects_active feature flag is removed + def destroyed_via_scope? + return false unless OpenProject::FeatureDecisions.scrum_projects_active? + return false unless destroyed_by_association + + foreign_key = destroyed_by_association.foreign_key + if foreign_key.is_a?(Array) + # Composite foreign key - check if any keys overlap with scope + (scope_condition.keys & foreign_key.map(&:to_sym)).any? + else + # Single foreign key + scope_condition.keys.include?(foreign_key.to_sym) + end + end + # rubocop:enable Style/ArrayIntersect + # rubocop:enable Performance/InefficientHashSearch + include InstanceMethods end module InstanceMethods def move_after(position: nil, prev_id: nil) - if acts_as_list_list.none?(:position) + if acts_as_list_list.all?(position: nil) # If no items have a position, create an order on position # silently. This can happen when sorting inside a version for the first # time after backlogs was activated and there have already been items @@ -83,11 +125,13 @@ def move_after(position: nil, prev_id: nil) # Override acts_as_list implementation to avoid it calling save. # Calling save would remove the changes/saved_changes information. - def set_list_position(new_position, _raise_exception_if_save_fails = false) + def set_list_position(new_position, _raise_exception_if_save_fails = false) # rubocop:disable Style/OptionalBooleanParameter update_columns(position: new_position) end - def fix_other_work_package_positions + def fix_other_work_package_positions # rubocop:disable Metrics/AbcSize, Metrics/PerceivedComplexity + return if OpenProject::FeatureDecisions.scrum_projects_active? + if changes.slice("project_id", "type_id", "version_id").present? if changes.slice("project_id", "version_id").blank? and Story.types.include?(type_id.to_i) and @@ -131,7 +175,9 @@ def fix_other_work_package_positions end end - def fix_own_work_package_position + def fix_own_work_package_position # rubocop:disable Metrics/AbcSize, Metrics/PerceivedComplexity + return if OpenProject::FeatureDecisions.scrum_projects_active? + if changes.slice("project_id", "type_id", "version_id").present? if changes.slice("project_id", "version_id").blank? and Story.types.include?(type_id.to_i) and diff --git a/modules/backlogs/spec/components/backlogs/sprint_component_spec.rb b/modules/backlogs/spec/components/backlogs/sprint_component_spec.rb index bc56e0db62ec..e5225be59e26 100644 --- a/modules/backlogs/spec/components/backlogs/sprint_component_spec.rb +++ b/modules/backlogs/spec/components/backlogs/sprint_component_spec.rb @@ -52,7 +52,7 @@ end def render_component - render_inline(described_class.new(sprint:, current_user: user)) + render_inline(described_class.new(sprint:, project:, current_user: user)) end describe "rendering" do diff --git a/modules/backlogs/spec/components/backlogs/sprint_header_component_spec.rb b/modules/backlogs/spec/components/backlogs/sprint_header_component_spec.rb index 2318fd41172c..0d8c7577681c 100644 --- a/modules/backlogs/spec/components/backlogs/sprint_header_component_spec.rb +++ b/modules/backlogs/spec/components/backlogs/sprint_header_component_spec.rb @@ -52,7 +52,7 @@ end def render_component(folded: false) - render_inline(described_class.new(sprint:, folded:, current_user: user)) + render_inline(described_class.new(sprint:, project:, folded:, current_user: user)) end describe "show state (default)" do diff --git a/modules/backlogs/spec/components/backlogs/story_component_spec.rb b/modules/backlogs/spec/components/backlogs/story_component_spec.rb index 8cd5e3207ebd..15ebdefa6993 100644 --- a/modules/backlogs/spec/components/backlogs/story_component_spec.rb +++ b/modules/backlogs/spec/components/backlogs/story_component_spec.rb @@ -35,7 +35,7 @@ shared_let(:type_task) { create(:type_task) } shared_let(:default_status) { create(:default_status) } shared_let(:default_priority) { create(:default_priority) } - shared_let(:user) { create(:admin) } + shared_let(:user) { create(:user) } current_user { user } let(:project) { create(:project, types: [type_feature, type_task]) } @@ -53,49 +53,54 @@ version: sprint) end let(:max_position) { 3 } + let(:permissions) { %i[manage_sprint_items] } before do allow(Setting) .to receive(:plugin_openproject_backlogs) .and_return("story_types" => [type_feature.id.to_s], "task_type" => type_task.id.to_s) + + mock_permissions_for(current_user) do |mock| + mock.allow_in_project(*permissions, project:) + end end def render_component - render_inline(described_class.new(story:, sprint:, max_position:, current_user: user)) + render_inline(described_class.new(story:, sprint:, project:, max_position:, current_user: user)) end - describe "rendering" do - it "renders Primer::OpenProject::DragHandle" do - render_component + it "renders WorkPackages::InfoLineComponent" do + render_component - # DragHandle renders with grabber icon - expect(page).to have_octicon(:grabber) - end + # InfoLine renders type and ID info + expect(page).to have_text("FEATURE") + expect(page).to have_text("##{story.id}") + end - it "renders WorkPackages::InfoLineComponent" do - render_component + it "shows story subject in semibold text" do + render_component - # InfoLine renders type and ID info - expect(page).to have_text("FEATURE") - expect(page).to have_text("##{story.id}") - end + expect(page).to have_text("Test Story Subject") + end - it "shows story subject in semibold text" do - render_component + it "shows story points" do + render_component - expect(page).to have_text("Test Story Subject") - end + expect(page).to have_text("5 points", normalize_ws: true) + end - it "shows story points" do - render_component + it "renders StoryMenuComponent" do + render_component - expect(page).to have_text("5 points", normalize_ws: true) - end + expect(page).to have_css("action-menu") + end - it "renders StoryMenuComponent" do + describe "drag handle behaviour" do + it "renders Primer::OpenProject::DragHandle" do render_component - expect(page).to have_css("action-menu") + # DragHandle renders with grabber icon + expect(page).to have_octicon(:grabber) end it "renders a drag handle compatible with GenericDragAndDropController" do @@ -104,6 +109,19 @@ def render_component expect(page).to have_css(".DragHandle[role='button'][tabindex='0']") expect(page).to have_css(".DragHandle[aria-label='Move Test Story Subject']") end + + context "when user is not allowed to drag" do + let(:permissions) { [] } + + it "does not render a drag handle" do + render_component + + # DragHandle renders with grabber icon + expect(page).not_to have_octicon(:grabber) + + expect(page).to have_no_css(".DragHandle") + end + end end describe "story points handling" do diff --git a/modules/backlogs/spec/components/backlogs/story_menu_component_spec.rb b/modules/backlogs/spec/components/backlogs/story_menu_component_spec.rb index 52665142937d..055db9dd78de 100644 --- a/modules/backlogs/spec/components/backlogs/story_menu_component_spec.rb +++ b/modules/backlogs/spec/components/backlogs/story_menu_component_spec.rb @@ -62,7 +62,7 @@ def render_component(position: 2, max_position: 3) story.update!(position:) - render_inline(described_class.new(story:, sprint:, max_position:, current_user: user)) + render_inline(described_class.new(story:, sprint:, project:, max_position:, current_user: user)) end describe "standard items" do diff --git a/modules/backlogs/spec/controllers/rb_stories_controller_spec.rb b/modules/backlogs/spec/controllers/rb_stories_controller_spec.rb index 6fd8581ca2cc..82dfe3edf38d 100644 --- a/modules/backlogs/spec/controllers/rb_stories_controller_spec.rb +++ b/modules/backlogs/spec/controllers/rb_stories_controller_spec.rb @@ -221,7 +221,7 @@ end end - describe "PUT #move" do + describe "PUT #move", with_flag: { scrum_projects: true } do let(:agile_sprint) { create(:agile_sprint, name: "Agile Sprint 1", project:) } let(:story_in_agile_sprint) { create(:work_package, status:, sprint: agile_sprint, project:) } diff --git a/modules/backlogs/spec/features/work_packages/create_work_package_spec.rb b/modules/backlogs/spec/features/work_packages/create_work_package_spec.rb new file mode 100644 index 000000000000..dae7f037e85c --- /dev/null +++ b/modules/backlogs/spec/features/work_packages/create_work_package_spec.rb @@ -0,0 +1,198 @@ +# 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" +require_relative "../../support/pages/sprint_planning" + +RSpec.describe "Create work package in sprint", :js, with_flag: { scrum_projects: true } do + let!(:project) do + create(:project, + types: [type, type2], + enabled_module_names: %w(work_package_tracking backlogs)) + end + let!(:project2) { create(:project) } + let(:create_role) do + create(:project_role, + permissions: %i(view_sprints + view_work_packages + manage_sprint_items + add_work_packages)) + end + let(:non_create_role) do + create(:project_role, + permissions: %i(view_sprints + view_work_packages)) + end + + let(:type) { create(:type) } + let(:type2) { create(:type) } + + let!(:priority) { create(:default_priority) } + let!(:status) { create(:default_status) } + + let!(:sprint1) { create(:agile_sprint, project:) } + let!(:sprint2) { create(:agile_sprint, project:) } + + let!(:sprint1_wp1) { create(:work_package, sprint: sprint1, type:, project:) } + let!(:sprint1_wp2) { create(:work_package, sprint: sprint1, type:, project:) } + let!(:sprint1_other_project_wp1) { create(:work_package, sprint: sprint1, type:, project: project2) } + + let(:backlogs_page) { Pages::SprintPlanning.new(project) } + + current_user do + create(:user, + member_with_roles: { + project => create_role, + project2 => create_role + }) + end + + before do + # Faulty and mostly irrelevant for the test. Only needed to make the sprints appear on the page. + # To be removed once the setting is removed. + Setting.plugin_openproject_backlogs = { + "story_types" => [type.id.to_s], + "task_type" => type.id.to_s + } + + backlogs_page.visit! + end + + context "in a non shared sprint" do + it "allows creating a new story" do + backlogs_page.click_in_sprint_menu(sprint1, "Add work package") + + within_dialog "New work package" do + fill_in "Subject", with: "The new item" + # TODO: removed in OP #57688, to be reimplemented + # fill_in "Story Points", with: "5" + + select_combo_box_option type2.name, from: "Type" + + # saving the new story + click_on "Create" + end + + expect_and_dismiss_flash type: :success, message: "New work package created" + + created_work_package = WorkPackage.last + + # velocity should be summed up immediately + # TODO: removed in OP #57688, to be reimplemented + # xpect(page).to have_css(".velocity", text: "12") + + # this will ensure that the page refresh is through before we check the order + backlogs_page.click_in_sprint_menu(sprint1, "Add work package") + + within_dialog "New work package" do + fill_in "Subject", with: "Another story" + end + + # the order is kept even after a page refresh -> it is persisted in the db + page.driver.refresh + + expect(page) + .to have_no_content "Another story" + + backlogs_page + .expect_work_packages_in_sprint_in_order(sprint1, + work_packages: [sprint1_wp1, + sprint1_wp2, + created_work_package]) + + # created with the selected type (HighlightedTypeComponent renders type name in uppercase) + backlogs_page.within_work_package_row(created_work_package) do + expect(page).to have_text(type2.name.upcase) + end + end + end + + context "in an empty non shared sprint" do + it "allows creating a new story" do + backlogs_page.click_in_sprint_menu(sprint2, "Add work package") + + within_dialog "New work package" do + fill_in "Subject", with: "The new item" + + click_on "Create" + end + + expect_and_dismiss_flash type: :success, message: "New work package created" + + created_work_package = WorkPackage.last + + backlogs_page + .expect_work_packages_in_sprint_in_order(sprint2, + work_packages: [created_work_package]) + end + end + + context "in a shared sprint" do + let(:backlogs_page) { Pages::SprintPlanning.new(project2) } + + it "allows creating a new story" do + backlogs_page.click_in_sprint_menu(sprint1, "Add work package") + + within_dialog "New work package" do + fill_in "Subject", with: "The new item" + + click_on "Create" + end + + expect_and_dismiss_flash type: :success, message: "New work package created" + + created_work_package = WorkPackage.last + + backlogs_page + .expect_work_packages_in_sprint_in_order(sprint1, + work_packages: [sprint1_other_project_wp1, + created_work_package]) + end + end + + context "when lacking the permission to create work packages" do + current_user do + create(:user, + member_with_roles: { + project => non_create_role + }) + end + + it "does not show a menu (item for creating a new work package)" do + # At the moment, since there's no menu item, the entire menu will not be visible. + # Once we add more and more menu items back, the menu will be rendered, but the action + # will be missing. When that happens, the expectation has to be adjusted for something like + # this: + # backlogs_page.expect_no_sprint_menu_item(sprint1, "Add work package") + + backlogs_page.expect_no_sprint_menu(sprint1) + end + end +end diff --git a/modules/backlogs/spec/features/work_packages/drag_in_sprint_spec.rb b/modules/backlogs/spec/features/work_packages/drag_in_sprint_spec.rb new file mode 100644 index 000000000000..6f330b02e851 --- /dev/null +++ b/modules/backlogs/spec/features/work_packages/drag_in_sprint_spec.rb @@ -0,0 +1,166 @@ +# 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" +require_relative "../../support/pages/sprint_planning" + +RSpec.describe "Dragging work packages in and between sprints", + :js, :settings_reset, with_flag: { scrum_projects: true } do + let!(:project) do + create(:project, + types: [type], + enabled_module_names: %w(work_package_tracking backlogs)) + end + let!(:project2) { create(:project) } + let(:manage_sprint_items_role) do + create(:project_role, + permissions: %i(view_sprints + manage_sprint_items + view_work_packages + edit_work_packages)) + end + let(:edit_role_without_manage_sprint_items) do + create(:project_role, + permissions: %i(view_sprints + view_work_packages + edit_work_packages)) + end + + let(:type) { create(:type) } + + let!(:sprint1) { create(:agile_sprint, project:) } + let!(:sprint2) { create(:agile_sprint, project:) } + + let!(:sprint1_wp1) { create(:work_package, sprint: sprint1, type:, project:) } + let!(:sprint1_wp2) { create(:work_package, sprint: sprint1, type:, project:) } + let!(:sprint1_wp3) { create(:work_package, sprint: sprint1, type:, project:) } + let!(:sprint1_wp4) { create(:work_package, sprint: sprint1, type:, project:) } + let!(:sprint1_other_project_wp1) { create(:work_package, sprint: sprint1, type:, project: project2) } + let!(:sprint1_other_project_wp2) { create(:work_package, sprint: sprint1, type:, project: project2) } + let!(:sprint1_other_project_wp3) { create(:work_package, sprint: sprint1, type:, project: project2) } + + let(:backlogs_page) { Pages::SprintPlanning.new(project) } + + current_user do + create(:user, + member_with_roles: { + project => manage_sprint_items_role, + project2 => manage_sprint_items_role + }) + end + + before do + # Faulty and mostly irrelevant for the test. Only needed to make the sprints appear on the page. + # To be removed once the setting is removed. + Setting.plugin_openproject_backlogs = { + "story_types" => [type.id.to_s], + "task_type" => type.id.to_s + } + + backlogs_page.visit! + end + + context "in a non shared sprint" do + it "displays work packages in correct order and allows dragging them around" do + backlogs_page + .expect_work_packages_in_sprint_in_order(sprint1, + work_packages: [sprint1_wp1, + sprint1_wp2, + sprint1_wp3, + sprint1_wp4]) + backlogs_page + .drag_work_package(sprint1_wp1, before: sprint1_wp4) + + backlogs_page + .expect_work_packages_in_sprint_in_order(sprint1, + work_packages: [sprint1_wp2, + sprint1_wp3, + sprint1_wp1, + sprint1_wp4]) + backlogs_page + .drag_work_package(sprint1_wp1, before: sprint1_wp3) + + backlogs_page + .expect_work_packages_in_sprint_in_order(sprint1, + work_packages: [sprint1_wp2, + sprint1_wp1, + sprint1_wp3, + sprint1_wp4]) + + backlogs_page + .drag_work_package(sprint1_wp1, into: sprint2) + + backlogs_page + .expect_work_packages_in_sprint_in_order(sprint1, + work_packages: [sprint1_wp2, + sprint1_wp3, + sprint1_wp4]) + backlogs_page + .expect_work_packages_in_sprint_in_order(sprint2, + work_packages: [sprint1_wp1]) + end + end + + context "when lacking the permission to manage sprint items" do + current_user do + create(:user, + member_with_roles: { + project => edit_role_without_manage_sprint_items + }) + end + + it "displays work packages in correct order but does not allow dragging them around" do + backlogs_page.expect_work_package_not_draggable(sprint1_wp1) + backlogs_page.expect_work_package_not_draggable(sprint1_wp2) + backlogs_page.expect_work_package_not_draggable(sprint1_wp3) + backlogs_page.expect_work_package_not_draggable(sprint1_wp4) + end + end + + context "in a shared sprint" do + let(:backlogs_page) { Pages::SprintPlanning.new(project2) } + + it "displays work packages in correct order and allows dragging them around in a shared sprint" do + backlogs_page + .expect_work_packages_in_sprint_in_order(sprint1, + work_packages: [sprint1_other_project_wp1, + sprint1_other_project_wp2, + sprint1_other_project_wp3]) + backlogs_page + .drag_work_package(sprint1_other_project_wp1, before: sprint1_other_project_wp3) + + backlogs_page + .expect_work_packages_in_sprint_in_order(sprint1, + work_packages: [sprint1_other_project_wp2, + sprint1_other_project_wp1, + sprint1_other_project_wp3]) + end + end +end diff --git a/modules/backlogs/spec/models/work_packages/position_spec.rb b/modules/backlogs/spec/models/work_packages/position_spec.rb new file mode 100644 index 000000000000..d0f85e49e6cf --- /dev/null +++ b/modules/backlogs/spec/models/work_packages/position_spec.rb @@ -0,0 +1,408 @@ +# 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 WorkPackage, "positions", with_flag: { scrum_projects: true } do # rubocop:disable RSpec/SpecFilePathFormat + def create_work_package(options) + create(:work_package, options.reverse_merge(project:, type_id: type.id)) + end + + shared_let(:project) { create(:project) } + shared_let(:type) { create(:type) } + shared_let(:sprint1) { create(:agile_sprint, project:, name: "Sprint 1") } + shared_let(:sprint2) { create(:agile_sprint, project:, name: "Sprint 2") } + + # Once the feature flag is removed, those can be changed into shared_let + let!(:sprint1_wp1) { create_work_package(subject: "Sprint 1 WorkPackage 1", sprint: sprint1) } + let!(:sprint1_wp2) { create_work_package(subject: "Sprint 1 WorkPackage 2", sprint: sprint1) } + let!(:sprint1_wp3) { create_work_package(subject: "Sprint 1 WorkPackage 3", sprint: sprint1) } + let!(:sprint1_wp4) { create_work_package(subject: "Sprint 1 WorkPackage 4", sprint: sprint1) } + let!(:sprint1_wp5) { create_work_package(subject: "Sprint 1 WorkPackage 5", sprint: sprint1) } + + let!(:sprint2_wp1) { create_work_package(subject: "Sprint 2 WorkPackage 1", sprint: sprint2) } + let!(:sprint2_wp2) { create_work_package(subject: "Sprint 2 WorkPackage 2", sprint: sprint2) } + let!(:sprint2_wp3) { create_work_package(subject: "Sprint 2 WorkPackage 3", sprint: sprint2) } + + let!(:no_sprint_wp1) { create_work_package(subject: "No sprint WorkPackage 1", sprint: nil) } + let!(:no_sprint_wp2) { create_work_package(subject: "No sprint WorkPackage 2", sprint: nil) } + let!(:no_sprint_wp3) { create_work_package(subject: "No sprint WorkPackage 3", sprint: nil) } + + def wp_of_sprint_by_id_and_position(sprint) + WorkPackage.where(sprint:).pluck(:id, :position).to_h + end + + context "when creating a work_package in a sprint" do + it "puts them in order" do + new_work_package = create_work_package(subject: "Newest WorkPackage", sprint: sprint1) + + expect(wp_of_sprint_by_id_and_position(sprint1)) + .to eq(sprint1_wp1.id => 1, + sprint1_wp2.id => 2, + sprint1_wp3.id => 3, + sprint1_wp4.id => 4, + sprint1_wp5.id => 5, + new_work_package.id => 6) + end + end + + context "when creating a work_package outside a sprint" do + it "puts them in order" do + new_work_package = create_work_package(subject: "Newest WorkPackage", sprint: nil) + + expect(wp_of_sprint_by_id_and_position(nil)) + .to eq(no_sprint_wp1.id => 1, + no_sprint_wp2.id => 2, + no_sprint_wp3.id => 3, + new_work_package.id => 4) + end + end + + context "when moving a work_package to a different sprint" do + it "reorders the remaining work_packages and the ones in the new sprint" do + sprint1_wp2.sprint = sprint2 + sprint1_wp2.save! + + expect(wp_of_sprint_by_id_and_position(sprint1)) + .to eq(sprint1_wp1.id => 1, + sprint1_wp3.id => 2, + sprint1_wp4.id => 3, + sprint1_wp5.id => 4) + + expect(wp_of_sprint_by_id_and_position(sprint2)) + .to eq(sprint2_wp1.id => 1, + sprint2_wp2.id => 2, + sprint2_wp3.id => 3, + sprint1_wp2.id => 4) + end + end + + context "when removing a work_package from a sprint" do + it "reorders the remaining work_packages and the ones outside of a sprint" do + sprint1_wp2.sprint = nil + sprint1_wp2.save! + + expect(wp_of_sprint_by_id_and_position(sprint1)) + .to eq(sprint1_wp1.id => 1, + sprint1_wp3.id => 2, + sprint1_wp4.id => 3, + sprint1_wp5.id => 4) + + expect(wp_of_sprint_by_id_and_position(nil)) + .to eq(no_sprint_wp1.id => 1, + no_sprint_wp2.id => 2, + no_sprint_wp3.id => 3, + sprint1_wp2.id => 4) + end + end + + context "when moving a work_package into a sprint" do + it "reorders the remaining work_packages and the ones in the new sprint" do + no_sprint_wp2.sprint = sprint1 + no_sprint_wp2.save! + + expect(wp_of_sprint_by_id_and_position(nil)) + .to eq(no_sprint_wp1.id => 1, + no_sprint_wp3.id => 2) + + expect(wp_of_sprint_by_id_and_position(sprint1)) + .to eq(sprint1_wp1.id => 1, + sprint1_wp2.id => 2, + sprint1_wp3.id => 3, + sprint1_wp4.id => 4, + sprint1_wp5.id => 5, + no_sprint_wp2.id => 6) + end + end + + context "when deleting a work_package in a sprint" do + it "reorders the existing work_packages" do + sprint1_wp3.destroy! + + expect(wp_of_sprint_by_id_and_position(sprint1)) + .to eq(sprint1_wp1.id => 1, + sprint1_wp2.id => 2, + sprint1_wp4.id => 3, + sprint1_wp5.id => 4) + end + end + + context "when deleting a work_package outside a sprint" do + it "reorders the existing work_packages" do + no_sprint_wp1.destroy! + + expect(wp_of_sprint_by_id_and_position(nil)) + .to eq(no_sprint_wp2.id => 1, + no_sprint_wp3.id => 2) + end + end + + describe "#move_after" do + context "when moving inside a sprint with a position of 1" do + it "moves the work_package to the beginning of the sprint" do + sprint1_wp4.move_after(position: 1) + + expect(wp_of_sprint_by_id_and_position(sprint1)) + .to eq(sprint1_wp4.id => 1, + sprint1_wp1.id => 2, + sprint1_wp2.id => 3, + sprint1_wp3.id => 4, + sprint1_wp5.id => 5) + end + end + + context "when moving down inside a sprint with a position in the middle of the sprint" do + it "moves the work_package to the middle of the sprint" do + sprint1_wp1.move_after(position: 3) + + expect(wp_of_sprint_by_id_and_position(sprint1)) + .to eq(sprint1_wp2.id => 1, + sprint1_wp3.id => 2, + sprint1_wp1.id => 3, + sprint1_wp4.id => 4, + sprint1_wp5.id => 5) + end + end + + context "when moving up inside a sprint with a position in the middle of the sprint" do + it "moves the work_package to the middle of the sprint" do + sprint1_wp5.move_after(position: 3) + + expect(wp_of_sprint_by_id_and_position(sprint1)) + .to eq(sprint1_wp1.id => 1, + sprint1_wp2.id => 2, + sprint1_wp5.id => 3, + sprint1_wp3.id => 4, + sprint1_wp4.id => 5) + end + end + + context "when moving inside a sprint with a position at the end of the sprint" do + it "moves the work_package to the end of the sprint" do + sprint1_wp2.move_after(position: 5) + + expect(wp_of_sprint_by_id_and_position(sprint1)) + .to eq(sprint1_wp1.id => 1, + sprint1_wp3.id => 2, + sprint1_wp4.id => 3, + sprint1_wp5.id => 4, + sprint1_wp2.id => 5) + end + end + + context "when moving inside a sprint with a position that is larger than the positions present in the sprint" do + it "moves the work_package to the top of the sprint" do + sprint1_wp2.move_after(position: 6) + + expect(wp_of_sprint_by_id_and_position(sprint1)) + .to eq(sprint1_wp2.id => 1, + sprint1_wp1.id => 2, + sprint1_wp3.id => 3, + sprint1_wp4.id => 4, + sprint1_wp5.id => 5) + end + end + + context "when moving inside a sprint with a previous that is the first element" do + it "moves the work_package to the top of the sprint" do + sprint1_wp4.move_after(prev_id: sprint1_wp1.id) + + expect(wp_of_sprint_by_id_and_position(sprint1)) + .to eq(sprint1_wp1.id => 1, + sprint1_wp4.id => 2, + sprint1_wp2.id => 3, + sprint1_wp3.id => 4, + sprint1_wp5.id => 5) + end + end + + context "when moving down inside a sprint with a previous in the middle" do + it "moves the work_package after the previous" do + sprint1_wp1.move_after(prev_id: sprint1_wp3.id) + + expect(wp_of_sprint_by_id_and_position(sprint1)) + .to eq(sprint1_wp2.id => 1, + sprint1_wp3.id => 2, + sprint1_wp1.id => 3, + sprint1_wp4.id => 4, + sprint1_wp5.id => 5) + end + end + + context "when moving up inside a sprint with a previous in the middle" do + it "moves the work_package after the previous" do + sprint1_wp5.move_after(prev_id: sprint1_wp3.id) + + expect(wp_of_sprint_by_id_and_position(sprint1)) + .to eq(sprint1_wp1.id => 1, + sprint1_wp2.id => 2, + sprint1_wp3.id => 3, + sprint1_wp5.id => 4, + sprint1_wp4.id => 5) + end + end + + context "when inside a sprint with a previous at the bottom" do + it "moves the work_package after the previous" do + sprint1_wp1.move_after(prev_id: sprint1_wp5.id) + + expect(wp_of_sprint_by_id_and_position(sprint1)) + .to eq(sprint1_wp2.id => 1, + sprint1_wp3.id => 2, + sprint1_wp4.id => 3, + sprint1_wp5.id => 4, + sprint1_wp1.id => 5) + end + end + + context "when inside a sprint with a previous that does not exist in that sprint" do + it "moves the work_package to the top of the sprint" do + sprint1_wp4.move_after(prev_id: sprint2_wp2.id) + + expect(wp_of_sprint_by_id_and_position(sprint1)) + .to eq(sprint1_wp4.id => 1, + sprint1_wp1.id => 2, + sprint1_wp2.id => 3, + sprint1_wp3.id => 4, + sprint1_wp5.id => 5) + end + end + + context "when moving outside a sprint with a position of 1" do + it "moves the work_package to the beginning of the sprint" do + no_sprint_wp3.move_after(position: 1) + + expect(wp_of_sprint_by_id_and_position(nil)) + .to eq(no_sprint_wp3.id => 1, + no_sprint_wp1.id => 2, + no_sprint_wp2.id => 3) + end + end + + context "when moving down outside a sprint with a position in the middle of the sprint" do + it "moves the work_package to the middle of the sprint" do + no_sprint_wp1.move_after(position: 2) + + expect(wp_of_sprint_by_id_and_position(nil)) + .to eq(no_sprint_wp2.id => 1, + no_sprint_wp1.id => 2, + no_sprint_wp3.id => 3) + end + end + + context "when moving up outside a sprint with a position in the middle of the sprint" do + it "moves the work_package to the middle of the sprint" do + no_sprint_wp3.move_after(position: 2) + + expect(wp_of_sprint_by_id_and_position(nil)) + .to eq(no_sprint_wp1.id => 1, + no_sprint_wp3.id => 2, + no_sprint_wp2.id => 3) + end + end + + context "when moving outside a sprint with a position at the end of the sprint" do + it "moves the work_package to the end of the sprint" do + no_sprint_wp1.move_after(position: 3) + + expect(wp_of_sprint_by_id_and_position(nil)) + .to eq(no_sprint_wp2.id => 1, + no_sprint_wp3.id => 2, + no_sprint_wp1.id => 3) + end + end + + context "when moving outside a sprint with a position that is larger than the positions present in the sprint" do + it "moves the work_package to the top of the sprint" do + no_sprint_wp2.move_after(position: 4) + + expect(wp_of_sprint_by_id_and_position(nil)) + .to eq(no_sprint_wp2.id => 1, + no_sprint_wp1.id => 2, + no_sprint_wp3.id => 3) + end + end + + context "when moving outside a sprint with a previous that is the first element" do + it "moves the work_package to the top of the sprint" do + no_sprint_wp3.move_after(prev_id: no_sprint_wp1.id) + + expect(wp_of_sprint_by_id_and_position(nil)) + .to eq(no_sprint_wp1.id => 1, + no_sprint_wp3.id => 2, + no_sprint_wp2.id => 3) + end + end + + context "when moving down outside a sprint with a previous in the middle" do + it "moves the work_package after the previous" do + no_sprint_wp1.move_after(prev_id: no_sprint_wp2.id) + + expect(wp_of_sprint_by_id_and_position(nil)) + .to eq(no_sprint_wp2.id => 1, + no_sprint_wp1.id => 2, + no_sprint_wp3.id => 3) + end + end + + context "when moving up outside a sprint with a previous in the middle" do + it "moves the work_package after the previous" do + no_sprint_wp3.move_after(prev_id: no_sprint_wp1.id) + + expect(wp_of_sprint_by_id_and_position(nil)) + .to eq(no_sprint_wp1.id => 1, + no_sprint_wp3.id => 2, + no_sprint_wp2.id => 3) + end + end + + context "when outside a sprint with a previous at the bottom" do + it "moves the work_package after the previous" do + no_sprint_wp1.move_after(prev_id: no_sprint_wp3.id) + + expect(wp_of_sprint_by_id_and_position(nil)) + .to eq(no_sprint_wp2.id => 1, + no_sprint_wp3.id => 2, + no_sprint_wp1.id => 3) + end + end + + context "when outside a sprint with a previous that does not exist outside" do + it "moves the work_package to the top position" do + no_sprint_wp3.move_after(prev_id: sprint2_wp2.id) + + expect(wp_of_sprint_by_id_and_position(nil)) + .to eq(no_sprint_wp3.id => 1, + no_sprint_wp1.id => 2, + no_sprint_wp2.id => 3) + end + end + end +end diff --git a/modules/backlogs/spec/services/work_packages/rebuild_positions_service_integration_spec.rb b/modules/backlogs/spec/services/work_packages/rebuild_positions_service_integration_spec.rb new file mode 100644 index 000000000000..69c721c65074 --- /dev/null +++ b/modules/backlogs/spec/services/work_packages/rebuild_positions_service_integration_spec.rb @@ -0,0 +1,184 @@ +# 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::RebuildPositionsService, "integration", type: :model do + def create_work_package(options) + create(:work_package, options.reverse_merge(project: options[:sprint]&.project, type_id: type.id)) do |wp| + wp.update_column(:position, options[:position]) if options.key?(:position) + end + end + + shared_let(:project1) { create(:project) } + shared_let(:project2) { create(:project) } + shared_let(:type) { create(:type) } + shared_let(:sprint1) { create(:agile_sprint, project: project1, name: "Sprint 1") } + shared_let(:sprint2) { create(:agile_sprint, project: project1, name: "Sprint 2") } + shared_let(:sprint3) { create(:agile_sprint, project: project2, name: "Sprint 2") } + + shared_let(:sprint1_wp1) { create_work_package(subject: "Sprint 1 WorkPackage 1", sprint: sprint1, position: nil) } + shared_let(:sprint1_wp2) { create_work_package(subject: "Sprint 1 WorkPackage 2", sprint: sprint1, position: 1) } + shared_let(:sprint1_wp3) { create_work_package(subject: "Sprint 1 WorkPackage 3", sprint: sprint1, position: 2) } + shared_let(:sprint1_wp4) { create_work_package(subject: "Sprint 1 WorkPackage 4", sprint: sprint1, position: 2) } + shared_let(:sprint1_wp5) { create_work_package(subject: "Sprint 1 WorkPackage 5", sprint: sprint1, position: nil) } + + shared_let(:sprint2_wp1) { create_work_package(subject: "Sprint 2 WorkPackage 1", sprint: sprint2, position: 3) } + shared_let(:sprint2_wp2) { create_work_package(subject: "Sprint 2 WorkPackage 2", sprint: sprint2, position: 2) } + shared_let(:sprint2_wp3) { create_work_package(subject: "Sprint 2 WorkPackage 3", sprint: sprint2, position: 1) } + + shared_let(:sprint3_wp1) { create_work_package(subject: "Sprint 3 WorkPackage 1", sprint: sprint3, position: nil) } + shared_let(:sprint3_wp2) { create_work_package(subject: "Sprint 3 WorkPackage 2", sprint: sprint3, position: nil) } + shared_let(:sprint3_wp3) { create_work_package(subject: "Sprint 3 WorkPackage 3", sprint: sprint3, position: nil) } + + shared_let(:no_sprint_wp1) do + create_work_package(subject: "No sprint WorkPackage 1", project: project1, sprint: nil, position: nil) + end + shared_let(:no_sprint_wp2) do + create_work_package(subject: "No sprint WorkPackage 2", project: project1, sprint: nil, position: nil) + end + shared_let(:no_sprint_wp3) do + create_work_package(subject: "No sprint WorkPackage 3", project: project1, sprint: nil, position: nil) + end + + context "with the project provided" do + before do + described_class.new(project: project1).call + end + + it "fixes the positions in that project while keeping those that have some in the same order" do # rubocop:disable Rspec/ExampleLength + expect(WorkPackage.where(sprint: sprint1).to_h { [it.subject, it.position] }) + .to eql( + sprint1_wp2.subject => 1, + sprint1_wp3.subject => 2, + sprint1_wp4.subject => 3, + sprint1_wp1.subject => 4, + sprint1_wp5.subject => 5 + ) + + expect(WorkPackage.where(sprint: sprint2).to_h { [it.subject, it.position] }) + .to eql( + sprint2_wp3.subject => 1, + sprint2_wp2.subject => 2, + sprint2_wp1.subject => 3 + ) + + expect(WorkPackage.where(sprint: nil).to_h { [it.subject, it.position] }) + .to eql( + no_sprint_wp1.subject => 1, + no_sprint_wp2.subject => 2, + no_sprint_wp3.subject => 3 + ) + + expect(WorkPackage.where(sprint: sprint3).to_h { [it.subject, it.position] }) + .to eql( + sprint3_wp1.subject => nil, + sprint3_wp2.subject => nil, + sprint3_wp3.subject => nil + ) + end + end + + context "with a different project provided" do + before do + described_class.new(project: project2).call + end + + it "fixes only the work packages in the other project" do # rubocop:disable Rspec/ExampleLength + expect(WorkPackage.where(sprint: sprint1).to_h { [it.subject, it.position] }) + .to eql( + sprint1_wp1.subject => nil, + sprint1_wp2.subject => 1, + sprint1_wp3.subject => 2, + sprint1_wp4.subject => 2, + sprint1_wp5.subject => nil + ) + + expect(WorkPackage.where(sprint: sprint2).to_h { [it.subject, it.position] }) + .to eql( + sprint2_wp3.subject => 1, + sprint2_wp2.subject => 2, + sprint2_wp1.subject => 3 + ) + + expect(WorkPackage.where(sprint: nil).to_h { [it.subject, it.position] }) + .to eql( + no_sprint_wp1.subject => nil, + no_sprint_wp2.subject => nil, + no_sprint_wp3.subject => nil + ) + + expect(WorkPackage.where(sprint: sprint3).to_h { [it.subject, it.position] }) + .to eql( + sprint3_wp1.subject => 1, + sprint3_wp2.subject => 2, + sprint3_wp3.subject => 3 + ) + end + end + + context "with a no project provided" do + before do + described_class.new(project: nil).call + end + + it "fixes the positions while keeping those that have some in the same order in all projects" do # rubocop:disable Rspec/ExampleLength + expect(WorkPackage.where(sprint: sprint1).to_h { [it.subject, it.position] }) + .to eql( + sprint1_wp2.subject => 1, + sprint1_wp3.subject => 2, + sprint1_wp4.subject => 3, + sprint1_wp1.subject => 4, + sprint1_wp5.subject => 5 + ) + + expect(WorkPackage.where(sprint: sprint2).to_h { [it.subject, it.position] }) + .to eql( + sprint2_wp3.subject => 1, + sprint2_wp2.subject => 2, + sprint2_wp1.subject => 3 + ) + + expect(WorkPackage.where(sprint: nil).to_h { [it.subject, it.position] }) + .to eql( + no_sprint_wp1.subject => 1, + no_sprint_wp2.subject => 2, + no_sprint_wp3.subject => 3 + ) + + expect(WorkPackage.where(sprint: sprint3).to_h { [it.subject, it.position] }) + .to eql( + sprint3_wp1.subject => 1, + sprint3_wp2.subject => 2, + sprint3_wp3.subject => 3 + ) + end + end +end diff --git a/modules/backlogs/spec/support/pages/backlogs.rb b/modules/backlogs/spec/support/pages/backlogs.rb index 69ada685be59..92f42672e814 100644 --- a/modules/backlogs/spec/support/pages/backlogs.rb +++ b/modules/backlogs/spec/support/pages/backlogs.rb @@ -106,13 +106,6 @@ def click_in_story_menu(story, item_name) end end - def drag_in_sprint(moved, target, before: true) - moved_element = find(story_selector(moved)) - target_element = find(story_selector(target)) - - drag_n_drop_element from: moved_element, to: target_element, offset_x: 0, offset_y: before ? -5 : +10 - end - def fold_backlog(backlog) within_backlog(backlog) do find(:button, aria: { controls: "backlog_#{backlog.id}-list" }).click @@ -205,10 +198,6 @@ def expect_details_view(story) details_view end - def expect_create_work_package_dialog - expect(page).to have_css("#create-work-package-dialog") - end - private def within_story(story, &) @@ -219,6 +208,14 @@ def within_backlog(backlog, &) within(backlog_selector(backlog), &) end + def within_sprint(sprint, &) + within(sprint_selector(sprint), &) + end + + def sprint_selector(sprint) + test_selector("sprint-#{sprint.id}") + end + def backlog_selector(backlog) "#backlog_#{backlog.id}" end @@ -227,8 +224,8 @@ def story_selector(story) "#story_#{story.id}" end - def work_package_selector(story) - "#work_package_#{story.id}" + def work_package_selector(work_package) + test_selector("work-package-#{work_package.id}") end def within_menu_controlled_by(button) diff --git a/modules/backlogs/spec/support/pages/sprint_planning.rb b/modules/backlogs/spec/support/pages/sprint_planning.rb index 21020e999f69..c3a2ced275bc 100644 --- a/modules/backlogs/spec/support/pages/sprint_planning.rb +++ b/modules/backlogs/spec/support/pages/sprint_planning.rb @@ -80,6 +80,51 @@ def sprint_names_in_order page.find_all("#sprint_backlogs_container > section .CollapsibleHeader-title").map(&:text) end + def expect_work_packages_in_sprint_in_order(sprint, + work_packages: []) + raise ArgumentError, "work_packages should not be empty" if work_packages.empty? + + within_sprint(sprint) do + selectors = work_packages.map { |wp| work_package_selector(wp) } + + expect(page) + .to have_css(selectors.join(" + ")) + end + end + + def drag_work_package(moved, before: nil, into: nil) + raise ArgumentError, "You must specify a either before or into" unless before || into || (before && into) + + moved_element = find("#{work_package_selector(moved)} .DragHandle") + target_element = if before + find(work_package_selector(before)) + else + find(sprint_selector(into)) + end + + moved_element.native.drag_to(target_element.native, delay: 0.1) + rescue Capybara::Cuprite::ObsoleteNode + retry + end + + def expect_work_package_not_draggable(work_package) + expect(page) + .to have_no_css("#{work_package_selector(work_package)} .DragHandle") + end + + def expect_no_sprint_menu(sprint) + within_sprint(sprint) do + expect(page).to have_no_button(accessible_name: "Sprint actions") + end + end + + def expect_no_sprint_menu_item(sprint, item_name) + within_sprint_menu(sprint) do |_menu| + expect(page) + .to have_no_selector(:menuitem, text: item_name) + end + end + def expect_sprint_names_in_order(*sprint_names) expect(sprint_names_in_order).to eq(sprint_names) end @@ -161,6 +206,10 @@ def within_sprint_menu(backlog, &) end end + def within_work_package_row(work_package, &) + within(work_package_selector(work_package), &) + end + private def within_story(story, &) @@ -172,7 +221,7 @@ def within_sprint(sprint, &) end def sprint_selector(sprint) - "#agile_sprint_#{sprint.id}" + test_selector("sprint-#{sprint.id}") end def backlog_selector(backlog) @@ -183,8 +232,8 @@ def story_selector(story) "#story_#{story.id}" end - def work_package_selector(story) - "#work_package_#{story.id}" + def work_package_selector(work_package) + test_selector("work-package-#{work_package.id}") end def within_menu_controlled_by(button)