From 75db0d387155bb9f1a135d08c25ebb24033a88bd Mon Sep 17 00:00:00 2001 From: Stephen Daly Date: Fri, 17 Jul 2026 12:45:32 +0100 Subject: [PATCH 1/9] Update reports to use delivery configurations Update reports to use the `delivery_configurations` rather then the deprecated `submission_type`, `submission_format`,`send_daily_submission_batch` and `send_weekly_submission_batch` attributes. --- .../reports/form_documents_service.rb | 18 ++++++--- .../reports/forms_csv_report_service.rb | 18 ++++++--- spec/factories/models/forms.rb | 10 +++++ spec/requests/reports_controller_spec.rb | 14 +++---- .../reports/feature_report_service_spec.rb | 20 ++++++---- .../reports/form_documents_service_spec.rb | 38 +++++++++++++------ .../reports/forms_csv_report_service_spec.rb | 13 ++++--- .../questions_csv_report_service_spec.rb | 2 +- 8 files changed, 88 insertions(+), 45 deletions(-) diff --git a/app/services/reports/form_documents_service.rb b/app/services/reports/form_documents_service.rb index 9961e9f78b..3c726ace62 100644 --- a/app/services/reports/form_documents_service.rb +++ b/app/services/reports/form_documents_service.rb @@ -41,23 +41,31 @@ def has_payments?(form_document) end def has_csv_submission_email_attachments(form_document) - form_document["content"]["submission_type"] == "email" && form_document["content"]["submission_format"].include?("csv") + form_document["content"]["delivery_configurations"].any? do |delivery_configuration| + delivery_configuration["delivery_method"] == "email" && + delivery_configuration["delivery_schedule"] == "immediate" && + delivery_configuration["formats"].include?("csv") + end end def has_json_submission_email_attachments(form_document) - form_document["content"]["submission_type"] == "email" && form_document["content"]["submission_format"].include?("json") + form_document["content"]["delivery_configurations"].any? do |delivery_configuration| + delivery_configuration["delivery_method"] == "email" && + delivery_configuration["delivery_schedule"] == "immediate" && + delivery_configuration["formats"].include?("json") + end end def has_daily_submission_csv(form_document) - form_document["content"]["send_daily_submission_batch"] + form_document["content"]["delivery_configurations"].any? { |c| c["delivery_schedule"] == "daily" } end def has_weekly_submission_csv(form_document) - form_document["content"]["send_weekly_submission_batch"] + form_document["content"]["delivery_configurations"].any? { |c| c["delivery_schedule"] == "weekly" } end def has_s3_submissions(form_document) - form_document["content"]["submission_type"] == "s3" + form_document["content"]["delivery_configurations"].any? { |c| c["delivery_method"] == "s3" } end def has_exit_pages?(form_document) diff --git a/app/services/reports/forms_csv_report_service.rb b/app/services/reports/forms_csv_report_service.rb index b42a2c8922..2f032aaefa 100644 --- a/app/services/reports/forms_csv_report_service.rb +++ b/app/services/reports/forms_csv_report_service.rb @@ -25,8 +25,7 @@ class Reports::FormsCsvReportService "Support phone", "Privacy policy URL", "What happens next markdown", - "Submission type", - "Submission formats", + "Delivery methods", "Daily submissions CSV enabled", "Weekly submissions CSV enabled", "Has Welsh translation", @@ -77,12 +76,19 @@ def form_row(form) form["content"]["support_phone"], form["content"]["privacy_policy_url"], form["content"]["what_happens_next_markdown"], - form["content"]["submission_type"], - form["content"]["submission_format"]&.sort&.join(" "), - form["content"]["send_daily_submission_batch"], - form["content"]["send_weekly_submission_batch"], + format_delivery_methods(form), + Reports::FormDocumentsService.has_daily_submission_csv(form), + Reports::FormDocumentsService.has_weekly_submission_csv(form), Reports::FormDocumentsService.has_welsh_translation(form), Reports::FormDocumentsService.copy_of_answers_enabled?(form), ] end + + def format_delivery_methods(form) + form["content"]["delivery_configurations"].map { |delivery_configuration| + next unless delivery_configuration["delivery_schedule"] == "immediate" + + "#{delivery_configuration['delivery_method']}: #{delivery_configuration['formats'].join(', ')}" + }.compact.join("\n") + end end diff --git a/spec/factories/models/forms.rb b/spec/factories/models/forms.rb index 4d1590cd6f..40ad9457b7 100644 --- a/spec/factories/models/forms.rb +++ b/spec/factories/models/forms.rb @@ -179,6 +179,16 @@ s3_bucket_aws_account_id { "123456789012" } s3_bucket_region { "eu-west-1" } end + + trait :with_email_delivery do + after(:create) do |form| + form.delivery_configurations.create!( + delivery_method: "email", + delivery_schedule: "immediate", + formats: [], + ) + end + end end end diff --git a/spec/requests/reports_controller_spec.rb b/spec/requests/reports_controller_spec.rb index 349ee3fd73..f1d0270365 100644 --- a/spec/requests/reports_controller_spec.rb +++ b/spec/requests/reports_controller_spec.rb @@ -2,7 +2,7 @@ RSpec.describe ReportsController, type: :request do let(:question_text) { "Question text" } - let(:forms) { create_list(:form, 4, :live) } + let(:forms) { create_list(:form, 4, :live, :with_email_delivery) } before do group = create :group @@ -255,7 +255,7 @@ describe "#forms_with_csv_submission_email_attachments" do let(:path) { report_forms_with_csv_submission_email_attachments_path(tag: :live) } - let(:form) { create(:form, :live, submission_type: "email", submission_format: %w[csv]) } + let(:form) { create(:form, :live, delivery_configurations: [create(:delivery_configuration, :immediate_email, formats: %w[csv])]) } let(:forms) { [form] } include_examples "unauthorized user is forbidden" @@ -280,7 +280,7 @@ describe "#forms_with_json_submission_email_attachments" do let(:path) { report_forms_with_json_submission_email_attachments_path(tag: :live) } - let(:form) { create(:form, :live, submission_type: "email", submission_format: %w[json]) } + let(:form) { create(:form, :live, delivery_configurations: [create(:delivery_configuration, :immediate_email, formats: %w[json])]) } let(:forms) { [form] } include_examples "unauthorized user is forbidden" @@ -305,7 +305,7 @@ describe "#forms_with_daily_submission_csv" do let(:path) { report_forms_with_daily_submission_csv_path(tag: :live) } - let(:form) { create(:form, :live, submission_type: "email", send_daily_submission_batch: true) } + let(:form) { create(:form, :live, delivery_configurations: [create(:delivery_configuration, :daily_email)]) } let(:forms) { [form] } include_examples "unauthorized user is forbidden" @@ -330,7 +330,7 @@ describe "#forms_with_weekly_submission_csv" do let(:path) { report_forms_with_weekly_submission_csv_path(tag: :live) } - let(:form) { create(:form, :live, submission_type: "email", send_weekly_submission_batch: true) } + let(:form) { create(:form, :live, delivery_configurations: [create(:delivery_configuration, :weekly_email)]) } let(:forms) { [form] } include_examples "unauthorized user is forbidden" @@ -355,7 +355,7 @@ describe "#forms_with_s3_submissions" do let(:path) { report_forms_with_s3_submissions_path(tag: :live) } - let(:form) { create(:form, :live, submission_type: "s3", submission_format: %w[json]) } + let(:form) { create(:form, :live, delivery_configurations: [create(:delivery_configuration, :s3, formats: %w[json])]) } let(:forms) { [form] } include_examples "unauthorized user is forbidden" @@ -667,7 +667,7 @@ end describe "#forms_with_csv_submission_enabled as csv" do - let(:form) { create(:form, :live, submission_type: "email", submission_format: %w[csv]) } + let(:form) { create(:form, :live, delivery_configurations: [create(:delivery_configuration, :immediate_email, formats: %w[csv])]) } let(:forms) { [form, *create_list(:form, 2, :live)] } let(:expected_csv_filename) { "live_forms_with_csv_submission_email_attachments_report-2025-05-15 15:31:57 UTC.csv" } diff --git a/spec/services/reports/feature_report_service_spec.rb b/spec/services/reports/feature_report_service_spec.rb index e976cffb0c..93d45e1b33 100644 --- a/spec/services/reports/feature_report_service_spec.rb +++ b/spec/services/reports/feature_report_service_spec.rb @@ -25,8 +25,6 @@ let(:form_with_all_answer_types) do create(:form, :live, :with_support, - submission_type: "email", - submission_format: %w[csv], payment_url: "https://www.gov.uk/payments/organisation/service", send_copy_of_answers: "enabled", pages: [ @@ -39,22 +37,28 @@ create(:page, answer_type: "phone_number"), create(:page, :with_selection_settings, is_optional: true), create(:page, :with_single_line_text_settings, is_repeatable: true), + ], + delivery_configurations: [ + create(:delivery_configuration, :immediate_email, formats: %w[csv]), ]) end let(:form_with_a_few_answer_types) do create(:form, :live, - submission_type: "email", - submission_format: %w[csv json], - send_daily_submission_batch: true, - send_weekly_submission_batch: true, pages: [ create(:page, answer_type: "email"), *create_list(:page, 3, answer_type: "name"), + ], + delivery_configurations: [ + create(:delivery_configuration, :immediate_email, formats: %w[csv json]), + create(:delivery_configuration, :daily_email), + create(:delivery_configuration, :weekly_email), ]) end let(:branch_route_form) do - form = create(:form, :live, :ready_for_routing, submission_type: "s3", submission_format: %w[csv]) + form = create(:form, :live, :ready_for_routing, delivery_configurations: [ + create(:delivery_configuration, :s3, formats: %w[csv]), + ]) create(:condition, :with_exit_page, routing_page_id: form.pages[0].id, check_page_id: form.pages[0].id, answer_value: "Option 1") create(:condition, routing_page_id: form.pages[1].id, check_page_id: form.pages[1].id, answer_value: "Option 1", goto_page_id: form.pages[3].id) create(:condition, routing_page_id: form.pages[2].id, check_page_id: form.pages[1].id, goto_page_id: form.pages[4].id) @@ -475,7 +479,7 @@ end describe "#forms_with_s3_submissions" do - it "returns live forms with json enabled" do + it "returns live forms with s3 submissions" do forms = described_class.new(form_documents).forms_with_s3_submissions expect(forms.length).to eq 1 expect(forms).to match [ diff --git a/spec/services/reports/form_documents_service_spec.rb b/spec/services/reports/form_documents_service_spec.rb index 1fcd09cff2..a650741a4f 100644 --- a/spec/services/reports/form_documents_service_spec.rb +++ b/spec/services/reports/form_documents_service_spec.rb @@ -242,37 +242,51 @@ end describe ".has_daily_submission_csv" do - context "when the form has send_daily_submission_batch enabled" do - let(:form_document) { create(:form, :live, send_daily_submission_batch: true).live_form_document } + subject(:daily_submission_batch_enabled) do + described_class.has_daily_submission_csv(form_document) + end + + context "when form has a daily delivery_configuration" do + let(:form_document) do + create(:form, :live, delivery_configurations: [create(:delivery_configuration, :daily_email)]) + .live_form_document + end it "returns true" do - expect(described_class.has_daily_submission_csv(form_document)).to be true + expect(daily_submission_batch_enabled).to be true end end - context "when the form has send_daily_submission_batch disabled" do - let(:form_document) { create(:form, :live, send_daily_submission_batch: false).live_form_document } + context "when form does not have a daily delivery_configuration" do + let(:form_document) { create(:form, :live).live_form_document } it "returns false" do - expect(described_class.has_daily_submission_csv(form_document)).to be false + expect(daily_submission_batch_enabled).to be false end end end describe ".has_weekly_submission_csv" do - context "when the form has send_weekly_submission_batch enabled" do - let(:form_document) { create(:form, :live, send_weekly_submission_batch: true).live_form_document } + subject(:weekly_submission_batch_enabled) do + described_class.has_weekly_submission_csv(form_document) + end + + context "when form has weekly delivery_configuration" do + let(:form_document) do + create(:form, :live, delivery_configurations: [create(:delivery_configuration, :weekly_email)]) + .live_form_document + end it "returns true" do - expect(described_class.has_weekly_submission_csv(form_document)).to be true + expect(weekly_submission_batch_enabled).to be true end end - context "when the form has send_weekly_submission_batch disabled" do - let(:form_document) { create(:form, :live, send_weekly_submission_batch: false).live_form_document } + context "when form does not have a weekly delivery_configuration" do + let(:form_document) { create(:form, :live).live_form_document } it "returns false" do - expect(described_class.has_weekly_submission_csv(form_document)).to be false + expect(weekly_submission_batch_enabled).to be false end end end diff --git a/spec/services/reports/forms_csv_report_service_spec.rb b/spec/services/reports/forms_csv_report_service_spec.rb index c1f9632fed..106c4cfaf3 100644 --- a/spec/services/reports/forms_csv_report_service_spec.rb +++ b/spec/services/reports/forms_csv_report_service_spec.rb @@ -26,11 +26,7 @@ create(:form, :live, :with_support, :with_welsh_translation, - submission_type: "email", - submission_format: %w[csv json], payment_url: "https://www.gov.uk/payments/organisation/service", - send_daily_submission_batch: true, - send_weekly_submission_batch: true, pages: [ create(:page, :with_address_settings, is_repeatable: true), create(:page, :with_date_settings), @@ -41,6 +37,12 @@ create(:page, answer_type: "phone_number"), create(:page, :with_selection_settings, is_optional: true), create(:page, :with_single_line_text_settings, is_repeatable: true), + ], + delivery_configurations: [ + create(:delivery_configuration, :immediate_email, formats: %w[csv json]), + create(:delivery_configuration, :s3, formats: %w[csv]), + create(:delivery_configuration, :daily_email), + create(:delivery_configuration, :weekly_email), ]) end let(:forms) { [form, create(:form, :live)] } @@ -79,8 +81,7 @@ form.support_phone, form.privacy_policy_url, form.what_happens_next_markdown, - "email", - "csv json", + "email: csv, json\ns3: csv", "true", "true", "true", diff --git a/spec/services/reports/questions_csv_report_service_spec.rb b/spec/services/reports/questions_csv_report_service_spec.rb index 99da30ffdb..1d22de53b6 100644 --- a/spec/services/reports/questions_csv_report_service_spec.rb +++ b/spec/services/reports/questions_csv_report_service_spec.rb @@ -24,7 +24,7 @@ end end let(:form_with_all_answer_types) do - create(:form, :live, :with_support, submission_type: "email", payment_url: "https://www.gov.uk/payments/organisation/service", pages: [ + create(:form, :live, :with_support, payment_url: "https://www.gov.uk/payments/organisation/service", pages: [ create(:page, :with_address_settings, is_repeatable: true), create(:page, :with_date_settings), create(:page, answer_type: "email"), From 584079580c87f927db1f10be5a650fe4a0a135f5 Mon Sep 17 00:00:00 2001 From: Stephen Daly Date: Fri, 17 Jul 2026 13:14:07 +0100 Subject: [PATCH 2/9] Only read/update delivery configurations when changing attachments When enabling/disabling CSV/JSON submission attachments, only read and upate the immediate email delivery configuration, rather than the deprecated submission_type and submission_format fields on the form. --- .../submission_attachments_controller.rb | 10 +- .../forms/submission_attachments_input.rb | 9 +- app/models/form.rb | 1 + .../submission_attachments_input_spec.rb | 165 +++++++++--------- .../submission_attachments_controller_spec.rb | 30 +++- 5 files changed, 115 insertions(+), 100 deletions(-) diff --git a/app/controllers/forms/submission_attachments_controller.rb b/app/controllers/forms/submission_attachments_controller.rb index c26b2eaf38..c393ea098b 100644 --- a/app/controllers/forms/submission_attachments_controller.rb +++ b/app/controllers/forms/submission_attachments_controller.rb @@ -1,6 +1,6 @@ module Forms class SubmissionAttachmentsController < FormsController - before_action :submission_type_email? + before_action :has_email_delivery_configuration? def new authorize current_form, :can_view_form? @@ -24,14 +24,14 @@ def submission_attachments_input_params params.require(:forms_submission_attachments_input).permit(submission_format: []).merge(form: current_form) end - def submission_type_email? - redirect_to error_404_path unless current_form.email? + def has_email_delivery_configuration? + redirect_to error_404_path if current_form.immediate_email_delivery_configuration.blank? end def success_message(form) - return nil unless form.submission_format_previously_changed? + return nil unless form.immediate_email_delivery_configuration.formats_previously_changed? - case form.submission_format + case form.immediate_email_delivery_configuration.formats when [] t("banner.success.form.receive_no_attachments") when %w[csv] diff --git a/app/input_objects/forms/submission_attachments_input.rb b/app/input_objects/forms/submission_attachments_input.rb index a98fc25bd4..d7061f2dce 100644 --- a/app/input_objects/forms/submission_attachments_input.rb +++ b/app/input_objects/forms/submission_attachments_input.rb @@ -9,11 +9,8 @@ class Forms::SubmissionAttachmentsInput < BaseInput def submit return false if invalid? - formats = submission_format.compact_blank - form.submission_format = formats - - delivery_configuration = form.delivery_configurations.where(delivery_method: "email", delivery_schedule: "immediate").first_or_initialize - delivery_configuration.formats = formats + delivery_configuration = form.immediate_email_delivery_configuration || form.build_immediate_email_delivery_configuration + delivery_configuration.formats = submission_format.compact_blank delivery_configuration.save! form.delivery_configurations.reload @@ -21,7 +18,7 @@ def submit end def assign_form_values - self.submission_format = form.submission_format + self.submission_format = form.immediate_email_delivery_configuration&.formats || [] self end diff --git a/app/models/form.rb b/app/models/form.rb index 2ef26ecc37..098692b90c 100644 --- a/app/models/form.rb +++ b/app/models/form.rb @@ -18,6 +18,7 @@ class Form < ApplicationRecord has_one :draft_form_document, -> { where tag: "draft", language: :en }, class_name: "FormDocument" has_many :conditions, through: :pages, source: :routing_conditions has_many :delivery_configurations, dependent: :destroy + has_one :immediate_email_delivery_configuration, -> { where delivery_method: "email", delivery_schedule: "immediate" }, class_name: "DeliveryConfiguration" translates :name, :privacy_policy_url, diff --git a/spec/input_objects/forms/submission_attachments_input_spec.rb b/spec/input_objects/forms/submission_attachments_input_spec.rb index 3dc7d361c6..db40e4b055 100644 --- a/spec/input_objects/forms/submission_attachments_input_spec.rb +++ b/spec/input_objects/forms/submission_attachments_input_spec.rb @@ -7,7 +7,7 @@ context "when given a valid CSV and JSON submission format" do let(:submission_format) { %w[csv json] } - it "validates succesfully" do + it "validates successfully" do submission_attachments_input = described_class.new(form:, submission_format:) expect(submission_attachments_input).to be_valid @@ -17,7 +17,7 @@ context "when given a valid no attachments submission format" do let(:submission_format) { [""] } - it "validates succesfully" do + it "validates successfully" do submission_attachments_input = described_class.new(form:, submission_format:) expect(submission_attachments_input).to be_valid @@ -50,92 +50,87 @@ describe "#submit" do subject(:submission_attachments_input) { described_class.new(form:, submission_format: updated_submission_format) } - let(:form) { create(:form, submission_format: []) } + let(:form) { create(:form) } context "when valid" do let(:updated_submission_format) { %w[csv] } - it "updates the form's submission_format" do - expect { - submission_attachments_input.submit - }.to change(form, :submission_format).to(updated_submission_format) - end - end - - context "when invalid" do - let(:updated_submission_format) { %w[banana] } - - it "does not update the form's submission_format" do - expect { - submission_attachments_input.submit - }.not_to change(form, :submission_format) - end - end - - context "when an immediate email DeliveryConfiguration exists" do - let(:immediate_delivery_configuration) { create(:delivery_configuration, form:, formats: %w[csv]) } - let(:daily_delivery_configuration) { create(:delivery_configuration, :daily_email, form:, formats: %w[csv]) } - let(:updated_submission_format) { %w[csv json] } - - before do - immediate_delivery_configuration - daily_delivery_configuration + context "when an immediate email DeliveryConfiguration exists" do + let(:immediate_delivery_configuration) { create(:delivery_configuration, form:, formats: %w[csv]) } + let(:daily_delivery_configuration) { create(:delivery_configuration, :daily_email, form:, formats: %w[csv]) } + let(:updated_submission_format) { %w[csv json] } + + before do + immediate_delivery_configuration + daily_delivery_configuration + end + + it "updates the immediate DeliveryConfiguration formats" do + expect { + submission_attachments_input.submit + }.to change { immediate_delivery_configuration.reload.formats }.from(%w[csv]).to(updated_submission_format) + + expect(form.draft_form_document.reload.content["delivery_configurations"]).to contain_exactly( + { + "delivery_method" => "email", + "delivery_schedule" => "immediate", + "formats" => updated_submission_format, + }, + { + "delivery_method" => "email", + "delivery_schedule" => "daily", + "formats" => %w[csv], + }, + ) + end + + it "does not change the daily DeliveryConfiguration formats" do + expect { + submission_attachments_input.submit + }.not_to(change { daily_delivery_configuration.reload.formats }) + end end - it "updates the immediate DeliveryConfiguration formats" do - expect { - submission_attachments_input.submit - }.to change { immediate_delivery_configuration.reload.formats }.from(%w[csv]).to(updated_submission_format) - - expect(form.draft_form_document.reload.content["delivery_configurations"]).to contain_exactly( - { - "delivery_method" => "email", - "delivery_schedule" => "immediate", - "formats" => updated_submission_format, - }, - { - "delivery_method" => "email", - "delivery_schedule" => "daily", - "formats" => %w[csv], - }, - ) - end - - it "does not change the daily DeliveryConfiguration formats" do - expect { - submission_attachments_input.submit - }.not_to(change { daily_delivery_configuration.reload.formats }) + context "when an immediate email DeliveryConfiguration does not exist" do + let(:s3_delivery_configuration) { create(:delivery_configuration, :s3, form:, formats: %w[csv]) } + let(:daily_delivery_configuration) { create(:delivery_configuration, :daily_email, form:, formats: %w[csv]) } + let(:updated_submission_format) { %w[csv json] } + + before do + s3_delivery_configuration + daily_delivery_configuration + end + + it "creates an immediate email DeliveryConfiguration" do + expect { + submission_attachments_input.submit + }.to change(DeliveryConfiguration, :count).by(1) + + expect(form.delivery_configurations.pluck(:delivery_method, :delivery_schedule, :formats)).to contain_exactly( + ["email", "immediate", updated_submission_format], + ["email", "daily", %w[csv]], + ["s3", "immediate", %w[csv]], + ) + + expect(form.draft_form_document.reload.content["delivery_configurations"]).to include( + { + "delivery_method" => "email", + "delivery_schedule" => "immediate", + "formats" => updated_submission_format, + }, + ) + end end end - context "when an immediate email DeliveryConfiguration does not exist" do - let(:s3_delivery_configuration) { create(:delivery_configuration, :s3, form:, formats: %w[csv]) } - let(:daily_delivery_configuration) { create(:delivery_configuration, :daily_email, form:, formats: %w[csv]) } - let(:updated_submission_format) { %w[csv json] } - - before do - s3_delivery_configuration - daily_delivery_configuration - end + context "when invalid" do + let(:updated_submission_format) { %w[banana] } + let(:form) { create(:form, delivery_configurations: [create(:delivery_configuration, :immediate_email)]) } - it "creates an immediate email DeliveryConfiguration" do + it "does not change the formats" do expect { submission_attachments_input.submit - }.to change(DeliveryConfiguration, :count).by(1) - - expect(form.delivery_configurations.pluck(:delivery_method, :delivery_schedule, :formats)).to contain_exactly( - ["email", "immediate", updated_submission_format], - ["email", "daily", %w[csv]], - ["s3", "immediate", %w[csv]], - ) - - expect(form.draft_form_document.reload.content["delivery_configurations"]).to include( - { - "delivery_method" => "email", - "delivery_schedule" => "immediate", - "formats" => updated_submission_format, - }, - ) + }.not_to(change { form.delivery_configurations.first.reload.formats }) end end end @@ -143,8 +138,12 @@ describe "#assign_form_values" do subject(:submission_attachments_input) { described_class.new(form:) } - context "when the original form has an empty array submission format" do - let(:form) { create(:form, submission_format: []) } + context "when the form has no formats" do + let(:form) do + create(:form, delivery_configurations: [ + create(:delivery_configuration, :immediate_email, formats: []), + ]) + end it "sets the submission format value to an empty array" do submission_attachments_input.assign_form_values @@ -153,10 +152,14 @@ end end - context "when the original form has a csv and json submission format" do - let(:form) { create(:form, submission_format: %w[csv json]) } + context "when the form has a csv and json submission format" do + let(:form) do + create(:form, delivery_configurations: [ + create(:delivery_configuration, :immediate_email, formats: %w[csv json]), + ]) + end - it "sets the submission format value to an empty array" do + it "sets the submission format value to the correct array" do submission_attachments_input.assign_form_values expect(submission_attachments_input.submission_format).to eq(%w[csv json]) diff --git a/spec/requests/forms/submission_attachments_controller_spec.rb b/spec/requests/forms/submission_attachments_controller_spec.rb index 9c21c83fc6..d1104a7010 100644 --- a/spec/requests/forms/submission_attachments_controller_spec.rb +++ b/spec/requests/forms/submission_attachments_controller_spec.rb @@ -1,7 +1,11 @@ require "rails_helper" RSpec.describe Forms::SubmissionAttachmentsController, type: :request do - let(:form) { create(:form, :live, submission_format: original_submission_format) } + let(:form) do + create(:form, :live, delivery_configurations: [ + create(:delivery_configuration, :immediate_email, formats: original_submission_format), + ]) + end let(:user) { standard_user } let(:original_submission_format) { [] } let(:group) { create(:group, organisation: user.organisation) } @@ -25,8 +29,13 @@ expect(assigns).to include submission_attachments_input: an_instance_of(Forms::SubmissionAttachmentsInput) end - context "when the submission_type is s3" do - let(:form) { create(:form, submission_type: "s3") } + context "when the form does not have an immediate email delivery configuration" do + let(:form) do + create(:form, delivery_configurations: [ + create(:delivery_configuration, :s3), + create(:delivery_configuration, :daily_email), + ]) + end it "returns a 404 page" do expect(response).to redirect_to :error_404 @@ -40,10 +49,10 @@ context "when params are valid" do let(:submission_format) { %w[csv json] } - it "updates the form submission format" do + it "updates the submission format for the email delivery method" do expect { post(submission_attachments_path(form_id: form.id), params:) - }.to change { form.reload.submission_format }.to(submission_format) + }.to change { form.reload.delivery_configurations.first.formats }.to(submission_format) end it "redirects you to the form overview page" do @@ -109,7 +118,7 @@ it "does not update the form" do expect { post(submission_attachments_path(form_id: form.id), params:) - }.not_to(change { form.reload.submission_format }) + }.not_to(change { form.reload.delivery_configurations.first.formats }) end it "re-renders the page with an error" do @@ -119,8 +128,13 @@ end end - context "when the submission_type is s3" do - let(:form) { create(:form, submission_type: "s3") } + context "when the form does not have an immediate email delivery configuration" do + let(:form) do + create(:form, delivery_configurations: [ + create(:delivery_configuration, :s3), + create(:delivery_configuration, :daily_email), + ]) + end let(:submission_format) { %w[csv] } it "returns a 404 page" do From 55ecd34331722ef335445e3e0434757624b4d669 Mon Sep 17 00:00:00 2001 From: Stephen Daly Date: Fri, 17 Jul 2026 16:47:03 +0100 Subject: [PATCH 3/9] Only use delivery configurations when updating batch settings Only read and update the form's DeliveryConfigurations when updating whether daily/weekly batch submission emails are enabled. This will allow us to remove the `send_daily_submission_batch` and `send_weekly_submission_batch` attributes from the Form model. --- .../forms/batch_submissions_controller.rb | 15 ++- .../forms/batch_submissions_input.rb | 25 +++- .../forms/batch_submissions_input_spec.rb | 122 +++++++++++++----- .../batch_submissions_controller_spec.rb | 26 ++-- .../batch_submissions/new.html.erb_spec.rb | 29 +++-- 5 files changed, 151 insertions(+), 66 deletions(-) diff --git a/app/controllers/forms/batch_submissions_controller.rb b/app/controllers/forms/batch_submissions_controller.rb index 299da43a18..2601073aad 100644 --- a/app/controllers/forms/batch_submissions_controller.rb +++ b/app/controllers/forms/batch_submissions_controller.rb @@ -10,7 +10,8 @@ def create @batch_submissions_input = Forms::BatchSubmissionsInput.new(batch_submissions_input_params) if @batch_submissions_input.submit - redirect_to form_path(current_form.id), success: success_message(current_form) + success_message = success_message(@batch_submissions_input.batch_frequencies, @batch_submissions_input.delivery_configurations_changed?) + redirect_to form_path(current_form.id), success: success_message else render :new, status: :unprocessable_content end @@ -22,14 +23,16 @@ def batch_submissions_input_params params.require(:forms_batch_submissions_input).permit(batch_frequencies: []).merge(form: current_form) end - def success_message(form) - return nil unless form.send_daily_submission_batch_previously_changed? || form.send_weekly_submission_batch_previously_changed? + def success_message(batch_frequencies, delivery_configurations_changed) + return nil unless delivery_configurations_changed - if form.send_daily_submission_batch && form.send_weekly_submission_batch + batch_frequencies = Array(batch_frequencies) + + if batch_frequencies.include?("daily") && batch_frequencies.include?("weekly") t("banner.success.form.batch_submissions.daily_and_weekly_enabled") - elsif form.send_daily_submission_batch + elsif batch_frequencies.include?("daily") t("banner.success.form.batch_submissions.daily_enabled") - elsif form.send_weekly_submission_batch + elsif batch_frequencies.include?("weekly") t("banner.success.form.batch_submissions.weekly_enabled") else t("banner.success.form.batch_submissions.disabled") diff --git a/app/input_objects/forms/batch_submissions_input.rb b/app/input_objects/forms/batch_submissions_input.rb index 12f1cff4fa..258f5b36f1 100644 --- a/app/input_objects/forms/batch_submissions_input.rb +++ b/app/input_objects/forms/batch_submissions_input.rb @@ -2,16 +2,16 @@ class Forms::BatchSubmissionsInput < BaseInput attr_accessor :form, :batch_frequencies def submit + @delivery_configurations_before = batch_delivery_configuration_snapshot selected_frequencies = Array(batch_frequencies) - form.send_daily_submission_batch = selected_frequencies.include?("daily") - form.send_weekly_submission_batch = selected_frequencies.include?("weekly") - %w[daily weekly].each do |frequency| + matching_delivery_configurations = form.delivery_configurations.where(delivery_method: "email", delivery_schedule: frequency) + if selected_frequencies.include?(frequency) form.delivery_configurations.find_or_create_by!(delivery_method: "email", delivery_schedule: frequency, formats: %w[csv]) else - form.delivery_configurations.where(delivery_method: "email", delivery_schedule: frequency).destroy_all + matching_delivery_configurations.destroy_all end end @@ -19,10 +19,23 @@ def submit form.save_draft! end + def delivery_configurations_changed? + @delivery_configurations_before != batch_delivery_configuration_snapshot + end + def assign_form_values self.batch_frequencies ||= [] - self.batch_frequencies << "daily" if form.send_daily_submission_batch - self.batch_frequencies << "weekly" if form.send_weekly_submission_batch + self.batch_frequencies << "daily" if form.delivery_configurations.daily.any? + self.batch_frequencies << "weekly" if form.delivery_configurations.weekly.any? self end + +private + + def batch_delivery_configuration_snapshot + form.delivery_configurations + .where(delivery_method: "email", delivery_schedule: %w[daily weekly]) + .pluck(:delivery_method, :delivery_schedule, :formats) + .sort + end end diff --git a/spec/input_objects/forms/batch_submissions_input_spec.rb b/spec/input_objects/forms/batch_submissions_input_spec.rb index 699f431535..6095aafe93 100644 --- a/spec/input_objects/forms/batch_submissions_input_spec.rb +++ b/spec/input_objects/forms/batch_submissions_input_spec.rb @@ -15,11 +15,6 @@ before { input.submit } - it "updates the form flags" do - expect(form.reload.send_daily_submission_batch).to be(true) - expect(form.reload.send_weekly_submission_batch).to be(true) - end - it "creates delivery configurations for both frequencies" do expect(form.delivery_configurations.pluck(:delivery_method, :delivery_schedule, :formats)).to contain_exactly( ["email", "daily", %w[csv]], ["email", "weekly", %w[csv]] @@ -40,24 +35,25 @@ }, ) end + + it "reports that the delivery configurations changed" do + expect(input.delivery_configurations_changed?).to be(true) + end end context "when only daily is selected and both delivery configurations already exist" do - let(:form) { create(:form, send_daily_submission_batch: true, send_weekly_submission_batch: true) } + let(:form) do + create(:form, delivery_configurations: [ + create(:delivery_configuration, :daily_email), + create(:delivery_configuration, :weekly_email), + ]) + end let(:batch_frequencies) { %w[daily] } before do - create(:delivery_configuration, :daily_email, form:) - create(:delivery_configuration, :weekly_email, form:) - input.submit end - it "updates the form flags" do - expect(form.reload.send_daily_submission_batch).to be(true) - expect(form.reload.send_weekly_submission_batch).to be(false) - end - it "keeps daily and removes weekly delivery configurations" do expect(form.delivery_configurations.order(:delivery_schedule).pluck(:delivery_method, :delivery_schedule, :formats)).to contain_exactly( ["email", "daily", %w[csv]], @@ -73,24 +69,25 @@ }, ) end + + it "reports that the delivery configurations changed" do + expect(input.delivery_configurations_changed?).to be(true) + end end context "when neither daily or weekly are selected" do - let(:form) { create(:form, send_daily_submission_batch: true, send_weekly_submission_batch: true) } + let(:form) do + create(:form, delivery_configurations: [ + create(:delivery_configuration, :daily_email), + create(:delivery_configuration, :weekly_email), + ]) + end let(:batch_frequencies) { [] } before do - create(:delivery_configuration, :daily_email, form:) - create(:delivery_configuration, :weekly_email, form:) - input.submit end - it "clears the form flags" do - expect(form.reload.send_daily_submission_batch).to be(false) - expect(form.reload.send_weekly_submission_batch).to be(false) - end - it "removes all batch delivery configurations" do expect(form.delivery_configurations).to be_empty end @@ -98,26 +95,81 @@ it "updates the draft form document" do expect(form.draft_form_document.reload.content["delivery_configurations"]).to be_empty end + + it "reports that the delivery configurations changed" do + expect(input.delivery_configurations_changed?).to be(true) + end + end + + context "when the selected batch frequencies match the existing delivery configurations" do + let(:form) do + create(:form, delivery_configurations: [ + create(:delivery_configuration, :daily_email), + ]) + end + let(:batch_frequencies) { %w[daily] } + + before do + input.submit + end + + it "keeps the batch delivery configurations unchanged" do + expect(form.delivery_configurations.pluck(:delivery_method, :delivery_schedule, :formats)).to contain_exactly( + ["email", "daily", %w[csv]], + ) + end + + it "reports that the delivery configurations did not change" do + expect(input.delivery_configurations_changed?).to be(false) + end end end describe "#assign_form_values" do subject(:input) { described_class.new(form:) } - [ - [true, false, %w[daily]], - [false, true, %w[weekly]], - [true, true, %w[daily weekly]], - [false, false, []], - ].each do |send_daily_submission_batch, send_weekly_submission_batch, expected| - context "when send_daily_submission_batch is #{send_daily_submission_batch} and send_weekly_submission_batch is #{send_weekly_submission_batch}" do - let(:form) { create(:form, send_daily_submission_batch:, send_weekly_submission_batch:) } + context "when the form has a daily batch delivery configuration" do + let(:form) { create(:form, delivery_configurations: [create(:delivery_configuration, :daily_email)]) } + + it "sets batch_frequencies to daily" do + input.assign_form_values + + expect(input.batch_frequencies).to match_array(%w[daily]) + end + end + + context "when the form has a weekly batch delivery configuration" do + let(:form) { create(:form, delivery_configurations: [create(:delivery_configuration, :weekly_email)]) } + + it "sets batch_frequencies to weekly" do + input.assign_form_values + + expect(input.batch_frequencies).to match_array(%w[weekly]) + end + end + + context "when the form has daily and weekly batch delivery configurations" do + let(:form) do + create(:form, delivery_configurations: [ + create(:delivery_configuration, :daily_email), + create(:delivery_configuration, :weekly_email), + ]) + end + + it "sets batch_frequencies to daily and weekly" do + input.assign_form_values + + expect(input.batch_frequencies).to match_array(%w[daily weekly]) + end + end + + context "when the form has no batch delivery configurations" do + let(:form) { create(:form) } - it "sets batch_frequencies to #{expected.inspect}" do - input.assign_form_values + it "sets batch_frequencies to an empty array" do + input.assign_form_values - expect(input.batch_frequencies).to match_array(expected) - end + expect(input.batch_frequencies).to be_empty end end end diff --git a/spec/requests/forms/batch_submissions_controller_spec.rb b/spec/requests/forms/batch_submissions_controller_spec.rb index f0a85b1c70..18a3fa7ab0 100644 --- a/spec/requests/forms/batch_submissions_controller_spec.rb +++ b/spec/requests/forms/batch_submissions_controller_spec.rb @@ -1,9 +1,7 @@ require "rails_helper" RSpec.describe Forms::BatchSubmissionsController, type: :request do - let(:form) { create(:form, :live, send_daily_submission_batch:, send_weekly_submission_batch:) } - let(:send_daily_submission_batch) { false } - let(:send_weekly_submission_batch) { false } + let(:form) { create(:form, :live) } let(:current_user) { standard_user } let(:group) { create(:group, organisation: standard_user.organisation) } @@ -45,8 +43,10 @@ end it "updates the form" do - expect(form.reload.send_daily_submission_batch).to be true - expect(form.reload.send_weekly_submission_batch).to be true + expect(form.reload.delivery_configurations.pluck(:delivery_method, :delivery_schedule, :formats)).to contain_exactly( + ["email", "daily", %w[csv]], + ["email", "weekly", %w[csv]], + ) end it "redirects to the form overview page" do @@ -78,8 +78,12 @@ end context "when daily and weekly batch checkboxes are unchecked" do - let(:send_daily_submission_batch) { true } - let(:send_weekly_submission_batch) { true } + let(:form) do + create(:form, :live, delivery_configurations: [ + create(:delivery_configuration, :daily_email), + create(:delivery_configuration, :weekly_email), + ]) + end let(:batch_frequencies) { [] } it "displays a success flash message indicating that batch emails are disabled" do @@ -87,8 +91,12 @@ end end - context "when the setting is unchanged" do - let(:send_daily_submission_batch) { true } + context "when the delivery configurations are unchanged" do + let(:form) do + create(:form, :live, delivery_configurations: [ + create(:delivery_configuration, :daily_email), + ]) + end let(:batch_frequencies) { %w[daily] } it "does not display a flash message" do diff --git a/spec/views/forms/batch_submissions/new.html.erb_spec.rb b/spec/views/forms/batch_submissions/new.html.erb_spec.rb index 8184cf3992..37720a5dcf 100644 --- a/spec/views/forms/batch_submissions/new.html.erb_spec.rb +++ b/spec/views/forms/batch_submissions/new.html.erb_spec.rb @@ -1,9 +1,7 @@ require "rails_helper" describe "forms/batch_submissions/new.html.erb" do - let(:send_daily_submission_batch) { true } - let(:send_weekly_submission_batch) { true } - let(:form) { build(:form, id: 1, send_daily_submission_batch:, send_weekly_submission_batch:) } + let(:form) { create(:form) } let(:batch_submissions_input) { Forms::BatchSubmissionsInput.new(form:).assign_form_values } before do @@ -39,25 +37,36 @@ expect(rendered).to have_css(".govuk-label[for='forms-batch-submissions-input-batch-frequencies-daily-field']", text: "Get a daily CSV of submissions") end - context "when the form has send_daily_submission_batch set to true" do - let(:send_daily_submission_batch) { true } + context "when the form has daily batches enabled" do + let(:form) do + create(:form, delivery_configurations: [ + create(:delivery_configuration, :daily_email), + ]) + end it "renders the checkbox as checked" do expect(rendered).to have_checked_field("forms-batch-submissions-input-batch-frequencies-daily-field") end end - context "when the form has send_weekly_submission_batch set to true" do - let(:send_weekly_submission_batch) { true } + context "when the form has weekly batches enabled" do + let(:form) do + create(:form, delivery_configurations: [ + create(:delivery_configuration, :weekly_email), + ]) + end - it "renders the checkboxes as unchecked" do + it "renders the checkbox as checked" do expect(rendered).to have_checked_field("forms-batch-submissions-input-batch-frequencies-weekly-field") end end context "when the form has batch submissions disabled" do - let(:send_daily_submission_batch) { false } - let(:send_weekly_submission_batch) { false } + let(:form) do + create(:form, delivery_configurations: [ + create(:delivery_configuration, :immediate_email), + ]) + end it "renders the checkboxes as unchecked" do expect(rendered).to have_unchecked_field("forms-batch-submissions-input-batch-frequencies-daily-field") From 602e3065db7d4a3dd010e68cfaac7aca9835d454 Mon Sep 17 00:00:00 2001 From: Stephen Daly Date: Fri, 17 Jul 2026 15:24:56 +0100 Subject: [PATCH 4/9] Remove unused translations These translations are for an input object that no longer exists --- config/locales/en.yml | 9 --------- 1 file changed, 9 deletions(-) diff --git a/config/locales/en.yml b/config/locales/en.yml index dac8e82d62..a06fbbfbbd 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -84,10 +84,6 @@ en: submission_format: blank: Sorry, there was a problem. Please try again. invalid_submission_format: Sorry, there was a problem. Please try again. - forms/submission_type_input: - attributes: - submission_type: - blank: Choose if you want to get completed forms as CSV files group_member_input: attributes: member_email_address: @@ -1055,9 +1051,6 @@ en: submission_format_options: csv: Get a CSV file of each completed form json: Get a JSON file of each completed form - forms_submission_type_input: - submission_type_options: - email_with_csv: Get CSV files group: name: Enter a name for your new group group_member_input: @@ -1214,8 +1207,6 @@ en: send_copy_of_answers: Do you want to give people the option to get a copy of their answers by email? forms_submission_attachments_input: submission_format: Do you want to get a CSV or JSON file of each completed form? - forms_submission_type_input: - submission_type: Do you want to get completed forms as CSV files? mou_signature: crown_agreed: Do you agree to the MOU? non_crown_agreed: Do you agree to the GOV.UK Forms agreement? From 2520cb59cab5c3f36284edfef543eb410f75e767 Mon Sep 17 00:00:00 2001 From: Stephen Daly Date: Fri, 17 Jul 2026 15:42:18 +0100 Subject: [PATCH 5/9] Update live form page to use delivery_configurations Use delivery_configurations instead of deprectated submission_type, submission_format, send_daily_submission_batch and send_weekly_submission_batch fields on the live form page. --- app/models/form_document/content.rb | 23 +++++++++ app/views/forms/_made_live_form.html.erb | 12 ++--- .../forms/_made_live_form.html.erb_spec.rb | 50 +++++++++++-------- 3 files changed, 57 insertions(+), 28 deletions(-) diff --git a/app/models/form_document/content.rb b/app/models/form_document/content.rb index a0b0e73cf8..e1b5d009f4 100644 --- a/app/models/form_document/content.rb +++ b/app/models/form_document/content.rb @@ -34,6 +34,7 @@ class FormDocument::Content attribute :send_daily_submission_batch, :boolean attribute :send_weekly_submission_batch, :boolean attribute :send_copy_of_answers, :string + attribute :delivery_configurations, array: true alias_attribute :id, :form_id @@ -54,4 +55,26 @@ def self.from_form_document(form_document) def has_welsh_translation? available_languages.present? && available_languages.include?("cy") end + + def has_email_delivery? + email_delivery_configuration.present? + end + + def email_delivery_configuration + delivery_configurations.find do |delivery_configuration| + delivery_configuration["delivery_method"] == "email" && delivery_configuration["delivery_schedule"] == "immediate" + end + end + + def daily_submission_batch_enabled? + delivery_configurations.any? do |delivery_configuration| + delivery_configuration["delivery_schedule"] == "daily" + end + end + + def weekly_submission_batch_enabled? + delivery_configurations.any? do |delivery_configuration| + delivery_configuration["delivery_schedule"] == "weekly" + end + end end diff --git a/app/views/forms/_made_live_form.html.erb b/app/views/forms/_made_live_form.html.erb index 713a9cd261..f513b8ccaf 100644 --- a/app/views/forms/_made_live_form.html.erb +++ b/app/views/forms/_made_live_form.html.erb @@ -150,18 +150,18 @@

<%= t('.how_you_get_completed_forms.submission_email') %>

<%= form_document.submission_email %>

- <% if form_document.submission_type == "email" %> - <% submission_format = ["email", *form_document.submission_format].join("_") %> + <% if form_document.has_email_delivery? %> + <% formats = ["email", *form_document.email_delivery_configuration["formats"]].join("_") %>

<%= t(".how_you_get_completed_forms.csv_and_json") %>

-

<%= t(".how_you_get_completed_forms.submission_format.email.#{submission_format}_html") %>

+

<%= t(".how_you_get_completed_forms.submission_format.email.#{formats}_html") %>

<% end %>

<%= t(".how_you_get_completed_forms.batch_submissions.title") %>

- <% if form_document.send_daily_submission_batch && form_document.send_weekly_submission_batch %> + <% if form_document.daily_submission_batch_enabled? && form_document.weekly_submission_batch_enabled? %>

<%= t(".how_you_get_completed_forms.batch_submissions.daily_and_weekly_enabled") %>

- <% elsif form_document.send_daily_submission_batch %> + <% elsif form_document.daily_submission_batch_enabled? %>

<%= t(".how_you_get_completed_forms.batch_submissions.daily_enabled") %>

- <% elsif form_document.send_weekly_submission_batch %> + <% elsif form_document.weekly_submission_batch_enabled? %>

<%= t(".how_you_get_completed_forms.batch_submissions.weekly_enabled") %>

<% else %>

<%= t(".how_you_get_completed_forms.batch_submissions.disabled") %>

diff --git a/spec/views/forms/_made_live_form.html.erb_spec.rb b/spec/views/forms/_made_live_form.html.erb_spec.rb index a649885016..3f3f543ea9 100644 --- a/spec/views/forms/_made_live_form.html.erb_spec.rb +++ b/spec/views/forms/_made_live_form.html.erb_spec.rb @@ -5,8 +5,7 @@ let(:past_week_metrics_data) { { weekly_submissions: 125, weekly_starts: 256 } } let(:what_happens_next_markdown) { "If you have not received a response within 5 working days, [contact our user support team](https://example.com)." } let(:form_metadata) do - create(:form, :live, declaration_markdown:, what_happens_next_markdown:, submission_type:, submission_format:, - send_daily_submission_batch:, send_weekly_submission_batch:, send_copy_of_answers:) + create(:form, :live, declaration_markdown:, what_happens_next_markdown:, send_copy_of_answers:) end let(:form_document) do form_document_content = FormDocument::Content.from_form_document(form_metadata.live_form_document) @@ -18,10 +17,6 @@ let(:status) { :live } let(:preview_mode) { :preview_live } let(:questions_path) { Faker::Internet.url } - let(:submission_type) { "email" } - let(:submission_format) { [] } - let(:send_daily_submission_batch) { false } - let(:send_weekly_submission_batch) { false } let(:send_copy_of_answers) { "disabled" } let(:cloudwatch_service) { instance_double(CloudWatchService, past_week_metrics_data:) } @@ -147,7 +142,9 @@ context "when the submission type is 'email'" do context "when CSV submission is enabled" do - let(:submission_format) { %w[csv] } + let(:form_metadata) do + create(:form, :live, delivery_configurations: [create(:delivery_configuration, :immediate_email, formats: %w[csv])]) + end it "tells the user they have CSVs enabled" do expect(rendered).to have_css("h4", text: I18n.t("forms.made_live_form.how_you_get_completed_forms.csv_and_json")) @@ -156,7 +153,9 @@ end context "when JSON submission is enabled" do - let(:submission_format) { %w[json] } + let(:form_metadata) do + create(:form, :live, delivery_configurations: [create(:delivery_configuration, :immediate_email, formats: %w[json])]) + end it "tells the user they have JSON submissions enabled" do expect(rendered).to have_css("h4", text: I18n.t("forms.made_live_form.how_you_get_completed_forms.csv_and_json")) @@ -165,7 +164,9 @@ end context "when both CSV and JSON submissions are enabled" do - let(:submission_format) { %w[csv json] } + let(:form_metadata) do + create(:form, :live, delivery_configurations: [create(:delivery_configuration, :immediate_email, formats: %w[csv json])]) + end it "tells the user they have CSV and JSON submissions enabled" do expect(rendered).to have_css("h4", text: I18n.t("forms.made_live_form.how_you_get_completed_forms.csv_and_json")) @@ -174,7 +175,9 @@ end context "when CSV submission is not enabled" do - let(:submission_format) { %w[] } + let(:form_metadata) do + create(:form, :live, delivery_configurations: [create(:delivery_configuration, :immediate_email, formats: [])]) + end it "tells the user they do not have CSVs enabled" do expect(rendered).to have_css("h4", text: I18n.t("forms.made_live_form.how_you_get_completed_forms.csv_and_json")) @@ -184,7 +187,9 @@ end context "when the submission type is 's3'" do - let(:submission_type) { "s3" } + let(:form_metadata) do + create(:form, :live, delivery_configurations: [create(:delivery_configuration, :s3)]) + end it "does not include the CSV and JSON section" do expect(rendered).not_to have_css("h4", text: I18n.t("forms.made_live_form.how_you_get_completed_forms.csv_and_json")) @@ -196,7 +201,7 @@ end context "when only daily batches are enabled" do - let(:send_daily_submission_batch) { true } + let(:form_metadata) { create(:form, :live, delivery_configurations: [create(:delivery_configuration, :daily_email)]) } it "tells the user they getting a daily CSV" do expect(rendered).to include(I18n.t("forms.made_live_form.how_you_get_completed_forms.batch_submissions.daily_enabled")) @@ -204,7 +209,7 @@ end context "when only weekly batches are enabled" do - let(:send_weekly_submission_batch) { true } + let(:form_metadata) { create(:form, :live, delivery_configurations: [create(:delivery_configuration, :weekly_email)]) } it "tells the user they getting a weekly CSV" do expect(rendered).to include(I18n.t("forms.made_live_form.how_you_get_completed_forms.batch_submissions.weekly_enabled")) @@ -212,8 +217,12 @@ end context "when both daily and weekly batches are enabled" do - let(:send_daily_submission_batch) { true } - let(:send_weekly_submission_batch) { true } + let(:form_metadata) do + create(:form, :live, delivery_configurations: [ + create(:delivery_configuration, :daily_email), + create(:delivery_configuration, :weekly_email), + ]) + end it "tells the user they getting a daily and weekly CSV" do expect(rendered).to include(I18n.t("forms.made_live_form.how_you_get_completed_forms.batch_submissions.daily_and_weekly_enabled")) @@ -221,9 +230,6 @@ end context "when neither daily or weekly batches are enabled" do - let(:send_daily_submission_batch) { false } - let(:send_weekly_submission_batch) { false } - it "tells the user they have not opted to get a daily or weekly CSV" do expect(rendered).to include(I18n.t("forms.made_live_form.how_you_get_completed_forms.batch_submissions.disabled")) end @@ -423,7 +429,7 @@ context "when the form has a Welsh translation" do let(:what_happens_next_markdown_cy) { "Os nad ydych wedi derbyn ymateb o fewn 5 diwrnod gwaith, [cysylltwch â’n tîm cymorth defnyddwyr](https://example.com)." } - let(:form_metadata) { create :form, :live, :with_welsh_translation, what_happens_next_markdown:, what_happens_next_markdown_cy:, submission_type:, submission_format: } + let(:form_metadata) { create :form, :live, :with_welsh_translation, what_happens_next_markdown:, what_happens_next_markdown_cy: } let(:welsh_form_document) do form_document_content = FormDocument::Content.from_form_document(form_metadata.live_welsh_form_document) form_document_content.first_made_live_at = 1.week.ago @@ -462,7 +468,7 @@ end context "when the form has a declaration" do - let(:form_metadata) { create :form, :live, :with_welsh_translation, declaration_markdown:, what_happens_next_markdown:, submission_type:, submission_format: } + let(:form_metadata) { create :form, :live, :with_welsh_translation, declaration_markdown:, what_happens_next_markdown: } it "contains a table displaying the declaration text in each language" do expect(rendered).to have_css(".govuk-summary-card__title", text: "Declaration") @@ -475,7 +481,7 @@ context "when the form has a GOV.UK Pay payment link" do let(:payment_url) { "https://www.gov.uk/payments/your-payment-link" } - let(:form_metadata) { create :form, :live, :with_welsh_translation, declaration_markdown:, what_happens_next_markdown:, submission_type:, submission_format:, payment_url: } + let(:form_metadata) { create :form, :live, :with_welsh_translation, declaration_markdown:, what_happens_next_markdown:, payment_url: } it "contains a table displaying the payment link in each language" do expect(rendered).to have_css(".govuk-summary-card__title", text: "GOV.UK Pay payment link") @@ -495,7 +501,7 @@ end context "with support details" do - let(:form_metadata) { create :form, :live, :with_welsh_translation, what_happens_next_markdown:, submission_type:, submission_format:, support_email: "support@example.gov.uk", support_phone: "phone details", support_url_text: "website", support_url: "www.example.gov.uk" } + let(:form_metadata) { create :form, :live, :with_welsh_translation, what_happens_next_markdown:, support_email: "support@example.gov.uk", support_phone: "phone details", support_url_text: "website", support_url: "www.example.gov.uk" } it "contains a table displaying the support details in each language" do expect(rendered).to have_css(".govuk-summary-card__title", text: "Your form’s contact details for support") From d22c52d07cf7d170ce2c322d0283409b003778e8 Mon Sep 17 00:00:00 2001 From: Stephen Daly Date: Fri, 17 Jul 2026 16:01:19 +0100 Subject: [PATCH 6/9] Use delivery configurations in task status service Use delivery_configurations rather than the deprected attributes when determining whether the tasks for email attachments and batch submissions are complete. --- app/services/form_task_list_service.rb | 2 +- app/services/task_status_service.rb | 5 ++- spec/services/form_task_list_service_spec.rb | 21 ++++++--- spec/services/task_status_service_spec.rb | 45 +++++++++++--------- 4 files changed, 43 insertions(+), 30 deletions(-) diff --git a/app/services/form_task_list_service.rb b/app/services/form_task_list_service.rb index 174fd2618e..49487844f7 100644 --- a/app/services/form_task_list_service.rb +++ b/app/services/form_task_list_service.rb @@ -97,7 +97,7 @@ def how_you_get_completed_forms_section_tasks def how_you_get_completed_forms_optional_subsection rows = [] - rows << submission_attachments_task if @form.email? + rows << submission_attachments_task if @form.immediate_email_delivery_configuration.present? rows << batch_submissions_task return nil if rows.empty? diff --git a/app/services/task_status_service.rb b/app/services/task_status_service.rb index 0cb338f0e1..499b7776e5 100644 --- a/app/services/task_status_service.rb +++ b/app/services/task_status_service.rb @@ -123,13 +123,14 @@ def welsh_language_status end def submission_attachments_status - return :completed if @form.email? && @form.submission_format.any? + email_delivery_configuration = @form.immediate_email_delivery_configuration + return :completed if email_delivery_configuration.present? && email_delivery_configuration.formats.any? :optional end def batch_submissions_status - return :completed if @form.send_daily_submission_batch || @form.send_weekly_submission_batch + return :completed if @form.delivery_configurations.daily.any? || @form.delivery_configurations.weekly.any? :optional end diff --git a/spec/services/form_task_list_service_spec.rb b/spec/services/form_task_list_service_spec.rb index 81e067aa16..9d1e52d2f2 100644 --- a/spec/services/form_task_list_service_spec.rb +++ b/spec/services/form_task_list_service_spec.rb @@ -300,6 +300,8 @@ let(:all_task_names) { all_sections.flat_map { |section| section[:rows] }.compact.map { |row| row[:task_name] } } context "when the submission type is email" do + let(:form) { create(:form, :with_email_delivery) } + it "has link to the submission attachments page" do expect(section_rows.first[:task_name]).to eq I18n.t("forms.task_list_create.how_you_get_completed_forms_section.optional_subsection.submission_attachments") expect(section_rows.first[:path]).to eq "/forms/#{form.id}/submission-attachments" @@ -308,10 +310,15 @@ it "has the subsection title 'Optional tasks' as there are multiple tasks" do expect(section[:title]).to eq I18n.t("forms.task_list.optional_tasks_title.other") end + + it "has link to the batch submissions page" do + expect(section_rows.second[:task_name]).to eq I18n.t("forms.task_list_create.how_you_get_completed_forms_section.optional_subsection.batch_submissions") + expect(section_rows.second[:path]).to eq "/forms/#{form.id}/batch-submissions" + end end - context "when the submission type is s3" do - let(:form) { create(:form, submission_type: "s3") } + context "when email delivery is not enabled" do + let(:form) { create(:form, delivery_configurations: [create(:delivery_configuration, :s3)]) } it "does not have link to the submission attachments page" do expect(all_task_names).not_to include I18n.t("forms.task_list_create.how_you_get_completed_forms_section.optional_subsection.submission_attachments") @@ -320,11 +327,11 @@ it "has the subsection title 'Optional task' as there is only one task" do expect(section[:title]).to eq I18n.t("forms.task_list.optional_tasks_title.one") end - end - it "has link to the batch submissions page" do - expect(section_rows.second[:task_name]).to eq I18n.t("forms.task_list_create.how_you_get_completed_forms_section.optional_subsection.batch_submissions") - expect(section_rows.second[:path]).to eq "/forms/#{form.id}/batch-submissions" + it "has link to the batch submissions page" do + expect(section_rows.first[:task_name]).to eq I18n.t("forms.task_list_create.how_you_get_completed_forms_section.optional_subsection.batch_submissions") + expect(section_rows.first[:path]).to eq "/forms/#{form.id}/batch-submissions" + end end end @@ -626,7 +633,7 @@ end context "when editing an existing form" do - let(:form) { create(:form, :live) } + let(:form) { create(:form, :live, :with_email_delivery) } let(:can_make_form_live) { true } it "has the expected section titles" do diff --git a/spec/services/task_status_service_spec.rb b/spec/services/task_status_service_spec.rb index cd52c76b00..92c7ec93db 100644 --- a/spec/services/task_status_service_spec.rb +++ b/spec/services/task_status_service_spec.rb @@ -199,27 +199,31 @@ end end - context "with submission_type set to 'email'" do - let(:form) { build(:form, :new_form, :with_group, submission_type: "email", submission_format:, group:) } + context "when email delivery is enabled" do + let(:form) do + create(:form, :new_form, :with_group, group:, delivery_configurations: [ + create(:delivery_configuration, :immediate_email, formats:), + ]) + end - context "with submission format empty" do - let(:submission_format) { [] } + context "with no submission formats enabled" do + let(:formats) { [] } it "returns optional" do expect(task_status_service.all_task_statuses[:submission_attachments_status]).to eq :optional end end - context "with submission format csv" do - let(:submission_format) { %w[csv] } + context "with CSV attachments enabled" do + let(:formats) { %w[csv] } it "returns completed" do expect(task_status_service.all_task_statuses[:submission_attachments_status]).to eq :completed end end - context "with submission format has multiple values" do - let(:submission_format) { %w[csv json] } + context "with multiple submission formats enabled" do + let(:formats) { %w[csv json] } it "returns completed" do expect(task_status_service.all_task_statuses[:submission_attachments_status]).to eq :completed @@ -227,8 +231,12 @@ end end - context "with submission_type set s3" do - let(:form) { build(:form, :new_form, :with_group, submission_type: "s3", submission_format: "csv", group:) } + context "with S3 delivery enabled" do + let(:form) do + build(:form, :new_form, :with_group, group:, delivery_configurations: [ + build(:delivery_configuration, :s3), + ]) + end it "returns optional" do expect(task_status_service.all_task_statuses[:submission_attachments_status]).to eq :optional @@ -237,29 +245,26 @@ end describe "batch_submissions_status" do - let(:form) { build(:form, :new_form, :with_group, group:, send_daily_submission_batch:, send_weekly_submission_batch:) } + let(:form) { create(:form, :new_form, :with_group, group:, delivery_configurations:) } - context "with send_daily_submission_batch set to true" do - let(:send_daily_submission_batch) { true } - let(:send_weekly_submission_batch) { false } + context "with daily emails configured" do + let(:delivery_configurations) { [create(:delivery_configuration, :daily_email)] } it "returns completed" do expect(task_status_service.all_task_statuses[:batch_submissions_status]).to eq :completed end end - context "with send_weekly_submission_batch set to true" do - let(:send_daily_submission_batch) { false } - let(:send_weekly_submission_batch) { true } + context "with weekly emails configured" do + let(:delivery_configurations) { [create(:delivery_configuration, :weekly_email)] } it "returns completed" do expect(task_status_service.all_task_statuses[:batch_submissions_status]).to eq :completed end end - context "with send daily and weekly batches set to false" do - let(:send_daily_submission_batch) { false } - let(:send_weekly_submission_batch) { false } + context "with no batch emails configured" do + let(:delivery_configurations) { [create(:delivery_configuration, :immediate_email)] } it "returns optional" do expect(task_status_service.all_task_statuses[:batch_submissions_status]).to eq :optional From 2d3869b21f9ae4500a8aec53934eed5e6b22d210 Mon Sep 17 00:00:00 2001 From: Stephen Daly Date: Fri, 17 Jul 2026 18:03:26 +0100 Subject: [PATCH 7/9] Update rake tasks for changing delivery configurations Update the rake tasks for enabling/disabling S3 and email submissions to have the following tasks: - enable_s3 - disable_s3 - enable_email - disable_email These tasks will now only edit the delivery_configurations and not edit the submission_type and submission_format which are deprecated and unused. The new tasks make it possible to have both S3 and email submissions enabled at the same time. --- lib/tasks/forms.rake | 142 +++++--- spec/lib/tasks/forms.rake_spec.rb | 585 +++++++++++------------------- 2 files changed, 319 insertions(+), 408 deletions(-) diff --git a/lib/tasks/forms.rake b/lib/tasks/forms.rake index 4c580fe8b8..34bd1b342f 100644 --- a/lib/tasks/forms.rake +++ b/lib/tasks/forms.rake @@ -73,75 +73,119 @@ namespace :forms do end end - namespace :submission_type do - desc "Set submission_type to email" - task :set_to_email, %i[form_id submission_formats] => :environment do |_, args| - submission_format = args[:submission_formats].nil? ? [] : args.to_a[1..] - submission_format = [] if submission_format == %w[email] + namespace :delivery_configurations do + desc "Enable email delivery for submissions" + task :enable_email, %i[form_id] => :environment do |_, args| + usage_message = "usage: rake forms:delivery_configurations:enable_email[]".freeze + abort usage_message if args[:form_id].blank? + + form = Form.find(args[:form_id]) + delivery_configuration = form.delivery_configurations.find_or_initialize_by(delivery_method: :email, delivery_schedule: :immediate) + delivery_configuration.save! - usage_message = "usage: rake forms:submission_type:set_to_email[(, )*]".freeze + # ensure draft form document is updated + form.delivery_configurations.reload + form.save! + + if form.is_live? + form.form_documents.where(tag: "live").find_each do |form_document| + delivery_configurations = form_document["content"]["delivery_configurations"] + has_email_delivery = delivery_configurations.any? { |d| d["delivery_method"] == "email" && d["delivery_schedule"] == "immediate" } + + unless has_email_delivery + delivery_configurations << DeliveryConfiguration.new(form: form, delivery_method: "email", delivery_schedule: "immediate") + end + form_document.save! + end + end + + Rails.logger.info "Enabled email delivery for #{fmt_form(form)}" + end + + desc "Disable email delivery for submissions" + task :disable_email, %i[form_id] => :environment do |_, args| + usage_message = "usage: rake forms:delivery_configurations:disable_email[]".freeze abort usage_message if args[:form_id].blank? - supported_formats = %w[csv json] - abort "submission_format must be one of #{supported_formats.join(', ')}" unless submission_format.all? { supported_formats.include? it } + form = Form.find(args[:form_id]) + form.delivery_configurations.where(delivery_method: :email, delivery_schedule: :immediate).destroy_all + + # ensure draft form document is updated + form.delivery_configurations.reload + form.save! + + if form.is_live? + form.form_documents.where(tag: "live").find_each do |form_document| + delivery_configurations = form_document["content"]["delivery_configurations"] + delivery_configurations.reject! { |d| d["delivery_method"] == "email" && d["delivery_schedule"] == "immediate" } + form_document.save! + end + end - form_id = args[:form_id] - Rails.logger.info("Setting submission_type to email with submission_format #{submission_format} for form: #{form_id}") + Rails.logger.info "Disabled email delivery for #{fmt_form(form)}" + end - form = Form.find(form_id) - form.submission_type = "email" - form.submission_format = submission_format + desc "Disable S3 delivery for submissions" + task :disable_s3, %i[form_id] => :environment do |_, args| + usage_message = "usage: rake forms:delivery_configurations:disable_s3[]".freeze + abort usage_message if args[:form_id].blank? - delivery_configuration = form.delivery_configurations.find_or_initialize_by(delivery_method: :email, delivery_schedule: :immediate) - delivery_configuration.formats = submission_format - delivery_configuration.save! + form = Form.find(args[:form_id]) + form.s3_bucket_name = nil + form.s3_bucket_aws_account_id = nil + form.s3_bucket_region = nil + form.delivery_configurations.where(delivery_method: :s3, delivery_schedule: :immediate).destroy_all - form.delivery_configurations.where(delivery_method: :s3).destroy_all + # ensure draft form document is updated + form.delivery_configurations.reload form.save! if form.is_live? - form_document = form.live_form_document - content = form_document.content + form.form_documents.where(tag: "live").find_each do |form_document| + content = form_document.content + + content["s3_bucket_name"] = nil + content["s3_bucket_aws_account_id"] = nil + content["s3_bucket_region"] = nil - content[:submission_type] = "email" - content[:submission_format] = submission_format - content[:delivery_configurations] = form.delivery_configurations.map(&:as_json) + delivery_configurations = content["delivery_configurations"] + delivery_configurations.reject! { |d| d["delivery_method"] == "s3" && d["delivery_schedule"] == "immediate" } - form_document.save! + form_document.save! + end end - Rails.logger.info("Set submission_type to email with submission_format #{submission_format} for form: #{form_id}") + Rails.logger.info "Disabled s3 delivery for #{fmt_form(form)}" end - desc "Set submission_type to s3" - task :set_to_s3, %i[form_id s3_bucket_name s3_bucket_aws_account_id s3_bucket_region format] => :environment do |_, args| - usage_message = "usage: rake forms:submission_type:set_to_s3[, , , , ]".freeze + desc "Enable S3 delivery for submissions" + task :enable_s3, %i[form_id s3_bucket_name s3_bucket_aws_account_id s3_bucket_region format disable_email] => :environment do |_, args| + usage_message = "usage: rake forms:delivery_configurations:enable_s3[, , , , , ]".freeze abort usage_message if args[:form_id].blank? abort usage_message if args[:s3_bucket_name].blank? abort usage_message if args[:s3_bucket_aws_account_id].blank? abort usage_message if args[:s3_bucket_region].blank? abort usage_message if args[:format].blank? + abort usage_message if args[:disable_email].blank? || !args[:disable_email].in?(%w[true false]) abort "s3_bucket_region must be one of eu-west-1 or eu-west-2" unless %w[eu-west-1 eu-west-2].include? args[:s3_bucket_region] abort "format must be one of csv or json" unless %w[csv json].include? args[:format] - submission_type = "s3" - submission_format = [args[:format]] + formats = [args[:format]] + disable_email = args[:disable_email] == "true" - Rails.logger.info("Setting submission_type to #{submission_type} and s3_bucket_name to #{args[:s3_bucket_name]} for form: #{args[:form_id]}") + Rails.logger.info("Enabling s3 submissions with s3_bucket_name #{args[:s3_bucket_name]} for form: #{args[:form_id]}") form = Form.find(args[:form_id]) - form.submission_type = submission_type - form.submission_format = submission_format form.s3_bucket_name = args[:s3_bucket_name] form.s3_bucket_aws_account_id = args[:s3_bucket_aws_account_id] form.s3_bucket_region = args[:s3_bucket_region] delivery_configuration = form.delivery_configurations.find_or_initialize_by(delivery_method: :s3, delivery_schedule: :immediate) - delivery_configuration.formats = submission_format + delivery_configuration.formats = formats delivery_configuration.save! - # For now, disable email delivery per submission. After we've migrated to using the DeliveryConfigurations in - # forms-runner we can allow enabling both email and s3. - form.delivery_configurations.where(delivery_method: :email, delivery_schedule: :immediate).destroy_all + if disable_email + form.delivery_configurations.where(delivery_method: :email, delivery_schedule: :immediate).destroy_all + end form.save! @@ -149,18 +193,30 @@ namespace :forms do form.form_documents.where(tag: "live").find_each do |form_document| content = form_document.content - content[:submission_type] = submission_type - content[:submission_format] = submission_format - content[:s3_bucket_name] = args[:s3_bucket_name] - content[:s3_bucket_aws_account_id] = args[:s3_bucket_aws_account_id] - content[:s3_bucket_region] = args[:s3_bucket_region] - content[:delivery_configurations] = form.delivery_configurations.map(&:as_json) + content["s3_bucket_name"] = args[:s3_bucket_name] + content["s3_bucket_aws_account_id"] = args[:s3_bucket_aws_account_id] + content["s3_bucket_region"] = args[:s3_bucket_region] + + delivery_configurations = content["delivery_configurations"] + has_s3_delivery = delivery_configurations.any? { |d| d["delivery_method"] == "s3" && d["delivery_schedule"] == "immediate" } + + unless has_s3_delivery + delivery_configurations << DeliveryConfiguration.new(form: form, delivery_method: "s3", delivery_schedule: "immediate") + end + + if disable_email + delivery_configurations.reject! { |d| d["delivery_method"] == "email" && d["delivery_schedule"] == "immediate" } + end form_document.save! end end - Rails.logger.info("Set submission_type to #{submission_type} and s3_bucket_name to #{args[:s3_bucket_name]} for form: #{args[:form_id]}") + if disable_email + Rails.logger.info("Enabled s3 submissions to bucket #{args[:s3_bucket_name]} and disabled email submissions for form: #{args[:form_id]}") + else + Rails.logger.info("Enabled s3 submissions to bucket #{args[:s3_bucket_name]} for form: #{args[:form_id]}. Email submissions are still enabled.") + end end end diff --git a/spec/lib/tasks/forms.rake_spec.rb b/spec/lib/tasks/forms.rake_spec.rb index 81f159299c..ae4b8cf6a2 100644 --- a/spec/lib/tasks/forms.rake_spec.rb +++ b/spec/lib/tasks/forms.rake_spec.rb @@ -347,463 +347,318 @@ end end - describe "forms:submission_type:set_to_email" do + describe "forms:delivery_configurations:enable_email" do subject(:task) do - Rake::Task["forms:submission_type:set_to_email"] + Rake::Task["forms:delivery_configurations:enable_email"] end - let(:form) do - create(:form, :live, submission_type: "s3", submission_format: [], delivery_configurations: [ - create(:delivery_configuration, :s3), - create(:delivery_configuration, :daily_email), - ]) - end - let!(:other_form) do - create(:form, :live, submission_type: "s3", submission_format: [], delivery_configurations: [ - create(:delivery_configuration, :s3), - ]) - end + let(:form) { create(:form) } - context "when the form is live" do - it "sets a form's submission_type to email" do - expect { task.invoke(form.id) } - .to change { form.reload.submission_type }.to("email") + context "with valid arguments" do + it "adds immediate email delivery to the form" do + expect { + task.invoke(form.id) + }.to change { form.reload.delivery_configurations.where(delivery_method: "email", delivery_schedule: "immediate").count } + .by(1) end - it "creates a DeliveryConfiguration for immediate email deliveries and removes the DeliveryConfiguration for s3" do + it "updates the draft form document delivery configurations" do task.invoke(form.id) - expect(form.reload.delivery_configurations.pluck(:delivery_method, :delivery_schedule, :formats)).to contain_exactly( - ["email", "immediate", []], - ["email", "daily", %w[csv]], - ) - end - it "updates a form's live form document" do - task.invoke(form.id) - content = form.live_form_document.reload.content - expect(content["submission_type"]).to eq("email") - expect(content["submission_format"]).to eq([]) - expect(content["delivery_configurations"]).to contain_exactly( - { - "delivery_method" => "email", - "delivery_schedule" => "immediate", - "formats" => [], - }, - { - "delivery_method" => "email", - "delivery_schedule" => "daily", - "formats" => %w[csv], - }, - ) - end - - it "updates the form's draft form document" do - task.invoke(form.id) - content = form.draft_form_document.reload.content - expect(content["submission_type"]).to eq("email") - expect(content["submission_format"]).to eq([]) - expect(content["delivery_configurations"]).to contain_exactly( - { - "delivery_method" => "email", - "delivery_schedule" => "immediate", - "formats" => [], - }, - { - "delivery_method" => "email", - "delivery_schedule" => "daily", - "formats" => %w[csv], - }, - ) + expect(form.reload.draft_form_document.content["delivery_configurations"]) + .to include(a_hash_including("delivery_method" => "email", "delivery_schedule" => "immediate")) end - it "does not update a different form" do - expect { task.invoke(form.id) } - .not_to(change { other_form.reload.submission_type }) - end - end + context "when the form is live" do + let(:form) { create(:form, :live, :with_welsh_translation) } - context "when the form is draft" do - let(:form) { create :form, submission_type: "s3" } + it "updates the live form documents delivery configurations" do + task.invoke(form.id) - it "sets a form's submission_type to email" do - expect { task.invoke(form.id) } - .to change { form.reload.submission_type }.to("email") + expect(form.reload.live_form_document.content["delivery_configurations"]) + .to include(a_hash_including("delivery_method" => "email", "delivery_schedule" => "immediate")) + expect(form.reload.live_welsh_form_document.content["delivery_configurations"]) + .to include(a_hash_including("delivery_method" => "email", "delivery_schedule" => "immediate")) + end end - it "updates the form's draft form document" do - task.invoke(form.id) - expect(form.draft_form_document.reload.content["submission_type"]).to eq("email") - expect(form.draft_form_document.reload.content["submission_format"]).to eq([]) - end - end + context "when email is already enabled" do + let(:form) { create(:form, :live, :with_welsh_translation) } - context "when the provided submission format is empty" do - let(:form) { create :form, :live, submission_type: "s3", submission_format: %w[json] } + it "does not change the delivery configurations or form documents" do + task.invoke(form.id) - it "sets a form's submission format to empty" do - expect { task.invoke(form.id) } - .to change { form.reload.submission_format }.to([]) - end + delivery_configuration_count = form.reload.delivery_configurations.where(delivery_method: "email", delivery_schedule: "immediate").count + draft_content = form.reload.draft_form_document.content.deep_dup + live_content = form.reload.live_form_document.content.deep_dup + live_welsh_content = form.reload.live_welsh_form_document.content.deep_dup - it "sets a form's submission_type to email" do - expect { task.invoke(form.id) } - .to change { form.reload.submission_type }.to("email") - end - end + task.invoke(form.id) - context "when the provided submission format is email" do - let(:form) { create :form, :live, submission_type: "s3", submission_format: %w[json] } + expect(form.reload.delivery_configurations.where(delivery_method: "email", delivery_schedule: "immediate").count) + .to eq(delivery_configuration_count) - it "sets a form's submission format to empty" do - expect { task.invoke(form.id, "email") } - .to change { form.reload.submission_format }.to([]) + expect(form.reload.draft_form_document.content).to eq(draft_content) + expect(form.reload.live_form_document.content).to eq(live_content) + expect(form.reload.live_welsh_form_document&.content).to eq(live_welsh_content) + end end + end - it "sets a form's submission_type to email" do - expect { task.invoke(form.id, "email") } - .to change { form.reload.submission_type }.to("email") + context "with invalid arguments" do + it "aborts with a usage message when the form id is missing" do + expect { + task.invoke + }.to raise_error(SystemExit) + .and output(/usage: rake forms:delivery_configurations:enable_email\[\]/).to_stderr end + end + end - it "creates a DeliveryConfiguration with blank format" do - task.invoke(form.id, "email") - expect(form.reload.delivery_configurations.pluck(:delivery_method, :delivery_schedule, :formats)).to include( - ["email", "immediate", []], - ) - end + describe "forms:delivery_configurations:disable_email" do + subject(:task) do + Rake::Task["forms:delivery_configurations:disable_email"] end - context "when the provided submission format is csv" do - it "sets a form's submission format to csv" do - expect { task.invoke(form.id, "csv") } - .to change { form.reload.submission_format }.to(%w[csv]) - end + let(:form) { create(:form, :live, :with_welsh_translation, :with_email_delivery) } - it "sets a form's submission_type to email" do - expect { task.invoke(form.id, "csv") } - .to change { form.reload.submission_type }.to("email") + context "with valid arguments" do + it "removes immediate email delivery from the form" do + expect { + task.invoke(form.id) + }.to change { form.reload.delivery_configurations.where(delivery_method: "email", delivery_schedule: "immediate").count } + .by(-1) end - it "creates a DeliveryConfiguration with csv format" do - task.invoke(form.id, "csv") - expect(form.reload.delivery_configurations.pluck(:delivery_method, :delivery_schedule, :formats)).to include( - ["email", "immediate", %w[csv]], - ) - end - end + it "updates the draft form document delivery configurations" do + task.invoke(form.id) - context "when the provided submission formate is json" do - it "sets a form's submission format to json" do - expect { task.invoke(form.id, "json") } - .to change { form.reload.submission_format }.to(%w[json]) + expect(form.reload.draft_form_document.content["delivery_configurations"]) + .not_to include(a_hash_including("delivery_method" => "email", "delivery_schedule" => "immediate")) end - it "sets a form's submission_type to email" do - expect { task.invoke(form.id, "json") } - .to change { form.reload.submission_type }.to("email") - end + context "when the form is live" do + it "updates the live form documents delivery configurations" do + task.invoke(form.id) - it "creates a DeliveryConfiguration with json format" do - task.invoke(form.id, "json") - expect(form.reload.delivery_configurations.pluck(:delivery_method, :delivery_schedule, :formats)).to include( - ["email", "immediate", %w[json]], - ) + expect(form.reload.live_form_document.content["delivery_configurations"]) + .not_to include(a_hash_including("delivery_method" => "email", "delivery_schedule" => "immediate")) + expect(form.reload.live_welsh_form_document.content["delivery_configurations"]) + .not_to include(a_hash_including("delivery_method" => "email", "delivery_schedule" => "immediate")) + end end - end - context "when the provided submission format is csv, json" do - it "sets a form's submission format to csv and json" do - expect { task.invoke(form.id, "csv", "json") } - .to change { form.reload.submission_format }.to(%w[csv json]) - end + context "when email is already disabled" do + let(:form) { create(:form, :live, :with_welsh_translation) } - it "sets a form's submission_type to email" do - expect { task.invoke(form.id, "csv", "json") } - .to change { form.reload.submission_type }.to("email") - end + it "does not change the delivery configurations or form documents" do + task.invoke(form.id) - it "creates a DeliveryConfiguration with the provided formats" do - task.invoke(form.id, "csv", "json") - expect(form.reload.delivery_configurations.pluck(:delivery_method, :delivery_schedule, :formats)).to include( - ["email", "immediate", %w[csv json]], - ) - end - end + delivery_configuration_count = form.reload.delivery_configurations.where(delivery_method: "email", delivery_schedule: "immediate").count + draft_content = form.reload.draft_form_document.content.deep_dup + live_content = form.reload.live_form_document.content.deep_dup + live_welsh_content = form.reload.live_welsh_form_document.content.deep_dup - context "when the provided submission_type is s3" do - it "aborts with a usage message" do - expect { task.invoke(form.id, "s3") } - .to raise_error(SystemExit) - .and output("submission_format must be one of csv, json\n").to_stderr + task.invoke(form.id) + + expect(form.reload.delivery_configurations.where(delivery_method: "email", delivery_schedule: "immediate").count) + .to eq(delivery_configuration_count) + + expect(form.reload.draft_form_document.content).to eq(draft_content) + expect(form.reload.live_form_document.content).to eq(live_content) + expect(form.reload.live_welsh_form_document.content).to eq(live_welsh_content) + end end end - context "without arguments" do - it "aborts with a usage message" do + context "with invalid arguments" do + it "aborts with a usage message when the form id is missing" do expect { task.invoke }.to raise_error(SystemExit) - .and output("usage: rake forms:submission_type:set_to_email[(, )*]\n").to_stderr + .and output(/usage: rake forms:delivery_configurations:disable_email\[\]/).to_stderr end end end - describe "forms:submission_type:set_to_s3" do + describe "forms:delivery_configurations:disable_s3" do subject(:task) do - Rake::Task["forms:submission_type:set_to_s3"] + Rake::Task["forms:delivery_configurations:disable_s3"] end let(:form) do - create(:form, :with_welsh_translation, :live, delivery_configurations: [ - create(:delivery_configuration, :immediate_email), - create(:delivery_configuration, :daily_email), + create(:form, :live, :with_welsh_translation, :with_s3_configuration, delivery_configurations: [ + create(:delivery_configuration, :s3), ]) end - let!(:other_form) { create :form, :live } - let(:s3_bucket_name) { "a-bucket" } - let(:s3_bucket_aws_account_id) { "an-aws-account-id" } - let(:s3_bucket_region) { "eu-west-1" } - let(:format) { "csv" } - let(:valid_args) { [form.id, s3_bucket_name, s3_bucket_aws_account_id, s3_bucket_region, format] } - context "when the form is live" do - context "when the format is csv" do - it "sets a form's submission_type to s3" do - expect { task.invoke(*valid_args) } - .to change { form.reload.submission_type }.to("s3") - end + context "with valid arguments" do + it "removes immediate S3 delivery from the form" do + expect { + task.invoke(form.id) + }.to change { form.reload.delivery_configurations.where(delivery_method: "s3", delivery_schedule: "immediate").count } + .by(-1) + end - it "sets a form's submission_format to csv" do - expect { task.invoke(*valid_args) } - .to change { form.reload.submission_format }.to(%w[csv]) - end + it "clears the form S3 configuration" do + expect { + task.invoke(form.id) + }.to change { form.reload.s3_bucket_name }.to(nil) + .and change { form.reload.s3_bucket_aws_account_id }.to(nil) + .and change { form.reload.s3_bucket_region }.to(nil) + end - it "replaces the immediate email DeliveryConfiguration with an s3 configuration" do - task.invoke(*valid_args) - expect(form.reload.delivery_configurations.pluck(:delivery_method, :delivery_schedule, :formats)).to contain_exactly( - ["s3", "immediate", %w[csv]], - ["email", "daily", %w[csv]], - ) - end + it "updates the draft form document" do + task.invoke(form.id) - it "updates the live form document" do - task.invoke(*valid_args) - form_document = form.live_form_document.reload - expect(form_document.content["submission_type"]).to eq("s3") - expect(form_document.content["submission_format"]).to eq(%w[csv]) - expect(form_document.content["s3_bucket_name"]).to eq(s3_bucket_name) - expect(form_document.content["s3_bucket_aws_account_id"]).to eq(s3_bucket_aws_account_id) - expect(form_document.content["s3_bucket_region"]).to eq(s3_bucket_region) - expect(form_document.content["delivery_configurations"]).to contain_exactly( - { - "delivery_method" => "s3", - "delivery_schedule" => "immediate", - "formats" => %w[csv], - }, - { - "delivery_method" => "email", - "delivery_schedule" => "daily", - "formats" => %w[csv], - }, - ) - end - - it "updates the live Welsh form document" do - task.invoke(*valid_args) - welsh_form_document = form.live_welsh_form_document.reload - expect(welsh_form_document.content["submission_type"]).to eq("s3") - expect(welsh_form_document.content["submission_format"]).to eq(%w[csv]) - expect(welsh_form_document.content["s3_bucket_name"]).to eq(s3_bucket_name) - expect(welsh_form_document.content["s3_bucket_aws_account_id"]).to eq(s3_bucket_aws_account_id) - expect(welsh_form_document.content["s3_bucket_region"]).to eq(s3_bucket_region) - expect(welsh_form_document.content["delivery_configurations"]).to contain_exactly( - { - "delivery_method" => "s3", - "delivery_schedule" => "immediate", - "formats" => %w[csv], - }, - { - "delivery_method" => "email", - "delivery_schedule" => "daily", - "formats" => %w[csv], - }, - ) - end - - it "updates the draft form document" do - task.invoke(*valid_args) - form_document = form.draft_form_document.reload - expect(form_document.content["submission_type"]).to eq("s3") - expect(form_document.content["submission_format"]).to eq(%w[csv]) - expect(form_document.content["s3_bucket_name"]).to eq(s3_bucket_name) - expect(form_document.content["s3_bucket_aws_account_id"]).to eq(s3_bucket_aws_account_id) - expect(form_document.content["s3_bucket_region"]).to eq(s3_bucket_region) - expect(form_document.content["delivery_configurations"]).to contain_exactly( - { - "delivery_method" => "s3", - "delivery_schedule" => "immediate", - "formats" => %w[csv], - }, - { - "delivery_method" => "email", - "delivery_schedule" => "daily", - "formats" => %w[csv], - }, - ) - end + expect(form.reload.draft_form_document.content["s3_bucket_name"]).to be_nil + expect(form.reload.draft_form_document.content["s3_bucket_aws_account_id"]).to be_nil + expect(form.reload.draft_form_document.content["s3_bucket_region"]).to be_nil + expect(form.reload.draft_form_document.content["delivery_configurations"]) + .not_to include(a_hash_including("delivery_method" => "s3", "delivery_schedule" => "immediate")) end - context "when the format is json" do - let(:format) { "json" } + context "when the form is live" do + it "updates the live form documents" do + task.invoke(form.id) - it "sets a form's submission_type to s3" do - expect { task.invoke(*valid_args) } - .to change { form.reload.submission_type }.to("s3") - end + expect(form.reload.live_form_document.content["s3_bucket_name"]).to be_nil + expect(form.reload.live_form_document.content["s3_bucket_aws_account_id"]).to be_nil + expect(form.reload.live_form_document.content["s3_bucket_region"]).to be_nil + expect(form.reload.live_form_document.content["delivery_configurations"]) + .not_to include(a_hash_including("delivery_method" => "s3", "delivery_schedule" => "immediate")) - it "sets a form's submission_format to json" do - expect { task.invoke(*valid_args) } - .to change { form.reload.submission_format }.to(%w[json]) + expect(form.reload.live_welsh_form_document.content["s3_bucket_name"]).to be_nil + expect(form.reload.live_welsh_form_document.content["s3_bucket_aws_account_id"]).to be_nil + expect(form.reload.live_welsh_form_document.content["s3_bucket_region"]).to be_nil + expect(form.reload.live_welsh_form_document.content["delivery_configurations"]) + .not_to include(a_hash_including("delivery_method" => "s3", "delivery_schedule" => "immediate")) end + end - it "replaces the immediate email DeliveryConfiguration with an s3 configuration with json format" do - task.invoke(*valid_args) - expect(form.reload.delivery_configurations.pluck(:delivery_method, :delivery_schedule, :formats)).to contain_exactly( - ["s3", "immediate", %w[json]], - ["email", "daily", %w[csv]], - ) - end + context "when S3 is already disabled" do + let(:form) { create(:form, :live, :with_welsh_translation) } - it "updates the live form document" do - task.invoke(*valid_args) - form_document = form.live_form_document.reload - expect(form_document.content["submission_type"]).to eq("s3") - expect(form_document.content["submission_format"]).to eq(%w[json]) - expect(form_document.content["delivery_configurations"]).to include( - { - "delivery_method" => "s3", - "delivery_schedule" => "immediate", - "formats" => %w[json], - }, - ) - end - - it "updates the draft form document" do - task.invoke(*valid_args) - form_document = form.draft_form_document.reload - expect(form_document.content["submission_type"]).to eq("s3") - expect(form_document.content["submission_format"]).to eq(%w[json]) - expect(form_document.content["delivery_configurations"]).to include( - { - "delivery_method" => "s3", - "delivery_schedule" => "immediate", - "formats" => %w[json], - }, - ) - end - end + it "does not change the delivery configurations or form documents" do + task.invoke(form.id) - it "sets a form's s3_bucket_name" do - expect { task.invoke(*valid_args) } - .to change { form.reload.s3_bucket_name }.to(s3_bucket_name) - end + delivery_configuration_count = form.reload.delivery_configurations.where(delivery_method: "s3", delivery_schedule: "immediate").count + draft_content = form.reload.draft_form_document.content.deep_dup + live_content = form.reload.live_form_document.content.deep_dup + live_welsh_content = form.reload.live_welsh_form_document.content.deep_dup - it "sets a form's s3_bucket_aws_account_id" do - expect { task.invoke(*valid_args) } - .to change { form.reload.s3_bucket_aws_account_id }.to(s3_bucket_aws_account_id) - end + task.invoke(form.id) - it "sets a form's s3_bucket_region" do - expect { task.invoke(*valid_args) } - .to change { form.reload.s3_bucket_region }.to(s3_bucket_region) - end + expect(form.reload.delivery_configurations.where(delivery_method: "s3", delivery_schedule: "immediate").count) + .to eq(delivery_configuration_count) - it "does not update a different form" do - expect { task.invoke(*valid_args) } - .not_to(change { other_form.reload.submission_type }) + expect(form.reload.draft_form_document.content).to eq(draft_content) + expect(form.reload.live_form_document.content).to eq(live_content) + expect(form.reload.live_welsh_form_document.content).to eq(live_welsh_content) + end end end - context "when the form is draft" do - let(:form) { create :form } - - it "updates the draft form document" do - task.invoke(*valid_args) - form_document = form.draft_form_document.reload - expect(form_document.content["submission_type"]).to eq("s3") - expect(form_document.content["submission_format"]).to eq([format]) - expect(form_document.content["s3_bucket_name"]).to eq(s3_bucket_name) - expect(form_document.content["s3_bucket_aws_account_id"]).to eq(s3_bucket_aws_account_id) - expect(form_document.content["s3_bucket_region"]).to eq(s3_bucket_region) - expect(form_document.content["delivery_configurations"]).to include( - { - "delivery_method" => "s3", - "delivery_schedule" => "immediate", - "formats" => %w[csv], - }, - ) - end - end - - context "without arguments" do - it "aborts with a usage message" do + context "with invalid arguments" do + it "aborts with a usage message when the form id is missing" do expect { task.invoke }.to raise_error(SystemExit) - .and output("usage: rake forms:submission_type:set_to_s3[, , , , ]\n").to_stderr + .and output(/usage: rake forms:delivery_configurations:disable_s3\[\]/).to_stderr end end + end - context "without bucket name argument" do - it "aborts with a usage message" do - expect { - task.invoke(1) - }.to raise_error(SystemExit) - .and output("usage: rake forms:submission_type:set_to_s3[, , , , ]\n").to_stderr - end + describe "forms:delivery_configurations:enable_s3" do + subject(:task) do + Rake::Task["forms:delivery_configurations:enable_s3"] end - context "without AWS account ID argument" do - it "aborts with a usage message" do + let(:form) { create(:form, :live, :with_welsh_translation, :with_email_delivery) } + let(:s3_bucket_name) { "test-bucket" } + let(:s3_bucket_aws_account_id) { "123456789012" } + let(:s3_bucket_region) { "eu-west-1" } + let(:format) { "csv" } + let(:disable_email) { "false" } + let(:valid_args) { [form.id, s3_bucket_name, s3_bucket_aws_account_id, s3_bucket_region, format, disable_email] } + + context "with valid arguments" do + it "updates the form S3 configuration" do expect { - task.invoke(1, s3_bucket_name) - }.to raise_error(SystemExit) - .and output("usage: rake forms:submission_type:set_to_s3[, , , , ]\n").to_stderr + task.invoke(*valid_args) + }.to change { form.reload.s3_bucket_name }.to(s3_bucket_name) + .and change { form.reload.s3_bucket_aws_account_id }.to(s3_bucket_aws_account_id) + .and change { form.reload.s3_bucket_region }.to(s3_bucket_region) end - end - context "without region argument" do - it "aborts with a usage message" do + it "adds S3 delivery and keeps email enabled" do expect { - task.invoke(1, s3_bucket_name, s3_bucket_aws_account_id) - }.to raise_error(SystemExit) - .and output("usage: rake forms:submission_type:set_to_s3[, , , , ]\n").to_stderr + task.invoke(*valid_args) + }.to change { form.reload.delivery_configurations.where(delivery_method: "s3", delivery_schedule: "immediate").count } + .by(1) + + expect(form.reload.delivery_configurations.where(delivery_method: "email", delivery_schedule: "immediate").count) + .to eq(1) + expect(form.reload.delivery_configurations.find_by(delivery_method: "s3", delivery_schedule: "immediate").formats) + .to eq([format]) end - end - context "without format argument" do - it "aborts with a usage message" do - expect { - task.invoke(1, s3_bucket_name, s3_bucket_aws_account_id, "eu-west-2") - }.to raise_error(SystemExit) - .and output("usage: rake forms:submission_type:set_to_s3[, , , , ]\n").to_stderr + it "updates the draft and live form documents" do + task.invoke(*valid_args) + + expect(form.reload.draft_form_document.content["s3_bucket_name"]).to eq(s3_bucket_name) + expect(form.reload.draft_form_document.content["s3_bucket_aws_account_id"]).to eq(s3_bucket_aws_account_id) + expect(form.reload.draft_form_document.content["s3_bucket_region"]).to eq(s3_bucket_region) + + expect(form.reload.live_form_document.content["s3_bucket_name"]).to eq(s3_bucket_name) + expect(form.reload.live_form_document.content["s3_bucket_aws_account_id"]).to eq(s3_bucket_aws_account_id) + expect(form.reload.live_form_document.content["s3_bucket_region"]).to eq(s3_bucket_region) + + expect(form.reload.live_welsh_form_document.content["s3_bucket_name"]).to eq(s3_bucket_name) + expect(form.reload.live_welsh_form_document.content["s3_bucket_aws_account_id"]).to eq(s3_bucket_aws_account_id) + expect(form.reload.live_welsh_form_document.content["s3_bucket_region"]).to eq(s3_bucket_region) + + expect(form.reload.draft_form_document.content["delivery_configurations"]) + .to include(a_hash_including("delivery_method" => "s3", "delivery_schedule" => "immediate")) + expect(form.reload.live_form_document.content["delivery_configurations"]) + .to include(a_hash_including("delivery_method" => "s3", "delivery_schedule" => "immediate")) + expect(form.reload.live_welsh_form_document.content["delivery_configurations"]) + .to include(a_hash_including("delivery_method" => "s3", "delivery_schedule" => "immediate")) end - end - context "when region is not allowed" do - it "aborts with message" do - expect { - task.invoke(1, s3_bucket_name, s3_bucket_aws_account_id, "eu-west-3", "csv") - }.to raise_error(SystemExit) - .and output("s3_bucket_region must be one of eu-west-1 or eu-west-2\n").to_stderr + context "when disable_email is true" do + let(:disable_email) { "true" } + + it "removes email delivery" do + expect { + task.invoke(*valid_args) + }.to change { form.reload.delivery_configurations.where(delivery_method: "email", delivery_schedule: "immediate").count } + .by(-1) + + expect(form.reload.draft_form_document.content["delivery_configurations"]) + .not_to include(a_hash_including("delivery_method" => "email", "delivery_schedule" => "immediate")) + expect(form.reload.live_form_document.content["delivery_configurations"]) + .not_to include(a_hash_including("delivery_method" => "email", "delivery_schedule" => "immediate")) + expect(form.reload.live_welsh_form_document.content["delivery_configurations"]) + .not_to include(a_hash_including("delivery_method" => "email", "delivery_schedule" => "immediate")) + + expect(form.reload.live_form_document.content["s3_bucket_name"]).to eq(s3_bucket_name) + expect(form.reload.live_form_document.content["s3_bucket_aws_account_id"]).to eq(s3_bucket_aws_account_id) + expect(form.reload.live_form_document.content["s3_bucket_region"]).to eq(s3_bucket_region) + + expect(form.reload.live_welsh_form_document.content["s3_bucket_name"]).to eq(s3_bucket_name) + expect(form.reload.live_welsh_form_document.content["s3_bucket_aws_account_id"]).to eq(s3_bucket_aws_account_id) + expect(form.reload.live_welsh_form_document.content["s3_bucket_region"]).to eq(s3_bucket_region) + end end end - context "when format is invalid" do - it "aborts with a usage message" do + context "with invalid arguments" do + it "aborts with a usage message when the form id is missing" do expect { - task.invoke(1, s3_bucket_name, s3_bucket_aws_account_id, "eu-west-2", "xml") + task.invoke }.to raise_error(SystemExit) - .and output("format must be one of csv or json\n").to_stderr + .and output(/usage: rake forms:delivery_configurations:enable_s3\[, , , , , \]/).to_stderr end end end From 99d8f536ef5436be04131a6b062963675afdcb56 Mon Sep 17 00:00:00 2001 From: Stephen Daly Date: Fri, 17 Jul 2026 19:02:26 +0100 Subject: [PATCH 8/9] Remove data migration rake task for adding DeliveryConfigurations This has been run on all environments and needs to be deleted so we can ignore the deprecated Form columns. --- lib/tasks/data_migrations.rake | 60 --------- spec/lib/tasks/data_migrations.rake_spec.rb | 128 -------------------- 2 files changed, 188 deletions(-) delete mode 100644 lib/tasks/data_migrations.rake delete mode 100644 spec/lib/tasks/data_migrations.rake_spec.rb diff --git a/lib/tasks/data_migrations.rake b/lib/tasks/data_migrations.rake deleted file mode 100644 index 956d3389b7..0000000000 --- a/lib/tasks/data_migrations.rake +++ /dev/null @@ -1,60 +0,0 @@ -namespace :data_migrations do - desc "create delivery configurations for existing forms" - task create_delivery_configurations: :environment do - updated_count = 0 - - Form.all.find_each do |form| - form.delivery_configurations.find_or_create_by!( - delivery_method: form.submission_type, - formats: form.submission_format, - delivery_schedule: "immediate", - ) - - if form.send_daily_submission_batch - form.delivery_configurations.find_or_create_by!( - delivery_method: "email", - formats: %w[csv], - delivery_schedule: "daily", - ) - end - - if form.send_weekly_submission_batch - form.delivery_configurations.find_or_create_by!( - delivery_method: "email", - formats: %w[csv], - delivery_schedule: "weekly", - ) - end - - form.form_documents.each do |form_document| - add_delivery_configurations_to_form_document(form, form_document) - end - - updated_count += 1 - end - - Rails.logger.info("Updated #{updated_count} forms") - end -end - -def add_delivery_configurations_to_form_document(form, form_document) - content = form_document.content - - delivery_configurations = [] - - delivery_method = content["submission_type"] - formats = content["submission_format"] - delivery_configurations << DeliveryConfiguration.new(form:, delivery_method:, formats:, delivery_schedule: "immediate") - - if content["send_daily_submission_batch"] - delivery_configurations << DeliveryConfiguration.new(form:, delivery_method: "email", formats: %w[csv], delivery_schedule: "daily") - end - - if content["send_weekly_submission_batch"] - delivery_configurations << DeliveryConfiguration.new(form:, delivery_method: "email", formats: %w[csv], delivery_schedule: "weekly") - end - - content["delivery_configurations"] = delivery_configurations - - form_document.save! -end diff --git a/spec/lib/tasks/data_migrations.rake_spec.rb b/spec/lib/tasks/data_migrations.rake_spec.rb deleted file mode 100644 index 3c491f4d9b..0000000000 --- a/spec/lib/tasks/data_migrations.rake_spec.rb +++ /dev/null @@ -1,128 +0,0 @@ -require "rails_helper" - -RSpec.describe "data_migrations.rake", type: :task do - describe "data_migrations:create_delivery_configurations" do - subject(:task) do - Rake::Task["data_migrations:create_delivery_configurations"] - end - - let(:form) do - create(:form, :live_with_draft, submission_type: "email", submission_format: %w[csv json]) - end - - let!(:draft_form_document) do - form.draft_form_document - end - - it "has no delivery configurations before the task is run" do - expect(form.delivery_configurations).to be_empty - end - - it "creates a delivery configuration for the form" do - task.invoke - - expect(form.reload.delivery_configurations.pluck(:delivery_method, :delivery_schedule, :formats)).to contain_exactly( - ["email", "immediate", %w[csv json]], - ) - end - - it "updates the draft form document" do - task.invoke - - expect(draft_form_document.reload.content["delivery_configurations"]).to contain_exactly( - { - "delivery_method" => "email", - "delivery_schedule" => "immediate", - "formats" => %w[csv json], - }, - ) - end - - context "when the live form document has different settings from the form" do - let(:form) do - create(:form, submission_type: "email", submission_format: []) - end - - let!(:live_form_document) do - create( - :form_document, - form:, - tag: "live", - language: "en", - content: form.as_form_document.merge( - "submission_type" => "s3", - "submission_format" => %w[json], - ), - ) - end - - it "updates the live form document with the delivery configuration for the live form settings" do - task.invoke - - expect(form.reload.delivery_configurations.pluck(:delivery_method, :delivery_schedule, :formats)).to contain_exactly( - ["email", "immediate", []], - ) - - expect(live_form_document.reload.content["delivery_configurations"]).to contain_exactly( - { - "delivery_method" => "s3", - "delivery_schedule" => "immediate", - "formats" => %w[json], - }, - ) - end - end - - context "when the form sends daily and weekly submission batches" do - let(:form) do - create(:form, :live_with_draft, send_daily_submission_batch: true, send_weekly_submission_batch: true) - end - - let!(:draft_form_document) do - form.draft_form_document - end - - it "creates immediate, daily, and weekly delivery configurations for the form" do - task.invoke - - expect(form.reload.delivery_configurations.pluck(:delivery_method, :delivery_schedule, :formats)).to contain_exactly( - ["email", "immediate", []], - ["email", "daily", %w[csv]], - ["email", "weekly", %w[csv]], - ) - end - - it "updates the draft form document with all delivery configurations" do - task.invoke - - expect(draft_form_document.reload.content["delivery_configurations"]).to contain_exactly( - { - "delivery_method" => "email", - "delivery_schedule" => "immediate", - "formats" => [], - }, - { - "delivery_method" => "email", - "delivery_schedule" => "daily", - "formats" => %w[csv], - }, - { - "delivery_method" => "email", - "delivery_schedule" => "weekly", - "formats" => %w[csv], - }, - ) - end - end - - it "does not duplicate delivery configurations when rerun" do - task.invoke - task.reenable - task.invoke - - expect(form.reload.delivery_configurations.pluck(:delivery_method, :delivery_schedule, :formats)).to contain_exactly( - ["email", "immediate", %w[csv json]], - ) - end - end -end From 5a2b297e2b40a9c46754d099ed766a7f11e18646 Mon Sep 17 00:00:00 2001 From: Stephen Daly Date: Fri, 17 Jul 2026 16:09:47 +0100 Subject: [PATCH 9/9] Ignore unused form attributes The following attributes are no longer used by forms-admin or forms-runner, so can be ignored in preparation for dropping the columns: - submission_type - submission_format - send_daily_submission_batch - send_weekly_submission_batch --- app/models/form.rb | 14 +------- app/models/form_document/content.rb | 4 --- db/seeds.rb | 2 -- spec/factories/models/forms.rb | 4 --- spec/models/form_document/content_spec.rb | 15 ++++++++- spec/models/form_spec.rb | 40 ----------------------- 6 files changed, 15 insertions(+), 64 deletions(-) diff --git a/app/models/form.rb b/app/models/form.rb index 098692b90c..177123a4c8 100644 --- a/app/models/form.rb +++ b/app/models/form.rb @@ -2,7 +2,7 @@ class Form < ApplicationRecord include FormStateMachine extend Mobility - self.ignored_columns += [:language] + self.ignored_columns += %i[language submission_type submission_format send_daily_submission_batch send_weekly_submission_batch] SUPPORTED_LANGUAGES = %w[en cy].freeze @@ -31,26 +31,14 @@ class Form < ApplicationRecord :what_happens_next_markdown, :payment_url - enum :submission_type, { - email: "email", - s3: "s3", - } - enum :send_copy_of_answers, { disabled: "disabled", enabled: "enabled", }, prefix: :send_copy_of_answers - # ActiveRecord doesn't support enums with arrays - # enum :submission_format, { - # csv: "csv", - # json: "json", - # } - validates :name, presence: true validates :payment_url, url: true, allow_blank: true validate :marking_complete_with_errors - validates :submission_type, presence: true validates :send_copy_of_answers, presence: true validates :available_languages, presence: true, inclusion: { in: SUPPORTED_LANGUAGES } validates :submission_email, email_address: { message: :invalid_email }, allow_blank: true diff --git a/app/models/form_document/content.rb b/app/models/form_document/content.rb index e1b5d009f4..904291b694 100644 --- a/app/models/form_document/content.rb +++ b/app/models/form_document/content.rb @@ -21,8 +21,6 @@ class FormDocument::Content attribute :support_email, :string attribute :support_phone, :string attribute :s3_bucket_name, :string - attribute :submission_type, :string - attribute :submission_format, array: true attribute :declaration_text, :string attribute :declaration_markdown, :string attribute :s3_bucket_region, :string @@ -31,8 +29,6 @@ class FormDocument::Content attribute :privacy_policy_url, :string attribute :s3_bucket_aws_account_id, :string attribute :what_happens_next_markdown, :string - attribute :send_daily_submission_batch, :boolean - attribute :send_weekly_submission_batch, :boolean attribute :send_copy_of_answers, :string attribute :delivery_configurations, array: true diff --git a/db/seeds.rb b/db/seeds.rb index 7a846eb867..05c32a6425 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -260,8 +260,6 @@ support_phone: "08000800", what_happens_next_markdown: "Test", share_preview_completed: true, - submission_type: "s3", - submission_format: %w[csv], s3_bucket_region: "eu-west-2", s3_bucket_name: "govuk-forms-submissions-to-s3-test", s3_bucket_aws_account_id: "711966560482", diff --git a/spec/factories/models/forms.rb b/spec/factories/models/forms.rb index 40ad9457b7..aae255bbb5 100644 --- a/spec/factories/models/forms.rb +++ b/spec/factories/models/forms.rb @@ -2,8 +2,6 @@ factory :form, class: "Form" do sequence(:name) { |n| "Form #{n}" } submission_email { Faker::Internet.email(domain: "example.gov.uk") } - submission_type { "email" } - submission_format { [] } privacy_policy_url { Faker::Internet.url(host: "gov.uk") } support_email { nil } support_phone { nil } @@ -18,8 +16,6 @@ state { :draft } payment_url { nil } external_id { nil } - send_daily_submission_batch { false } - send_weekly_submission_batch { false } send_copy_of_answers { "disabled" } brand_id { nil } s3_bucket_aws_account_id { nil } diff --git a/spec/models/form_document/content_spec.rb b/spec/models/form_document/content_spec.rb index 4d8d397df0..0841ee8695 100644 --- a/spec/models/form_document/content_spec.rb +++ b/spec/models/form_document/content_spec.rb @@ -35,7 +35,20 @@ end it "has all form attributes the original form has" do - expected_attributes = form.attributes.except(*%w[id state external_id pages question_section_completed declaration_section_completed share_preview_completed welsh_completed copied_from_id]) + excluded_attributes = %w[id + state + external_id + pages + question_section_completed + declaration_section_completed + share_preview_completed + welsh_completed + copied_from_id + submission_type + submission_format + send_daily_submission_batch + send_weekly_submission_batch] + expected_attributes = form.attributes.except(*excluded_attributes) expect(form_document_content).to have_attributes(expected_attributes) end diff --git a/spec/models/form_spec.rb b/spec/models/form_spec.rb index 358a749361..3b8c0cf65e 100644 --- a/spec/models/form_spec.rb +++ b/spec/models/form_spec.rb @@ -234,13 +234,6 @@ expect(form).to be_valid end end - - context "when there is no submission type" do - it "returns invalid" do - form.submission_type = nil - expect(form).to be_invalid - end - end end end @@ -900,15 +893,6 @@ end end - describe "submission type" do - describe "enum" do - it "returns a list of submission types" do - expect(described_class.submission_types.keys).to eq(%w[email s3]) - expect(described_class.submission_types.values).to eq(%w[email s3]) - end - end - end - describe "answer email copy" do describe "enum" do it "returns a list of email copy answers values" do @@ -924,30 +908,6 @@ end end - describe "submission format" do - let(:form) { create :form } - - it "can be empty" do - form.update!(submission_format: []) - expect(form.submission_format).to be_empty - end - - it "stores an array of strings" do - form.update!(submission_format: %w[csv json]) - expect(form.submission_format).to include "csv" - expect(form.submission_format).to include "json" - end - - # ActiveRecord doesn't support enums with arrays - # describe "enum" do - # it "returns a list of submission formats" do - # formats = %w[csv json] - # expect(described_class.submission_formats.keys).to eq formats - # expect(described_class.submission_formats.values).to eq formats - # end - # end - end - describe "#destroy" do let(:form) { create :form }