Address CR comments.

app/contracts/project_life_cycle_steps/base_contract.rb

@@ -31,9 +31,9 @@ module ProjectLifeCycleSteps
     attribute :start_date
     attribute :finish_date
 
-    validate :edit_project_phase_permission
+    validate :validate_edit_project_phase_permission
 
-    def edit_project_phase_permission
+    def validate_edit_project_phase_permission
       return if user.allowed_in_project?(:edit_project_phases, model.project)
 
       errors.add :base, :error_unauthorized

app/services/project_life_cycle_steps/preview_attributes_service.rb

@@ -1,32 +0,0 @@
-#-- copyright
-# OpenProject is an open source project management software.
-# Copyright (C) the OpenProject GmbH
-#
-# This program is free software; you can redistribute it and/or
-# modify it under the terms of the GNU General Public License version 3.
-#
-# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
-# Copyright (C) 2006-2013 Jean-Philippe Lang
-# Copyright (C) 2010-2013 the ChiliProject Team
-#
-# This program is free software; you can redistribute it and/or
-# modify it under the terms of the GNU General Public License
-# as published by the Free Software Foundation; either version 2
-# of the License, or (at your option) any later version.
-#
-# This program is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-# GNU General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License
-# along with this program; if not, write to the Free Software
-# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
-#
-# See COPYRIGHT and LICENSE files for more details.
-#++
-
-module ProjectLifeCycleSteps
-  class PreviewAttributesService < ::BaseServices::SetAttributes
-  end
-end

app/services/project_life_cycle_steps/update_service.rb

@@ -29,9 +29,7 @@
 module ProjectLifeCycleSteps
   class UpdateService < ::BaseServices::Update
     def after_perform(call)
-      life_cycle_step = call.result
-
-      project = life_cycle_step.project
+      project = call.result.project
       project.touch_and_save_journals
 
       call

modules/overviews/app/controllers/overviews/project_phases_controller.rb

@@ -41,7 +41,7 @@ module ::Overviews
     end
 
     def preview
-      service_call = ::ProjectLifeCycleSteps::PreviewAttributesService
+      service_call = ::ProjectLifeCycleSteps::SetAttributesService
                 .new(user: current_user,
                      model: @project_phase,
                      contract_class: ProjectLifeCycleSteps::UpdateContract)
@@ -57,25 +57,27 @@ module ::Overviews
     end
 
     def update
-      service_call = ::ProjectLifeCycleSteps::UpdateService.new(user: current_user, model: @project_phase)
+      service_call = ::ProjectLifeCycleSteps::UpdateService
+                      .new(user: current_user, model: @project_phase)
                       .call(permitted_params.project_phase)
-      if service_call.success?
-        update_via_turbo_stream(
-          component: Overviews::ProjectPhases::SidePanelComponent.new(project: @project)
-        )
-      else
-        update_via_turbo_stream(
-          component: Overviews::ProjectPhases::EditComponent.new(service_call.result)
-        )
-      end
 
-      respond_to_with_turbo_streams(status: service_call.success? ? :ok : :unprocessable_entity)
+      component, status =
+        if service_call.success?
+          [Overviews::ProjectPhases::SidePanelComponent.new(project: @project), :ok]
+        else
+          [Overviews::ProjectPhases::EditComponent.new(service_call.result), :unprocessable_entity]
+        end
+
+      update_via_turbo_stream(component:)
+      respond_to_with_turbo_streams(status:)
     end
 
     private
 
     def find_project_phase_and_project
-      @project_phase = Project::Phase.where(active: true).eager_load(:definition, :project).find(params[:id])
+      @project_phase = Project::Phase.where(active: true)
+                                     .eager_load(:definition, :project)
+                                     .find(params[:id])
       @project = @project_phase.project
     rescue ActiveRecord::RecordNotFound
       # TODO: Use rescue from in the controller, and remove all the rescue instances

spec/features/projects/life_cycle/overview_page/dialog/permission_spec.rb

@@ -72,8 +72,7 @@ RSpec.describe "Edit project phases on project overview page", :js, with_flag: {
     it "does not show the edit buttons" do
       overview_page.within_life_cycles_sidebar do
         project_life_cycles.each do |lc|
-          expect(page)
-            .to have_no_css("[data-test-selector='project-life-cycle-edit-button-#{lc.id}']")
+          expect(page).to have_no_link(href: edit_project_phase_path(lc))
         end
       end
     end
@@ -85,8 +84,7 @@ RSpec.describe "Edit project phases on project overview page", :js, with_flag: {
     it "shows the edit buttons" do
       overview_page.within_life_cycles_sidebar do
         project_life_cycles.each do |lc|
-          expect(page)
-            .to have_css("[data-test-selector='project-life-cycle-edit-button-#{lc.id}']")
+          expect(page).to have_link(href: edit_project_phase_path(lc))
         end
       end
     end

spec/features/projects/life_cycle/overview_page/dialog/update_spec.rb

@@ -83,12 +83,12 @@ RSpec.describe "Edit project phases on project overview page", :js, with_flag: {
         retry_block do
           # Retrying due to a race condition between filling the input vs submitting the form preview.
           original_dates = [life_cycle_initiating.start_date, life_cycle_initiating.finish_date]
-          dialog.set_date_for(life_cycle_initiating, values: original_dates)
+          dialog.set_date_for(values: original_dates)
 
           page.driver.clear_network_traffic
-          dialog.set_date_for(life_cycle_initiating, values: initiating_dates)
+          dialog.set_date_for(values: initiating_dates)
 
-          dialog.expect_caption(life_cycle_initiating, text: "Duration: 8 working days")
+          dialog.expect_caption(text: "Duration: 8 working days")
           # Ensure that only 1 ajax request is triggered after setting the date range.
           expect(page.driver.browser.network.traffic.size).to eq(1)
         end
@@ -97,22 +97,23 @@ RSpec.describe "Edit project phases on project overview page", :js, with_flag: {
         dialog.submit
         dialog.expect_closed
 
-        # Clear the value of life_cycle_planning
-        dialog = overview_page.open_edit_dialog_for_life_cycle(life_cycle_planning)
-        expect_angular_frontend_initialized
-
-        dialog.expect_input_for(life_cycle_planning)
-        dialog.clear_date_for(life_cycle_planning)
-
-        # Saving the dialog is successful
-        dialog.submit
-        dialog.expect_closed
-
         # Sidebar is refreshed with the updated values
         overview_page.within_life_cycle_container(life_cycle_initiating) do
           expect(page).to have_text initiating_dates.map { I18n.l(it) }.join("\n-\n")
         end
 
+        # Clear the value of life_cycle_planning
+        dialog = overview_page.open_edit_dialog_for_life_cycle(life_cycle_planning)
+        expect_angular_frontend_initialized
+
+        dialog.expect_input_for(life_cycle_planning)
+        dialog.clear_date
+
+        # Saving the dialog is successful
+        dialog.submit
+        dialog.expect_closed
+
+        # Sidebar is refreshed with the updated values
         ready_for_planning_dates = [
           life_cycle_planning.start_date,
           life_cycle_planning.finish_date

spec/support/components/projects/project_life_cycles/edit_dialog.rb

@@ -49,12 +49,12 @@ module Components
           close if close_after_yield
         end
 
-        def clear_date_for(step)
+        def clear_date
           find("input[id^='project_phase_date_range']").set ""
           find_by_id("edit-project-life-cycles-dialog-title").click
         end
 
-        def set_date_for(step, values:)
+        def set_date_for(values:)
           dialog_selector = "##{Overviews::ProjectPhases::EditDialogComponent::DIALOG_ID}"
 
           datepicker = Components::RangeDatepicker.new(dialog_selector)
@@ -115,27 +115,27 @@ module Components
           expect_input(step.name, value:)
         end
 
-        def expect_caption(step, text: nil, present: true)
+        def expect_caption(text: nil, present: true)
           selector = 'span[id^="caption"]'
-          expect_selector_for(step, selector:, text:, present:)
+          expect_selector_for(selector:, text:, present:)
         end
 
-        def expect_no_caption(step)
-          expect_caption(step, present: false)
+        def expect_no_caption
+          expect_caption(present: false)
         end
 
-        def expect_validation_message(step, text: nil, present: true)
+        def expect_validation_message(text: nil, present: true)
           selector = 'div[id^="validation"]'
-          expect_selector_for(step, selector:, text:, present:)
+          expect_selector_for(selector:, text:, present:)
         end
 
-        def expect_no_validation_message(step)
-          expect_validation_message(step, present: false)
+        def expect_no_validation_message
+          expect_validation_message(present: false)
         end
 
         private
 
-        def expect_selector_for(step, selector:, text: nil, present: true)
+        def expect_selector_for(selector:, text: nil, present: true)
           within_async_content do
             input_id = "#project_phase_date_range"
             parent = find(input_id).ancestor("primer-datepicker-field")