diff --git a/app/models/concerns/condition_methods.rb b/app/models/concerns/condition_methods.rb index 424f2c27c9..c537ee5b76 100644 --- a/app/models/concerns/condition_methods.rb +++ b/app/models/concerns/condition_methods.rb @@ -1,6 +1,6 @@ module ConditionMethods def is_exit_page? - !exit_page_markdown.nil? + !exit_page_markdown.nil? || try(:exit_page_id).present? end alias_method :exit_page?, :is_exit_page? diff --git a/app/models/condition.rb b/app/models/condition.rb index db7062fe89..e4e9591dd1 100644 --- a/app/models/condition.rb +++ b/app/models/condition.rb @@ -9,6 +9,7 @@ class Condition < ApplicationRecord belongs_to :goto_page, class_name: "Page", optional: true has_one :form, through: :routing_page + belongs_to :exit_page, optional: true before_destroy :destroy_postconditions @@ -101,11 +102,12 @@ def errors_with_fields def as_json(options = {}) super(options.reverse_merge( methods: %i[validation_errors has_routing_errors], + except: %i[exit_page_id], )) end def as_form_document_condition - data = as_json(methods: %i[validation_errors]) + data = as_json(methods: %i[validation_errors], except: %i[exit_page_id]) data.merge( "routing_page_id" => routing_page&.external_id, "check_page_id" => check_page&.external_id, diff --git a/app/models/exit_page.rb b/app/models/exit_page.rb new file mode 100644 index 0000000000..3fe739775c --- /dev/null +++ b/app/models/exit_page.rb @@ -0,0 +1,17 @@ +class ExitPage < ApplicationRecord + extend Mobility + belongs_to :question_page, class_name: "Page", optional: false + + validates :heading, presence: true + validates :markdown, presence: true + + translates :heading, :markdown + + def as_form_document_exit_page + { + "id" => id, + "heading" => heading, + "markdown" => markdown, + } + end +end diff --git a/app/models/form_document/condition.rb b/app/models/form_document/condition.rb index f4b4be460a..bb7b47aa0f 100644 --- a/app/models/form_document/condition.rb +++ b/app/models/form_document/condition.rb @@ -15,6 +15,7 @@ class FormDocument::Condition attribute :exit_page_heading, :string attribute :validation_errors, DataStructType.new attribute :exit_page_markdown, :string + attribute :exit_page_id, :integer def initialize(attributes = {}) attributes.slice!(*self.class.attribute_names) diff --git a/app/models/page.rb b/app/models/page.rb index 908b16e118..4c873f8d8a 100644 --- a/app/models/page.rb +++ b/app/models/page.rb @@ -6,6 +6,8 @@ class Page < ApplicationRecord 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: :destroy has_many :goto_conditions, class_name: "Condition", foreign_key: "goto_page_id", dependent: :destroy + has_many :exit_pages, foreign_key: :question_page_id, dependent: :destroy + acts_as_list scope: :form translates :question_text, @@ -52,10 +54,15 @@ def destroy_and_update_form! def save_and_update_form return true unless has_changes_to_save? - save! - form.save_question_changes! - check_conditions.destroy_all if answer_type_changed_from_selection - check_conditions.destroy_all if answer_settings_changed_from_only_one_option + ActiveRecord::Base.transaction do + save! + form.save_question_changes! + + if answer_type_changed_from_selection || answer_settings_changed_from_only_one_option + exit_pages.destroy_all + check_conditions.destroy_all + end + end true end @@ -110,6 +117,7 @@ def as_form_document_step(next_page) "type" => "question", "data" => slice(*%w[question_text hint_text answer_type is_optional answer_settings page_heading guidance_markdown is_repeatable]), "routing_conditions" => routing_conditions.map(&:as_form_document_condition), + "exit_pages" => exit_pages.map(&:as_form_document_exit_page), } end diff --git a/db/migrate/20260625153556_add_exit_page.rb b/db/migrate/20260625153556_add_exit_page.rb new file mode 100644 index 0000000000..03e489fcb4 --- /dev/null +++ b/db/migrate/20260625153556_add_exit_page.rb @@ -0,0 +1,10 @@ +class AddExitPage < ActiveRecord::Migration[8.1] + def change + create_table :exit_pages do |t| + t.text :heading, comment: "The title used for the exit page " + t.text :markdown, comment: "The body content in markdown format" + t.belongs_to :question_page, foreign_key: { to_table: :pages, on_delete: :cascade }, null: false, comment: "The page that the exit page belongs to" + t.timestamps + end + end +end diff --git a/db/migrate/20260629121653_create_exit_pages_heading_and_markdown_translations_for_mobility_table_backend.rb b/db/migrate/20260629121653_create_exit_pages_heading_and_markdown_translations_for_mobility_table_backend.rb new file mode 100644 index 0000000000..7b4c03d2c6 --- /dev/null +++ b/db/migrate/20260629121653_create_exit_pages_heading_and_markdown_translations_for_mobility_table_backend.rb @@ -0,0 +1,16 @@ +class CreateExitPagesHeadingAndMarkdownTranslationsForMobilityTableBackend < ActiveRecord::Migration[8.1] + def change + create_table :exit_page_translations do |t| + t.text :heading + t.text :markdown + + t.string :locale, null: false + t.references :exit_page, null: false, foreign_key: true, index: false + + t.timestamps null: false + end + + add_index :exit_page_translations, :locale, name: :index_exit_page_translations_on_locale + add_index :exit_page_translations, %i[exit_page_id locale], name: :index_exit_page_translations_on_exit_page_id_and_locale, unique: true + end +end diff --git a/db/migrate/20260629145441_add_exit_page_to_condition.rb b/db/migrate/20260629145441_add_exit_page_to_condition.rb new file mode 100644 index 0000000000..2966ab02cd --- /dev/null +++ b/db/migrate/20260629145441_add_exit_page_to_condition.rb @@ -0,0 +1,5 @@ +class AddExitPageToCondition < ActiveRecord::Migration[8.1] + def change + add_reference :conditions, :exit_page, foreign_key: { to_table: :exit_pages }, null: true + end +end diff --git a/db/schema.rb b/db/schema.rb index f9401740c5..9fb6d762d0 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.1].define(version: 2026_06_08_112508) do +ActiveRecord::Schema[8.1].define(version: 2026_06_29_145441) do # These are extensions that must be enabled in order to support this database enable_extension "pg_catalog.plpgsql" @@ -30,12 +30,14 @@ t.bigint "check_page_id", comment: "The question page this condition looks at to compare answers" t.datetime "created_at", null: false t.text "exit_page_heading", comment: "Text for the heading of the exit page" + t.bigint "exit_page_id" t.text "exit_page_markdown", comment: "When not nil this condition should be treated as an exit page. When set it contains the markdown for the body of the exit page" t.bigint "goto_page_id", comment: "The question page which this conditions will skip forwards to" t.bigint "routing_page_id", comment: "The question page at which this conditional route takes place" t.boolean "skip_to_end", default: false t.datetime "updated_at", null: false t.index ["check_page_id"], name: "index_conditions_on_check_page_id" + t.index ["exit_page_id"], name: "index_conditions_on_exit_page_id" t.index ["goto_page_id"], name: "index_conditions_on_goto_page_id" t.index ["routing_page_id"], name: "index_conditions_on_routing_page_id" end @@ -71,6 +73,26 @@ t.index ["user_id"], name: "index_draft_questions_on_user_id" end + create_table "exit_page_translations", force: :cascade do |t| + t.datetime "created_at", null: false + t.bigint "exit_page_id", null: false + t.text "heading" + t.string "locale", null: false + t.text "markdown" + t.datetime "updated_at", null: false + t.index ["exit_page_id", "locale"], name: "index_exit_page_translations_on_exit_page_id_and_locale", unique: true + t.index ["locale"], name: "index_exit_page_translations_on_locale" + end + + create_table "exit_pages", force: :cascade do |t| + t.datetime "created_at", null: false + t.text "heading", comment: "The title used for the exit page " + t.text "markdown", comment: "The body content in markdown format" + t.bigint "question_page_id", null: false, comment: "The page that the exit page belongs to" + t.datetime "updated_at", null: false + t.index ["question_page_id"], name: "index_exit_pages_on_question_page_id" + end + create_table "form_documents", force: :cascade do |t| t.jsonb "content", comment: "The JSON which describes the form" t.datetime "created_at", null: false @@ -285,9 +307,12 @@ end add_foreign_key "condition_translations", "conditions" + add_foreign_key "conditions", "exit_pages" add_foreign_key "create_form_events", "groups", on_delete: :cascade add_foreign_key "create_form_events", "users", on_delete: :cascade add_foreign_key "draft_questions", "users" + add_foreign_key "exit_page_translations", "exit_pages" + add_foreign_key "exit_pages", "pages", column: "question_page_id", on_delete: :cascade add_foreign_key "form_documents", "forms" add_foreign_key "form_translations", "forms" add_foreign_key "groups", "users", column: "creator_id" diff --git a/spec/factories/models/conditions.rb b/spec/factories/models/conditions.rb index a7e582d1e0..593a4e2fe6 100644 --- a/spec/factories/models/conditions.rb +++ b/spec/factories/models/conditions.rb @@ -12,6 +12,10 @@ 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/factories/models/exit_page.rb b/spec/factories/models/exit_page.rb new file mode 100644 index 0000000000..c5a8822db8 --- /dev/null +++ b/spec/factories/models/exit_page.rb @@ -0,0 +1,7 @@ +FactoryBot.define do + factory :exit_page, class: "ExitPage" do + heading { Faker::Lorem.sentence } + markdown { Faker::Lorem.paragraph } + association :question_page, factory: :page + end +end diff --git a/spec/models/condition_spec.rb b/spec/models/condition_spec.rb index 5dea834225..7e6bf46a0b 100644 --- a/spec/models/condition_spec.rb +++ b/spec/models/condition_spec.rb @@ -264,13 +264,22 @@ end end - context "when is_exit_page?" do + context "when is_exit_page? using exit_page_markdown" do let(:condition) { create :condition, routing_page_id: routing_page.id, goto_page_id: nil, exit_page_markdown: "exit page" } it "returns nil" do expect(condition.warning_goto_page_doesnt_exist).to be_nil end end + + context "when exit_page_id is present using ExitPage" do + let(:exit_page) { create :exit_page } + let(:condition) { create :condition, routing_page_id: routing_page.id, goto_page_id: nil, exit_page_id: exit_page.id } + + it "returns nil" do + expect(condition.warning_goto_page_doesnt_exist).to be_nil + end + end end describe "#warning_answer_doesnt_exist" do diff --git a/spec/models/exit_page_spec.rb b/spec/models/exit_page_spec.rb new file mode 100644 index 0000000000..b7b5378144 --- /dev/null +++ b/spec/models/exit_page_spec.rb @@ -0,0 +1,72 @@ +require "rails_helper" + +RSpec.describe ExitPage, type: :model do + it "has a valid factory" do + expect(build(:exit_page)).to be_valid + end + + describe "validations" do + context "when heading is blank" do + it "is invalid" do + expect(build(:exit_page, heading: nil)).not_to be_valid + end + end + + context "when markdown is blank" do + it "is invalid" do + expect(build(:exit_page, markdown: nil)).not_to be_valid + end + end + end + + describe "associations" do + let!(:question_page) { create(:page) } + let!(:exit_page) { create(:exit_page, question_page:) } + + it "has a question page" do + expect(exit_page.question_page).to eq(question_page) + end + + it "is deleted when the question page is deleted" do + expect { question_page.destroy! }.to change(described_class, :count).by(-1) + end + + it "the page has exit pages" do + expect(question_page.exit_pages).to eq([exit_page]) + end + end + + describe "translations" do + let!(:question_page) { create(:page) } + let!(:exit_page) { create(:exit_page, question_page:) } + + it "can set and read translated attributes for :en and :cy locales" do + exit_page.heading = "English heading" + exit_page.markdown = "English markdown" + + exit_page.heading_cy = "Welsh heading" + exit_page.markdown_cy = "Welsh markdown" + exit_page.save! + + exit_page.reload + expect(exit_page.heading).to eq("English heading") + expect(exit_page.heading_cy).to eq("Welsh heading") + + expect(exit_page.markdown).to eq("English markdown") + expect(exit_page.markdown_cy).to eq("Welsh markdown") + end + end + + describe "#as_form_document_exit_page" do + let!(:question_page) { create(:page) } + let!(:exit_page) { create(:exit_page, question_page:) } + + it "returns a hash" do + expect(exit_page.as_form_document_exit_page).to be_a(Hash) + end + + it "returns the exit page attributes" do + expect(exit_page.as_form_document_exit_page).to match a_hash_including("id" => exit_page.id, "heading" => exit_page.heading, "markdown" => exit_page.markdown) + end + end +end diff --git a/spec/models/form_document/condition_spec.rb b/spec/models/form_document/condition_spec.rb index cf7e27d821..4eb34f5b70 100644 --- a/spec/models/form_document/condition_spec.rb +++ b/spec/models/form_document/condition_spec.rb @@ -11,7 +11,7 @@ end it "has all the attributes the original condition has" do - expect(form_document_condition.attributes.except("routing_page_id")).to include(condition.attributes.except("routing_page_id")) + expect(form_document_condition.attributes.except("routing_page_id")).to include(condition.attributes.except("routing_page_id", "exit_page_id")) expect(form_document_condition.routing_page_id).to eq(condition.routing_page.external_id) end diff --git a/spec/models/page_spec.rb b/spec/models/page_spec.rb index ab678060e1..88a4b294f8 100644 --- a/spec/models/page_spec.rb +++ b/spec/models/page_spec.rb @@ -480,14 +480,21 @@ end end - context "when page has routing conditions" do - let(:routing_conditions) { [create(:condition)] } + context "when page has routing conditions and ExitPages" do + let(:routing_conditions) { [create(:condition, exit_page:)] } let(:check_conditions) { routing_conditions } + let(:exit_page) { create :exit_page } - it "does not delete existing conditions" do + before do + exit_page.question_page = page + exit_page.save! + end + + it "does not delete existing conditions or ExitPages" do page.save_and_update_form expect(page.reload.routing_conditions.to_a).to eq(routing_conditions) expect(page.reload.check_conditions.to_a).to eq(check_conditions) + expect(page.reload.exit_pages.to_a).to eq([exit_page]) end context "when answer type is updated to one doesn't support routing" do @@ -496,13 +503,20 @@ page.save_and_update_form expect(page.reload.check_conditions).to be_empty end + + it "deletes any ExitPages" do + page.answer_type = "number" + page.save_and_update_form + expect(page.reload.exit_pages).to be_empty + end end context "when the page is saved without changing the answer type" do - it "does not delete the conditions" do + it "does not delete the conditions or ExitPages" do page.question_text = "test" page.save_and_update_form expect(page.reload.check_conditions).not_to be_empty + expect(page.reload.exit_pages).not_to be_empty end end @@ -512,13 +526,20 @@ page.save_and_update_form expect(page.reload.check_conditions).to be_empty end + + it "deletes any ExitPages" do + page.answer_settings["only_one_option"] = "0" + page.save_and_update_form + expect(page.reload.exit_pages).to be_empty + end end context "when the answer settings change while still restricting to only one option" do - it "does not delete any conditions" do + it "does not delete any conditions or ExitPages" do page.answer_settings["selection_options"].first["name"] = "New option name" page.save_and_update_form expect(page.reload.check_conditions).not_to be_empty + expect(page.reload.exit_pages).not_to be_empty end end end @@ -742,6 +763,10 @@ expect(page.as_form_document_step(second_page)["data"]).to match a_hash_including(*page_attributes) end + it "includes exit_pages as an array" do + expect(page.as_form_document_step(second_page)["exit_pages"]).to be_a(Array) + end + context "when there are conditions associated with the page" do let!(:condition) { create :condition, routing_page_id: page.id, check_page_id: page.id } @@ -755,6 +780,18 @@ expect(page.as_form_document_step(nil)["next_step_id"]).to be_nil end 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 + + 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) + end + end end describe "#secondary_skip_condition" do diff --git a/spec/support/shared_examples/condition_methods.rb b/spec/support/shared_examples/condition_methods.rb index a90b67686e..52f18861a8 100644 --- a/spec/support/shared_examples/condition_methods.rb +++ b/spec/support/shared_examples/condition_methods.rb @@ -2,11 +2,19 @@ describe "#is_exit_page?" do it "returns false when exit_page_markdown is nil" do subject.exit_page_markdown = nil + subject.exit_page_id = nil expect(subject.is_exit_page?).to be false end it "returns true when exit_page_markdown is not nil" do subject.exit_page_markdown = "" + subject.exit_page_id = nil + expect(subject.is_exit_page?).to be true + end + + it "returns true when exit_page_id is present" do + subject.exit_page_markdown = nil + subject.exit_page_id = 1 expect(subject.is_exit_page?).to be true end end