From b60c0a127ac8118f2cfe92ccb28c7da6c3d581d1 Mon Sep 17 00:00:00 2001 From: Stephen Daly Date: Fri, 21 Aug 2026 17:19:50 +0100 Subject: [PATCH 1/9] Add module to share content labels for Welsh CSV download We will use the first column in the CSV to match translations to the form/page attribute they are for when importing the translations from an uploaded CSV. Add a module to share constants/methods for generating these labels so they can be re-used when importing to retrieve the translations from the CSV. Also add a column header to the first column in the CSV download so we can more easily validate the headers. --- app/lib/welsh_translation_content_labels.rb | 51 +++++++++++++++++++++ app/services/welsh_csv_service.rb | 43 +++++++++-------- spec/services/welsh_csv_service_spec.rb | 16 +++---- 3 files changed, 81 insertions(+), 29 deletions(-) create mode 100644 app/lib/welsh_translation_content_labels.rb diff --git a/app/lib/welsh_translation_content_labels.rb b/app/lib/welsh_translation_content_labels.rb new file mode 100644 index 0000000000..a889a11c0a --- /dev/null +++ b/app/lib/welsh_translation_content_labels.rb @@ -0,0 +1,51 @@ +module WelshTranslationContentLabels + FORM_ATTRIBUTE_LABELS = { + name: "Form name", + declaration_markdown: "Declaration", + what_happens_next_markdown: "Information about what happens next", + payment_url: "GOV.UK Pay payment link", + privacy_policy_url: "Link to privacy information for this form", + support_email: "Contact details for support - email address", + support_phone: "Contact details for support - phone number and opening times", + support_url: "Contact details for support - online contact link", + support_url_text: "Contact details for support - online contact link text", + }.freeze + + PAGE_ATTRIBUTE_LABELS = { + question_text: "question text", + hint_text: "hint text", + page_heading: "page heading", + guidance_markdown: "guidance text", + none_of_the_above_question: "question or label if 'None of the above' is selected", + }.freeze + + CONDITION_ATTRIBUTE_LABELS = { + exit_page_heading: "exit page heading", + exit_page_markdown: "exit page content", + }.freeze + + EXIT_PAGE_ATTRIBUTE_LABELS = { + heading: "heading", + markdown: "content", + }.freeze + + def page_label(page, attribute) + "#{question_name(page)} - #{PAGE_ATTRIBUTE_LABELS.fetch(attribute)}" + end + + def selection_option_label(page, option_index) + "#{question_name(page)} - option #{option_index + 1}" + end + + def condition_label(page, attribute) + "#{question_name(page)} - #{CONDITION_ATTRIBUTE_LABELS.fetch(attribute)}" + end + + def exit_page_label(page, exit_page_position, attribute) + "#{question_name(page)} - exit page #{exit_page_position} #{EXIT_PAGE_ATTRIBUTE_LABELS.fetch(attribute)}" + end + + def question_name(page) + "Question #{page.position}" + end +end diff --git a/app/services/welsh_csv_service.rb b/app/services/welsh_csv_service.rb index addf53bce7..cd436a1f27 100644 --- a/app/services/welsh_csv_service.rb +++ b/app/services/welsh_csv_service.rb @@ -1,6 +1,11 @@ class WelshCsvService + include WelshTranslationContentLabels + MAX_FILENAME_LENGTH = 80 FILENAME_SEPARATOR = "_".freeze + CONTENT_ID_HEADER = "Content ID".freeze + ENGLISH_CONTENT_HEADER = "English content".freeze + WELSH_CONTENT_HEADER = "Welsh content".freeze attr_reader :form @@ -29,7 +34,7 @@ def filename private def add_header(csv) - csv << ["", "English content", "Welsh content"] + csv << [CONTENT_ID_HEADER, ENGLISH_CONTENT_HEADER, WELSH_CONTENT_HEADER] end def add_form_name(csv) @@ -53,16 +58,16 @@ def add_page_content(csv) end def add_question_content(csv, page) - csv << ["#{question_name(page)} - question text", page.question_text, page.question_text_cy] + csv << [page_label(page, :question_text), page.question_text, page.question_text_cy] if page.hint_text.present? - csv << ["#{question_name(page)} - hint text", page.hint_text, page.hint_text_cy] + csv << [page_label(page, :hint_text), page.hint_text, page.hint_text_cy] end end def add_selection_options(csv, page) page.answer_settings.selection_options.each_with_index do |option, index| welsh_option_name = page.answer_settings_cy&.selection_options&.dig(index)&.name || "" - csv << ["#{question_name(page)} - option #{index + 1}", option.name, welsh_option_name] + csv << [selection_option_label(page, index), option.name, welsh_option_name] end end @@ -71,7 +76,7 @@ def add_none_of_above_question(csv, page) welsh_question = page.answer_settings_cy&.none_of_the_above_question&.question_text || "" csv << [ - "#{question_name(page)} - question or label if ‘None of the above’ is selected", + page_label(page, :none_of_the_above_question), english_question, welsh_question, ] @@ -85,8 +90,8 @@ def has_none_of_the_above?(page) def add_routing_conditions(csv, page) page.routing_conditions.each do |condition| if condition.is_exit_page? - csv << ["#{question_name(page)} - exit page heading", condition.exit_page_heading, condition.exit_page_heading_cy] - csv << ["#{question_name(page)} - exit page content", condition.exit_page_markdown, condition.exit_page_markdown_cy] + csv << [condition_label(page, :exit_page_heading), condition.exit_page_heading, condition.exit_page_heading_cy] + csv << [condition_label(page, :exit_page_markdown), condition.exit_page_markdown, condition.exit_page_markdown_cy] end end end @@ -95,20 +100,20 @@ def add_exit_pages(csv, page) exit_page_positions = ExitPage.positions_for_page(page) page.exit_pages.each do |exit_page| exit_page_position = exit_page_positions[exit_page.id] - csv << ["#{question_name(page)} - exit page #{exit_page_position} heading", exit_page.heading, exit_page.heading_cy] - csv << ["#{question_name(page)} - exit page #{exit_page_position} content", exit_page.markdown, exit_page.markdown_cy] + csv << [exit_page_label(page, exit_page_position, :heading), exit_page.heading, exit_page.heading_cy] + csv << [exit_page_label(page, exit_page_position, :markdown), exit_page.markdown, exit_page.markdown_cy] end end def add_page_heading(csv, page) if page.page_heading.present? - csv << ["#{question_name(page)} - page heading", page.page_heading, page.page_heading_cy] + csv << [page_label(page, :page_heading), page.page_heading, page.page_heading_cy] end end def add_guidance_text(csv, page) if page.guidance_markdown.present? - csv << ["#{question_name(page)} - guidance text", page.guidance_markdown, page.guidance_markdown_cy] + csv << [page_label(page, :guidance_markdown), page.guidance_markdown, page.guidance_markdown_cy] end end @@ -117,19 +122,19 @@ def question_name(page) end def add_form_metadata(csv) - add_field_if_present(csv, "Declaration", form.declaration_text, form.declaration_text_cy) - add_field_if_present(csv, "Information about what happens next", form.what_happens_next_markdown, form.what_happens_next_markdown_cy) - add_field_if_present(csv, "GOV⁠.⁠UK Pay payment link", form.payment_url, form.payment_url_cy) - add_field_if_present(csv, "Link to privacy information for this form", form.privacy_policy_url, form.privacy_policy_url_cy) + add_field_if_present(csv, FORM_ATTRIBUTE_LABELS[:declaration_markdown], form.declaration_text, form.declaration_text_cy) + add_field_if_present(csv, FORM_ATTRIBUTE_LABELS[:what_happens_next_markdown], form.what_happens_next_markdown, form.what_happens_next_markdown_cy) + add_field_if_present(csv, FORM_ATTRIBUTE_LABELS[:payment_url], form.payment_url, form.payment_url_cy) + add_field_if_present(csv, FORM_ATTRIBUTE_LABELS[:privacy_policy_url], form.privacy_policy_url, form.privacy_policy_url_cy) add_support_details(csv) end def add_support_details(csv) - add_field_if_present(csv, "Contact details for support - email address", form.support_email, form.support_email_cy) - add_field_if_present(csv, "Contact details for support - phone number and opening times", form.support_phone, form.support_phone_cy) - add_field_if_present(csv, "Contact details for support - online contact link", form.support_url, form.support_url_cy) - add_field_if_present(csv, "Contact details for support - online contact link text", form.support_url_text, form.support_url_text_cy) + add_field_if_present(csv, FORM_ATTRIBUTE_LABELS[:support_email], form.support_email, form.support_email_cy) + add_field_if_present(csv, FORM_ATTRIBUTE_LABELS[:support_phone], form.support_phone, form.support_phone_cy) + add_field_if_present(csv, FORM_ATTRIBUTE_LABELS[:support_url], form.support_url, form.support_url_cy) + add_field_if_present(csv, FORM_ATTRIBUTE_LABELS[:support_url_text], form.support_url_text, form.support_url_text_cy) end def add_field_if_present(csv, label, english_value, welsh_value) diff --git a/spec/services/welsh_csv_service_spec.rb b/spec/services/welsh_csv_service_spec.rb index 613807c067..cfa999f685 100644 --- a/spec/services/welsh_csv_service_spec.rb +++ b/spec/services/welsh_csv_service_spec.rb @@ -5,11 +5,7 @@ let(:form) { build :form } it "contains the header row" do - expect(csv_rows(form)[0]).to contain_exactly( - "", - "English content", - "Welsh content", - ) + expect(csv_rows(form)[0]).to eq(["Content ID", "English content", "Welsh content"]) end it "contains the form name" do @@ -49,7 +45,7 @@ it "contains the payment URL" do expect(csv_rows(form)).to include([ - "GOV⁠.⁠UK Pay payment link", + "GOV.UK Pay payment link", "https://www.gov.uk/payment", "https://www.gov.uk/payment_cy", ]) @@ -201,7 +197,7 @@ it "contains the none of the above question" do expect(csv_rows(form)).to include([ - "Question 1 - question or label if ‘None of the above’ is selected", + "Question 1 - question or label if 'None of the above' is selected", "None of the above question?", "Welsh None of the above question?", ]) @@ -347,12 +343,12 @@ it "returns a CSV with a header row and the expected rows" do csv = csv_rows(form) - expected_csv = [["", "English content", "Welsh content"], + expected_csv = [["Content ID", "English content", "Welsh content"], ["Form name", "A form", "Welsh A form"], ["Question 1 - question text", "None of the above question?", "Welsh None of the above question?"], ["Question 1 - option 1", "Option 1", "Option 1"], ["Question 1 - option 2", "Option 2", "Option 2"], - ["Question 1 - question or label if ‘None of the above’ is selected", "None of the above question?", "Welsh None of the above question?"], + ["Question 1 - question or label if 'None of the above' is selected", "None of the above question?", "Welsh None of the above question?"], ["Question 1 - exit page heading", "Exit page heading", "Welsh exit page heading"], ["Question 1 - exit page content", "Exit page markdown", "Welsh exit page markdown"], ["Question 2 - page heading", "Page heading", "Welsh Page heading"], @@ -360,7 +356,7 @@ ["Question 2 - question text", "What?", "Welsh What?"], ["Declaration", "Declaration text", ""], ["Information about what happens next", "English what happens next", "Welsh what happens next"], - ["GOV⁠.⁠UK Pay payment link", "https://www.gov.uk/payment", "https://www.gov.uk/payment_cy"], + ["GOV.UK Pay payment link", "https://www.gov.uk/payment", "https://www.gov.uk/payment_cy"], ["Link to privacy information for this form", "https://www.gov.uk/privacy", ""], ["Contact details for support - email address", "support@example.gov.uk", "support@example.gov.uk"], ["Contact details for support - phone number and opening times", "English support phone", "Welsh support phone"], From 94879ab08ce6fda2e2f7fc21607e57a17abdfafc Mon Sep 17 00:00:00 2001 From: Stephen Daly Date: Tue, 25 Aug 2026 13:09:48 +0100 Subject: [PATCH 2/9] Add service to upload Welsh CSV Add a service that will validate the format of a Welsh translation CSV and return a hash of content label to Welsh content. We will use the output of this service to populate the fields on the Welsh translations page with the translations from the CSV. --- app/services/welsh_csv_import_service.rb | 40 ++++++ .../services/welsh_csv_import_service_spec.rb | 118 ++++++++++++++++++ 2 files changed, 158 insertions(+) create mode 100644 app/services/welsh_csv_import_service.rb create mode 100644 spec/services/welsh_csv_import_service_spec.rb diff --git a/app/services/welsh_csv_import_service.rb b/app/services/welsh_csv_import_service.rb new file mode 100644 index 0000000000..50a0c9f3e9 --- /dev/null +++ b/app/services/welsh_csv_import_service.rb @@ -0,0 +1,40 @@ +class WelshCsvImportService + include WelshTranslationContentLabels + + class InvalidHeadersError < StandardError; end + + attr_reader :file + + HEADER_INDEXES = { + WelshCsvService::CONTENT_ID_HEADER => 0, + WelshCsvService::ENGLISH_CONTENT_HEADER => 1, + WelshCsvService::WELSH_CONTENT_HEADER => 2, + }.freeze + + def initialize(file, form) + @file = file + @form = form + end + + def read + file_content = file.read.force_encoding("UTF-8") + csv = CSV.parse(file_content, headers: true) + + raise InvalidHeadersError unless headers_valid?(csv) + + csv.each_with_object({}) do |row, values| + content_id = row[WelshCsvService::CONTENT_ID_HEADER] + next if content_id.nil? + + values[content_id] = row[WelshCsvService::WELSH_CONTENT_HEADER].to_s + end + end + +private + + def headers_valid?(csv) + HEADER_INDEXES.all? do |header, index| + csv.headers[index] == header + end + end +end diff --git a/spec/services/welsh_csv_import_service_spec.rb b/spec/services/welsh_csv_import_service_spec.rb new file mode 100644 index 0000000000..8e3c506d20 --- /dev/null +++ b/spec/services/welsh_csv_import_service_spec.rb @@ -0,0 +1,118 @@ +require "rails_helper" + +RSpec.describe WelshCsvImportService do + subject(:service) { described_class.new(file, form) } + + let(:form) do + create :form, + :with_pages, + name: "A form", + what_happens_next_markdown: "English what happens next", + privacy_policy_url: "https://www.gov.uk/privacy", + payment_url: "https://www.gov.uk/payment", + support_email: "support@example.gov.uk", + support_phone: "English support phone", + support_url: "https://www.gov.uk/support", + support_url_text: "Support URL text", + declaration_text: "Declaration text", + pages: [page, another_page] + end + let(:page) do + create :page, + :selection_with_none_of_the_above_question, + question_text: "None of the above question?", + none_of_the_above_question_text: "None of the above question?", + routing_conditions: [condition] + end + let(:condition) { create :condition, :with_exit_page, exit_page_heading: "Exit page heading" } + let(:another_page) { create :page, question_text: "What?", page_heading: "Page heading", guidance_markdown: "This is the guidance." } + + let(:file) { Tempfile.new } + + after do + file.unlink + end + + context "when the CSV is valid" do + context "when the rows in the CSV match the current form" do + before do + rows = [ + ["Content ID", "English content", "Welsh content"], + ["Form name", "A form", "Welsh A form ôÂŵéï"], + ["Question 1 - question text", "None of the above question?", "Welsh None of the above question?"], + ["Question 1 - option 1", "Option 1", "Welsh Option 1"], + ["Question 1 - option 2", "Option 2", "Welsh Option 2"], + ["Question 1 - question or label if 'None of the above' is selected", "None of the above question?", "Welsh None of the above question?"], + ["Question 1 - exit page heading", "Exit page heading", "Welsh exit page heading"], + ["Question 1 - exit page content", "Exit page markdown", "Welsh exit page markdown"], + ["Question 2 - page heading", "Page heading", "Welsh Page heading"], + ["Question 2 - guidance text", "This is the guidance.", "Welsh This is the guidance."], + ["Question 2 - question text", "What?", "Welsh What?"], + ["Declaration", "Declaration text", "Welsh declaration text"], + ["Information about what happens next", "English what happens next", "Welsh what happens next"], + ["GOV.UK Pay payment link", "https://www.gov.uk/payment", "https://www.gov.uk/payment_cy"], + ["Link to privacy information for this form", "https://www.gov.uk/privacy", "https://www.gov.uk/privacy_cy"], + ["Contact details for support - email address", "support@example.gov.uk", "support@example.gov.uk"], + ["Contact details for support - phone number and opening times", "English support phone", "Welsh support phone"], + ["Contact details for support - online contact link", "https://www.gov.uk/support", "https://www.gov.uk/support_cy"], + ["Contact details for support - online contact link text", "Support URL text", "Welsh Support URL text"], + ] + file.write(rows.map(&:to_csv).join) + file.rewind + end + + it "returns the translations data" do + expect(service.read).to eq({ + "Form name" => "Welsh A form ôÂŵéï", + "Question 1 - question text" => "Welsh None of the above question?", + "Question 1 - option 1" => "Welsh Option 1", + "Question 1 - option 2" => "Welsh Option 2", + "Question 1 - question or label if 'None of the above' is selected" => "Welsh None of the above question?", + "Question 1 - exit page heading" => "Welsh exit page heading", + "Question 1 - exit page content" => "Welsh exit page markdown", + "Question 2 - page heading" => "Welsh Page heading", + "Question 2 - guidance text" => "Welsh This is the guidance.", + "Question 2 - question text" => "Welsh What?", + "Declaration" => "Welsh declaration text", + "Information about what happens next" => "Welsh what happens next", + "GOV.UK Pay payment link" => "https://www.gov.uk/payment_cy", + "Link to privacy information for this form" => "https://www.gov.uk/privacy_cy", + "Contact details for support - email address" => "support@example.gov.uk", + "Contact details for support - phone number and opening times" => "Welsh support phone", + "Contact details for support - online contact link" => "https://www.gov.uk/support_cy", + "Contact details for support - online contact link text" => "Welsh Support URL text", + }) + end + end + end + + context "when the CSV has invalid headers" do + before do + rows = [ + ["Content ID", "Unexpected", "Welsh content"], + ["Form name", "A form", "Welsh A form"], + ] + file.write(rows.map(&:to_csv).join) + file.rewind + end + + it "raises an InvalidHeadersError" do + expect { service.read }.to raise_error(WelshCsvImportService::InvalidHeadersError) + end + end + + context "when the headers are in the wrong order" do + before do + rows = [ + ["Welsh content", "English content", "Content ID"], + ["Form name", "Welsh A form", "A form"], + ] + file.write(rows.map(&:to_csv).join) + file.rewind + end + + it "raises an InvalidHeadersError" do + expect { service.read }.to raise_error(WelshCsvImportService::InvalidHeadersError) + end + end +end From d0c7187bf5a877bcb960c8031fe991e2ceb1a9b5 Mon Sep 17 00:00:00 2001 From: Stephen Daly Date: Fri, 21 Aug 2026 17:29:57 +0100 Subject: [PATCH 3/9] Add method to populate Welsh translations using CSV data Add a method to the WelshTranslationInput that accepts the data extracted from an uploaded CSV of translations and assigns value to the inputs on the Welsh translations page. --- .../welsh_condition_translation_input.rb | 12 +++ .../welsh_exit_page_translation_input.rb | 12 +++ .../forms/welsh_page_translation_input.rb | 22 +++++ .../forms/welsh_page_translation_input2.rb | 22 +++++ ...elsh_selection_option_translation_input.rb | 8 ++ .../forms/welsh_translation_input.rb | 25 +++++ .../forms/welsh_translation_input2.rb | 25 +++++ .../welsh_condition_translation_input_spec.rb | 61 +++++++++++- .../welsh_exit_page_translation_input_spec.rb | 69 ++++++++++++-- .../welsh_page_translation_input2_spec.rb | 95 +++++++++++++++++++ .../welsh_page_translation_input_spec.rb | 94 +++++++++++++++++- .../forms/welsh_translation_input2_spec.rb | 87 ++++++++++++++++- .../forms/welsh_translation_input_spec.rb | 89 ++++++++++++++++- 13 files changed, 606 insertions(+), 15 deletions(-) diff --git a/app/input_objects/forms/welsh_condition_translation_input.rb b/app/input_objects/forms/welsh_condition_translation_input.rb index c7459b29c8..8b99897946 100644 --- a/app/input_objects/forms/welsh_condition_translation_input.rb +++ b/app/input_objects/forms/welsh_condition_translation_input.rb @@ -1,6 +1,7 @@ class Forms::WelshConditionTranslationInput < BaseInput include ActionView::Helpers::FormTagHelper include ActiveModel::Attributes + include WelshTranslationContentLabels attr_accessor :condition @@ -39,6 +40,17 @@ def assign_condition_values self end + def assign_from_spreadsheet(data) + assign_condition_values + + %i[exit_page_heading exit_page_markdown].each do |attr| + content_label = condition_label(condition.routing_page, attr) + send(:"#{attr}_cy=", data[content_label]) if data.key?(content_label) && data[content_label].present? + end + + self + end + def form_field_id(attribute) field_id(:forms_welsh_condition_translation_input, condition.id, :condition_translations, attribute) end diff --git a/app/input_objects/forms/welsh_exit_page_translation_input.rb b/app/input_objects/forms/welsh_exit_page_translation_input.rb index c715fab37a..f5145cf59e 100644 --- a/app/input_objects/forms/welsh_exit_page_translation_input.rb +++ b/app/input_objects/forms/welsh_exit_page_translation_input.rb @@ -1,6 +1,7 @@ class Forms::WelshExitPageTranslationInput < BaseInput include ActionView::Helpers::FormTagHelper include ActiveModel::Attributes + include WelshTranslationContentLabels attr_accessor :exit_page, :position @@ -41,6 +42,17 @@ def assign_exit_page_values self end + def assign_from_spreadsheet(data) + assign_exit_page_values + + %i[heading markdown].each do |attr| + content_label = exit_page_label(exit_page.question_page, position, attr) + send(:"#{attr}_cy=", data[content_label]) if data.key?(content_label) && data[content_label].present? + end + + self + end + def form_field_id(attribute) field_id(:forms_welsh_exit_page_translation_input, exit_page.id, :exit_page_translations, attribute) end diff --git a/app/input_objects/forms/welsh_page_translation_input.rb b/app/input_objects/forms/welsh_page_translation_input.rb index 20d90a655b..4310ebfbd2 100644 --- a/app/input_objects/forms/welsh_page_translation_input.rb +++ b/app/input_objects/forms/welsh_page_translation_input.rb @@ -2,6 +2,7 @@ class Forms::WelshPageTranslationInput < BaseInput include TextInputHelper include ActionView::Helpers::FormTagHelper include ActiveModel::Attributes + include WelshTranslationContentLabels attr_accessor :condition_translations, :selection_options_cy attr_reader :page @@ -82,6 +83,27 @@ def assign_page_values self end + def assign_from_spreadsheet(data) + # assign values from the form first, and override only those that are set in the spreadsheet + assign_page_values + + %i[question_text hint_text page_heading guidance_markdown none_of_the_above_question].each do |attr| + content_label = page_label(page, attr) + send(:"#{attr}_cy=", data[content_label]) if data.key?(content_label) && data[content_label].present? + end + + selection_options_cy&.each_with_index do |option, index| + content_label = selection_option_label(page, index) + option.name_cy = data[content_label] if data.key?(content_label) && data[content_label].present? + end + + condition_translations.each do |condition_translation| + condition_translation.assign_from_spreadsheet(data) + end + + self + end + def condition_translations_attributes=(attributes) submitted_condition_ids = attributes.values.map { |attrs| attrs["id"] }.compact diff --git a/app/input_objects/forms/welsh_page_translation_input2.rb b/app/input_objects/forms/welsh_page_translation_input2.rb index 84c2f2fa6b..aa6a62d4d7 100644 --- a/app/input_objects/forms/welsh_page_translation_input2.rb +++ b/app/input_objects/forms/welsh_page_translation_input2.rb @@ -2,6 +2,7 @@ class Forms::WelshPageTranslationInput2 < BaseInput include TextInputHelper include ActionView::Helpers::FormTagHelper include ActiveModel::Attributes + include WelshTranslationContentLabels attr_accessor :exit_page_translations, :selection_options_cy attr_reader :page @@ -84,6 +85,27 @@ def assign_page_values self end + def assign_from_spreadsheet(data) + # assign values from the form first, and override only those that are set in the spreadsheet + assign_page_values + + %i[question_text hint_text page_heading guidance_markdown none_of_the_above_question].each do |attr| + content_label = page_label(page, attr) + send(:"#{attr}_cy=", data[content_label]) if data.key?(content_label) && data[content_label].present? + end + + selection_options_cy&.each_with_index do |option, index| + content_label = selection_option_label(page, index) + option.name_cy = data[content_label] if data.key?(content_label) && data[content_label].present? + end + + exit_page_translations&.each do |exit_page_translation| + exit_page_translation.assign_from_spreadsheet(data) + end + + self + end + def exit_page_translations_attributes=(attributes) submitted_exit_page_ids = attributes.values.map { |attrs| attrs["id"] }.compact diff --git a/app/input_objects/forms/welsh_selection_option_translation_input.rb b/app/input_objects/forms/welsh_selection_option_translation_input.rb index e94025ac3d..00c9bad28f 100644 --- a/app/input_objects/forms/welsh_selection_option_translation_input.rb +++ b/app/input_objects/forms/welsh_selection_option_translation_input.rb @@ -26,6 +26,14 @@ def assign_selection_option_values self end + def assign_selection_option_values_from_csv_values(csv_values) + return self unless selection_option + + spreadsheet_id = page_content_id(page.id, "option_#{index}") + self.name_cy = csv_values[spreadsheet_id] if csv_values.key?(spreadsheet_id) + self + end + def as_selection_option { name: name_cy, value: selection_option.value } end diff --git a/app/input_objects/forms/welsh_translation_input.rb b/app/input_objects/forms/welsh_translation_input.rb index 7c0aca357b..4e70c37af2 100644 --- a/app/input_objects/forms/welsh_translation_input.rb +++ b/app/input_objects/forms/welsh_translation_input.rb @@ -1,6 +1,7 @@ class Forms::WelshTranslationInput < Forms::MarkCompleteInput include TextInputHelper include ActiveModel::Attributes + include WelshTranslationContentLabels attr_accessor :form, :page_translations @@ -122,6 +123,30 @@ def assign_form_values self end + def assign_from_spreadsheet(data) + # assign values from the form first, and override only those that are set in the spreadsheet + assign_form_values + + %i[name + privacy_policy_url + support_email + support_phone + support_url + support_url_text + declaration_markdown + what_happens_next_markdown + payment_url].each do |attr| + content_label = FORM_ATTRIBUTE_LABELS.fetch(attr) + send(:"#{attr}_cy=", data[content_label]) if data.key?(content_label) && data[content_label].present? + end + + self.page_translations = form.pages.map do |page| + Forms::WelshPageTranslationInput.new(page:).assign_from_spreadsheet(data) + end + + self + end + def blanked? all_fields_empty? && page_translations.all?(&:blanked?) end diff --git a/app/input_objects/forms/welsh_translation_input2.rb b/app/input_objects/forms/welsh_translation_input2.rb index 9a729aaa64..6a7281e477 100644 --- a/app/input_objects/forms/welsh_translation_input2.rb +++ b/app/input_objects/forms/welsh_translation_input2.rb @@ -1,6 +1,7 @@ class Forms::WelshTranslationInput2 < Forms::MarkCompleteInput include TextInputHelper include ActiveModel::Attributes + include WelshTranslationContentLabels attr_accessor :form, :page_translations @@ -122,6 +123,30 @@ def assign_form_values self end + def assign_from_spreadsheet(data) + # assign values from the form first, and override only those that are set in the spreadsheet + assign_form_values + + %i[name + privacy_policy_url + support_email + support_phone + support_url + support_url_text + declaration_markdown + what_happens_next_markdown + payment_url].each do |attr| + content_label = FORM_ATTRIBUTE_LABELS.fetch(attr) + send(:"#{attr}_cy=", data[content_label]) if data.key?(content_label) && data[content_label].present? + end + + self.page_translations = form.pages.map do |page| + Forms::WelshPageTranslationInput2.new(page:).assign_from_spreadsheet(data) + end + + self + end + def blanked? all_fields_empty? && page_translations.all?(&:blanked?) end diff --git a/spec/input_objects/forms/welsh_condition_translation_input_spec.rb b/spec/input_objects/forms/welsh_condition_translation_input_spec.rb index 3b91a11fad..30e6ef4526 100644 --- a/spec/input_objects/forms/welsh_condition_translation_input_spec.rb +++ b/spec/input_objects/forms/welsh_condition_translation_input_spec.rb @@ -4,7 +4,7 @@ subject(:welsh_condition_translation_input) { described_class.new(new_input_data) } let(:condition) { create_condition } - let(:page) { create :page } + let(:page) { create :page, position: 1 } let(:new_input_data) do { @@ -16,12 +16,12 @@ def create_condition(attributes = {}) default_attributes = { - id: 1, answer_value: "Yes", exit_page_markdown: "You are ineligible", exit_page_heading: "Sorry, you are ineligible for this service.", exit_page_markdown_cy: "", exit_page_heading_cy: "", + routing_page: page, } create(:condition, default_attributes.merge(attributes)) end @@ -178,7 +178,7 @@ def create_condition(attributes = {}) end end - describe "#assign_page_values" do + describe "#assign_condition_values" do it "loads the existing welsh attributes from the page" do welsh_condition_translation_input = described_class.new(condition:) welsh_condition_translation_input.assign_condition_values @@ -188,6 +188,61 @@ def create_condition(attributes = {}) end end + describe "#assign_from_spreadsheet" do + subject(:welsh_condition_translation_input) { described_class.new(condition:) } + + context "when the spreadsheet data contains Welsh translations for all fields" do + let(:spreadsheet_data) do + { + "Question 1 - exit page heading" => "Welsh heading from spreadsheet", + "Question 1 - exit page content" => "Welsh markdown from spreadsheet", + "Question 2 - exit page heading" => "Another condition translation (ignored)", + "Question 1 - question text" => "Page field translation (ignored)", + } + end + + it "assigns the welsh attributes from the spreadsheet data" do + welsh_condition_translation_input.assign_from_spreadsheet(spreadsheet_data) + + expect(welsh_condition_translation_input.exit_page_heading_cy).to eq("Welsh heading from spreadsheet") + expect(welsh_condition_translation_input.exit_page_markdown_cy).to eq("Welsh markdown from spreadsheet") + end + end + + context "when the spreadsheet data does not include keys for all fields" do + let(:condition) { create_condition(exit_page_markdown_cy: "Welsh markdown on form") } + let(:spreadsheet_data) do + { + "Question 1 - exit page heading" => "Welsh heading from spreadsheet", + } + end + + it "uses the Welsh already set on the condition for fields not present in the spreadsheet data" do + welsh_condition_translation_input.assign_from_spreadsheet(spreadsheet_data) + + expect(welsh_condition_translation_input.exit_page_heading_cy).to eq("Welsh heading from spreadsheet") + expect(welsh_condition_translation_input.exit_page_markdown_cy).to eq("Welsh markdown on form") + end + end + + context "when the spreadsheet data includes blank values" do + let(:condition) { create_condition(exit_page_markdown_cy: "Welsh markdown on form", exit_page_heading_cy: "Welsh heading on form") } + let(:spreadsheet_data) do + { + "Question 1 - exit page heading" => "Welsh heading from spreadsheet", + "Question 1 - exit page content" => "", + } + end + + it "uses the Welsh already set on the condition for fields with blank values in the spreadsheet data" do + welsh_condition_translation_input.assign_from_spreadsheet(spreadsheet_data) + + expect(welsh_condition_translation_input.exit_page_heading_cy).to eq("Welsh heading from spreadsheet") + expect(welsh_condition_translation_input.exit_page_markdown_cy).to eq("Welsh markdown on form") + end + end + end + describe "#condition_has_exit_page?" do context "when the condition has an exit page" do let(:condition) do diff --git a/spec/input_objects/forms/welsh_exit_page_translation_input_spec.rb b/spec/input_objects/forms/welsh_exit_page_translation_input_spec.rb index 44a1f36e41..9184a7f8e8 100644 --- a/spec/input_objects/forms/welsh_exit_page_translation_input_spec.rb +++ b/spec/input_objects/forms/welsh_exit_page_translation_input_spec.rb @@ -4,7 +4,7 @@ subject(:welsh_exit_page_translation_input) { described_class.new(new_input_data) } let(:exit_page) { create_exit_page } - let(:page) { create :page } + let(:page) { create :page, position: 3 } let(:new_input_data) do { @@ -17,7 +17,6 @@ def create_exit_page(attributes = {}) default_attributes = { - id: 1, question_page: page, markdown: "You are ineligible", heading: "Sorry, you are ineligible for this service.", @@ -36,7 +35,7 @@ def create_exit_page(attributes = {}) it "is not valid" do expect(welsh_exit_page_translation_input).not_to be_valid(validation_context) - expect(welsh_exit_page_translation_input.errors.full_messages_for(:heading_cy)).to include "Heading cy #{I18n.t('activemodel.errors.models.forms/welsh_condition_translation_input.attributes.exit_page_heading_cy.blank', question_number: exit_page.question_page.position)}" + expect(welsh_exit_page_translation_input.errors.full_messages_for(:heading_cy)).to include "Heading cy #{I18n.t('activemodel.errors.models.forms/welsh_exit_page_translation_input.attributes.heading_cy.blank', question_number: exit_page.question_page.position)}" end end @@ -46,7 +45,7 @@ def create_exit_page(attributes = {}) it "is not valid" do expect(welsh_exit_page_translation_input).not_to be_valid(validation_context) - expect(welsh_exit_page_translation_input.errors.full_messages_for(:heading_cy)).to include "Heading cy #{I18n.t('activemodel.errors.models.forms/welsh_condition_translation_input.attributes.exit_page_heading_cy.too_long', question_number: exit_page.question_page.position, count: 250)}" + expect(welsh_exit_page_translation_input.errors.full_messages_for(:heading_cy)).to include "Heading cy #{I18n.t('activemodel.errors.models.forms/welsh_exit_page_translation_input.attributes.heading_cy.too_long', question_number: exit_page.question_page.position, count: 250)}" end end @@ -65,7 +64,7 @@ def create_exit_page(attributes = {}) it "is not valid" do expect(welsh_exit_page_translation_input).not_to be_valid(validation_context) - expect(welsh_exit_page_translation_input.errors.full_messages_for(:markdown_cy)).to include "Markdown cy #{I18n.t('activemodel.errors.models.forms/welsh_condition_translation_input.attributes.exit_page_markdown_cy.blank', question_number: exit_page.question_page.position)}" + expect(welsh_exit_page_translation_input.errors.full_messages_for(:markdown_cy)).to include "Markdown cy #{I18n.t('activemodel.errors.models.forms/welsh_exit_page_translation_input.attributes.markdown_cy.blank', question_number: exit_page.question_page.position)}" end end @@ -95,7 +94,7 @@ def create_exit_page(attributes = {}) it "is not valid" do expect(welsh_exit_page_translation_input).not_to be_valid(validation_context) - expect(welsh_exit_page_translation_input.errors.full_messages_for(:heading_cy)).to include "Heading cy #{I18n.t('activemodel.errors.models.forms/welsh_condition_translation_input.attributes.exit_page_heading_cy.too_long', question_number: exit_page.question_page.position, count: 250)}" + expect(welsh_exit_page_translation_input.errors.full_messages_for(:heading_cy)).to include "Heading cy #{I18n.t('activemodel.errors.models.forms/welsh_exit_page_translation_input.attributes.heading_cy.too_long', question_number: exit_page.question_page.position, count: 250)}" end end @@ -157,6 +156,62 @@ def create_exit_page(attributes = {}) end end + describe "#assign_from_spreadsheet" do + subject(:welsh_exit_page_translation_input) { described_class.new(exit_page: exit_page, position: 1) } + + context "when the spreadsheet data contains Welsh translations for all fields" do + let(:spreadsheet_data) do + { + "Question 3 - exit page 1 heading" => "Welsh heading from spreadsheet", + "Question 3 - exit page 1 content" => "Welsh markdown from spreadsheet", + "Question 3 - exit page 2 heading" => "Another exit page heading translation (ignored)", + "Question 2 - exit page 1 heading" => "Another page's exit page heading translation (ignored)", + "Question 1 - question text" => "Page field translation (ignored)", + } + end + + it "assigns the welsh attributes from the spreadsheet data" do + welsh_exit_page_translation_input.assign_from_spreadsheet(spreadsheet_data) + + expect(welsh_exit_page_translation_input.heading_cy).to eq("Welsh heading from spreadsheet") + expect(welsh_exit_page_translation_input.markdown_cy).to eq("Welsh markdown from spreadsheet") + end + end + + context "when the spreadsheet data does not include keys for all fields" do + let(:exit_page) { create_exit_page(markdown_cy: "Welsh markdown on form") } + let(:spreadsheet_data) do + { + "Question 3 - exit page 1 heading" => "Welsh heading from spreadsheet", + } + end + + it "uses the Welsh already set on the exit page for fields not present in the spreadsheet data" do + welsh_exit_page_translation_input.assign_from_spreadsheet(spreadsheet_data) + + expect(welsh_exit_page_translation_input.heading_cy).to eq("Welsh heading from spreadsheet") + expect(welsh_exit_page_translation_input.markdown_cy).to eq("Welsh markdown on form") + end + end + + context "when the spreadsheet data includes blank values" do + let(:exit_page) { create_exit_page(markdown_cy: "Welsh markdown on form", heading_cy: "Welsh heading on form") } + let(:spreadsheet_data) do + { + "Question 3 - exit page 1 heading" => "Welsh heading from spreadsheet", + "Question 3 - exit page 1 content" => "", + } + end + + it "uses the Welsh already set on the exit page for fields with blank values in the spreadsheet data" do + welsh_exit_page_translation_input.assign_from_spreadsheet(spreadsheet_data) + + expect(welsh_exit_page_translation_input.heading_cy).to eq("Welsh heading from spreadsheet") + expect(welsh_exit_page_translation_input.markdown_cy).to eq("Welsh markdown on form") + end + end + end + describe "#form_field_id" do let(:exit_page) do create_exit_page(id: 999) @@ -175,7 +230,7 @@ def create_exit_page(attributes = {}) end end - context "when the welsh condition fields are all empty" do + context "when the welsh exit page fields are all empty" do let(:new_input_data) { { exit_page:, markdown_cy: "", heading_cy: "" } } it "returns true" do diff --git a/spec/input_objects/forms/welsh_page_translation_input2_spec.rb b/spec/input_objects/forms/welsh_page_translation_input2_spec.rb index a661034cde..6882e3ecea 100644 --- a/spec/input_objects/forms/welsh_page_translation_input2_spec.rb +++ b/spec/input_objects/forms/welsh_page_translation_input2_spec.rb @@ -585,6 +585,101 @@ def create_page(attributes = {}) end end + describe "#assign_from_spreadsheet" do + let(:page) do + create_page(attributes_for(:page, :selection_with_none_of_the_above_question, position: 3)) + end + + before do + page.exit_pages = [exit_page, another_exit_page] + end + + context "when the spreadsheet data contains Welsh translations for all fields" do + let(:spreadsheet_data) do + { + "Question 3 - question text" => "Welsh question text from spreadsheet", + "Question 3 - hint text" => "Welsh hint text from spreadsheet", + "Question 3 - page heading" => "Welsh page heading from spreadsheet", + "Question 3 - guidance text" => "Welsh guidance markdown from spreadsheet", + "Question 3 - option 1" => "Welsh Option 1 from spreadsheet", + "Question 3 - option 2" => "Welsh Option 2 from spreadsheet", + "Question 3 - question or label if 'None of the above' is selected" => "Welsh None of the above question? from spreadsheet", + "Question 3 - exit page 1 heading" => "Welsh exit page heading from spreadsheet", + "Question 3 - exit page 1 content" => "Welsh exit page markdown from spreadsheet", + "Question 3 - exit page 2 heading" => "Another exit page heading from spreadsheet", + "Question 3 - exit page 2 content" => "Another exit page markdown from spreadsheet", + "Question 1 - page heading" => "Translation for a different page (ignored)", + "Declaration" => "Translation for a form field (ignored)", + } + end + + it "sets the welsh attributes from the spreadsheet data" do + welsh_page_translation_input = described_class.new(page:) + welsh_page_translation_input.assign_from_spreadsheet(spreadsheet_data) + + expect(welsh_page_translation_input.question_text_cy).to eq("Welsh question text from spreadsheet") + expect(welsh_page_translation_input.hint_text_cy).to eq("Welsh hint text from spreadsheet") + expect(welsh_page_translation_input.page_heading_cy).to eq("Welsh page heading from spreadsheet") + expect(welsh_page_translation_input.guidance_markdown_cy).to eq("Welsh guidance markdown from spreadsheet") + expect(welsh_page_translation_input.none_of_the_above_question_cy).to eq("Welsh None of the above question? from spreadsheet") + + selection_options_cy = welsh_page_translation_input.selection_options_cy.map(&:as_selection_option) + + expect(selection_options_cy).to eq([ + { name: "Welsh Option 1 from spreadsheet", value: "Option 1" }, + { name: "Welsh Option 2 from spreadsheet", value: "Option 2" }, + ]) + + exit_page_translation = welsh_page_translation_input.exit_page_translations.find { |ct| ct.id == exit_page.id } + expect(exit_page_translation.heading_cy).to eq("Welsh exit page heading from spreadsheet") + expect(exit_page_translation.markdown_cy).to eq("Welsh exit page markdown from spreadsheet") + + another_exit_page_translation = welsh_page_translation_input.exit_page_translations.find { |ct| ct.id == another_exit_page.id } + expect(another_exit_page_translation.heading_cy).to eq("Another exit page heading from spreadsheet") + expect(another_exit_page_translation.markdown_cy).to eq("Another exit page markdown from spreadsheet") + end + end + + context "when the spreadsheet data does not include keys for all fields" do + let(:page) { create_page(hint_text_cy: "Page Welsh hint text", position: 3) } + let(:spreadsheet_data) do + { + "Question 3 - question text" => "Welsh question text from spreadsheet", + } + end + + it "uses the Welsh already set on the page for fields not present in the spreadsheet data" do + welsh_page_translation_input = described_class.new(page:) + welsh_page_translation_input.assign_from_spreadsheet(spreadsheet_data) + + expect(welsh_page_translation_input.question_text_cy).to eq("Welsh question text from spreadsheet") + expect(welsh_page_translation_input.hint_text_cy).to eq("Page Welsh hint text") + end + end + + context "when the spreadsheet data includes blank values" do + let(:page) do + create_page(question_text_cy: "Page Welsh question text", + hint_text_cy: "Page Welsh hint text", + position: 3) + end + let(:spreadsheet_data) do + { + "Question 3 - question text" => "Welsh question text from spreadsheet", + "Question 3 - hint text" => "", + } + end + + it "uses the Welsh already set on the page for fields with blank values in the spreadsheet data" do + welsh_page_translation_input = described_class.new(page:) + welsh_page_translation_input.assign_from_spreadsheet(spreadsheet_data) + + expect(welsh_page_translation_input.question_text_cy).to eq("Welsh question text from spreadsheet") + expect(welsh_page_translation_input.hint_text_cy).to eq("Page Welsh hint text") + end + end + end + describe "#page_has_hint_text?" do context "when the page has hint_text" do let(:page) { create_page(hint_text: "Choose 'Yes' if you already have a valid licence.") } diff --git a/spec/input_objects/forms/welsh_page_translation_input_spec.rb b/spec/input_objects/forms/welsh_page_translation_input_spec.rb index eaf59b06d5..48e5fb4b5f 100644 --- a/spec/input_objects/forms/welsh_page_translation_input_spec.rb +++ b/spec/input_objects/forms/welsh_page_translation_input_spec.rb @@ -25,7 +25,6 @@ def create_page(attributes = {}) default_attributes = { - id: 1, question_text: "Are you renewing a licence?", hint_text: "Choose 'Yes' if you already have a valid licence.", page_heading: "Licencing", @@ -541,8 +540,8 @@ def create_page(attributes = {}) expect(selection_options_cy).to eq([ { name: "Welsh option 1", value: "Yes" }, - { name: "", value: "No" }, - { name: "", value: "Maybe" }, + { name: "", value: "No" }, + { name: "", value: "Maybe" }, ]) end end @@ -581,6 +580,95 @@ def create_page(attributes = {}) end end + describe "#assign_from_spreadsheet" do + let(:page) do + create_page(attributes_for(:page, :selection_with_none_of_the_above_question, position: 3)) + end + + before do + page.routing_conditions = [condition, another_condition] + end + + context "when the spreadsheet data contains Welsh translations for all fields" do + let(:spreadsheet_data) do + { + "Question 3 - question text" => "Welsh question text from spreadsheet", + "Question 3 - hint text" => "Welsh hint text from spreadsheet", + "Question 3 - page heading" => "Welsh page heading from spreadsheet", + "Question 3 - guidance text" => "Welsh guidance markdown from spreadsheet", + "Question 3 - option 1" => "Welsh Option 1 from spreadsheet", + "Question 3 - option 2" => "Welsh Option 2 from spreadsheet", + "Question 3 - question or label if 'None of the above' is selected" => "Welsh None of the above question? from spreadsheet", + "Question 3 - exit page heading" => "Welsh exit page heading from spreadsheet", + "Question 3 - exit page content" => "Welsh exit page markdown from spreadsheet", + "Question 1 - page heading" => "Translation for a different page (ignored)", + "Declaration" => "Translation for a form field (ignored)", + } + end + + it "sets the welsh attributes from the spreadsheet data" do + welsh_page_translation_input = described_class.new(page:) + welsh_page_translation_input.assign_from_spreadsheet(spreadsheet_data) + + expect(welsh_page_translation_input.question_text_cy).to eq("Welsh question text from spreadsheet") + expect(welsh_page_translation_input.hint_text_cy).to eq("Welsh hint text from spreadsheet") + expect(welsh_page_translation_input.page_heading_cy).to eq("Welsh page heading from spreadsheet") + expect(welsh_page_translation_input.guidance_markdown_cy).to eq("Welsh guidance markdown from spreadsheet") + expect(welsh_page_translation_input.none_of_the_above_question_cy).to eq("Welsh None of the above question? from spreadsheet") + + selection_options_cy = welsh_page_translation_input.selection_options_cy.map(&:as_selection_option) + + expect(selection_options_cy).to eq([ + { name: "Welsh Option 1 from spreadsheet", value: "Option 1" }, + { name: "Welsh Option 2 from spreadsheet", value: "Option 2" }, + ]) + + condition_translation = welsh_page_translation_input.condition_translations.find { |ct| ct.id == condition.id } + expect(condition_translation.exit_page_heading_cy).to eq("Welsh exit page heading from spreadsheet") + expect(condition_translation.exit_page_markdown_cy).to eq("Welsh exit page markdown from spreadsheet") + end + end + + context "when the spreadsheet data does not include keys for all fields" do + let(:page) { create_page(hint_text_cy: "Page Welsh hint text", position: 3) } + let(:spreadsheet_data) do + { + "Question 3 - question text" => "Welsh question text from spreadsheet", + } + end + + it "uses the Welsh already set on the page for fields not present in the spreadsheet data" do + welsh_page_translation_input = described_class.new(page:) + welsh_page_translation_input.assign_from_spreadsheet(spreadsheet_data) + + expect(welsh_page_translation_input.question_text_cy).to eq("Welsh question text from spreadsheet") + expect(welsh_page_translation_input.hint_text_cy).to eq("Page Welsh hint text") + end + end + + context "when the spreadsheet data includes blank values" do + let(:page) do + create_page(question_text_cy: "Page Welsh question text", + hint_text_cy: "Page Welsh hint text", + position: 3) + end + let(:spreadsheet_data) do + { + "Question 3 - question text" => "Welsh question text from spreadsheet", + "Question 3 - hint text" => "", + } + end + + it "uses the Welsh already set on the page for fields with blank values in the spreadsheet data" do + welsh_page_translation_input = described_class.new(page:) + welsh_page_translation_input.assign_from_spreadsheet(spreadsheet_data) + + expect(welsh_page_translation_input.question_text_cy).to eq("Welsh question text from spreadsheet") + expect(welsh_page_translation_input.hint_text_cy).to eq("Page Welsh hint text") + end + end + end + describe "#page_has_hint_text?" do context "when the page has hint_text" do let(:page) { create_page(hint_text: "Choose 'Yes' if you already have a valid licence.") } diff --git a/spec/input_objects/forms/welsh_translation_input2_spec.rb b/spec/input_objects/forms/welsh_translation_input2_spec.rb index 11ead9e6ef..b45d9dcbf8 100644 --- a/spec/input_objects/forms/welsh_translation_input2_spec.rb +++ b/spec/input_objects/forms/welsh_translation_input2_spec.rb @@ -12,7 +12,7 @@ guidance_markdown: "This part of the form concerns licencing.", position: 1 end - let(:another_page) { create :page } + let(:another_page) { create :page, position: 2 } let(:mark_complete) { "true" } @@ -692,6 +692,91 @@ def build_empty_welsh_form end end + describe "#assign_from_spreadsheet" do + before do + form.pages = [page, another_page] + end + + context "when the spreadsheet data contains Welsh translations for all fields" do + let(:spreadsheet_data) do + { + "Form name" => "Welsh A form from spreadsheet", + "Question 1 - question text" => "Welsh question text from spreadsheet", + "Question 1 - option 1" => "Welsh Option 1 from spreadsheet", + "Question 1 - option 2" => "Welsh Option 2 from spreadsheet", + "Question 1 - question or label if 'None of the above' is selected" => "Welsh None of the above question? from spreadsheet", + "Question 1 - exit page 1 heading" => "Welsh exit page heading from spreadsheet", + "Question 1 - exit page 1 content" => "Welsh exit page content from spreadsheet", + "Question 2 - page heading" => "Welsh question 2 page heading from spreadsheet", + "Question 2 - guidance text" => "Welsh question 2 guidance markdown from spreadsheet", + "Question 2 - question text" => "Welsh question 2 text from spreadsheet", + "Declaration" => "Welsh declaration text from spreadsheet", + "Information about what happens next" => "Welsh what happens next from spreadsheet", + "GOV.UK Pay payment link" => "https://www.gov.uk/payment_cy_spreadsheet", + "Link to privacy information for this form" => "https://www.gov.uk/privacy_cy_spreadsheet", + "Contact details for support - email address" => "support-spreadsheet@example.gov.wales", + "Contact details for support - phone number and opening times" => "Welsh support phone from spreadsheet", + "Contact details for support - online contact link" => "https://www.gov.uk/support_cy_spreadsheet", + "Contact details for support - online contact link text" => "Welsh Support URL text from spreadsheet", + } + end + + it "sets the welsh attributes from the spreadsheet data" do + welsh_translation_input = described_class.new(form:) + welsh_translation_input.assign_from_spreadsheet(spreadsheet_data) + + expect(welsh_translation_input.name_cy).to eq("Welsh A form from spreadsheet") + expect(welsh_translation_input.declaration_markdown_cy).to eq("Welsh declaration text from spreadsheet") + expect(welsh_translation_input.what_happens_next_markdown_cy).to eq("Welsh what happens next from spreadsheet") + expect(welsh_translation_input.payment_url_cy).to eq("https://www.gov.uk/payment_cy_spreadsheet") + expect(welsh_translation_input.privacy_policy_url_cy).to eq("https://www.gov.uk/privacy_cy_spreadsheet") + expect(welsh_translation_input.support_email_cy).to eq("support-spreadsheet@example.gov.wales") + expect(welsh_translation_input.support_phone_cy).to eq("Welsh support phone from spreadsheet") + expect(welsh_translation_input.support_url_cy).to eq("https://www.gov.uk/support_cy_spreadsheet") + expect(welsh_translation_input.support_url_text_cy).to eq("Welsh Support URL text from spreadsheet") + + page_translation = welsh_translation_input.page_translations.find { |pt| pt.id == page.id } + expect(page_translation.question_text_cy).to eq("Welsh question text from spreadsheet") + + another_page_translation = welsh_translation_input.page_translations.find { |pt| pt.id == another_page.id } + expect(another_page_translation.page_heading_cy).to eq("Welsh question 2 page heading from spreadsheet") + end + end + + context "when the spreadsheet data does not include keys for all fields" do + let(:spreadsheet_data) do + { + "Form name" => "Welsh A form from spreadsheet", + } + end + + it "uses the Welsh already set on the form for fields not present in the spreadsheet data" do + welsh_translation_input = described_class.new(form:) + welsh_translation_input.assign_from_spreadsheet(spreadsheet_data) + + expect(welsh_translation_input.name_cy).to eq("Welsh A form from spreadsheet") + expect(welsh_translation_input.support_email_cy).to eq("welsh-support@example.gov.uk") + end + end + + context "when the spreadsheet data includes blank values" do + let(:spreadsheet_data) do + { + "Form name" => "Welsh A form from spreadsheet", + "Contact details for support - email address" => "", + } + end + + it "uses the Welsh already set on the form for fields with blank values in the spreadsheet data" do + welsh_translation_input = described_class.new(form:) + welsh_translation_input.assign_from_spreadsheet(spreadsheet_data) + + expect(welsh_translation_input.name_cy).to eq("Welsh A form from spreadsheet") + expect(welsh_translation_input.support_email_cy).to eq("welsh-support@example.gov.uk") + end + end + end + describe "#all_fields_empty?" do context "when the welsh translation fields are not empty" do let(:form) { build_form } diff --git a/spec/input_objects/forms/welsh_translation_input_spec.rb b/spec/input_objects/forms/welsh_translation_input_spec.rb index cebb22d993..ff61b523fe 100644 --- a/spec/input_objects/forms/welsh_translation_input_spec.rb +++ b/spec/input_objects/forms/welsh_translation_input_spec.rb @@ -12,7 +12,7 @@ guidance_markdown: "This part of the form concerns licencing.", position: 1 end - let(:another_page) { create :page } + let(:another_page) { create :page, position: 2 } let(:mark_complete) { "true" } @@ -692,6 +692,93 @@ def build_empty_welsh_form end end + describe "#assign_from_spreadsheet" do + before do + form.pages = [page, another_page] + end + + context "when the spreadsheet data contains Welsh translations for all fields" do + let(:spreadsheet_data) do + { + "Form name" => "Welsh A form from spreadsheet", + "Question 1 - question text" => "Welsh question text from spreadsheet", + "Question 1 - option 1" => "Welsh Option 1 from spreadsheet", + "Question 1 - option 2" => "Welsh Option 2 from spreadsheet", + "Question 1 - question or label if 'None of the above' is selected" => "Welsh None of the above question? from spreadsheet", + "Question 1 - exit page heading" => "Welsh exit page heading from spreadsheet", + "Question 1 - exit page content" => "Welsh exit page content from spreadsheet", + "Question 2 - exit page 1 heading" => "Welsh question 2 exit page heading from spreadsheet", + "Question 2 - exit page 1 content" => "Welsh question 2 exit page content from spreadsheet", + "Question 2 - page heading" => "Welsh question 2 page heading from spreadsheet", + "Question 2 - guidance text" => "Welsh question 2 guidance markdown from spreadsheet", + "Question 2 - question text" => "Welsh question 2 text from spreadsheet", + "Declaration" => "Welsh declaration text from spreadsheet", + "Information about what happens next" => "Welsh what happens next from spreadsheet", + "GOV.UK Pay payment link" => "https://www.gov.uk/payment_cy_spreadsheet", + "Link to privacy information for this form" => "https://www.gov.uk/privacy_cy_spreadsheet", + "Contact details for support - email address" => "support-spreadsheet@example.gov.wales", + "Contact details for support - phone number and opening times" => "Welsh support phone from spreadsheet", + "Contact details for support - online contact link" => "https://www.gov.uk/support_cy_spreadsheet", + "Contact details for support - online contact link text" => "Welsh Support URL text from spreadsheet", + } + end + + it "sets the welsh attributes from the spreadsheet data" do + welsh_translation_input = described_class.new(form:) + welsh_translation_input.assign_from_spreadsheet(spreadsheet_data) + + expect(welsh_translation_input.name_cy).to eq("Welsh A form from spreadsheet") + expect(welsh_translation_input.declaration_markdown_cy).to eq("Welsh declaration text from spreadsheet") + expect(welsh_translation_input.what_happens_next_markdown_cy).to eq("Welsh what happens next from spreadsheet") + expect(welsh_translation_input.payment_url_cy).to eq("https://www.gov.uk/payment_cy_spreadsheet") + expect(welsh_translation_input.privacy_policy_url_cy).to eq("https://www.gov.uk/privacy_cy_spreadsheet") + expect(welsh_translation_input.support_email_cy).to eq("support-spreadsheet@example.gov.wales") + expect(welsh_translation_input.support_phone_cy).to eq("Welsh support phone from spreadsheet") + expect(welsh_translation_input.support_url_cy).to eq("https://www.gov.uk/support_cy_spreadsheet") + expect(welsh_translation_input.support_url_text_cy).to eq("Welsh Support URL text from spreadsheet") + + page_translation = welsh_translation_input.page_translations.find { |pt| pt.id == page.id } + expect(page_translation.question_text_cy).to eq("Welsh question text from spreadsheet") + + another_page_translation = welsh_translation_input.page_translations.find { |pt| pt.id == another_page.id } + expect(another_page_translation.page_heading_cy).to eq("Welsh question 2 page heading from spreadsheet") + end + end + + context "when the spreadsheet data does not include keys for all fields" do + let(:spreadsheet_data) do + { + "Form name" => "Welsh A form from spreadsheet", + } + end + + it "uses the Welsh already set on the form for fields not present in the spreadsheet data" do + welsh_translation_input = described_class.new(form:) + welsh_translation_input.assign_from_spreadsheet(spreadsheet_data) + + expect(welsh_translation_input.name_cy).to eq("Welsh A form from spreadsheet") + expect(welsh_translation_input.support_email_cy).to eq("welsh-support@example.gov.uk") + end + end + + context "when the spreadsheet data includes blank values" do + let(:spreadsheet_data) do + { + "Form name" => "Welsh A form from spreadsheet", + "Contact details for support - email address" => "", + } + end + + it "uses the Welsh already set on the form for fields with blank values in the spreadsheet data" do + welsh_translation_input = described_class.new(form:) + welsh_translation_input.assign_from_spreadsheet(spreadsheet_data) + + expect(welsh_translation_input.name_cy).to eq("Welsh A form from spreadsheet") + expect(welsh_translation_input.support_email_cy).to eq("welsh-support@example.gov.uk") + end + end + end + describe "#all_fields_empty?" do context "when the welsh translation fields are not empty" do let(:form) { build_form } From 677e6a24feda735c9dd5e3aa252f20f5c2d0eb3f Mon Sep 17 00:00:00 2001 From: Stephen Daly Date: Thu, 27 Aug 2026 14:25:13 +0100 Subject: [PATCH 4/9] Add model for page to upload a CSV of translations Add an input object model for the web form for a new page to upload a CSV of Welsh translations. This does basic validation on the file type and size and then calls the WelshCsvImportService to parse the CSV. We handle any exceptions thrown by WelshCsvImportService by mapping them to appropriate errors to show in an error summary. --- .../forms/welsh_translation_upload_input.rb | 31 ++++++ .../welsh_translation_upload.yml | 17 +++ spec/fixtures/files/valid.csv | 2 + .../welsh_translation_upload_input_spec.rb | 100 ++++++++++++++++++ 4 files changed, 150 insertions(+) create mode 100644 app/input_objects/forms/welsh_translation_upload_input.rb create mode 100644 config/locales/input_objects/welsh_translation_upload.yml create mode 100644 spec/fixtures/files/valid.csv create mode 100644 spec/input_objects/forms/welsh_translation_upload_input_spec.rb diff --git a/app/input_objects/forms/welsh_translation_upload_input.rb b/app/input_objects/forms/welsh_translation_upload_input.rb new file mode 100644 index 0000000000..4242d3d3ae --- /dev/null +++ b/app/input_objects/forms/welsh_translation_upload_input.rb @@ -0,0 +1,31 @@ +class Forms::WelshTranslationUploadInput < BaseInput + FILE_TYPES = %w[ + text/csv + ].freeze + MAX_SIZE_IN_MB = 10 + + attr_accessor :form, :file + + validates :file, presence: true, file_content_type: { in: FILE_TYPES } + validate :validate_file_size + + def read_file + return false if invalid? + + WelshCsvImportService.new(file, form).read + rescue CSV::MalformedCSVError + errors.add(:file, :malformed) + false + rescue WelshCsvImportService::InvalidHeadersError + errors.add(:file, :invalid_headers) + false + end + +private + + def validate_file_size + if file.present? && file.size > MAX_SIZE_IN_MB.megabytes + errors.add(:file, :too_big) + end + end +end diff --git a/config/locales/input_objects/welsh_translation_upload.yml b/config/locales/input_objects/welsh_translation_upload.yml new file mode 100644 index 0000000000..97b0055125 --- /dev/null +++ b/config/locales/input_objects/welsh_translation_upload.yml @@ -0,0 +1,17 @@ +--- +en: + activemodel: + errors: + models: + forms/welsh_translation_upload_input: + attributes: + file: + blank: Select a file + invalid_file_type: The selected file must be a CSV + invalid_headers: We couldn’t upload the CSV because the column headings are wrong - check them and try uploading again + malformed: The CSV has invalid formatting - try downloading a new version, then uploading it again + too_big: The selected file must be smaller than 10MB + helpers: + label: + forms_welsh_translation_upload_input: + file: Upload CSV file diff --git a/spec/fixtures/files/valid.csv b/spec/fixtures/files/valid.csv new file mode 100644 index 0000000000..f39ee9076d --- /dev/null +++ b/spec/fixtures/files/valid.csv @@ -0,0 +1,2 @@ +header1,header2 +value1,value2 \ No newline at end of file diff --git a/spec/input_objects/forms/welsh_translation_upload_input_spec.rb b/spec/input_objects/forms/welsh_translation_upload_input_spec.rb new file mode 100644 index 0000000000..031a487914 --- /dev/null +++ b/spec/input_objects/forms/welsh_translation_upload_input_spec.rb @@ -0,0 +1,100 @@ +require "rails_helper" + +RSpec.describe Forms::WelshTranslationUploadInput do + let(:form) { create(:form) } + + describe "validations" do + it "is invalid without a file" do + input = described_class.new(form: form, file: nil) + expect(input).to be_invalid + expect(input.errors.full_messages_for(:file)).to include("File Select a file") + end + + it "is invalid with an unsupported file type" do + file = fixture_file_upload("invalid.txt", "text/plain") + input = described_class.new(form: form, file: file) + expect(input).to be_invalid + expect(input.errors.full_messages_for(:file)).to include("File The selected file must be a CSV") + end + + it "is invalid with a file that exceeds the maximum size" do + file = fixture_file_upload("valid.csv", "text/csv") + allow(file).to receive(:size).and_return((Forms::WelshTranslationUploadInput::MAX_SIZE_IN_MB + 1).megabytes) + + input = described_class.new(form: form, file: file) + expect(input).to be_invalid + expect(input.errors.full_messages_for(:file)).to include("File The selected file must be smaller than 10MB") + end + + it "is valid with a supported file type" do + Tempfile.create do |file| + file.write("header1,header2\nvalue1,value2") + file.rewind + + input = described_class.new(form: form, file: Rack::Test::UploadedFile.new(file, "text/csv")) + expect(input).to be_valid + end + end + end + + describe "#read_file" do + let(:mock_welsh_csv_import_service) { instance_double(WelshCsvImportService) } + let(:file) do + Tempfile.create do |file| + file.write("header1,header2\nvalue1,value2") + file.rewind + Rack::Test::UploadedFile.new(file, "text/csv") + end + end + + before do + allow(WelshCsvImportService).to receive(:new).and_return(mock_welsh_csv_import_service) + end + + after do + file.unlink + end + + it "returns false when the input is invalid" do + input = described_class.new(form: form, file: nil) + expect(input.read_file).to be false + end + + context "when the CSV is valid" do + let(:translations) { { "key1" => "translation1", "key2" => "translation2" } } + + before do + allow(mock_welsh_csv_import_service).to receive(:read).and_return(translations) + end + + it "returns the translations from the CSV" do + input = described_class.new(form: form, file: file) + expect(input.read_file).to eq(translations) + end + end + + context "when the CSV is malformed" do + before do + allow(mock_welsh_csv_import_service).to receive(:read).and_raise(CSV::MalformedCSVError.new("error", 1)) + end + + it "returns false and adds an error" do + input = described_class.new(form: form, file: file) + expect(input.read_file).to be false + expect(input.errors.full_messages_for(:file)).to include("File The CSV has invalid formatting - try downloading a new version, then uploading it again") + end + end + + context "when the CSV has invalid headers" do + before do + allow(mock_welsh_csv_import_service).to receive(:read).and_raise(WelshCsvImportService::InvalidHeadersError) + end + + it "returns false and adds an error" do + input = described_class.new(form: form, file: file) + expect(input.read_file).to be false + expect(input.errors.full_messages_for(:file)).to include("File We couldn’t upload the CSV because the column headings are wrong - check them and try uploading again") + end + end + end +end From 9bef72de328f1b1ee860013bc6c9356c7961cd62 Mon Sep 17 00:00:00 2001 From: Stephen Daly Date: Thu, 27 Aug 2026 14:25:28 +0100 Subject: [PATCH 5/9] Add a page to upload a CSV of Welsh translations This page has a file upload input that allows the user to upload a CSV of their Welsh translations. If the file is valid, the Welsh translations page is rendered with the inputs populated from the CSV. The Welsh is not saved until the user submits the translations page. We run validation on the input model so that we will show an error summary if any of the uploaded translations were invalid. For this validation, the input is not marked as complete so it won't show validation errors for translations that have not been filled in. --- .../forms/welsh_translation_controller.rb | 34 ++++ .../welsh_translation/show_upload.html.erb | 32 ++++ config/locales/en.yml | 11 ++ config/routes.rb | 2 + .../welsh_translation_controller_spec.rb | 181 ++++++++++++++++-- .../show_upload.html.erb_spec.rb | 44 +++++ 6 files changed, 291 insertions(+), 13 deletions(-) create mode 100644 app/views/forms/welsh_translation/show_upload.html.erb create mode 100644 spec/views/forms/welsh_translation/show_upload.html.erb_spec.rb diff --git a/app/controllers/forms/welsh_translation_controller.rb b/app/controllers/forms/welsh_translation_controller.rb index 83ce83430b..b022eb0225 100644 --- a/app/controllers/forms/welsh_translation_controller.rb +++ b/app/controllers/forms/welsh_translation_controller.rb @@ -81,6 +81,34 @@ def download disposition: "attachment; filename=#{form_content_service.filename}" end + def show_upload + authorize current_form, :can_edit_form? + welsh_translation_upload_input = WelshTranslationUploadInput.new(form: current_form) + render :show_upload, locals: { current_form:, welsh_translation_upload_input: } + end + + def upload + authorize current_form, :can_edit_form? + + welsh_translation_upload_input = WelshTranslationUploadInput.new(**welsh_translation_upload_params) + + data = welsh_translation_upload_input.read_file + unless data + return render :show_upload, status: :unprocessable_entity, locals: { current_form:, welsh_translation_upload_input: } + end + + @welsh_translation_input = if FeatureService.new(group: current_form.group).enabled?(:multiple_branches) + WelshTranslationInput2.new(form: form_with_pages_and_exit_pages) + else + WelshTranslationInput.new(form: form_with_pages_and_conditions) + end + + @welsh_translation_input.assign_from_spreadsheet(data).validate + @table_presenter = Forms::TranslationTablePresenter.new + + render :new + end + private def preview_html @@ -122,5 +150,11 @@ def form_with_pages_and_conditions def form_with_pages_and_exit_pages Form.includes(pages: [:exit_pages]).find(current_form.id) end + + def welsh_translation_upload_params + params.fetch(:forms_welsh_translation_upload_input, ActionController::Parameters.new) + .permit(:file) + .merge(form: current_form) + end end end diff --git a/app/views/forms/welsh_translation/show_upload.html.erb b/app/views/forms/welsh_translation/show_upload.html.erb new file mode 100644 index 0000000000..ef4950a310 --- /dev/null +++ b/app/views/forms/welsh_translation/show_upload.html.erb @@ -0,0 +1,32 @@ +<% set_page_title(t("page_titles.welsh_translation_upload")) %> +<% content_for :back_link, govuk_back_link_to(welsh_translation_path(current_form)) %> + +
+
+ <%= form_with(model: welsh_translation_upload_input, url: welsh_translation_upload_path(current_form), method: 'POST') do |f| %> + <% if welsh_translation_upload_input&.errors.any? %> + <%= f.govuk_error_summary %> + <% end %> + +

+ <%= current_form.name %> + <%= t("page_titles.welsh_translation_upload") %> +

+ + <%= govuk_warning_text(text: t(".warning")) %> + + <%= govuk_details(summary_text: t(".details_summary")) do %> + <%= t(".details_html")%> + <% end %> + + <%= f.govuk_file_field :file, + label: { tag: 'h2', size: 'm' }, + accept: Forms::WelshTranslationUploadInput::FILE_TYPES.join(", "), + javascript: true + %> + + <%= f.govuk_submit t("save_and_continue") %> + <% end %> +
+
+ diff --git a/config/locales/en.yml b/config/locales/en.yml index 4c27868a96..9598e6b04b 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -751,6 +751,16 @@ en: support_url: Online contact link support_url_text: Text to describe the contact link welsh_header: Welsh content + show_upload: + details_html: | +

To make sure the formatting is correct:

+
    +
  • Download the existing content as a CSV.
  • +
  • Add or update Welsh content - without making any other changes to the formatting.
  • +
  • Save, then upload the new Welsh content as a CSV.
  • +
+ details_summary: How to format a CSV for uploading + warning: This will replace any Welsh content you already have saved in the form group_forms: edit: body_html: "

We’ll send an email to members of the current group to let them know the form has moved and they may no longer have access to it.

\n" @@ -1839,6 +1849,7 @@ en: type_of_answer: What kind of answer do you need to this question? unarchive_form: Make your form live again welsh_translation: Add a Welsh version of your form + welsh_translation_upload: Upload new Welsh content as a CSV what_happens_next: Add information about what happens next your_changes_are_live: Your changes are live your_english_form_is_live: Your English form is live diff --git a/config/routes.rb b/config/routes.rb index a604928485..8296d19653 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -81,6 +81,8 @@ delete "/welsh-translation/delete" => "forms/welsh_translation#destroy", as: :welsh_translation_destroy post "/welsh-translation-preview" => "forms/welsh_translation#render_preview", as: :welsh_translation_render_preview get "/welsh-translation-download" => "forms/welsh_translation#download", as: :welsh_translation_download + get "/welsh-translation-upload" => "forms/welsh_translation#show_upload", as: :welsh_translation_show_upload + post "/welsh-translation-upload" => "forms/welsh_translation#upload", as: :welsh_translation_upload get "/submission-attachments" => "forms/submission_attachments#new", as: :submission_attachments post "/submission-attachments" => "forms/submission_attachments#create", as: :submission_attachments_create get "/batch-submissions" => "forms/batch_submissions#new", as: :batch_submissions diff --git a/spec/requests/forms/welsh_translation_controller_spec.rb b/spec/requests/forms/welsh_translation_controller_spec.rb index 8df87a54e5..86594c1df6 100644 --- a/spec/requests/forms/welsh_translation_controller_spec.rb +++ b/spec/requests/forms/welsh_translation_controller_spec.rb @@ -46,8 +46,8 @@ expect { post(welsh_translation_create_path(id), params:) }.to change { form.reload.welsh_completed }.to(true) - .and change { form.pages.first.reload.question_text_cy }.to("Ydych chi'n adnewyddu trwydded?") - .and change { condition.reload.exit_page_heading_cy }.to("Nid ydych yn gymwys") + .and change { form.pages.first.reload.question_text_cy }.to("Ydych chi'n adnewyddu trwydded?") + .and change { condition.reload.exit_page_heading_cy }.to("Nid ydych yn gymwys") end it "redirects to the form task list and displays a success banner including text about being marked complete" do @@ -81,8 +81,8 @@ expect { post(welsh_translation_create_path(id), params:) }.to not_change { form.reload.welsh_completed } - .and not_change { form.pages.first.reload.question_text_cy } - .and(not_change { condition.reload.exit_page_markdown_cy }) + .and not_change { form.pages.first.reload.question_text_cy } + .and(not_change { condition.reload.exit_page_markdown_cy }) end it "returns a 422, re-renders the page with an error, and does not display a success banner" do @@ -149,8 +149,8 @@ expect { post(welsh_translation_create_path(id), params:) }.to not_change { form.reload.welsh_completed } - .and not_change { form.pages.first.reload.question_text_cy } - .and(not_change { condition.reload.exit_page_markdown_cy }) + .and not_change { form.pages.first.reload.question_text_cy } + .and(not_change { condition.reload.exit_page_markdown_cy }) end it "returns 403" do @@ -172,8 +172,8 @@ expect { post(welsh_translation_create_path(id), params:) }.to change { form.reload.welsh_completed }.to(true) - .and change { form.pages.first.reload.question_text_cy }.to("Ydych chi'n adnewyddu trwydded?") - .and change { exit_page.reload.heading_cy }.to("Nid ydych yn gymwys") + .and change { form.pages.first.reload.question_text_cy }.to("Ydych chi'n adnewyddu trwydded?") + .and change { exit_page.reload.heading_cy }.to("Nid ydych yn gymwys") end it "redirects to the form task list and displays a success banner including text about being marked complete" do @@ -207,8 +207,8 @@ expect { post(welsh_translation_create_path(id), params:) }.to not_change { form.reload.welsh_completed } - .and not_change { form.pages.first.reload.question_text_cy } - .and(not_change { exit_page.reload.markdown_cy }) + .and not_change { form.pages.first.reload.question_text_cy } + .and(not_change { exit_page.reload.markdown_cy }) end it "returns a 422, re-renders the page with an error, and does not display a success banner" do @@ -275,8 +275,8 @@ expect { post(welsh_translation_create_path(id), params:) }.to not_change { form.reload.welsh_completed } - .and not_change { form.pages.first.reload.question_text_cy } - .and(not_change { exit_page.reload.markdown_cy }) + .and not_change { form.pages.first.reload.question_text_cy } + .and(not_change { exit_page.reload.markdown_cy }) end it "returns 403" do @@ -423,8 +423,163 @@ it "returns a CSV with a header row and and content" do csv = CSV.parse(response.body) - expect(csv.first).to eq(["", "English content", "Welsh content"]) + expect(csv.first).to eq(["Content ID", "English content", "Welsh content"]) expect(csv.second).to eq(["Form name", "A form with Welsh", "Welsh A form with Welsh"]) end end + + describe "#show_upload" do + before do + get welsh_translation_show_upload_path(id) + end + + it "renders the template" do + expect(response).to have_http_status(:ok) + expect(response).to render_template(:show_upload) + end + + context "when the user is not authorized" do + let(:current_user) { build :user } + + it "returns 403" do + expect(response).to have_http_status(:forbidden) + end + end + end + + describe "#upload" do + let(:form) { create(:form, :ready_for_live, welsh_completed: false) } + let(:csv_data) { "foo,bar" } + let(:file) do + file = Tempfile.new(["translations", ".csv"]) + file.write(csv_data) + file.rewind + Rack::Test::UploadedFile.new(file.path, "text/csv", original_filename: "translations.csv") + end + + after do + file.unlink + end + + context "when a valid CSV is uploaded" do + context "when the multiple branches feature is disabled", feature_multiple_branches: false do + let(:condition) do + create(:condition, routing_page: form.pages.first, answer_value: "No", + exit_page_heading: "You are ineligible", + exit_page_markdown: "Sorry, you are ineligible for this service.") + end + + let(:csv_data) do + CSV.generate do |csv| + csv << ["Content ID", "English content", "Welsh content"] + csv << ["Form name", form.name, "Fy Ffurflen"] + csv << ["Question 1 - exit page heading", "You are ineligible", "Welsh exit page heading"] + csv << ["Question 1 - exit page content", "Sorry, you are ineligible for this service.", "Welsh exit page content"] + end + end + + before do + condition + post welsh_translation_upload_path(id), params: { forms_welsh_translation_upload_input: { file: } } + end + + it "renders the new template" do + expect(response).to have_http_status(:ok) + expect(response).to render_template(:new) + end + + it "pre-populates fields from CSV" do + expect(response.body).not_to include(I18n.t("error_summary.heading")) + expect(response.body).to include("Fy Ffurflen") + expect(response.body).to include("Welsh exit page heading") + expect(response.body).to include("Welsh exit page content") + end + + context "when there are invalid translations" do + let(:csv_data) do + CSV.generate do |csv| + csv << ["Content ID", "English content", "Welsh content"] + csv << ["Link to privacy information for this form", form.privacy_policy_url, "this is not a URL"] + end + end + + it "pre-validates the fields and shows errors for invalid data" do + expect(response.body).to include(I18n.t("error_summary.heading")) + expect(response.body).to include(I18n.t("activemodel.errors.models.forms/welsh_translation_input.attributes.privacy_policy_url_cy.url")) + expect(response.body).to include("this is not a URL") + end + end + end + + context "when the multiple branches feature is enabled", :feature_multiple_branches do + let(:exit_page) { create :exit_page, question_page: form.pages.first } + + let(:csv_data) do + CSV.generate do |csv| + csv << ["Content ID", "English content", "Welsh content"] + csv << ["Form name", form.name, "Fy Ffurflen"] + csv << ["Question 1 - exit page 1 heading", "You are ineligible", "Welsh exit page heading"] + csv << ["Question 1 - exit page 1 content", "Sorry, you are ineligible for this service.", "Welsh exit page content"] + end + end + + before do + exit_page + post welsh_translation_upload_path(id), params: { forms_welsh_translation_upload_input: { file: } } + end + + it "renders the new template" do + expect(response).to have_http_status(:ok) + expect(response).to render_template(:new) + end + + it "pre-populates fields from CSV" do + expect(response.body).not_to include(I18n.t("error_summary.heading")) + expect(response.body).to include("Fy Ffurflen") + expect(response.body).to include("Welsh exit page heading") + expect(response.body).to include("Welsh exit page content") + end + + context "when there are invalid translations" do + let(:csv_data) do + CSV.generate do |csv| + csv << ["Content ID", "English content", "Welsh content"] + csv << ["Link to privacy information for this form", form.privacy_policy_url, "this is not a URL"] + end + end + + it "pre-validates the fields and shows errors for invalid data" do + expect(response.body).to include(I18n.t("error_summary.heading")) + expect(response.body).to include(I18n.t("activemodel.errors.models.forms/welsh_translation_input.attributes.privacy_policy_url_cy.url")) + expect(response.body).to include("this is not a URL") + end + end + end + + context "when no file is provided" do + before do + post welsh_translation_upload_path(id), params: { forms_welsh_translation_upload_input: { file: nil } } + end + + it "renders the upload file page with an error" do + expect(response).to have_http_status(:unprocessable_content) + expect(response).to render_template(:show_upload) + expect(response.body).to include(I18n.t("activemodel.errors.models.forms/welsh_translation_upload_input.attributes.file.blank")) + expect(flash).to be_empty + end + end + end + + context "when the user is not authorized" do + let(:current_user) { build :user } + + before do + post welsh_translation_upload_path(id), params: { file: } + end + + it "returns 403" do + expect(response).to have_http_status(:forbidden) + end + end + end end diff --git a/spec/views/forms/welsh_translation/show_upload.html.erb_spec.rb b/spec/views/forms/welsh_translation/show_upload.html.erb_spec.rb new file mode 100644 index 0000000000..8e4b795eab --- /dev/null +++ b/spec/views/forms/welsh_translation/show_upload.html.erb_spec.rb @@ -0,0 +1,44 @@ +require "rails_helper" + +describe "forms/welsh_translation/show_upload.html.erb" do + let(:form) { create :form } + let(:welsh_translation_upload_input) { Forms::WelshTranslationUploadInput.new(form: form) } + + before do + render template: "forms/welsh_translation/show_upload", locals: { + current_form: form, + welsh_translation_upload_input: welsh_translation_upload_input, + } + end + + it "contains a top-level heading" do + expect(rendered).to have_css("h1", text: I18n.t("page_titles.welsh_translation_upload")) + end + + it "has a back link to the welsh translation page" do + expect(view.content_for(:back_link)).to have_link("Back", href: welsh_translation_path(form)) + end + + it "contains a form for uploading a Welsh translation CSV file" do + expect(rendered).to have_css("form[action='#{welsh_translation_upload_path(form)}'][method='post'][enctype='multipart/form-data']") + end + + it "contains a file input field for uploading the CSV file" do + expect(rendered).to have_css("input[type='file'][name='forms_welsh_translation_upload_input[file]'][accept='text/csv']") + end + + context "when the form has errors" do + before do + welsh_translation_upload_input.errors.add(:file, "an error occurred") + render template: "forms/welsh_translation/show_upload", locals: { + current_form: form, + welsh_translation_upload_input: welsh_translation_upload_input, + } + end + + it "displays the error message" do + expect(rendered).to have_css(".govuk-error-summary") + expect(rendered).to have_css(".govuk-error-message", text: "an error occurred") + end + end +end From e460dd3f93f4dd83f605fb0353bb70f933814a3c Mon Sep 17 00:00:00 2001 From: Stephen Daly Date: Thu, 27 Aug 2026 15:54:49 +0100 Subject: [PATCH 6/9] Don't validate the page is marked as complete after translation upload When pre-validating the fields after uploading a translation CSV, don't validate that a radio has been selected for whether to mark the step as complete. The user will still have to select an option before they can submit the translation page after it has been populated with translations from the CSV. --- app/controllers/forms/welsh_translation_controller.rb | 2 +- app/input_objects/forms/welsh_translation_input.rb | 10 +++++++++- app/input_objects/forms/welsh_translation_input2.rb | 10 +++++++++- .../forms/welsh_translation_input_spec.rb | 7 +++++++ 4 files changed, 26 insertions(+), 3 deletions(-) diff --git a/app/controllers/forms/welsh_translation_controller.rb b/app/controllers/forms/welsh_translation_controller.rb index b022eb0225..864e04428c 100644 --- a/app/controllers/forms/welsh_translation_controller.rb +++ b/app/controllers/forms/welsh_translation_controller.rb @@ -103,7 +103,7 @@ def upload WelshTranslationInput.new(form: form_with_pages_and_conditions) end - @welsh_translation_input.assign_from_spreadsheet(data).validate + @welsh_translation_input.assign_from_spreadsheet(data).validate(:upload) @table_presenter = Forms::TranslationTablePresenter.new render :new diff --git a/app/input_objects/forms/welsh_translation_input.rb b/app/input_objects/forms/welsh_translation_input.rb index 4e70c37af2..2d5834a270 100644 --- a/app/input_objects/forms/welsh_translation_input.rb +++ b/app/input_objects/forms/welsh_translation_input.rb @@ -1,4 +1,4 @@ -class Forms::WelshTranslationInput < Forms::MarkCompleteInput +class Forms::WelshTranslationInput < BaseInput include TextInputHelper include ActiveModel::Attributes include WelshTranslationContentLabels @@ -17,6 +17,10 @@ class Forms::WelshTranslationInput < Forms::MarkCompleteInput attribute :what_happens_next_markdown_cy attribute :payment_url_cy + with_options except_on: :upload do + validates :mark_complete, presence: true + end + validates :name_cy, presence: true, if: -> { marked_complete? } validates :name_cy, length: { maximum: 500 }, if: -> { name_cy.present? } @@ -147,6 +151,10 @@ def assign_from_spreadsheet(data) self end + def marked_complete? + ["true", true].include?(mark_complete) + end + def blanked? all_fields_empty? && page_translations.all?(&:blanked?) end diff --git a/app/input_objects/forms/welsh_translation_input2.rb b/app/input_objects/forms/welsh_translation_input2.rb index 6a7281e477..6be694438b 100644 --- a/app/input_objects/forms/welsh_translation_input2.rb +++ b/app/input_objects/forms/welsh_translation_input2.rb @@ -1,4 +1,4 @@ -class Forms::WelshTranslationInput2 < Forms::MarkCompleteInput +class Forms::WelshTranslationInput2 < BaseInput include TextInputHelper include ActiveModel::Attributes include WelshTranslationContentLabels @@ -17,6 +17,10 @@ class Forms::WelshTranslationInput2 < Forms::MarkCompleteInput attribute :what_happens_next_markdown_cy attribute :payment_url_cy + with_options except_on: :upload do + validates :mark_complete, presence: true + end + validates :name_cy, presence: true, if: -> { marked_complete? } validates :name_cy, length: { maximum: 500 }, if: -> { name_cy.present? } @@ -147,6 +151,10 @@ def assign_from_spreadsheet(data) self end + def marked_complete? + ["true", true].include?(mark_complete) + end + def blanked? all_fields_empty? && page_translations.all?(&:blanked?) end diff --git a/spec/input_objects/forms/welsh_translation_input_spec.rb b/spec/input_objects/forms/welsh_translation_input_spec.rb index ff61b523fe..427a7816d9 100644 --- a/spec/input_objects/forms/welsh_translation_input_spec.rb +++ b/spec/input_objects/forms/welsh_translation_input_spec.rb @@ -89,6 +89,13 @@ def build_empty_welsh_form expect(welsh_translation_input.errors.full_messages_for(:mark_complete)).to include "Mark complete #{I18n.t('activemodel.errors.models.forms/welsh_translation_input.attributes.mark_complete.blank')}" end + it "is valid if mark complete is blank and the scope is :upload" do + form = OpenStruct.new(welsh_completed: false, name: "Apply for a juggling licence") + welsh_translation_input = described_class.new(mark_complete: nil, form:) + + expect(welsh_translation_input.valid?(:upload)).to be true + end + context "when the form is marked complete" do let(:mark_complete) { "true" } From 8928c0825344cde65d6e717198dc99ca6b851f10 Mon Sep 17 00:00:00 2001 From: Stephen Daly Date: Fri, 4 Sep 2026 12:15:23 +0100 Subject: [PATCH 7/9] Show a success banner when the CSV is uploaded Show a success banner when rending the translations page after the CSV of Welsh translations has been uploaded. --- app/controllers/forms/welsh_translation_controller.rb | 1 + config/locales/en.yml | 1 + spec/requests/forms/welsh_translation_controller_spec.rb | 2 ++ 3 files changed, 4 insertions(+) diff --git a/app/controllers/forms/welsh_translation_controller.rb b/app/controllers/forms/welsh_translation_controller.rb index 864e04428c..8933ac694b 100644 --- a/app/controllers/forms/welsh_translation_controller.rb +++ b/app/controllers/forms/welsh_translation_controller.rb @@ -106,6 +106,7 @@ def upload @welsh_translation_input.assign_from_spreadsheet(data).validate(:upload) @table_presenter = Forms::TranslationTablePresenter.new + flash.now[:success] = t("banner.success.form.welsh_translation_uploaded") render :new end diff --git a/config/locales/en.yml b/config/locales/en.yml index 9598e6b04b..1a3853a53b 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -311,6 +311,7 @@ en: support_details_saved: Your contact details for support have been saved welsh_translation_saved: The Welsh version of your form has been saved welsh_translation_saved_and_completed: The Welsh version of your form has been saved and marked as complete + welsh_translation_uploaded: Your CSV has been uploaded what_happens_next_saved: Your information about what happens next has been saved page_routes_deleted: Question %{question_number}’s routes have been deleted route_created: Route %{route_number} has been added diff --git a/spec/requests/forms/welsh_translation_controller_spec.rb b/spec/requests/forms/welsh_translation_controller_spec.rb index 86594c1df6..d796c88ec3 100644 --- a/spec/requests/forms/welsh_translation_controller_spec.rb +++ b/spec/requests/forms/welsh_translation_controller_spec.rb @@ -486,6 +486,7 @@ it "renders the new template" do expect(response).to have_http_status(:ok) expect(response).to render_template(:new) + expect(response.body).to include(I18n.t("banner.success.form.welsh_translation_uploaded")) end it "pre-populates fields from CSV" do @@ -531,6 +532,7 @@ it "renders the new template" do expect(response).to have_http_status(:ok) expect(response).to render_template(:new) + expect(response.body).to include(I18n.t("banner.success.form.welsh_translation_uploaded")) end it "pre-populates fields from CSV" do From 4d548e348db13bb35594fe9a0eef5e8968b2fe37 Mon Sep 17 00:00:00 2001 From: Stephen Daly Date: Fri, 4 Sep 2026 17:04:40 +0100 Subject: [PATCH 8/9] Add UTF-8 BOM to translations CSV download Add the BOM byte sequence to indicate UTF-8 encoding to the translations CSV download so that Excel will recognise the file as UTF-8 and preserve special characters if the file is directly opened in Excel. Without this, special characters such as curly quotes and Welsh diacritics are replaced by other characters when the file is opened in Excel. --- app/controllers/forms/welsh_translation_controller.rb | 5 +++-- app/services/welsh_csv_service.rb | 7 ++++++- spec/services/welsh_csv_service_spec.rb | 6 +++--- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/app/controllers/forms/welsh_translation_controller.rb b/app/controllers/forms/welsh_translation_controller.rb index 8933ac694b..43f1bc15b9 100644 --- a/app/controllers/forms/welsh_translation_controller.rb +++ b/app/controllers/forms/welsh_translation_controller.rb @@ -77,8 +77,9 @@ def download form_content_service = WelshCsvService.new(form_with_pages_and_conditions) send_data form_content_service.as_csv, - type: "text/csv; charset=iso-8859-1", - disposition: "attachment; filename=#{form_content_service.filename}" + type: "text/csv; charset=utf-8", + filename: form_content_service.filename, + disposition: "attachment" end def show_upload diff --git a/app/services/welsh_csv_service.rb b/app/services/welsh_csv_service.rb index cd436a1f27..bb0822b2af 100644 --- a/app/services/welsh_csv_service.rb +++ b/app/services/welsh_csv_service.rb @@ -7,6 +7,8 @@ class WelshCsvService ENGLISH_CONTENT_HEADER = "English content".freeze WELSH_CONTENT_HEADER = "Welsh content".freeze + UTF_8_BOM = "\uFEFF".freeze + attr_reader :form def initialize(form) @@ -14,12 +16,15 @@ def initialize(form) end def as_csv - CSV.generate do |csv| + csv = CSV.generate do |csv| add_header(csv) add_form_name(csv) add_page_content(csv) add_form_metadata(csv) end + + # Prepend UTF-8 BOM so Excel recognises the file as UTF-8 and preserves special characters + "#{UTF_8_BOM}#{csv}" end def filename diff --git a/spec/services/welsh_csv_service_spec.rb b/spec/services/welsh_csv_service_spec.rb index cfa999f685..c3ae445597 100644 --- a/spec/services/welsh_csv_service_spec.rb +++ b/spec/services/welsh_csv_service_spec.rb @@ -4,8 +4,8 @@ describe "#as_csv" do let(:form) { build :form } - it "contains the header row" do - expect(csv_rows(form)[0]).to eq(["Content ID", "English content", "Welsh content"]) + it "contains the header row, including the UTF-8 BOM" do + expect(csv_rows(form)[0]).to eq(["\uFEFFContent ID", "English content", "Welsh content"]) end it "contains the form name" do @@ -343,7 +343,7 @@ it "returns a CSV with a header row and the expected rows" do csv = csv_rows(form) - expected_csv = [["Content ID", "English content", "Welsh content"], + expected_csv = [["\uFEFFContent ID", "English content", "Welsh content"], ["Form name", "A form", "Welsh A form"], ["Question 1 - question text", "None of the above question?", "Welsh None of the above question?"], ["Question 1 - option 1", "Option 1", "Option 1"], From ea46ba39ebad502fd923936372c6526cfb7ac24d Mon Sep 17 00:00:00 2001 From: Stephen Daly Date: Fri, 4 Sep 2026 17:16:15 +0100 Subject: [PATCH 9/9] Handle uploaded CSV having a UTF-8 BOM When Excel saves a CSV file with UTF-8 encoding, it adds a BOM to the start of the file. Delete this when reading the file so we can successfully validate the headers with or without the BOM being present. --- app/services/welsh_csv_import_service.rb | 1 + spec/services/welsh_csv_import_service_spec.rb | 12 +++++++++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/app/services/welsh_csv_import_service.rb b/app/services/welsh_csv_import_service.rb index 50a0c9f3e9..b7d75d854a 100644 --- a/app/services/welsh_csv_import_service.rb +++ b/app/services/welsh_csv_import_service.rb @@ -18,6 +18,7 @@ def initialize(file, form) def read file_content = file.read.force_encoding("UTF-8") + file_content.delete_prefix!("\xEF\xBB\xBF") # delete UTF-8 BOM if present csv = CSV.parse(file_content, headers: true) raise InvalidHeadersError unless headers_valid?(csv) diff --git a/spec/services/welsh_csv_import_service_spec.rb b/spec/services/welsh_csv_import_service_spec.rb index 8e3c506d20..7594a8c2ad 100644 --- a/spec/services/welsh_csv_import_service_spec.rb +++ b/spec/services/welsh_csv_import_service_spec.rb @@ -35,9 +35,11 @@ context "when the CSV is valid" do context "when the rows in the CSV match the current form" do + let(:bom) { "" } + before do rows = [ - ["Content ID", "English content", "Welsh content"], + ["#{bom}Content ID", "English content", "Welsh content"], ["Form name", "A form", "Welsh A form ôÂŵéï"], ["Question 1 - question text", "None of the above question?", "Welsh None of the above question?"], ["Question 1 - option 1", "Option 1", "Welsh Option 1"], @@ -83,6 +85,14 @@ "Contact details for support - online contact link text" => "Welsh Support URL text", }) end + + context "when the CSV has a BOM" do + let(:bom) { "\uFEFF" } + + it "returns the translations data" do + expect(service.read).to include("Form name" => "Welsh A form ôÂŵéï") + end + end end end