Add StartContract for sprint start validation

Introduces `Sprints::StartContract` inheriting from
`BaseContract` to validate permissions, sprint status, and
active sprint uniqueness at the service layer.

Refactors `Sprints::StartService` to use
`BaseServices::BaseContracted` with contract integration.

Author: myabc

config/locales/en.yml

@@ -2002,6 +2002,7 @@ en:
         not_editable: "cannot be edited because it is already in effect."
         not_current_user: "is not the current user."
         system_wide_non_working_day_exists: "conflicts with an existing system-wide non-working day for this date."
+        must_be_in_planning: "must be in planning to start."
         only_one_active_sprint_allowed: "only one active sprint is allowed per project."
         not_found: "not found."
         not_a_date: "is not a valid date."

modules/backlogs/app/contracts/sprints/start_contract.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.
+#++
+
+module Sprints
+  class StartContract < ::BaseContract
+    validate :validate_permission
+    validate :validate_status_in_planning
+    validate :validate_no_other_active_sprint
+
+    private
+
+    def validate_permission
+      return if user.allowed_in_project?(:start_complete_sprint, model.project)
+
+      errors.add :base, :error_unauthorized
+    end
+
+    def validate_status_in_planning
+      return if model.in_planning?
+
+      errors.add :status, :must_be_in_planning
+    end
+
+    def validate_no_other_active_sprint
+      return unless model.in_planning?
+      return unless Agile::Sprint.where(project: model.project).active.where.not(id: model.id).exists?
+
+      errors.add :status, :only_one_active_sprint_allowed
+    end
+  end
+end

modules/backlogs/app/controllers/rb_sprints_controller.rb

@@ -232,7 +232,7 @@ def converted_agile_sprint_params
   def start_sprint
     Sprints::StartService
       .new(user: current_user, model: @sprint)
-      .call
+      .call(send_notifications: false)
   end
 
   def finish_sprint

modules/backlogs/app/services/sprints/start_service.rb

@@ -28,45 +28,24 @@
 # See COPYRIGHT and LICENSE files for more details.
 #++
 
-class Sprints::StartService < BaseServices::BaseCallable
-  include Shared::ServiceContext
-
-  attr_reader :user, :model
-
-  def initialize(user:, model:)
-    super()
-    @user = user
-    @model = model
-  end
-
-  def perform
-    in_context(model, send_notifications: false) do
-      start_sprint
-    end
+class Sprints::StartService < BaseServices::BaseContracted
+  def initialize(user:, model:, contract_class: Sprints::StartContract)
+    super(user:, contract_class:)
+    self.model = model
   end
 
   private
 
-  def start_sprint
-    return unsuccessful_start_result unless model.in_planning?
-
+  def persist(service_call)
     result = ensure_task_board
     return result if result.failure?
 
     model.active!
 
-    ServiceResult.success(result: model)
-  rescue ActiveRecord::RecordInvalid
-    unsuccessful_start_result
+    service_call
   rescue ActiveRecord::RecordNotUnique
     add_only_one_active_sprint_error
-    unsuccessful_start_result
-  end
-
-  def unsuccessful_start_result
-    ServiceResult.failure(result: model,
-                          errors: model.errors,
-                          message: unsuccessful_start_message)
+    ServiceResult.failure(result: model, errors: model.errors)
   end
 
   def ensure_task_board
@@ -78,10 +57,6 @@ def ensure_task_board
       .call(project: model.project, sprint: model, name: model.board_name)
   end
 
-  def unsuccessful_start_message
-    model.errors.full_messages.to_sentence if model.errors.any?
-  end
-
   def add_only_one_active_sprint_error
     return if model.errors.added?(:status, :only_one_active_sprint_allowed)
 

modules/backlogs/spec/contracts/sprints/start_contract_spec.rb

@@ -0,0 +1,111 @@
+# 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 Sprints::StartContract do
+  let(:project) { create(:project) }
+  let(:user) { create(:user) }
+  let(:sprint) do
+    create(:agile_sprint,
+           project:,
+           status: sprint_status)
+  end
+  let(:sprint_status) { "in_planning" }
+  let(:permissions) { [:start_complete_sprint] }
+
+  subject(:contract) { described_class.new(sprint, user) }
+
+  before do
+    mock_permissions_for(user) do |mock|
+      mock.allow_in_project(*permissions, project:)
+    end
+  end
+
+  describe "validation" do
+    context "with valid sprint and permissions" do
+      it "is valid" do
+        expect(contract.validate).to be(true)
+      end
+    end
+
+    context "without start_complete_sprint permission" do
+      let(:permissions) { [:view_work_packages] }
+
+      it "is invalid" do
+        expect(contract.validate).to be(false)
+        expect(contract.errors.symbols_for(:base)).to include(:error_unauthorized)
+      end
+    end
+
+    context "when sprint is active" do
+      let(:sprint_status) { "active" }
+
+      it "is invalid" do
+        expect(contract.validate).to be(false)
+        expect(contract.errors.symbols_for(:status)).to include(:must_be_in_planning)
+      end
+    end
+
+    context "when sprint is completed" do
+      let(:sprint_status) { "completed" }
+
+      it "is invalid" do
+        expect(contract.validate).to be(false)
+        expect(contract.errors.symbols_for(:status)).to include(:must_be_in_planning)
+      end
+    end
+
+    context "when another active sprint exists in the project" do
+      before do
+        create(:agile_sprint,
+               project:,
+               status: "active")
+      end
+
+      it "is invalid" do
+        expect(contract.validate).to be(false)
+        expect(contract.errors.symbols_for(:status)).to include(:only_one_active_sprint_allowed)
+      end
+    end
+
+    context "when an active sprint exists in a different project" do
+      before do
+        create(:agile_sprint,
+               project: create(:project),
+               status: "active")
+      end
+
+      it "is valid" do
+        expect(contract.validate).to be(true)
+      end
+    end
+  end
+end

modules/backlogs/spec/services/sprints/start_service_spec.rb

@@ -40,7 +40,7 @@
   let(:user) { create(:admin) }
   let(:instance) { described_class.new(user:, model: sprint) }
 
-  subject(:result) { instance.call }
+  subject(:result) { instance.call(send_notifications: false) }
 
   before do
     create(:workflow, type: type_task, old_status: status1, new_status: status2, role: create(:project_role))
@@ -96,20 +96,19 @@
 
     it "returns failure and leaves the sprint in planning", :aggregate_failures do
       expect(result).not_to be_success
-      expect(result.message).to eq("something went wrong")
       expect(sprint.reload).to be_in_planning
       expect(sprint.task_board_for(project)).to be_nil
     end
   end
 
-  context "when sprint activation fails after board creation" do
+  context "when another active sprint exists in the project" do
     let!(:active_sprint) { create(:agile_sprint, project:, status: "active") }
 
-    it "rolls back the created board", :aggregate_failures do
+    it "fails contract validation without creating a board", :aggregate_failures do
       expect(result).not_to be_success
+      expect(result.errors.symbols_for(:status)).to include(:only_one_active_sprint_allowed)
       expect(sprint.reload).to be_in_planning
       expect(sprint.task_board_for(project)).to be_nil
-      expect(result.message).to eq(sprint.errors.full_messages.to_sentence)
     end
   end
 
@@ -123,7 +122,7 @@
     it "returns failure with the active sprint error", :aggregate_failures do
       expect(result).not_to be_success
       expect(result.errors[:status]).to include("only one active sprint is allowed per project.")
-      expect(result.message).to eq(sprint.errors.full_messages.to_sentence)
+      expect(result.message).to be_present
       expect(sprint.reload).to be_in_planning
       expect(sprint.task_board_for(project)).to be_nil
     end
@@ -132,22 +131,30 @@
   context "when the sprint is already active" do
     let(:status) { "active" }
 
-    it "returns failure and leaves the sprint unchanged", :aggregate_failures do
+    it "fails contract validation", :aggregate_failures do
       expect(result).not_to be_success
-      expect(result.message).to be_blank
+      expect(result.errors.symbols_for(:status)).to include(:must_be_in_planning)
       expect(sprint.reload).to be_active
-      expect(sprint.task_board_for(project)).to be_nil
     end
   end
 
   context "when the sprint is already completed" do
     let(:status) { "completed" }
 
-    it "returns failure and leaves the sprint unchanged", :aggregate_failures do
+    it "fails contract validation", :aggregate_failures do
       expect(result).not_to be_success
-      expect(result.message).to be_blank
+      expect(result.errors.symbols_for(:status)).to include(:must_be_in_planning)
       expect(sprint.reload).to be_completed
-      expect(sprint.task_board_for(project)).to be_nil
+    end
+  end
+
+  context "when the user lacks permission" do
+    let(:user) { create(:user) }
+
+    it "fails contract validation", :aggregate_failures do
+      expect(result).not_to be_success
+      expect(result.errors.symbols_for(:base)).to include(:error_unauthorized)
+      expect(sprint.reload).to be_in_planning
     end
   end
 end