From 6695ee1fa1d562acc22e8e2772331f5254938bb7 Mon Sep 17 00:00:00 2001 From: Stephen Daly Date: Tue, 7 Apr 2026 15:07:06 +0100 Subject: [PATCH 1/4] Stop setting unused `id` attribute on form in specs --- spec/components/check_your_answers_component/view_spec.rb | 2 +- spec/integration/sentry_spec.rb | 2 +- spec/lib/flow/context_spec.rb | 1 - spec/lib/flow/journey_spec.rb | 1 - spec/lib/flow/step_factory_spec.rb | 2 +- spec/models/repeatable_step_spec.rb | 2 +- spec/views/forms/add_another_answer/show.html.erb_spec.rb | 2 +- spec/views/forms/check_your_answers/show.html.erb_spec.rb | 2 +- spec/views/forms/copy_of_answers/show.html.erb_spec.rb | 2 +- spec/views/forms/exit_pages/show.html.erb_spec.rb | 2 +- spec/views/forms/remove_answer/show.html.erb_spec.rb | 2 +- spec/views/forms/remove_file/show.html.erb_spec.rb | 2 +- spec/views/forms/review_file/show.html.erb_spec.rb | 2 +- spec/views/forms/step/show.html.erb_spec.rb | 2 +- spec/views/forms/submitted/submitted.html.erb_spec.rb | 2 +- 15 files changed, 13 insertions(+), 15 deletions(-) diff --git a/spec/components/check_your_answers_component/view_spec.rb b/spec/components/check_your_answers_component/view_spec.rb index 178232882..f4c35066a 100644 --- a/spec/components/check_your_answers_component/view_spec.rb +++ b/spec/components/check_your_answers_component/view_spec.rb @@ -3,7 +3,7 @@ RSpec.describe CheckYourAnswersComponent::View, type: :component do include Rails.application.routes.url_helpers - let(:form) { build :form, id: 1 } + let(:form) { build :form } let(:question) { build :text, question_text: "Do you want to remain anonymous?", text: "Yes" } let(:optional_question) { build :text, question_text: "Optional question", is_optional: true, text: "" } let(:steps) do diff --git a/spec/integration/sentry_spec.rb b/spec/integration/sentry_spec.rb index c92036ce1..cba3ba84b 100644 --- a/spec/integration/sentry_spec.rb +++ b/spec/integration/sentry_spec.rb @@ -20,7 +20,7 @@ end context "when an exception is raised containing personally identifying information" do - let(:form) { build :form, id: 1, submission_email: } + let(:form) { build :form, submission_email: } before do raise "Something went wrong: #{form.inspect}" diff --git a/spec/lib/flow/context_spec.rb b/spec/lib/flow/context_spec.rb index d9b18871c..c9469b7ba 100644 --- a/spec/lib/flow/context_spec.rb +++ b/spec/lib/flow/context_spec.rb @@ -15,7 +15,6 @@ let(:form) do build(:form, :with_support, - id: 2, start_page: 1, privacy_policy_url: "http://www.example.gov.uk/privacy_policy", what_happens_next_markdown: "Good things come to those that wait", diff --git a/spec/lib/flow/journey_spec.rb b/spec/lib/flow/journey_spec.rb index d3393028b..53bc2c606 100644 --- a/spec/lib/flow/journey_spec.rb +++ b/spec/lib/flow/journey_spec.rb @@ -14,7 +14,6 @@ let(:form) do build(:form, :with_support, - id: 2, start_page: first_step_id, steps: form_document_steps) end diff --git a/spec/lib/flow/step_factory_spec.rb b/spec/lib/flow/step_factory_spec.rb index 7c4d6e41d..8fff61afa 100644 --- a/spec/lib/flow/step_factory_spec.rb +++ b/spec/lib/flow/step_factory_spec.rb @@ -1,7 +1,7 @@ require "rails_helper" RSpec.describe Flow::StepFactory do - let(:form) { build :form, id: "form-123", form_slug: "test-form", start_page: "page-1", steps: [] } + let(:form) { build :form, form_slug: "test-form", start_page: "page-1", steps: [] } let(:factory) { described_class.new(form:) } describe "#create_step" do diff --git a/spec/models/repeatable_step_spec.rb b/spec/models/repeatable_step_spec.rb index faca79438..750a74165 100644 --- a/spec/models/repeatable_step_spec.rb +++ b/spec/models/repeatable_step_spec.rb @@ -3,7 +3,7 @@ RSpec.describe RepeatableStep, type: :model do subject(:repeatable_step) { described_class.new(question:, form_document_step:) } - let(:form) { build :form, id: 1, form_slug: "form-slug", steps: [form_document_step, build(:v2_question_step, id: 2)] } + let(:form) { build :form, form_slug: "form-slug", steps: [form_document_step, build(:v2_question_step, id: 2)] } let(:form_document_step) { build :v2_question_step } let(:question) { build :name, is_optional: false } let(:submission_reference) { "abc123" } diff --git a/spec/views/forms/add_another_answer/show.html.erb_spec.rb b/spec/views/forms/add_another_answer/show.html.erb_spec.rb index 6731b2f14..76d6b9d4a 100644 --- a/spec/views/forms/add_another_answer/show.html.erb_spec.rb +++ b/spec/views/forms/add_another_answer/show.html.erb_spec.rb @@ -1,7 +1,7 @@ require "rails_helper" describe "forms/add_another_answer/show.html.erb" do - let(:form) { build :form, id: 1 } + let(:form) { build :form } let(:mode) { OpenStruct.new(preview_draft?: false, preview_archived?: false, preview_live?: false) } let(:step) { OpenStruct.new({ form_id: 1, form_slug: "form-1", step_slug: "1", mode:, questions:, question: OpenStruct.new({ question_text: "Question text" }), max_answers?: max_answers }) } let(:add_another_answer_input) { AddAnotherAnswerInput.new } diff --git a/spec/views/forms/check_your_answers/show.html.erb_spec.rb b/spec/views/forms/check_your_answers/show.html.erb_spec.rb index 5c15d0503..c64f4a36b 100644 --- a/spec/views/forms/check_your_answers/show.html.erb_spec.rb +++ b/spec/views/forms/check_your_answers/show.html.erb_spec.rb @@ -1,7 +1,7 @@ require "rails_helper" describe "forms/check_your_answers/show.html.erb" do - let(:form) { build :form, :with_support, id: 1, declaration_text:, declaration_markdown: } + let(:form) { build :form, :with_support, declaration_text:, declaration_markdown: } let(:support_details) { OpenStruct.new(email: form.support_email) } let(:context) { OpenStruct.new(form:) } let(:full_width) { false } diff --git a/spec/views/forms/copy_of_answers/show.html.erb_spec.rb b/spec/views/forms/copy_of_answers/show.html.erb_spec.rb index d873afaf5..f43c8c03e 100644 --- a/spec/views/forms/copy_of_answers/show.html.erb_spec.rb +++ b/spec/views/forms/copy_of_answers/show.html.erb_spec.rb @@ -1,7 +1,7 @@ require "rails_helper" describe "forms/copy_of_answers/show.html.erb" do - let(:form) { build :form, id: 1 } + let(:form) { build :form } let(:mode) { OpenStruct.new(preview_draft?: false, preview_archived?: false, preview_live?: false) } let(:copy_of_answers_input) { CopyOfAnswersInput.new } let(:back_link) { "/back" } diff --git a/spec/views/forms/exit_pages/show.html.erb_spec.rb b/spec/views/forms/exit_pages/show.html.erb_spec.rb index 08236329e..d8128cf60 100644 --- a/spec/views/forms/exit_pages/show.html.erb_spec.rb +++ b/spec/views/forms/exit_pages/show.html.erb_spec.rb @@ -1,7 +1,7 @@ require "rails_helper" describe "forms/exit_pages/show.html.erb" do - let(:form) { build :form, :with_support, id: 1, name: "exit page form" } + let(:form) { build :form, :with_support, name: "exit page form" } let(:mode) { OpenStruct.new(preview_draft?: false, preview_archived?: false, preview_live?: false) } let(:condition) { OpenStruct.new({ exit_page_heading: "heading", exit_page_markdown: " * first line\n * second line\n" }) } let(:support_details) { OpenStruct.new(email: form.support_email) } diff --git a/spec/views/forms/remove_answer/show.html.erb_spec.rb b/spec/views/forms/remove_answer/show.html.erb_spec.rb index b29ecf1fb..9fedddb24 100644 --- a/spec/views/forms/remove_answer/show.html.erb_spec.rb +++ b/spec/views/forms/remove_answer/show.html.erb_spec.rb @@ -1,7 +1,7 @@ require "rails_helper" describe "forms/remove_answer/show.html.erb" do - let(:form) { build :form, id: 1 } + let(:form) { build :form } let(:mode) { OpenStruct.new(preview_draft?: false, preview_archived?: false, preview_live?: false) } let(:question) { OpenStruct.new({ allow_multiple_answers?: allow_multiple_answers?, has_long_answer?: has_long_answer? }) } let(:step) { OpenStruct.new({ form_id: 1, form_slug: "form-1", step_slug: "1", questions:, question:, answer_index: 1, mode: }) } diff --git a/spec/views/forms/remove_file/show.html.erb_spec.rb b/spec/views/forms/remove_file/show.html.erb_spec.rb index 3e506008f..9a53c6a53 100644 --- a/spec/views/forms/remove_file/show.html.erb_spec.rb +++ b/spec/views/forms/remove_file/show.html.erb_spec.rb @@ -1,7 +1,7 @@ require "rails_helper" describe "forms/remove_file/show.html.erb" do - let(:form) { build :form, :with_support, id: 1 } + let(:form) { build :form, :with_support } let(:mode) { OpenStruct.new(preview_draft?: false, preview_archived?: false, preview_live?: false) } let(:back_link) { "/back" } let(:continue_url) { "/review_file" } diff --git a/spec/views/forms/review_file/show.html.erb_spec.rb b/spec/views/forms/review_file/show.html.erb_spec.rb index bb2ef29ad..e297c8652 100644 --- a/spec/views/forms/review_file/show.html.erb_spec.rb +++ b/spec/views/forms/review_file/show.html.erb_spec.rb @@ -1,7 +1,7 @@ require "rails_helper" describe "forms/review_file/show.html.erb" do - let(:form) { build :form, :with_support, id: 1 } + let(:form) { build :form, :with_support } let(:mode) { OpenStruct.new(preview_draft?: false, preview_archived?: false, preview_live?: false) } let(:back_link) { "/back" } let(:continue_url) { "/review_file" } diff --git a/spec/views/forms/step/show.html.erb_spec.rb b/spec/views/forms/step/show.html.erb_spec.rb index f01755dba..ecb4f9db7 100644 --- a/spec/views/forms/step/show.html.erb_spec.rb +++ b/spec/views/forms/step/show.html.erb_spec.rb @@ -1,7 +1,7 @@ require "rails_helper" describe "forms/check_your_answers/show.html.erb" do - let(:form) { build :form, :with_support, id: 1 } + let(:form) { build :form, :with_support } let(:support_details) { OpenStruct.new(email: form.support_email) } let(:question) { build :full_name_question } let(:step) { build :step, question: question } diff --git a/spec/views/forms/submitted/submitted.html.erb_spec.rb b/spec/views/forms/submitted/submitted.html.erb_spec.rb index aa02e4630..96357aae2 100644 --- a/spec/views/forms/submitted/submitted.html.erb_spec.rb +++ b/spec/views/forms/submitted/submitted.html.erb_spec.rb @@ -1,7 +1,7 @@ require "rails_helper" describe "forms/submitted/submitted.html.erb" do - let(:form) { build :form, id: 1, what_happens_next_markdown:, payment_url: } + let(:form) { build :form, what_happens_next_markdown:, payment_url: } let(:what_happens_next_markdown) { nil } let(:requested_email_confirmation) { false } let(:payment_url) { nil } From 7dc56404ce910d34138201847659f65e0060df05 Mon Sep 17 00:00:00 2001 From: Stephen Daly Date: Tue, 7 Apr 2026 15:08:56 +0100 Subject: [PATCH 2/4] Use FormDocumentResource model within Form model Previously, the Form model was an ActiveResource model that was constructed using the JSON retrieved from the forms-admin API using the FormDocumentResource model. Make it a plain Ruby object and instead delegate attributes to the FormDocumentResource model. This means we are not using the ActiveResource model directly in the rest of the codebase, and instead consuming the Form model which has a defined interface. Future changes to the API can be handled more easily within the Form model. As part of this change, remove the code that expects that the API response can include an `id` attribute rather than `form_id` as this will no longer be the case. --- app/models/form.rb | 69 +++++++---- app/models/submission.rb | 3 +- .../api/v2/form_document_repository.rb | 9 +- spec/factories/models/forms.rb | 2 + spec/models/form_spec.rb | 108 +++++++++--------- spec/requests/forms/step_controller_spec.rb | 2 +- spec/services/form_submission_service_spec.rb | 36 ++++-- 7 files changed, 133 insertions(+), 96 deletions(-) diff --git a/app/models/form.rb b/app/models/form.rb index 5a68dac18..fa137946e 100644 --- a/app/models/form.rb +++ b/app/models/form.rb @@ -1,22 +1,33 @@ -class Form < ActiveResource::Base - self.site = Settings.forms_api.base_url - self.prefix = "/api/v2/" - self.include_format_in_path = false +class Form + attr_reader :document_json + private attr_reader :form_document - class Step < ActiveResource::Base - self.site = Form.site - self.prefix = Form.prefix_source - self.include_format_in_path = false + def initialize(form_document, form_document_json = nil) + @form_document = form_document + @document_json = form_document_json end - has_many :steps, class_name: "Api::V2::StepResource" - attr_accessor :document_json - - alias_method :form_document_steps, :steps - - def form_id - @attributes["form_id"] || @attributes["id"] - end + delegate :steps, to: :form_document, prefix: true + delegate :declaration_text, + :form_id, + :form_slug, + :name, + :payment_url, + :privacy_policy_url, + :s3_bucket_aws_account_id, + :s3_bucket_name, + :s3_bucket_region, + :start_page, + :send_daily_submission_batch, + :send_weekly_submission_batch, + :submission_email, + :submission_type, + :support_email, + :support_phone, + :support_url, + :support_url_text, + :what_happens_next_markdown, + to: :form_document alias_method :id, :form_id @@ -25,27 +36,27 @@ def step_by_id(step_id) end def payment_url_with_reference(reference) - return nil if payment_url.blank? + return nil if form_document.payment_url.blank? - "#{payment_url}?reference=#{reference}" + "#{form_document.payment_url}?reference=#{reference}" end def submission_format - @attributes["submission_format"] || [] + form_document.try(:submission_format) || [] end def support_details OpenStruct.new({ - email: support_email, - phone: support_phone, + email: form_document.support_email, + phone: form_document.support_phone, call_charges_url: "https://www.gov.uk/call-charges", - url: support_url, - url_text: support_url_text, + url: form_document.support_url, + url_text: form_document.support_url_text, }) end def language - @attributes["language"]&.to_sym || :en + form_document.try(:language)&.to_sym || :en end def english? @@ -57,6 +68,14 @@ def welsh? end def multilingual? - @attributes["available_languages"].present? && @attributes["available_languages"].count > 1 + available_languages.count > 1 + end + + def available_languages + form_document.try(:available_languages) || [] + end + + def declaration_markdown + form_document.try(:declaration_markdown) end end diff --git a/app/models/submission.rb b/app/models/submission.rb index 7ad08edce..f5439de1a 100644 --- a/app/models/submission.rb +++ b/app/models/submission.rb @@ -60,6 +60,7 @@ def answer_store end def form_from_document - Form.new(form_document, true) + form_document_resource = Api::V2::FormDocumentResource.new(form_document, true) + Form.new(form_document_resource, form_document) end end diff --git a/app/services/api/v2/form_document_repository.rb b/app/services/api/v2/form_document_repository.rb index b95407019..3405a80ac 100644 --- a/app/services/api/v2/form_document_repository.rb +++ b/app/services/api/v2/form_document_repository.rb @@ -4,12 +4,9 @@ def find(form_id:, tag:, language: :en) return nil unless form_id.to_s =~ /^[[:alnum:]]+$/ begin - form_document = Api::V2::FormDocumentResource.get(form_id, tag, **options_for_language(language)) - - form = Form.new(form_document, true) - form.document_json = form_document - form.prefix_options = { form_id:, tag: } - form + form_document_json = Api::V2::FormDocumentResource.get(form_id, tag, **options_for_language(language)) + form_document = Api::V2::FormDocumentResource.new(form_document_json) + Form.new(form_document, form_document_json) rescue ActiveResource::ResourceNotFound nil end diff --git a/spec/factories/models/forms.rb b/spec/factories/models/forms.rb index 6cb9b9360..736451ef9 100644 --- a/spec/factories/models/forms.rb +++ b/spec/factories/models/forms.rb @@ -1,5 +1,7 @@ FactoryBot.define do factory :form, class: "Form" do + initialize_with { new(build(:v2_form_document, **attributes), document_json) } + sequence(:name) { |n| "Form #{n}" } sequence(:form_slug) { |n| "form-#{n}" } has_draft_version { true } diff --git a/spec/models/form_spec.rb b/spec/models/form_spec.rb index ccc684528..1848e54c9 100644 --- a/spec/models/form_spec.rb +++ b/spec/models/form_spec.rb @@ -1,41 +1,15 @@ require "rails_helper" RSpec.describe Form, type: :model do - subject(:form) { described_class.new(attributes) } + subject(:form) { described_class.new(form_document) } - let(:attributes) { { id: 1, name: "form name", submission_email: "user@example.com", start_page: 1, steps: } } + let(:form_document) { build :v2_form_document } + let(:payment_url) { nil } + let(:language) { "en" } + let(:available_languages) { [] } - let(:form_document_steps) do - [ - { id: 9, next_page: 10, answer_type: "date", question_text: "Question one" }, - { id: 10, answer_type: "address", question_text: "Question two" }, - ] - end - - describe "#form_id" do - context "when the form is initialised with attribute form_id" do - let(:attributes) { { form_id: "1" } } - - it "returns the form ID" do - expect(form).to have_attributes form_id: "1" - end - - it "equals #id" do - expect(form.form_id).to eq form.id - end - end - - context "when the form is initialised with attribute id" do - let(:attributes) { { id: 1 } } - - it "returns the form ID" do - expect(form).to have_attributes form_id: 1 - end - - it "equals #id" do - expect(form.form_id).to eq form.id - end - end + it "returns the form ID" do + expect(form).to have_attributes form_id: form_document.form_id end describe "#form_document_steps" do @@ -61,7 +35,7 @@ end describe "#payment_url_with_reference" do - let(:attributes) { { id: 1, name: "form name", payment_url:, start_page: 1 } } + let(:form_document) { build :v2_form_document, payment_url: } let(:reference) { SecureRandom.base58(8).upcase } context "when there is a payment_url" do @@ -82,8 +56,10 @@ end describe "#submission_format" do + let(:form_document) { build :v2_form_document, submission_format: } + context "when the submission format attribute is nil" do - let(:attributes) { { submission_format: nil } } + let(:submission_format) { nil } it "returns no submission delivery formats" do expect(form.submission_format).to eq [] @@ -91,7 +67,7 @@ end context "when the submission format attribute is an array of strings" do - let(:attributes) { { submission_format: %w[csv json] } } + let(:submission_format) { %w[csv json] } it "returns the submission format attribute" do expect(form.submission_format).to eq %w[csv json] @@ -100,16 +76,12 @@ end describe "#support_details" do - let(:attributes) do - { - id: 1, - name: "form name", - support_email: "help@example.gov.uk", - support_phone: "0203 222 2222", - support_url: "https://example.gov.uk/help", - support_url_text: "Get help with this form", - start_page: 1, - } + let(:form_document) do + build :v2_form_document, + support_email: "help@example.gov.uk", + support_phone: "0203 222 2222", + support_url: "https://example.gov.uk/help", + support_url_text: "Get help with this form" end it "returns an OpenStruct with support details" do @@ -124,8 +96,26 @@ end describe "#language" do - context "when the form is initialised with \"cy\" attribute language" do - let(:attributes) { { id: 1, name: "form name", language: "cy" } } + let(:form_document) { build :v2_form_document, language: } + + context "when the form is initialised with \"en\" attribute language" do + let(:language) { "en" } + + it "returns the language of the form" do + expect(form.language).to eq(:en) + end + + it "#english? returns false" do + expect(form.english?).to be true + end + + it "#welsh? returns true" do + expect(form.welsh?).to be false + end + end + + context "when the form is initialised with \"cn\" attribute language" do + let(:language) { "cy" } it "returns the language of the form" do expect(form.language).to eq(:cy) @@ -141,7 +131,7 @@ end context "when the form is initialised without attribute language" do - let(:attributes) { { id: 1, name: "form name" } } + let(:form_document) { OpenStruct.new } it "returns the default language of the form" do expect(form.language).to eq(:en) @@ -157,7 +147,7 @@ end context "when the form is initialised with attribute language as nil" do - let(:attributes) { { id: 1, name: "form name", language: nil } } + let(:language) { nil } it "returns the default language of the form" do expect(form.language).to eq(:en) @@ -175,16 +165,26 @@ describe "#multilingual?" do context "when the form does not have an available_languages field" do - let(:attributes) { { id: 1, name: "form name", submission_email: "user@example.com", start_page: 1 } } + let(:form_document) { OpenStruct.new } - it "returns false" do + it "#multilingual returns false" do expect(form.multilingual?).to be false end end context "when the form has an available_languages field" do + let(:form_document) { build :v2_form_document, available_languages: } + + context "when the available_languages field is empty" do + let(:available_languages) { [] } + + it "returns false" do + expect(form.multilingual?).to be false + end + end + context "when the form has only one available language" do - let(:attributes) { { id: 1, name: "form name", submission_email: "user@example.com", start_page: 1, available_languages: %w[en] } } + let(:available_languages) { %w[en] } it "returns false" do expect(form.multilingual?).to be false @@ -192,7 +192,7 @@ end context "when the form has more than one available language" do - let(:attributes) { { id: 1, name: "form name", submission_email: "user@example.com", start_page: 1, available_languages: %w[en cy] } } + let(:available_languages) { %w[en cy] } it "returns true" do expect(form.multilingual?).to be true diff --git a/spec/requests/forms/step_controller_spec.rb b/spec/requests/forms/step_controller_spec.rb index 439ae4c2a..f41bce265 100644 --- a/spec/requests/forms/step_controller_spec.rb +++ b/spec/requests/forms/step_controller_spec.rb @@ -59,7 +59,7 @@ context "when setting logging context" do let(:step_id) { 101 } let(:form_data) do - build(:form, :with_support, + build(:v2_form_document, :with_support, id: 200, start_page: step_id, declaration_text: "agree to the declaration", diff --git a/spec/services/form_submission_service_spec.rb b/spec/services/form_submission_service_spec.rb index 6349913a5..901c21ffb 100644 --- a/spec/services/form_submission_service_spec.rb +++ b/spec/services/form_submission_service_spec.rb @@ -8,10 +8,10 @@ let(:mode) { Mode.new } let(:confirmation_email_address) { "testing@gov.uk" } let(:email_confirmation_input) { build :email_confirmation_input_opted_in, confirmation_email_address: } - let(:form) { build(:form, **document_json, document_json:) } - let(:welsh_form) { build(:form, **welsh_document_json, document_json: welsh_document_json) } + let(:form) { Form.new(form_document, document_json) } + let(:welsh_form) { Form.new(welsh_form_document, welsh_document_json) } - let(:document_json) do + let(:form_document) do build( :v2_form_document, form_id: 1, @@ -26,11 +26,30 @@ submission_type:, submission_format:, steps:, - language:, - ).as_json + language: "en", + ) end + let(:document_json) { form_document.as_json } - let(:welsh_document_json) { document_json.merge("language" => "cy", "name" => "Welsh Form 1") } + let(:welsh_form_document) do + build( + :v2_form_document, + form_id: 1, + name: "Welsh Form 1", + what_happens_next_markdown:, + support_email:, + support_phone:, + support_url:, + support_url_text:, + submission_email:, + payment_url:, + submission_type:, + submission_format:, + steps:, + language: "cy", + ) + end + let(:welsh_document_json) { welsh_form_document.as_json } let(:steps) { [build(:v2_question_step, id: 2, answer_type: "text")] } let(:submission_type) { "email" } @@ -42,7 +61,6 @@ let(:support_url_text) { Faker::Lorem.sentence(word_count: 1, random_words_to_add: 4) } let(:payment_url) { nil } let(:submission_email) { "testing@gov.uk" } - let(:language) { "en" } let(:reference) { Faker::Alphanumeric.alphanumeric(number: 8).upcase } @@ -105,7 +123,7 @@ expect { service.submit }.to change(Submission, :count).by(1) - .and change(Delivery, :count).by(1) + .and change(Delivery, :count).by(1) expect(Submission.last).to have_attributes(reference:, form_id: form.id, answers: answers.deep_stringify_keys, mode: "form", form_document: document_json, @@ -170,7 +188,7 @@ expect { service.submit }.to change(Submission, :count).by(1) - .and change(Delivery, :count).by(1) + .and change(Delivery, :count).by(1) expect(Submission.last).to have_attributes(reference:, form_id: form.id, answers: answers.deep_stringify_keys, mode: "form", form_document: document_json, From a002d7d208db0b9dfd4b41dc52c9450e2655c6d6 Mon Sep 17 00:00:00 2001 From: Stephen Daly Date: Tue, 7 Apr 2026 16:03:25 +0100 Subject: [PATCH 3/4] Remove unused traits and attributes from form factory and fixture --- spec/factories/models/forms.rb | 38 ++++------------------ spec/fixtures/all_question_types_form.json | 2 -- 2 files changed, 6 insertions(+), 34 deletions(-) diff --git a/spec/factories/models/forms.rb b/spec/factories/models/forms.rb index 736451ef9..51ea81ce9 100644 --- a/spec/factories/models/forms.rb +++ b/spec/factories/models/forms.rb @@ -2,13 +2,11 @@ factory :form, class: "Form" do initialize_with { new(build(:v2_form_document, **attributes), document_json) } + form_id { Faker::Number.number(digits: 5) } sequence(:name) { |n| "Form #{n}" } sequence(:form_slug) { |n| "form-#{n}" } - has_draft_version { true } - has_live_version { false } submission_email { Faker::Internet.email(domain: "example.gov.uk") } privacy_policy_url { Faker::Internet.url(host: "gov.uk") } - org { "test-org" } what_happens_next_markdown { nil } support_email { nil } support_phone { nil } @@ -19,40 +17,26 @@ document_json { nil } declaration_text { nil } - declaration_section_completed { false } submission_type { "email" } s3_bucket_name { nil } s3_bucket_aws_account_id { nil } - trait :new_form do - submission_email { nil } - privacy_policy_url { nil } - end - - trait :ready_for_live do - with_pages + trait :live? do + with_steps support_email { Faker::Internet.email(domain: "example.gov.uk") } what_happens_next_markdown { "We usually respond to applications within 10 working days." } end - trait :live? do - ready_for_live - has_draft_version { false } - has_live_version { true } - end - - trait :with_pages do + trait :with_steps do transient do - pages_count { 5 } + steps_count { 5 } end form_document_steps do - Array.new(pages_count) { association(:form_document_step) } + Array.new(steps_count) { association(:form_document_step) } end - - question_section_completed { true } end trait :with_support do @@ -62,16 +46,6 @@ support_url_text { Faker::Lorem.sentence(word_count: 1, random_words_to_add: 4) } end - trait :ready_for_routing do - transient do - pages_count { 5 } - end - - form_document_steps do - Array.new(pages_count) { association(:form_document_step, :with_selections_settings) } - end - end - trait :with_payment_url do payment_url { "https://www.gov.uk/payments/test-service/pay-for-licence" } end diff --git a/spec/fixtures/all_question_types_form.json b/spec/fixtures/all_question_types_form.json index ec34a8699..015f15dc5 100644 --- a/spec/fixtures/all_question_types_form.json +++ b/spec/fixtures/all_question_types_form.json @@ -9,8 +9,6 @@ "support_url": null, "support_url_text": null, "declaration_text": "", - "question_section_completed": true, - "declaration_section_completed": true, "created_at": "2024-09-05T06:25:25.558Z", "updated_at": "2024-09-05T06:25:25.637Z", "creator_id": null, From 33adc283c6d21038a8400f1fd20439629dc5bab8 Mon Sep 17 00:00:00 2001 From: Stephen Daly Date: Tue, 7 Apr 2026 16:04:17 +0100 Subject: [PATCH 4/4] Rename live form trait The question mark on the trait doesn't make sense. --- spec/factories/models/forms.rb | 2 +- spec/factories/v2_form_document.rb | 2 +- spec/features/email_confirmation_spec.rb | 2 +- spec/features/fill_in_and_submit_form_spec.rb | 2 +- spec/features/fill_in_and_submit_form_with_csv_spec.rb | 2 +- spec/features/fill_in_autocomplete_question_spec.rb | 2 +- spec/features/fill_in_file_upload_question_spec.rb | 2 +- spec/features/fill_in_form_with_exit_page_spec.rb | 2 +- spec/features/fill_in_single_repeatable_form_spec.rb | 2 +- spec/requests/forms/remove_file_controller_spec.rb | 2 +- spec/requests/forms/review_file_controller_spec.rb | 2 +- 11 files changed, 11 insertions(+), 11 deletions(-) diff --git a/spec/factories/models/forms.rb b/spec/factories/models/forms.rb index 51ea81ce9..fc5aefcce 100644 --- a/spec/factories/models/forms.rb +++ b/spec/factories/models/forms.rb @@ -23,7 +23,7 @@ s3_bucket_name { nil } s3_bucket_aws_account_id { nil } - trait :live? do + trait :live do with_steps support_email { Faker::Internet.email(domain: "example.gov.uk") } what_happens_next_markdown { "We usually respond to applications within 10 working days." } diff --git a/spec/factories/v2_form_document.rb b/spec/factories/v2_form_document.rb index b66136dad..3700b74eb 100644 --- a/spec/factories/v2_form_document.rb +++ b/spec/factories/v2_form_document.rb @@ -65,7 +65,7 @@ what_happens_next_markdown { "We usually respond to applications within 10 working days." } end - trait :live? do + trait :live do ready_for_live end diff --git a/spec/features/email_confirmation_spec.rb b/spec/features/email_confirmation_spec.rb index 105f58b42..6329f8ddf 100644 --- a/spec/features/email_confirmation_spec.rb +++ b/spec/features/email_confirmation_spec.rb @@ -2,7 +2,7 @@ feature "Email confirmation", type: :feature do let(:steps) { [build(:v2_question_step, :with_text_settings, id: 1, routing_conditions: [], question_text:)] } - let(:form) { build :v2_form_document, :live?, form_id: 1, name: "Apply for a juggling license", steps:, start_page: 1 } + let(:form) { build :v2_form_document, :live, form_id: 1, name: "Apply for a juggling license", steps:, start_page: 1 } let(:question_text) { Faker::Lorem.question } let(:text_answer) { Faker::Lorem.sentence } diff --git a/spec/features/fill_in_and_submit_form_spec.rb b/spec/features/fill_in_and_submit_form_spec.rb index 6f6a691e3..6345440f4 100644 --- a/spec/features/fill_in_and_submit_form_spec.rb +++ b/spec/features/fill_in_and_submit_form_spec.rb @@ -2,7 +2,7 @@ feature "Fill in and submit a form", type: :feature do let(:steps) { [build(:v2_question_step, :with_text_settings, id: 1, routing_conditions: [], question_text:)] } - let(:form) { build :v2_form_document, :live?, form_id: 1, name: "Fill in this form", steps:, start_page: 1 } + let(:form) { build :v2_form_document, :live, form_id: 1, name: "Fill in this form", steps:, start_page: 1 } let(:question_text) { Faker::Lorem.question } let(:answer_text) { "Answer text" } let(:reference) { Faker::Alphanumeric.alphanumeric(number: 8).upcase } diff --git a/spec/features/fill_in_and_submit_form_with_csv_spec.rb b/spec/features/fill_in_and_submit_form_with_csv_spec.rb index 67da3e398..1f3f49f88 100644 --- a/spec/features/fill_in_and_submit_form_with_csv_spec.rb +++ b/spec/features/fill_in_and_submit_form_with_csv_spec.rb @@ -7,7 +7,7 @@ build(:v2_selection_question_step, only_one_option: false, id: 2, question_text: "Skipped question", next_step_id: 3), ] end - let(:form) { build :v2_form_document, :live?, form_id: 1, name: "Fill in this form", steps:, start_page: steps.first.id, submission_type: "email", submssion_format: %w[csv] } + let(:form) { build :v2_form_document, :live, form_id: 1, name: "Fill in this form", steps:, start_page: steps.first.id, submission_type: "email", submssion_format: %w[csv] } let(:reference) { Faker::Alphanumeric.alphanumeric(number: 8).upcase } let(:req_headers) { { "Accept" => "application/json" } } diff --git a/spec/features/fill_in_autocomplete_question_spec.rb b/spec/features/fill_in_autocomplete_question_spec.rb index 4a9bce727..372f285b0 100644 --- a/spec/features/fill_in_autocomplete_question_spec.rb +++ b/spec/features/fill_in_autocomplete_question_spec.rb @@ -2,7 +2,7 @@ feature "Fill in and submit a form with an autocomplete question", type: :feature do let(:steps) { [build(:v2_selection_question_step, id: 1, routing_conditions: [], question_text:, selection_options:)] } - let(:form) { build :v2_form_document, :live?, form_id: 1, name: "Fill in this form", steps:, start_page: 1 } + let(:form) { build :v2_form_document, :live, form_id: 1, name: "Fill in this form", steps:, start_page: 1 } let(:selection_options) { Array.new(31).each_with_index.map { |_element, index| { name: "Answer #{index}", value: "Answer #{index}" } } } let(:question_text) { Faker::Lorem.question } let(:answer_text) { "Answer 1" } diff --git a/spec/features/fill_in_file_upload_question_spec.rb b/spec/features/fill_in_file_upload_question_spec.rb index eab23a010..103990333 100644 --- a/spec/features/fill_in_file_upload_question_spec.rb +++ b/spec/features/fill_in_file_upload_question_spec.rb @@ -4,7 +4,7 @@ include ActiveJob::TestHelper let(:steps) { [build(:v2_question_step, answer_type: "file", id: 1, routing_conditions: [], question_text:)] } - let(:form) { build :v2_form_document, :live?, form_id: 1, name: "Fill in this form", steps:, start_page: 1 } + let(:form) { build :v2_form_document, :live, form_id: 1, name: "Fill in this form", steps:, start_page: 1 } let(:question_text) { Faker::Lorem.question } let(:answer_text) { "Answer 1" } let(:reference) { Faker::Alphanumeric.alphanumeric(number: 8).upcase } diff --git a/spec/features/fill_in_form_with_exit_page_spec.rb b/spec/features/fill_in_form_with_exit_page_spec.rb index 8302822bb..3c81b83e7 100644 --- a/spec/features/fill_in_form_with_exit_page_spec.rb +++ b/spec/features/fill_in_form_with_exit_page_spec.rb @@ -3,7 +3,7 @@ feature "Fill in and submit a form with an exit page", type: :feature do let(:routing_conditions) { [DataStruct.new(routing_page_id: 1, check_page_id: 1, answer_value: "Option 1", goto_page_id: nil, exit_page_heading: "This is an exit_page", exit_page_markdown: "This is the contents", validation_errors: [])] } let(:steps) { [build(:v2_selection_question_step, id: 1, routing_conditions:, question_text:)] } - let(:form) { build :v2_form_document, :live?, form_id: 1, name: "Fill in this form", steps:, start_page: 1 } + let(:form) { build :v2_form_document, :live, form_id: 1, name: "Fill in this form", steps:, start_page: 1 } let(:question_text) { Faker::Lorem.question } let(:reference) { Faker::Alphanumeric.alphanumeric(number: 8).upcase } diff --git a/spec/features/fill_in_single_repeatable_form_spec.rb b/spec/features/fill_in_single_repeatable_form_spec.rb index f2d8c2c60..62326cdb4 100644 --- a/spec/features/fill_in_single_repeatable_form_spec.rb +++ b/spec/features/fill_in_single_repeatable_form_spec.rb @@ -2,7 +2,7 @@ feature "Fill in and submit a form with a single repeatable question", type: :feature do let(:steps) { [build(:v2_question_step, :with_repeatable, answer_type: "number", question_text:)] } - let(:form) { build :v2_form_document, :live?, form_id: 42, name: "Form with repeating question", steps:, start_page: steps.first.id } + let(:form) { build :v2_form_document, :live, form_id: 42, name: "Form with repeating question", steps:, start_page: steps.first.id } let(:question_text) { Faker::Lorem.question } let(:first_answer_text) { "99" } diff --git a/spec/requests/forms/remove_file_controller_spec.rb b/spec/requests/forms/remove_file_controller_spec.rb index 6f82dd31d..23a972fd7 100644 --- a/spec/requests/forms/remove_file_controller_spec.rb +++ b/spec/requests/forms/remove_file_controller_spec.rb @@ -2,7 +2,7 @@ RSpec.describe Forms::RemoveFileController, type: :request do let(:form_data) do - build(:v2_form_document, :with_support, :live?, + build(:v2_form_document, :with_support, :live, form_id: 1, start_page: 1, privacy_policy_url: "http://www.example.gov.uk/privacy_policy", diff --git a/spec/requests/forms/review_file_controller_spec.rb b/spec/requests/forms/review_file_controller_spec.rb index 14be7ac07..670ca84ee 100644 --- a/spec/requests/forms/review_file_controller_spec.rb +++ b/spec/requests/forms/review_file_controller_spec.rb @@ -2,7 +2,7 @@ RSpec.describe Forms::ReviewFileController, type: :request do let(:form_data) do - build(:v2_form_document, :with_support, :live?, + build(:v2_form_document, :with_support, :live, form_id: 1, start_page: 1, privacy_policy_url: "http://www.example.gov.uk/privacy_policy",