From 13b8226a8b383f62a92ac8798e21d0f62fc3ee95 Mon Sep 17 00:00:00 2001 From: Laurence de Bruxelles Date: Fri, 6 Dec 2024 10:18:54 +0200 Subject: [PATCH 1/2] Add specs for associations from pages to conditions --- spec/models/page_spec.rb | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/spec/models/page_spec.rb b/spec/models/page_spec.rb index 1767f032..bed168e2 100644 --- a/spec/models/page_spec.rb +++ b/spec/models/page_spec.rb @@ -12,6 +12,45 @@ expect(page).to be_valid end + describe "associations" do + let(:form) { create :form } + let(:page) { create :page, form: } + + context "when it has a routing condition" do + let!(:condition) { page.routing_conditions.create! } + + it "deletes the condition if it is deleted" do + page.destroy! + + expect(condition).to be_destroyed + end + end + + context "when it has a check condition" do + let(:routing_page) { create :page, form: } + let!(:condition) { page.check_conditions.create! routing_page: } + + it "removes association from the condition if it is deleted" do + page.destroy! + + expect(condition).not_to be_destroyed + expect(condition.goto_page).to be_nil + end + end + + context "when it has a go to condition" do + let(:routing_page) { create :page, form: } + let!(:condition) { page.goto_conditions.create! routing_page: } + + it "removes association from the condition if it is deleted" do + page.destroy! + + expect(condition).not_to be_destroyed + expect(condition.goto_page).to be_nil + end + end + end + describe "versioning", :versioning do it "enables paper trail" do expect(page).to be_versioned From 62bd560d06e589f93b5e50d903050805769dd0dc Mon Sep 17 00:00:00 2001 From: Laurence de Bruxelles Date: Fri, 6 Dec 2024 10:29:02 +0200 Subject: [PATCH 2/2] Change page model to delete dependent check conditions It doesn't makes sense to have a secondary skip condition when the precondition skip route has been deleted, but that's currently what happens when you delete a page with a skip and secondary skip. Instead let's just delete all check conditions for a page when that page is deleted, same as we do for the routing conditions. This shouldn't have any other side effects, as the only case currently where a check condition isn't also a routing condition for a page is for a secondary skip. --- app/models/page.rb | 2 +- spec/models/page_spec.rb | 20 +++++++++++++++++--- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/app/models/page.rb b/app/models/page.rb index 4fb7e893..4412b84c 100644 --- a/app/models/page.rb +++ b/app/models/page.rb @@ -5,7 +5,7 @@ class Page < ApplicationRecord belongs_to :form has_many :routing_conditions, class_name: "Condition", foreign_key: "routing_page_id", dependent: :destroy - has_many :check_conditions, class_name: "Condition", foreign_key: "check_page_id", dependent: :nullify + has_many :check_conditions, class_name: "Condition", foreign_key: "check_page_id", dependent: :destroy has_many :goto_conditions, class_name: "Condition", foreign_key: "goto_page_id", dependent: :nullify acts_as_list scope: :form diff --git a/spec/models/page_spec.rb b/spec/models/page_spec.rb index bed168e2..6f07afb4 100644 --- a/spec/models/page_spec.rb +++ b/spec/models/page_spec.rb @@ -30,11 +30,10 @@ let(:routing_page) { create :page, form: } let!(:condition) { page.check_conditions.create! routing_page: } - it "removes association from the condition if it is deleted" do + it "deletes the condition if it is deleted" do page.destroy! - expect(condition).not_to be_destroyed - expect(condition.goto_page).to be_nil + expect(condition).to be_destroyed end end @@ -49,6 +48,21 @@ expect(condition.goto_page).to be_nil end end + + context "when it has a branch route with skip and secondary skip" do + let(:first_branch) { create :page, form: } + let(:second_branch) { create :page, form: } + + let!(:skip_condition) { page.routing_conditions.create! goto_page: second_branch } + let!(:secondary_skip_condition) { page.check_conditions.create! routing_page: first_branch, skip_to_end: true } + + it "deletes all the conditions if it is deleted" do + page.destroy! + + expect(skip_condition).to be_destroyed + expect(secondary_skip_condition).to be_destroyed + end + end end describe "versioning", :versioning do