move backlog bucket validations from model to contract patch of wp

Author: toy

modules/backlogs/lib/open_project/backlogs/patches/base_contract_patch.rb

@@ -40,10 +40,25 @@ module OpenProject::Backlogs::Patches::BaseContractPatch
               # This also covers the check for backlogs being active
               permission: :manage_sprint_items
 
+    validate :backlog_bucket_xor_sprint
+    validate :backlog_bucket_belongs_to_project
     validate :sprint_shared_with_project
 
     private
 
+    def backlog_bucket_xor_sprint
+      return unless model.backlog_bucket && model.sprint
+
+      errors.add :base, :backlog_bucket_xor_sprint
+    end
+
+    def backlog_bucket_belongs_to_project
+      return unless model.backlog_bucket
+      return if model.backlog_bucket.project == model.project
+
+      errors.add :backlog_bucket, :backlog_bucket_from_another_project
+    end
+
     def sprint_shared_with_project
       return if model.sprint.nil? ||
                 Agile::Sprint.for_project(model.project).exists?(id: model.sprint_id)

modules/backlogs/lib/open_project/backlogs/patches/work_package_patch.rb

@@ -46,9 +46,6 @@ module OpenProject::Backlogs::Patches::WorkPackagePatch
     belongs_to :backlog_bucket, class_name: "Agile::BacklogBucket", optional: true
     belongs_to :sprint, class_name: "Agile::Sprint", optional: true
 
-    validate :backlog_bucket_xor_sprint
-    validate :backlog_bucket_belongs_to_project
-
     include OpenProject::Backlogs::List
   end
 
@@ -66,21 +63,6 @@ def done?
     def backlogs_enabled?
       project&.backlogs_enabled?
     end
-
-    private
-
-    def backlog_bucket_xor_sprint
-      return unless backlog_bucket && sprint
-
-      errors.add :base, :backlog_bucket_xor_sprint
-    end
-
-    def backlog_bucket_belongs_to_project
-      return unless backlog_bucket
-      return if backlog_bucket.project == project
-
-      errors.add :backlog_bucket, :backlog_bucket_from_another_project
-    end
   end
 end
 

modules/backlogs/spec/contracts/work_packages/shared_contract_examples.rb

@@ -149,6 +149,32 @@
 
       it_behaves_like "contract is invalid", position: :error_readonly
     end
+
+    describe "when attaching to a backlog bucket" do
+      before do
+        work_package.backlog_bucket = backlog_bucket
+      end
+
+      context "when only backlog bucket is set" do
+        let(:work_package_sprint) { nil }
+        let(:backlog_bucket) { build_stubbed(:backlog_bucket, project: work_package_project) }
+
+        it_behaves_like "contract is valid"
+      end
+
+      context "when both sprint and backlog bucket are set" do
+        let(:backlog_bucket) { build_stubbed(:backlog_bucket, project: work_package_project) }
+
+        it_behaves_like "contract is invalid", base: :backlog_bucket_xor_sprint
+      end
+
+      context "when backlog bucket belongs to a different project" do
+        let(:work_package_sprint) { nil }
+        let(:backlog_bucket) { build_stubbed(:backlog_bucket, project: build_stubbed(:project)) }
+
+        it_behaves_like "contract is invalid", backlog_bucket: :backlog_bucket_from_another_project
+      end
+    end
   end
 
   describe "writable_attributes" do

modules/backlogs/spec/models/work_package_spec.rb

@@ -36,97 +36,6 @@
     it { is_expected.to belong_to(:sprint).class_name("Agile::Sprint").optional(true) }
   end
 
-  describe "can't be both in backlog bucket and sprint" do
-    shared_let(:project) { create(:project) }
-    shared_let(:backlog_bucket) { create(:backlog_bucket, project:) }
-    shared_let(:sprint) { create(:agile_sprint, project:) }
-
-    context "when already in a backlog bucket" do
-      let(:work_package) { create(:work_package, project:, backlog_bucket:) }
-
-      it "fails validation when being added to sprint" do
-        work_package.assign_attributes(sprint:)
-
-        expect(work_package).not_to be_valid
-        expect(work_package.errors[:base]).to include(/cannot be assigned to both a sprint and a backlog bucket/)
-      end
-
-      it "fails being added to sprint" do
-        work_package.assign_attributes(sprint:)
-
-        expect do
-          work_package.save!(validate: false)
-        end.to raise_error(ActiveRecord::CheckViolation)
-      end
-
-      it "allows changing backlog bucket" do
-        work_package.update(backlog_bucket: create(:backlog_bucket, project:))
-      end
-
-      it "allows replacing backlog bucket with sprint" do
-        work_package.update(backlog_bucket: nil, sprint:)
-      end
-    end
-
-    context "when already in a sprint" do
-      let(:work_package) { create(:work_package, project:, sprint:) }
-
-      it "fails validation when being added to backlog bucket" do
-        work_package.assign_attributes(backlog_bucket:)
-
-        expect(work_package).not_to be_valid
-        expect(work_package.errors[:base]).to include(/cannot be assigned to both a sprint and a backlog bucket/)
-      end
-
-      it "fails being added to backlog bucket" do
-        work_package.assign_attributes(backlog_bucket:)
-
-        expect do
-          work_package.save!(validate: false)
-        end.to raise_error(ActiveRecord::CheckViolation)
-      end
-
-      it "allows changing sprint" do
-        work_package.update(sprint: create(:agile_sprint, project:))
-      end
-
-      it "allows replacing sprint with backlog bucket" do
-        work_package.update(backlog_bucket:, sprint: nil)
-      end
-    end
-  end
-
-  describe "backlog bucket must belong to the same project" do
-    shared_let(:project) { create(:project) }
-    let(:other_project) { create(:project) }
-    let(:work_package) { build(:work_package, project:, backlog_bucket:) }
-
-    context "when backlog bucket belongs to the same project" do
-      let(:backlog_bucket) { create(:backlog_bucket, project:) }
-
-      it "is valid" do
-        expect(work_package).to be_valid
-      end
-    end
-
-    context "when backlog bucket belongs to a different project" do
-      let(:backlog_bucket) { create(:backlog_bucket, project: other_project) }
-
-      it "is invalid" do
-        expect(work_package).not_to be_valid
-        expect(work_package.errors[:backlog_bucket]).to include(/must belong to the same project/)
-      end
-    end
-
-    context "when backlog bucket is nil" do
-      let(:backlog_bucket) { nil }
-
-      it "is valid" do
-        expect(work_package).to be_valid
-      end
-    end
-  end
-
   describe ".order_by_position" do
     let(:work_packages) { create_list(:work_package, 3) }