Merge pull request #2828 from govuk-forms/fix-stale-condition-check

Fix stale_conditions same answer value conditions

Author: SamJamCul

app/services/routes/sync_service.rb

@@ -30,6 +30,7 @@ def update_or_create_conditions
       condition.assign_attributes(
         route.condition_attributes,
       )
+
       condition.save!
     end
   end
@@ -40,10 +41,14 @@ def destroy_stale_conditions
     # If there are no routes marked as default, there's nothing to destroy.
     return if default_routes.none?
 
+    stale_route_pairs = default_routes.map do |route|
+      [route.page_id, route.answer_value.presence]
+    end
+
     stale_conditions = form.conditions.where(
-      routing_page_id: default_routes.map(&:page_id),
-      answer_value: default_routes.map { |r| r.answer_value.presence },
+      %i[routing_page_id answer_value] => stale_route_pairs,
     )
+
     stale_conditions.destroy_all
   end
 end

spec/services/routes/sync_service_spec.rb

@@ -71,17 +71,32 @@
         create(:condition, routing_page_id: pages.first.id, answer_value: "Yes", goto_page_id: pages.second.id)
       end
 
+      # Create a condition that shouldn't be removed
+      let!(:healthy_condition) do
+        create(:condition, routing_page_id: pages.second.id, answer_value: "Yes", goto_page_id: pages.third.id)
+      end
+
       # Provide a route that matches the stale condition's key, but is now a "default" route.
       let(:routes) do
         [
+          # Route for the first page, which is default so conditions are removed
           build(:route_input, :default, page: pages.first, answer_value: "Yes"),
+          # Two routes for the second page, which should not be removed
+          build(:route_input, page: pages.second, goto: pages.third.id, answer_value: "Yes"),
+          # This is default but has no conditions to remove
+          build(:route_input, :default, page: pages.second, answer_value: "No"),
         ]
       end
 
       it "destroys the condition that is now a default route" do
         expect { service.sync_conditions_from_routes }.to change(Condition, :count).by(-1)
         expect(Condition.exists?(stale_condition.id)).to be false
       end
+
+      it "does not destroy other page's conditions with the same answer value" do
+        service.sync_conditions_from_routes
+        expect(Condition.exists?(healthy_condition.id)).to be true
+      end
     end
 
     context "with an answer_value of an empty string" do