diff --git a/app/controllers/pages/exit_page_controller.rb b/app/controllers/pages/exit_page_controller.rb index 9ba623ada1..2f9c74d823 100644 --- a/app/controllers/pages/exit_page_controller.rb +++ b/app/controllers/pages/exit_page_controller.rb @@ -1,6 +1,6 @@ class Pages::ExitPageController < PagesController before_action :can_add_page_routing, only: %i[new create delete destroy] - before_action :ensure_answer_value_present, only: %i[new create] + before_action :ensure_answer_value_present, only: %i[new create], unless: -> { current_form&.group.present? && FeatureService.new(group: current_form.group).enabled?(:multiple_branches) } def new exit_page_input = Pages::ExitPageInput.new(form: current_form, page:, answer_value: params[:answer_value]) @@ -81,7 +81,11 @@ def render_preview private def can_add_page_routing - authorize current_form, :can_add_page_routing_conditions? + if current_form&.group.present? && FeatureService.new(group: current_form.group).enabled?(:multiple_branches) + authorize current_form, :can_edit_form? + else + authorize current_form, :can_add_page_routing_conditions? + end end def exit_page_input_params diff --git a/app/models/condition.rb b/app/models/condition.rb index 30a1250a2b..a086d18903 100644 --- a/app/models/condition.rb +++ b/app/models/condition.rb @@ -9,26 +9,53 @@ class Condition < ApplicationRecord belongs_to :goto_page, class_name: "Page", optional: true has_one :form, through: :routing_page - belongs_to :exit_page, optional: true + belongs_to :exit_page, optional: true, autosave: true + validates_associated :exit_page before_destroy :destroy_postconditions + before_validation :sync_exit_page translates :exit_page_heading translates :exit_page_markdown, presence: false # Without presence here, the value is nil if set to "" - def self.create_and_update_form!(...) - condition = Condition.new(...) + alias_method :legacy_exit_page_heading, :exit_page_heading + alias_method :legacy_exit_page_heading=, :exit_page_heading= + alias_method :legacy_exit_page_heading_cy, :exit_page_heading_cy + alias_method :legacy_exit_page_markdown, :exit_page_markdown + alias_method :legacy_exit_page_markdown=, :exit_page_markdown= + alias_method :legacy_exit_page_markdown_cy, :exit_page_markdown_cy + + def sync_exit_page + if legacy_exit_page_heading.present? && legacy_exit_page_markdown.present? + self.exit_page ||= build_exit_page(question_page: routing_page) + + exit_page.heading = legacy_exit_page_heading + exit_page.markdown = legacy_exit_page_markdown + exit_page.heading_cy = legacy_exit_page_heading_cy + exit_page.markdown_cy = legacy_exit_page_markdown_cy + end + + if legacy_exit_page_heading.blank? || legacy_exit_page_markdown.blank? + exit_page&.mark_for_destruction + end + end + + def self.create_and_update_form!(**args) + condition = Condition.new(**args) + condition.save_and_update_form condition end - def save_and_update_form + def save_and_update_form(**args) save! form.save_question_changes! end def destroy_and_update_form! + exit_page_to_delete = exit_page destroy! && form.save_question_changes! + exit_page_to_delete.presence&.destroy! # unless FeatureService.new(group: form.group).enabled?(:multiple_branches) end def validation_errors @@ -40,6 +67,58 @@ def validation_errors ].compact end + # def exit_page_heading(**args) + # return exit_page&.heading(**args) if exit_page.present? + + # super(**args) + # end + + # def exit_page_markdown(**args) + # return exit_page&.markdown(**args) if exit_page.present? + + # super(**args) + # end + + # def exit_page_heading=(value, **args) + # if exit_page.present? + # exit_page.send(:heading=, value, **args) + # end + + # super(value, **args) + # end + + # def exit_page_markdown=(value, **args) + # if exit_page.present? + # exit_page.send(:markdown=, value, **args) + # end + + # super(value, **args) + # end + + def exit_page_heading(locale: nil, **options) + exit_page&.heading(locale:, **options) || legacy_exit_page_heading(locale:, **options) + end + + def exit_page_heading=(value, locale: nil, **options) + public_send(:legacy_exit_page_heading=, value, locale: locale, **options) + end + + def exit_page_heading_cy + exit_page&.heading_cy || legacy_exit_page_heading_cy + end + + def exit_page_markdown(locale: nil, **options) + exit_page&.markdown(locale:, **options) || legacy_exit_page_markdown(locale:, **options) + end + + def exit_page_markdown=(value, locale: nil, **options) + public_send(:legacy_exit_page_markdown=, value, locale: locale, **options) + end + + def exit_page_markdown_cy + exit_page&.markdown_cy || legacy_exit_page_markdown_cy + end + def warning_goto_page_doesnt_exist return nil if is_exit_page? # goto_page_id isn't needed if the route is skipping to the end of the form diff --git a/app/models/exit_page.rb b/app/models/exit_page.rb index 3fe739775c..82ac0e5c74 100644 --- a/app/models/exit_page.rb +++ b/app/models/exit_page.rb @@ -1,6 +1,7 @@ class ExitPage < ApplicationRecord extend Mobility belongs_to :question_page, class_name: "Page", optional: false + has_many :conditions, class_name: "Condition", dependent: :nullify validates :heading, presence: true validates :markdown, presence: true diff --git a/app/models/form.rb b/app/models/form.rb index 3ab473a3b7..38f42ac480 100644 --- a/app/models/form.rb +++ b/app/models/form.rb @@ -17,6 +17,7 @@ class Form < ApplicationRecord has_one :draft_welsh_form_document, -> { where tag: "draft", language: :cy }, class_name: "FormDocument" has_one :draft_form_document, -> { where tag: "draft", language: :en }, class_name: "FormDocument" has_many :conditions, through: :pages, source: :routing_conditions + has_many :exit_pages, through: :pages, source: :exit_pages has_many :delivery_configurations, dependent: :destroy has_one :immediate_email_delivery_configuration, -> { where delivery_method: "email", delivery_schedule: "immediate" }, class_name: "DeliveryConfiguration" has_one :s3_delivery_configuration, -> { where delivery_method: "s3", delivery_schedule: "immediate" }, class_name: "DeliveryConfiguration" diff --git a/app/models/page.rb b/app/models/page.rb index 7928806a33..079d2ca39c 100644 --- a/app/models/page.rb +++ b/app/models/page.rb @@ -59,8 +59,8 @@ def save_and_update_form form.save_question_changes! if answer_type_changed_from_selection_only_one_option || answer_settings_changed_from_only_one_option - exit_pages.destroy_all check_conditions.destroy_all + exit_pages.destroy_all end end diff --git a/app/services/revert_draft_form_service.rb b/app/services/revert_draft_form_service.rb index 49c1b8f7e0..065adefec2 100644 --- a/app/services/revert_draft_form_service.rb +++ b/app/services/revert_draft_form_service.rb @@ -188,6 +188,7 @@ def clear_welsh_translations form.translations.where(locale: :cy).delete_all Page::Translation.where(locale: :cy, page_id: form.page_ids).delete_all Condition::Translation.where(locale: :cy, condition_id: form.condition_ids).delete_all + ExitPage::Translation.where(locale: :cy, exit_page_id: form.exit_page_ids).delete_all end def revert_delivery_configurations(form_document_content) diff --git a/spec/components/page_list_component/view_spec.rb b/spec/components/page_list_component/view_spec.rb index a340e8b300..928fc97f73 100644 --- a/spec/components/page_list_component/view_spec.rb +++ b/spec/components/page_list_component/view_spec.rb @@ -112,7 +112,7 @@ end context "when the condition has an exit page heading" do - let!(:condition) { create :condition, :with_exit_page, routing_page_id: pages.first.id, check_page_id: pages.first.id, answer_value: "Option 1" } + let!(:condition) { create :condition, :with_exit_page, routing_page: pages.first, check_page_id: pages.first.id, answer_value: "Option 1" } before do render_inline(page_list_component) diff --git a/spec/factories/models/conditions.rb b/spec/factories/models/conditions.rb index 593a4e2fe6..a7e582d1e0 100644 --- a/spec/factories/models/conditions.rb +++ b/spec/factories/models/conditions.rb @@ -12,10 +12,6 @@ exit_page_heading { nil } exit_page_markdown { nil } - # Define the association but we want to set it to nil by default - association :exit_page - exit_page_id { nil } - trait :with_exit_page do goto_page { nil } exit_page_heading { "Exit page heading" } diff --git a/spec/models/condition_spec.rb b/spec/models/condition_spec.rb index b121be1340..0e981fcc36 100644 --- a/spec/models/condition_spec.rb +++ b/spec/models/condition_spec.rb @@ -720,4 +720,50 @@ end end end + + describe "using the new ExitPage model" do + describe "#exit_page_heading" do + let(:condition) { create :condition, exit_page_heading: "English heading", exit_page_markdown: "English markdown", exit_page_heading_cy: "Welsh heading", exit_page_markdown_cy: "Welsh markdown" } + + it "returns the heading" do + expect(condition.exit_page_heading).to eq("English heading") + end + + context "when the condition has an ExitPage" do + before do + condition.exit_page = create :exit_page, question_page: condition.routing_page, heading: "Exit page english heading", heading_cy: "Exit page welsh heading" + end + + it "returns the heading from the ExitPage" do + expect(condition.exit_page_heading).to eq("Exit page english heading") + end + + it "returns the welsh heading if the condition an ExitPage" do + expect(condition.exit_page_heading_cy).to eq("Exit page welsh heading") + end + end + + describe "changing the heading" do + let(:new_heading) { "New heading" } + + it "updates the heading" do + condition.exit_page_heading = new_heading + expect(condition.exit_page_heading).to eq(new_heading) + end + + it "updates the ExitPage model" do + condition.exit_page_heading = new_heading + + condition.save! + expect(condition.exit_page.heading).to eq(new_heading) + end + + it "changing the welsh heading, changes the ExitPage welsh heading" do + condition.exit_page_heading_cy = "welsh heading" + condition.save! + expect(condition.exit_page.reload.heading_cy).to eq("welsh heading") + end + end + end + end end diff --git a/spec/models/page_spec.rb b/spec/models/page_spec.rb index 918c67117d..cbb5cc405f 100644 --- a/spec/models/page_spec.rb +++ b/spec/models/page_spec.rb @@ -481,13 +481,12 @@ end context "when page has routing conditions and ExitPages" do - let(:routing_conditions) { [create(:condition, exit_page:)] } + let(:routing_conditions) { [create(:condition)] } let(:check_conditions) { routing_conditions } - let(:exit_page) { create :exit_page } + let(:exit_page) { create :exit_page, question_page: page } before do - exit_page.question_page = page - exit_page.save! + routing_conditions.first.exit_page = exit_page end it "does not delete existing conditions or ExitPages" do @@ -792,14 +791,10 @@ end context "when the page has an ExitPage" do - let!(:exit_page) { create :exit_page, question_page: page } - - before do - create(:condition, routing_page_id: page.id, check_page_id: page.id, exit_page:) - end + let!(:condition) { create(:condition, :with_exit_page, routing_page_id: page.id, check_page_id: page.id) } it "includes the exit page" do - expect(page.reload.as_form_document_step(second_page)["exit_pages"].first).to match a_hash_including("id" => exit_page.id, "markdown" => exit_page.markdown, "heading" => exit_page.heading) + expect(page.reload.as_form_document_step(second_page)["exit_pages"].first).to match a_hash_including("id" => condition.exit_page_id, "markdown" => condition.exit_page.markdown, "heading" => condition.exit_page.heading) end end end diff --git a/spec/requests/pages/exit_page_controller_spec.rb b/spec/requests/pages/exit_page_controller_spec.rb index 92459c6e69..9e943c3e13 100644 --- a/spec/requests/pages/exit_page_controller_spec.rb +++ b/spec/requests/pages/exit_page_controller_spec.rb @@ -93,6 +93,13 @@ expect(response.status).to eq(403) expect(response).to render_template("errors/forbidden") end + + context "when mutlple branches is enabled", :multiple_branches do + it "Renders the forbidden page" do + expect(response.status).to eq(403) + expect(response).to render_template("errors/forbidden") + end + end end end