Merge branch 'release/15.1' into release/15.2

app/components/work_package_relations_tab/add_work_package_child_form_component.rb

@@ -47,6 +47,6 @@ class WorkPackageRelationsTab::AddWorkPackageChildFormComponent < ApplicationCom
 
   def submit_url_options
     { method: :post,
-      url: work_package_children_path(@work_package) }
+      url: work_package_children_relations_path(@work_package) }
   end
 end

app/components/work_package_relations_tab/index_component.html.erb

@@ -37,7 +37,7 @@
             if should_render_add_child?
               menu.with_item(
                 label: t("#{I18N_NAMESPACE}.relations.label_child_singular").capitalize,
-                href: new_work_package_child_path(@work_package),
+                href: new_work_package_children_relation_path(@work_package),
                 test_selector: new_button_test_selector(relation_type: :child),
                 content_arguments: {
                   data: { turbo_stream: true }

app/components/work_package_relations_tab/index_component.rb

@@ -28,6 +28,8 @@ class WorkPackageRelationsTab::IndexComponent < ApplicationComponent
   private
 
   def should_render_add_child?
+    return false if @work_package.milestone?
+
     helpers.current_user.allowed_in_project?(:manage_subtasks, @work_package.project)
   end
 

app/components/work_package_relations_tab/relation_component.rb

@@ -89,7 +89,7 @@ class WorkPackageRelationsTab::RelationComponent < ApplicationComponent
 
   def destroy_path
     if parent_child_relationship?
-      work_package_child_path(@work_package, @child)
+      work_package_children_relation_path(@work_package, @child)
     else
       work_package_relation_path(@work_package, @relation)
     end

app/controllers/work_package_children_relations_controller.rb

@@ -28,7 +28,7 @@
 # See COPYRIGHT and LICENSE files for more details.
 #++
 
-class WorkPackageChildrenController < ApplicationController
+class WorkPackageChildrenRelationsController < ApplicationController
   include OpTurbo::ComponentStream
   include OpTurbo::DialogStreamHelper
 
@@ -36,9 +36,6 @@ class WorkPackageChildrenController < ApplicationController
 
   before_action :authorize # Short-circuit early if not authorized
 
-  before_action :set_child, except: %i[new create]
-  before_action :set_relations, except: %i[new create]
-
   def new
     component = WorkPackageRelationsTab::AddWorkPackageChildDialogComponent
       .new(work_package: @work_package)
@@ -46,39 +43,34 @@ class WorkPackageChildrenController < ApplicationController
   end
 
   def create
-    target_work_package_id = params[:work_package][:id]
-    target_child_work_package = WorkPackage.find(target_work_package_id)
+    child = WorkPackage.find(params[:work_package][:id])
+    service_result = set_relation(child:, parent: @work_package)
 
-    target_child_work_package.parent = @work_package
-
-    if target_child_work_package.save
-      @children = @work_package.children.visible
-      @relations = @work_package.relations.visible
-
-      component = WorkPackageRelationsTab::IndexComponent.new(
-        work_package: @work_package,
-        relations: @relations,
-        children: @children,
-        scroll_to_id: target_work_package_id
-      )
-      replace_via_turbo_stream(component:)
-      update_flash_message_via_turbo_stream(
-        message: I18n.t(:notice_successful_update), scheme: :success
-      )
-      respond_with_turbo_streams
-    end
+    respond_with_relations_tab_update(service_result, scroll_to_id: target_work_package_id)
   end
 
   def destroy
-    @child.parent = nil
+    child = WorkPackage.find(params[:id])
+    service_result = set_relation(child:, parent: nil)
 
-    if @child.save
+    respond_with_relations_tab_update(service_result)
+  end
+
+  private
+
+  def set_relation(child:, parent:)
+    WorkPackages::UpdateService.new(user: current_user, model: child)
+                               .call(parent:)
+  end
+
+  def respond_with_relations_tab_update(service_result, **)
+    if service_result.success?
       @work_package.reload
-      @children = @work_package.children.visible
       component = WorkPackageRelationsTab::IndexComponent.new(
         work_package: @work_package,
-        relations: @relations,
-        children: @children
+        relations: @work_package.relations.visible,
+        children: @work_package.children.visible,
+        **
       )
       replace_via_turbo_stream(component:)
       update_flash_message_via_turbo_stream(
@@ -86,21 +78,13 @@ class WorkPackageChildrenController < ApplicationController
       )
 
       respond_with_turbo_streams
+    else
+      respond_with_turbo_streams(status: :unprocessable_entity)
     end
   end
 
-  private
-
   def set_work_package
     @work_package = WorkPackage.find(params[:work_package_id])
     @project = @work_package.project
   end
-
-  def set_child
-    @child = WorkPackage.find(params[:id])
-  end
-
-  def set_relations
-    @relations = @work_package.relations.visible
-  end
 end

app/controllers/work_package_relations_controller.rb

@@ -63,8 +63,8 @@ class WorkPackageRelationsController < ApplicationController
       target_work_package_id = params[:relation][:to_id]
       @work_package.reload
       component = WorkPackageRelationsTab::IndexComponent.new(work_package: @work_package,
-                                                              relations: @work_package.relations,
-                                                              children: @work_package.children,
+                                                              relations: @work_package.relations.visible,
+                                                              children: @work_package.children.visible,
                                                               scroll_to_id: target_work_package_id)
       replace_via_turbo_stream(component:)
       respond_with_turbo_streams
@@ -82,8 +82,8 @@ class WorkPackageRelationsController < ApplicationController
     if service_result.success?
       @work_package.reload
       component = WorkPackageRelationsTab::IndexComponent.new(work_package: @work_package,
-                                                              relations: @work_package.relations,
-                                                              children: @work_package.children)
+                                                              relations: @work_package.relations.visible,
+                                                              children: @work_package.children.visible)
       replace_via_turbo_stream(component:)
       respond_with_turbo_streams
     else
@@ -95,11 +95,12 @@ class WorkPackageRelationsController < ApplicationController
     service_result = Relations::DeleteService.new(user: current_user, model: @relation).call
 
     if service_result.success?
-      @children = WorkPackage.where(parent_id: @work_package.id)
+      @children = WorkPackage.where(parent_id: @work_package.id).visible
       @relations = @work_package
         .relations
         .reload
         .includes(:to, :from)
+        .visible
 
       component = WorkPackageRelationsTab::IndexComponent.new(work_package: @work_package,
                                                               relations: @relations,

app/controllers/work_package_relations_tab_controller.rb

@@ -33,9 +33,10 @@ class WorkPackageRelationsTabController < ApplicationController
   before_action :authorize_global
 
   def index
-    @children = WorkPackage.where(parent_id: @work_package.id)
+    @children = WorkPackage.where(parent_id: @work_package.id).visible
     @relations = @work_package
       .relations
+      .visible
       .includes(:to, :from)
 
     component = WorkPackageRelationsTab::IndexComponent.new(

config/initializers/menus.rb

@@ -680,7 +680,7 @@ Redmine::MenuManager.map :work_package_split_view do |menu|
             { tab: :relations },
             skip_permissions_check: true,
             badge: ->(work_package:, **) {
-              work_package.relations.count + work_package.children.count
+              work_package.relations.visible.count + work_package.children.visible.count
             },
             caption: :"js.work_packages.tabs.relations"
   menu.push :watchers,

config/initializers/permissions.rb

@@ -354,7 +354,7 @@ Rails.application.reloader.to_prepare do
 
       wpt.permission :manage_subtasks,
                      {
-                       work_package_children: %i[new create destroy]
+                       work_package_children_relations: %i[new create destroy]
                      },
                      permissible_on: :project,
                      dependencies: :view_work_packages

config/routes.rb

@@ -610,7 +610,7 @@ Rails.application.routes.draw do
       end
     end
 
-    resources :children, only: %i[new create destroy], controller: "work_package_children"
+    resources :children_relations, only: %i[new create destroy], controller: "work_package_children_relations"
 
     resource :progress, only: %i[new edit update], controller: "work_packages/progress"
     collection do

spec/controllers/work_package_children_relations_controller_spec.rb

@@ -30,11 +30,13 @@
 
 require "spec_helper"
 
-RSpec.describe WorkPackageChildrenController do
+RSpec.describe WorkPackageChildrenRelationsController do
   shared_let(:user) { create(:admin) }
-  shared_let(:project) { create(:project) }
-  shared_let(:work_package) { create(:work_package, project:) }
-  shared_let(:child_work_package) { create(:work_package, parent: work_package, project:) }
+  shared_let(:task_type) { create(:type_task) }
+  shared_let(:milestone_type) { create(:type_milestone) }
+  shared_let(:project) { create(:project, types: [task_type, milestone_type]) }
+  shared_let(:work_package) { create(:work_package, project:, type: task_type) }
+  shared_let(:child_work_package) { create(:work_package, parent: work_package, project:, type: task_type) }
 
   current_user { user }
 
@@ -55,25 +57,64 @@ RSpec.describe WorkPackageChildrenController do
     end
   end
 
-  describe "DELETE /work_packages/:work_package_id/children/:id" do
-    before do
-      allow(WorkPackageRelationsTab::IndexComponent).to receive(:new).and_call_original
-      allow(controller).to receive(:replace_via_turbo_stream).and_call_original
+  describe "POST /work_packages/:work_package_id/children" do
+    shared_let(:future_child_work_package) { create(:work_package, project:) }
+
+    it "creates a child relationship" do
+      post("create", params: { work_package_id: work_package.id,
+                               work_package: { id: future_child_work_package.id } },
+                     as: :turbo_stream)
+      expect(response).to have_http_status(:ok)
+      expect(future_child_work_package.reload.parent).to eq(work_package)
     end
 
-    it "deletes the child relationship" do
+    it "can't create a child relationship for a milestone work package" do
+      work_package.update(type: milestone_type)
+      post("create", params: { work_package_id: work_package.id,
+                               work_package: { id: future_child_work_package.id } },
+                     as: :turbo_stream)
+      expect(response).to have_http_status(:unprocessable_entity)
+      expect(future_child_work_package.reload.parent).to be_nil
+    end
+  end
+
+  describe "DELETE /work_packages/:work_package_id/children/:id" do
+    def send_delete_request
       delete("destroy",
              params: { work_package_id: work_package.id,
                        id: child_work_package.id },
              as: :turbo_stream)
+    end
 
-      expect(response).to be_successful
+    it "deletes the child relationship" do
+      send_delete_request
+
+      expect(response).to have_http_status(:ok)
+      expect(child_work_package.reload.parent).to be_nil
+    end
+
+    it "renders the relations tab index component" do
+      allow(WorkPackageRelationsTab::IndexComponent).to receive(:new).and_call_original
+      allow(controller).to receive(:replace_via_turbo_stream).and_call_original
+
+      send_delete_request
 
       expect(WorkPackageRelationsTab::IndexComponent).to have_received(:new)
         .with(work_package:, relations: [], children: [])
       expect(controller).to have_received(:replace_via_turbo_stream)
         .with(component: an_instance_of(WorkPackageRelationsTab::IndexComponent))
-      expect(child_work_package.reload.parent).to be_nil
+    end
+
+    it "updates dependent work packages" do
+      allow(WorkPackages::UpdateAncestorsService).to receive(:new).and_call_original
+      allow(WorkPackages::SetScheduleService).to receive(:new).and_call_original
+
+      send_delete_request
+
+      expect(WorkPackages::UpdateAncestorsService).to have_received(:new)
+        .with(user: user, work_package: child_work_package)
+      expect(WorkPackages::SetScheduleService).to have_received(:new)
+        .with(a_hash_including(work_package: [child_work_package]))
     end
   end
 end

spec/features/work_packages/details/relations/hierarchy_milestone_spec.rb

@@ -28,30 +28,34 @@
 
 require "spec_helper"
 
-RSpec.describe "work package hierarchies for milestones", :js, :selenium do
+RSpec.describe "work package hierarchies for milestones", :js, :with_cuprite do
   let(:user) { create(:admin) }
-  let(:type) { create(:type, is_milestone: true) }
-  let(:project) { create(:project, types: [type]) }
-  let(:work_package) { create(:work_package, project:, type:) }
-  let(:relations) { Components::WorkPackages::Relations.new(work_package) }
-  let(:tabs) { Components::WorkPackages::Tabs.new(work_package) }
-  let(:wp_page) { Pages::FullWorkPackage.new(work_package) }
-
-  let(:relations_tab) { find(".op-tab-row--link_selected", text: "RELATIONS") }
-  let(:visit) { true }
+  let(:task_type) { create(:type_task) }
+  let(:milestone_type) { create(:type_milestone) }
+  let(:project) { create(:project, types: [task_type, milestone_type]) }
+  let!(:milestone_work_package) { create(:work_package, subject: "milestone_work_package", project:, type: milestone_type) }
+  let!(:task_work_package) { create(:work_package, subject: "task_work_package", project:, type: task_type) }
+  let(:relations) { Components::WorkPackages::Relations.new }
 
   before do
     login_as user
+  end
+
+  def visit_relations_tab_for(work_package)
+    wp_page = Pages::FullWorkPackage.new(work_package)
     wp_page.visit_tab!("relations")
     expect_angular_frontend_initialized
     wp_page.expect_subject
     loading_indicator_saveguard
   end
 
-  it "does not provide links to add children or existing children (Regression #28745)" do
-    expect(page).to have_no_text("Add existing child")
-    expect(page).to have_no_text("Create new child")
-    expect(page).to have_no_css("wp-inline-create--add-link")
-    expect(page).to have_no_text("Children")
+  it "does not provide links to add children or existing children (Regression #28745 and #60512)" do
+    # A work package has a menu entry to link a child
+    visit_relations_tab_for(task_work_package)
+    relations.expect_new_relation_type("Child")
+
+    # A milestone work package does NOT have a menu entry to link a child
+    visit_relations_tab_for(milestone_work_package)
+    relations.expect_no_new_relation_type("Child")
   end
 end

spec/features/work_packages/details/relations/primerized_relations_spec.rb

@@ -32,8 +32,16 @@ RSpec.describe "Primerized work package relations tab",
                :js, :with_cuprite do
   include Components::Autocompleter::NgSelectAutocompleteHelpers
 
-  shared_let(:user) { create(:admin) }
   shared_let(:project) { create(:project) }
+  shared_let(:user) do
+    create(:user,
+           member_with_permissions: {
+             project => %i[add_work_packages
+                           manage_subtasks
+                           manage_work_package_relations
+                           view_work_packages]
+           })
+  end
 
   before_all do
     set_factory_default(:user, user)
@@ -41,17 +49,17 @@ RSpec.describe "Primerized work package relations tab",
     set_factory_default(:project_with_types, project)
   end
 
-  shared_let(:parent_work_package) { create(:work_package, subject: "parent") }
-  shared_let(:work_package) { create(:work_package, subject: "main", parent: parent_work_package) }
+  shared_let(:parent_work_package) { create(:work_package, subject: "parent_work_package") }
+  shared_let(:work_package) { create(:work_package, subject: "work_package (main)", parent: parent_work_package) }
   shared_let(:type1) { create(:type) }
   shared_let(:type2) { create(:type) }
 
   shared_let(:wp_predecessor) do
-    create(:work_package, type: type1, subject: "predecessor of main",
+    create(:work_package, type: type1, subject: "wp_predecessor",
                           start_date: Date.current, due_date: Date.current + 1.week)
   end
-  shared_let(:wp_related) { create(:work_package, type: type2, subject: "related to main") }
-  shared_let(:wp_blocker) { create(:work_package, type: type1, subject: "blocks main") }
+  shared_let(:wp_related) { create(:work_package, type: type2, subject: "wp_related") }
+  shared_let(:wp_blocker) { create(:work_package, type: type1, subject: "wp_blocker") }
 
   shared_let(:relation_follows) do
     create(:relation,
@@ -73,16 +81,39 @@ RSpec.describe "Primerized work package relations tab",
   end
   shared_let(:child_wp) do
     create(:work_package,
+           subject: "child_wp",
            parent: work_package,
            type: type1,
            project: project)
   end
-  shared_let(:not_yet_child_wp) do
+  shared_let(:not_child_yet_wp) do
     create(:work_package,
+           subject: "not_child_yet_wp",
            type: type1,
            project:)
   end
 
+  # The user should not be able to see any relations to work packages from this
+  # project because the user does not have the permissions to view this project
+  shared_let(:restricted_project) { create(:project) }
+  shared_let(:restricted_work_package) do
+    create(:work_package,
+           subject: "restricted_work_package",
+           project: restricted_project)
+  end
+  shared_let(:restricted_child_work_package) do
+    create(:work_package,
+           subject: "restricted_child_work_package",
+           parent: work_package,
+           project: restricted_project)
+  end
+  shared_let(:restricted_relation_relates) do
+    create(:relation,
+           from: work_package,
+           to: restricted_work_package,
+           relation_type: Relation::TYPE_RELATES)
+  end
+
   let(:relations_tab) { Components::WorkPackages::Relations.new(work_package) }
   let(:relations_panel_selector) { ".detail-panel--relations" }
   let(:relations_panel) { find(relations_panel_selector) }
@@ -119,6 +150,10 @@ RSpec.describe "Primerized work package relations tab",
       relations_tab.expect_relation(relation_follows)
       relations_tab.expect_relation(relation_relates)
       relations_tab.expect_relation(relation_blocked)
+
+      # Relations not visible due to lack of permissions on the project
+      relations_tab.expect_no_relation(restricted_relation_relates)
+      relations_tab.expect_no_relation(restricted_child_work_package)
     end
   end
 
@@ -133,6 +168,10 @@ RSpec.describe "Primerized work package relations tab",
       expect { relation_follows.reload }.to raise_error(ActiveRecord::RecordNotFound)
 
       tabs.expect_counter("relations", 3)
+
+      # Relations not visible due to lack of permissions on the project
+      relations_tab.expect_no_relation(restricted_relation_relates)
+      relations_tab.expect_no_relation(restricted_child_work_package)
     end
 
     it "can delete children" do
@@ -144,6 +183,10 @@ RSpec.describe "Primerized work package relations tab",
       expect(child_wp.reload.parent).to be_nil
 
       tabs.expect_counter("relations", 3)
+
+      # Relations not visible due to lack of permissions on the project
+      relations_tab.expect_no_relation(restricted_relation_relates)
+      relations_tab.expect_no_relation(restricted_child_work_package)
     end
   end
 
@@ -165,6 +208,8 @@ RSpec.describe "Primerized work package relations tab",
 
       # Unchanged
       tabs.expect_counter("relations", 4)
+      relations_tab.expect_no_relation(restricted_relation_relates)
+      relations_tab.expect_no_relation(restricted_child_work_package)
 
       # Edit again
       relations_tab.edit_relation_description(relation_follows, "And they can be edited!")
@@ -174,6 +219,10 @@ RSpec.describe "Primerized work package relations tab",
 
       # Unchanged
       tabs.expect_counter("relations", 4)
+
+      # Relations not visible due to lack of permissions on the project
+      relations_tab.expect_no_relation(restricted_relation_relates)
+      relations_tab.expect_no_relation(restricted_child_work_package)
     end
 
     it "does not have an edit action for children" do
@@ -246,6 +295,10 @@ RSpec.describe "Primerized work package relations tab",
       tabs.expect_counter("relations", 5)
       # Relation is created
       expect(Relation.follows.where(from: wp_successor, to: work_package)).to exist
+
+      # Relations not visible due to lack of permissions on the project
+      relations_tab.expect_no_relation(restricted_relation_relates)
+      relations_tab.expect_no_relation(restricted_child_work_package)
     end
 
     it "does not autocomplete unrelatable work packages" do
@@ -278,11 +331,18 @@ RSpec.describe "Primerized work package relations tab",
 
       tabs.expect_counter("relations", 4)
 
-      relations_tab.add_existing_child(not_yet_child_wp)
-      relations_tab.expect_child(not_yet_child_wp)
+      relations_tab.add_existing_child(not_child_yet_wp)
+      relations_tab.expect_child(not_child_yet_wp)
 
       # Bumped by one
       tabs.expect_counter("relations", 5)
+
+      # Child relation is created
+      expect(not_child_yet_wp.reload.parent).to eq work_package
+
+      # Relations not visible due to lack of permissions on the project
+      relations_tab.expect_no_relation(restricted_relation_relates)
+      relations_tab.expect_no_relation(restricted_child_work_package)
     end
 
     it "doesn't autocomplete parent, children, and WP itself" do

spec/support/components/work_packages/relations.rb

@@ -39,7 +39,7 @@ module Components
 
       attr_reader :work_package
 
-      def initialize(work_package)
+      def initialize(work_package = nil)
         @work_package = work_package
       end
 
@@ -77,17 +77,44 @@ module Components
 
       def expect_no_row(relatable)
         actual_relatable = find_relatable(relatable)
-        expect(page).not_to have_test_selector("op-relation-row-#{actual_relatable.id}")
+        expect(page).not_to have_test_selector("op-relation-row-#{actual_relatable.id}"),
+                            "expected no relation row for work package " \
+                            "##{actual_relatable.id} #{actual_relatable.subject.inspect}"
       end
 
       def select_relation_type(relation_type)
-        page.find_test_selector("new-relation-action-menu").click
-
-        within page.find_by_id("new-relation-action-menu-list") do
+        within_new_relation_action_menu do
           click_link_or_button relation_type
         end
       end
 
+      def expect_new_relation_type(relation_type)
+        within_new_relation_action_menu do
+          expect(page).to have_link(relation_type, wait: 1)
+        end
+      end
+
+      def expect_no_new_relation_type(relation_type)
+        within_new_relation_action_menu do
+          expect(page).to have_no_link(relation_type, wait: 1)
+        end
+      end
+
+      def open_new_relation_action_menu
+        return if new_relation_action_menu.visible?
+
+        new_relation_button.click
+      end
+
+      def new_relation_action_menu
+        action_menu_id = new_relation_button["aria-controls"]
+        page.find(id: action_menu_id, visible: :all)
+      end
+
+      def new_relation_button
+        page.find_test_selector("new-relation-action-menu").find_button
+      end
+
       def remove_relation(relatable)
         actual_relatable = find_relatable(relatable)
         relatable_row = find_row(actual_relatable)
@@ -317,6 +344,13 @@ module Components
 
         expect_no_row(work_package)
       end
+
+      private
+
+      def within_new_relation_action_menu(&)
+        open_new_relation_action_menu
+        within(new_relation_action_menu, &)
+      end
     end
   end
 end