From 9c19c3799c0d3bf7408ccb4f6ab9a4c2bed3cddd Mon Sep 17 00:00:00 2001 From: Stephen Daly Date: Tue, 30 Jun 2026 10:33:21 +0100 Subject: [PATCH 1/3] SPIKE: Add support for uploading translations via CSV Allow uploading a CSV file in the same format as the existing CSV download for the form content. Add an identifier to each CSV row so we can identify which translation to update. When the CSV is uploaded, show the add/edit translations page with the translations filled from the CSV file. The translations will not be saved until the user submits this page. This code was mainly generated by Claude code for the purpose of spiking this, so might not all be the best way to do it. --- .../forms/welsh_translation_controller.rb | 26 ++++ .../welsh_condition_translation_input.rb | 12 +- .../forms/welsh_page_translation_input.rb | 21 ++- ...elsh_selection_option_translation_input.rb | 8 + .../forms/welsh_translation_input.rb | 13 ++ .../forms/welsh_translation_upload_input.rb | 15 ++ app/lib/welsh_translation_content_id.rb | 9 ++ app/services/welsh_csv_import_service.rb | 36 +++++ app/services/welsh_csv_service.rb | 43 +++--- .../forms/welsh_translation/new.html.erb | 1 + .../welsh_translation/show_upload.html.erb | 21 +++ config/locales/en.yml | 4 + .../welsh_translation_upload.yml | 14 ++ config/routes.rb | 2 + .../welsh_condition_translation_input_spec.rb | 4 +- .../welsh_translation_controller_spec.rb | 62 +++++++- .../services/welsh_csv_import_service_spec.rb | 141 ++++++++++++++++++ spec/services/welsh_csv_service_spec.rb | 66 ++++---- .../welsh_translation/new.html.erb_spec.rb | 16 +- 19 files changed, 454 insertions(+), 60 deletions(-) create mode 100644 app/input_objects/forms/welsh_translation_upload_input.rb create mode 100644 app/lib/welsh_translation_content_id.rb create mode 100644 app/services/welsh_csv_import_service.rb create mode 100644 app/views/forms/welsh_translation/show_upload.html.erb create mode 100644 config/locales/input_objects/welsh_translation_upload.yml create mode 100644 spec/services/welsh_csv_import_service_spec.rb diff --git a/app/controllers/forms/welsh_translation_controller.rb b/app/controllers/forms/welsh_translation_controller.rb index f85d152e3c..a95118e8ff 100644 --- a/app/controllers/forms/welsh_translation_controller.rb +++ b/app/controllers/forms/welsh_translation_controller.rb @@ -70,6 +70,28 @@ def download disposition: "attachment; filename=#{form_content_service.filename}" end + def show_upload + authorize current_form, :can_edit_form? + render :show_upload, locals: { current_form:, welsh_translation_upload_input: WelshTranslationUploadInput.new } + end + + def upload + authorize current_form, :can_edit_form? + + welsh_translation_upload_input = WelshTranslationUploadInput.new(**welsh_translation_upload_params) + + csv_data = welsh_translation_upload_input.read_file + unless csv_data + return render :show_upload, status: :unprocessable_entity, locals: { current_form:, welsh_translation_upload_input: } + end + + @welsh_translation_input = WelshTranslationInput.new(form: form_with_pages_and_conditions).assign_form_values + @welsh_translation_input.assign_from_csv_values(csv_data) + @table_presenter = Forms::TranslationTablePresenter.new + + render :new + end + private def preview_html @@ -96,5 +118,9 @@ def delete_welsh_translation_params def form_with_pages_and_conditions Form.includes(pages: [:routing_conditions]).find(current_form.id) end + + def welsh_translation_upload_params + params.require(:forms_welsh_translation_upload_input).permit(:file) + end end end diff --git a/app/input_objects/forms/welsh_condition_translation_input.rb b/app/input_objects/forms/welsh_condition_translation_input.rb index c7459b29c8..2755b94b3f 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 WelshTranslationContentId attr_accessor :condition @@ -39,8 +40,17 @@ def assign_condition_values self end + def assign_from_csv_values(csv_values) + %i[exit_page_heading exit_page_markdown].each do |attr| + csv_id = condition_content_id(condition.id, attr) + send(:"#{attr}_cy=", csv_values[csv_id]) if csv_values.key?(csv_id) + end + + self + end + def form_field_id(attribute) - field_id(:forms_welsh_condition_translation_input, condition.id, :condition_translations, attribute) + condition_content_id(condition.id, attribute) end def condition_has_exit_page? diff --git a/app/input_objects/forms/welsh_page_translation_input.rb b/app/input_objects/forms/welsh_page_translation_input.rb index 20d90a655b..b45c4d3799 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 WelshTranslationContentId attr_accessor :condition_translations, :selection_options_cy attr_reader :page @@ -82,6 +83,24 @@ def assign_page_values self end + def assign_from_csv_values(csv_values) + %i[question_text hint_text page_heading guidance_markdown none_of_the_above_question].each do |attr| + csv_id = page_content_id(page.id, attr) + send(:"#{attr}_cy=", csv_values[csv_id]) if csv_values.key?(csv_id) + end + + selection_options_cy&.each_with_index do |option, index| + csv_id = page_content_id(page.id, "option_#{index}") + option.name_cy = csv_values[csv_id] if csv_values.key?(csv_id) + end + + condition_translations.each do |condition_translation| + condition_translation.assign_from_csv_values(csv_values) + end + + self + end + def condition_translations_attributes=(attributes) submitted_condition_ids = attributes.values.map { |attrs| attrs["id"] }.compact @@ -112,7 +131,7 @@ def selection_options_cy_attributes=(attributes) end def form_field_id(attribute) - field_id(:forms_welsh_page_translation_input, page.id, :page_translations, attribute) + page_content_id(page.id, attribute) end def page_has_hint_text? 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..59ddfadfbc 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 + + csv_id = page_content_id(page.id, "option_#{index}") + self.name_cy = csv_values[csv_id] if csv_values.key?(csv_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 22b6e4d9f1..bd523ce14b 100644 --- a/app/input_objects/forms/welsh_translation_input.rb +++ b/app/input_objects/forms/welsh_translation_input.rb @@ -122,6 +122,19 @@ def assign_form_values self end + def assign_from_csv_values(csv_values) + WelshCsvImportService::FORM_FIELD_IDS.each do |field_id| + attr = :"#{field_id}_cy" + send(:"#{attr}=", csv_values[field_id]) if csv_values.key?(field_id) + end + + page_translations.each do |page_translation| + page_translation.assign_from_csv_values(csv_values) + end + + self + end + def blanked? all_fields_empty? && page_translations.all?(&:blanked?) end 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..528ac59c83 --- /dev/null +++ b/app/input_objects/forms/welsh_translation_upload_input.rb @@ -0,0 +1,15 @@ +class Forms::WelshTranslationUploadInput < BaseInput + attr_accessor :file + + validates :file, presence: true + + def read_file + return false if invalid? + + csv_data = file.read.force_encoding("UTF-8") + WelshCsvImportService.new(csv_data).read + rescue CSV::MalformedCSVError, WelshCsvImportService::InvalidHeadersError + errors.add(:file, :invalid_csv) + false + end +end diff --git a/app/lib/welsh_translation_content_id.rb b/app/lib/welsh_translation_content_id.rb new file mode 100644 index 0000000000..df53c79383 --- /dev/null +++ b/app/lib/welsh_translation_content_id.rb @@ -0,0 +1,9 @@ +module WelshTranslationContentId + def page_content_id(page_id, attribute) + "page_#{page_id}_#{attribute}" + end + + def condition_content_id(condition_id, attribute) + "condition_#{condition_id}_#{attribute}" + end +end diff --git a/app/services/welsh_csv_import_service.rb b/app/services/welsh_csv_import_service.rb new file mode 100644 index 0000000000..672a569c09 --- /dev/null +++ b/app/services/welsh_csv_import_service.rb @@ -0,0 +1,36 @@ +class WelshCsvImportService + class InvalidHeadersError < StandardError; end + + FORM_FIELD_IDS = %w[ + name + declaration_markdown + what_happens_next_markdown + payment_url + privacy_policy_url + support_email + support_phone + support_url + support_url_text + ].freeze + + ID_COLUMN = "Identifier (do not change)".freeze + WELSH_CONTENT_COLUMN = "Welsh content".freeze + + attr_reader :csv_data + + def initialize(csv_data) + @csv_data = csv_data + end + + def read + rows = CSV.parse(csv_data, headers: true) + raise InvalidHeadersError unless rows.headers.include?(ID_COLUMN) && rows.headers.include?(WELSH_CONTENT_COLUMN) + + rows.each_with_object({}) do |row, values| + id = row[ID_COLUMN] + next if id.nil? + + values[id] = row[WELSH_CONTENT_COLUMN].to_s + end + end +end diff --git a/app/services/welsh_csv_service.rb b/app/services/welsh_csv_service.rb index 73032eec9e..64d86c9f39 100644 --- a/app/services/welsh_csv_service.rb +++ b/app/services/welsh_csv_service.rb @@ -1,4 +1,6 @@ class WelshCsvService + include WelshTranslationContentId + MAX_FILENAME_LENGTH = 80 FILENAME_SEPARATOR = "_".freeze @@ -29,11 +31,11 @@ def filename private def add_header(csv) - csv << ["", "English content", "Welsh content"] + csv << [WelshCsvImportService::ID_COLUMN, "Content", "English content", "Welsh content"] end def add_form_name(csv) - csv << ["Form name", form.name, form.name_cy] + csv << ["name", "Form name", form.name, form.name_cy] end def add_page_content(csv) @@ -48,16 +50,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_content_id(page.id, :question_text), "#{question_name(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_content_id(page.id, :hint_text), "#{question_name(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 << [page_content_id(page.id, "option_#{index}"), "#{question_name(page)} - option #{index + 1}", option.name, welsh_option_name] end end @@ -66,7 +68,8 @@ 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_content_id(page.id, :none_of_the_above_question), + "#{question_name(page)} - question or label if 'None of the above' is selected", english_question, welsh_question, ] @@ -80,21 +83,21 @@ 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_content_id(condition.id, :exit_page_heading), "#{question_name(page)} - exit page heading", condition.exit_page_heading, condition.exit_page_heading_cy] + csv << [condition_content_id(condition.id, :exit_page_markdown), "#{question_name(page)} - exit page content", condition.exit_page_markdown, condition.exit_page_markdown_cy] end 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_content_id(page.id, :page_heading), "#{question_name(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_content_id(page.id, :guidance_markdown), "#{question_name(page)} - guidance text", page.guidance_markdown, page.guidance_markdown_cy] end end @@ -103,24 +106,24 @@ 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, "declaration_markdown", "Declaration", form.declaration_text, form.declaration_text_cy) + add_field_if_present(csv, "what_happens_next_markdown", "Information about what happens next", form.what_happens_next_markdown, form.what_happens_next_markdown_cy) + add_field_if_present(csv, "payment_url", "GOV\u2060.\u2060UK Pay payment link", form.payment_url, form.payment_url_cy) + add_field_if_present(csv, "privacy_policy_url", "Link to privacy information for this form", 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, "support_email", "Contact details for support - email address", form.support_email, form.support_email_cy) + add_field_if_present(csv, "support_phone", "Contact details for support - phone number and opening times", form.support_phone, form.support_phone_cy) + add_field_if_present(csv, "support_url", "Contact details for support - online contact link", form.support_url, form.support_url_cy) + add_field_if_present(csv, "support_url_text", "Contact details for support - online contact link text", form.support_url_text, form.support_url_text_cy) end - def add_field_if_present(csv, label, english_value, welsh_value) + def add_field_if_present(csv, id, label, english_value, welsh_value) if english_value.present? - csv << [label, english_value, welsh_value || ""] + csv << [id, label, english_value, welsh_value || ""] end end end diff --git a/app/views/forms/welsh_translation/new.html.erb b/app/views/forms/welsh_translation/new.html.erb index c4e72dfd1a..57601d4e29 100644 --- a/app/views/forms/welsh_translation/new.html.erb +++ b/app/views/forms/welsh_translation/new.html.erb @@ -15,6 +15,7 @@
<%= govuk_button_link_to t(".csv_download"), welsh_translation_download_path(@welsh_translation_input.form, format: :csv), secondary: true, data: { "track-link": true } %> + <%= govuk_link_to t(".csv_upload"), welsh_translation_show_upload_path(@welsh_translation_input.form), secondary: true %> <%= render PreviewLinkComponent::View.new(@welsh_translation_input.form.pages, preview_link(@welsh_translation_input.form, locale: :cy), t(".preview_link_text")) %>
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..0c18140a7d --- /dev/null +++ b/app/views/forms/welsh_translation/show_upload.html.erb @@ -0,0 +1,21 @@ +<% set_page_title(t(".title")) %> +<% 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 %> + + <%= f.govuk_file_field :file, + label: { tag: 'h1', size: 'l' }, + accept: "text/csv", + javascript: true + %> + + <%= f.govuk_submit t(".submit") %> + <% end %> +
+
+ diff --git a/config/locales/en.yml b/config/locales/en.yml index 680812b282..4fa6763384 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -611,6 +611,7 @@ en: condition: heading: Question %{question_number}’s exit page csv_download: Download content as CSV + csv_upload: Upload translations delete_welsh_version: Delete Welsh version english_header: English content exit_page_heading: Exit page heading @@ -643,6 +644,9 @@ en: support_url: Online contact link support_url_text: Text to describe the contact link welsh_header: Welsh content + show_upload: + submit: Upload translations + title: Upload Welsh translations from CSV 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" 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..010cbb708f --- /dev/null +++ b/config/locales/input_objects/welsh_translation_upload.yml @@ -0,0 +1,14 @@ +--- +en: + activemodel: + errors: + models: + forms/welsh_translation_upload_input: + attributes: + file: + blank: Choose a file + invalid_csv: Invalid CSV file. Download the content as a CSV file, fill in the translations, and upload it again. + helpers: + label: + forms_welsh_translation_upload_input: + file: Upload a CSV file with your Welsh translations diff --git a/config/routes.rb b/config/routes.rb index 9a02f5faaf..6f41427a26 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -78,6 +78,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/input_objects/forms/welsh_condition_translation_input_spec.rb b/spec/input_objects/forms/welsh_condition_translation_input_spec.rb index 3b91a11fad..27bebdc51d 100644 --- a/spec/input_objects/forms/welsh_condition_translation_input_spec.rb +++ b/spec/input_objects/forms/welsh_condition_translation_input_spec.rb @@ -218,8 +218,8 @@ def create_condition(attributes = {}) end it "returns the custom ID for each attribute" do - expect(welsh_condition_translation_input.form_field_id(:exit_page_markdown_cy)).to eq "forms_welsh_condition_translation_input_#{condition.id}_condition_translations_exit_page_markdown_cy" - expect(welsh_condition_translation_input.form_field_id(:exit_page_heading_cy)).to eq "forms_welsh_condition_translation_input_#{condition.id}_condition_translations_exit_page_heading_cy" + expect(welsh_condition_translation_input.form_field_id(:exit_page_markdown_cy)).to eq "condition_#{condition.id}_exit_page_markdown_cy" + expect(welsh_condition_translation_input.form_field_id(:exit_page_heading_cy)).to eq "condition_#{condition.id}_exit_page_heading_cy" end end diff --git a/spec/requests/forms/welsh_translation_controller_spec.rb b/spec/requests/forms/welsh_translation_controller_spec.rb index 8afcec163b..40bb1f8d9f 100644 --- a/spec/requests/forms/welsh_translation_controller_spec.rb +++ b/spec/requests/forms/welsh_translation_controller_spec.rb @@ -293,8 +293,66 @@ 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.second).to eq(["Form name", "A form with Welsh", "Welsh A form with Welsh"]) + expect(csv.first).to eq(["Identifier (do not change)", "Content", "English content", "Welsh content"]) + expect(csv.second).to eq(["name", "Form name", "A form with Welsh", "Welsh A form with Welsh"]) + end + end + + describe "#upload" do + let(:form) { create(:form, :ready_for_routing, welsh_completed: false) } + + let(:csv_data) do + CSV.generate do |csv| + csv << [WelshCsvImportService::ID_COLUMN, "Content", "English content", "Welsh content"] + csv << ["name", "Form name", form.name, "Fy Ffurflen"] + end + end + + 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 + + context "when a valid CSV is uploaded" do + before do + 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 form name from CSV" do + expect(response.body).to include("Fy Ffurflen") + 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 + + 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/services/welsh_csv_import_service_spec.rb b/spec/services/welsh_csv_import_service_spec.rb new file mode 100644 index 0000000000..c9fa92df6f --- /dev/null +++ b/spec/services/welsh_csv_import_service_spec.rb @@ -0,0 +1,141 @@ +require "rails_helper" + +RSpec.describe WelshCsvImportService do + describe "#import" do + subject(:import_values) { described_class.new(csv_data).read } + + context "with a valid CSV containing all form-level fields" do + let(:csv_data) do + CSV.generate do |csv| + csv << ["Identifier (do not change)", "Content", "English content", "Welsh content"] + csv << ["name", "Form name", "My Form", "Fy Ffurflen"] + csv << ["declaration_markdown", "Declaration", "Declaration text", "Welsh declaration"] + csv << ["what_happens_next_markdown", "What happens next", "Next steps", "Welsh next steps"] + csv << ["payment_url", "Payment link", "https://pay.gov.uk/en", "https://pay.gov.uk/cy"] + csv << ["privacy_policy_url", "Privacy policy", "https://privacy.gov.uk", "https://privacy.gov.uk/cy"] + csv << ["support_email", "Support email", "help@example.gov.uk", "help@example.gov.uk"] + csv << ["support_phone", "Support phone", "0800 123 456", "0800 123 456 cy"] + csv << ["support_url", "Support URL", "https://contact.gov.uk", "https://contact.gov.uk/cy"] + csv << ["support_url_text", "Support URL text", "Contact us", "Cysylltu â ni"] + end + end + + it "maps name" do + expect(import_values["name"]).to eq("Fy Ffurflen") + end + + it "maps declaration_markdown" do + expect(import_values["declaration_markdown"]).to eq("Welsh declaration") + end + + it "maps what_happens_next_markdown" do + expect(import_values["what_happens_next_markdown"]).to eq("Welsh next steps") + end + + it "maps all form-level IDs" do + expect(import_values.keys).to include( + "name", + "declaration_markdown", + "what_happens_next_markdown", + "payment_url", + "privacy_policy_url", + "support_email", + "support_phone", + "support_url", + "support_url_text", + ) + end + end + + context "with a valid CSV containing page-level fields" do + let(:page_id) { 42 } + let(:condition_id) { 99 } + let(:csv_data) do + CSV.generate do |csv| + csv << ["Identifier (do not change)", "Content", "English content", "Welsh content"] + csv << ["page_#{page_id}_question_text", "Question 1 - question text", "What is your name?", "Beth yw eich enw?"] + csv << ["page_#{page_id}_hint_text", "Question 1 - hint text", "Enter your full name", "Rhowch eich enw llawn"] + csv << ["page_#{page_id}_page_heading", "Question 1 - page heading", "Page heading", "Pennawd tudalen"] + csv << ["page_#{page_id}_guidance_markdown", "Question 1 - guidance text", "Some guidance", "Rhywfaint o arweiniad"] + csv << ["page_#{page_id}_option_0", "Question 1 - option 1", "Yes", "Ydy"] + csv << ["page_#{page_id}_option_1", "Question 1 - option 2", "No", "Nac ydy"] + csv << ["page_#{page_id}_none_of_the_above_question", "NoneOfAbove", "None of the above?", "Dim un o'r uchod?"] + csv << ["condition_#{condition_id}_exit_page_heading", "Exit heading", "You cannot continue", "Ni allwch barhau"] + csv << ["condition_#{condition_id}_exit_page_markdown", "Exit content", "Sorry", "Mae'n ddrwg"] + end + end + + it "maps page question text" do + expect(import_values["page_#{page_id}_question_text"]).to eq("Beth yw eich enw?") + end + + it "maps page hint text" do + expect(import_values["page_#{page_id}_hint_text"]).to eq("Rhowch eich enw llawn") + end + + it "maps selection option by index" do + expect(import_values["page_#{page_id}_option_0"]).to eq("Ydy") + expect(import_values["page_#{page_id}_option_1"]).to eq("Nac ydy") + end + + it "maps condition exit page fields" do + expect(import_values["condition_#{condition_id}_exit_page_heading"]).to eq("Ni allwch barhau") + expect(import_values["condition_#{condition_id}_exit_page_markdown"]).to eq("Mae'n ddrwg") + end + end + + context "with empty Welsh content" do + let(:csv_data) do + CSV.generate do |csv| + csv << ["Identifier (do not change)", "Content", "English content", "Welsh content"] + csv << ["form_name", "Form name", "My Form", ""] + end + end + + it "returns an empty string for the Welsh value" do + expect(import_values["form_name"]).to eq("") + end + end + + context "with rows missing an identifier" do + let(:csv_data) do + CSV.generate do |csv| + csv << ["Identifier (do not change)", "Content", "English content", "Welsh content"] + csv << [nil, "Some label", "English value", "Welsh value"] + csv << ["name", "Form name", "My Form", "Fy Ffurflen"] + end + end + + it "skips rows without an identifier" do + expect(import_values.keys).not_to include(nil) + expect(import_values["name"]).to eq("Fy Ffurflen") + end + end + + context "when the CSV does not have the identifier column header" do + let(:csv_data) do + CSV.generate do |csv| + csv << ["", "English content", "Welsh content"] + csv << ["Form name", "My Form", "Fy Ffurflen"] + end + end + + it "raises an error" do + expect { import_values }.to raise_error(WelshCsvImportService::InvalidHeadersError) + end + end + + context "when the CSV does not have the Welsh Content column header" do + let(:csv_data) do + CSV.generate do |csv| + csv << [WelshCsvImportService::ID_COLUMN, "English content", ""] + csv << ["Form name", "My Form", "Fy Ffurflen"] + end + end + + it "raises an error" do + expect { import_values }.to raise_error(WelshCsvImportService::InvalidHeadersError) + end + end + end +end diff --git a/spec/services/welsh_csv_service_spec.rb b/spec/services/welsh_csv_service_spec.rb index 17eb6e4473..5a4b769733 100644 --- a/spec/services/welsh_csv_service_spec.rb +++ b/spec/services/welsh_csv_service_spec.rb @@ -5,15 +5,12 @@ 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(["Identifier (do not change)", "Content", "English content", "Welsh content"]) end it "contains the form name" do expect(csv_rows(form)).to include([ + "name", "Form name", form.name, form.name_cy, @@ -25,6 +22,7 @@ it "contains the declaration" do expect(csv_rows(form)).to include([ + "declaration_markdown", "Declaration", "Declaration text", "Welsh Declaration text", @@ -37,6 +35,7 @@ it "contains the what happens next" do expect(csv_rows(form)).to include([ + "what_happens_next_markdown", "Information about what happens next", "What happens next text", "Welsh What happens next text", @@ -49,7 +48,8 @@ it "contains the payment URL" do expect(csv_rows(form)).to include([ - "GOV⁠.⁠UK Pay payment link", + "payment_url", + "GOV\u2060.\u2060UK Pay payment link", "https://www.gov.uk/payment", "https://www.gov.uk/payment_cy", ]) @@ -62,6 +62,7 @@ it "contains the support email" do expect(csv_rows(form)).to include([ + "support_email", "Contact details for support - email address", "support@example.gov.uk", "support@example.gov.uk", @@ -74,6 +75,7 @@ it "contains the support phone" do expect(csv_rows(form)).to include([ + "support_phone", "Contact details for support - phone number and opening times", "English support phone", "Welsh support phone", @@ -86,6 +88,7 @@ it "contains the support URL" do expect(csv_rows(form)).to include([ + "support_url", "Contact details for support - online contact link", "https://www.gov.uk/support", "https://www.gov.uk/support_cy", @@ -94,6 +97,7 @@ it "contains the support URL text" do expect(csv_rows(form)).to include([ + "support_url_text", "Contact details for support - online contact link text", "Text to describe the contact link", "Welsh Text to describe the contact link", @@ -116,6 +120,7 @@ it "contains the question text" do expect(csv_rows(form)).to include([ + "page_#{page.id}_question_text", "Question 1 - question text", "Question text", "Welsh question text", @@ -129,6 +134,7 @@ it "contains the hint text" do expect(csv_rows(form)).to include([ + "page_#{page.id}_hint_text", "Question 1 - hint text", "Hint text", "Welsh hint text", @@ -154,10 +160,12 @@ it "contains the options" do expect(csv_rows(form)).to include([ + "page_#{page.id}_option_0", "Question 1 - option 1", "Yes", "Ydy", ], [ + "page_#{page.id}_option_1", "Question 1 - option 2", "No", "Nac ydy", @@ -177,10 +185,12 @@ it "contains the guidance" do expect(csv_rows(form)).to include([ + "page_#{page.id}_guidance_markdown", "Question 1 - guidance text", "Markdown", "Welsh markdown", ], [ + "page_#{page.id}_page_heading", "Question 1 - page heading", "Page heading", "Welsh page heading", @@ -201,7 +211,8 @@ 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", + "page_#{page.id}_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?", ]) @@ -215,6 +226,7 @@ it "contains the exit page heading" do expect(csv_rows(form)).to include([ + "condition_#{condition.id}_exit_page_heading", "Question 1 - exit page heading", "Exit page heading", "Welsh exit page heading", @@ -261,25 +273,27 @@ it "returns a CSV with a header row and the expected rows" do csv = csv_rows(form) - expected_csv = [["", "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 - 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", ""], - ["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", ""], - ["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"]] + expected_csv = [ + ["Identifier (do not change)", "Content", "English content", "Welsh content"], + ["name", "Form name", "A form", "Welsh A form"], + ["page_#{page.id}_question_text", "Question 1 - question text", "None of the above question?", "Welsh None of the above question?"], + ["page_#{page.id}_option_0", "Question 1 - option 1", "Option 1", "Option 1"], + ["page_#{page.id}_option_1", "Question 1 - option 2", "Option 2", "Option 2"], + ["page_#{page.id}_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?"], + ["condition_#{condition.id}_exit_page_heading", "Question 1 - exit page heading", "Exit page heading", "Welsh exit page heading"], + ["condition_#{condition.id}_exit_page_markdown", "Question 1 - exit page content", "Exit page markdown", "Welsh exit page markdown"], + ["page_#{another_page.id}_page_heading", "Question 2 - page heading", "Page heading", "Welsh Page heading"], + ["page_#{another_page.id}_guidance_markdown", "Question 2 - guidance text", "This is the guidance.", "Welsh This is the guidance."], + ["page_#{another_page.id}_question_text", "Question 2 - question text", "What?", "Welsh What?"], + ["declaration_markdown", "Declaration", "Declaration text", ""], + ["what_happens_next_markdown", "Information about what happens next", "English what happens next", "Welsh what happens next"], + ["payment_url", "GOV\u2060.\u2060UK Pay payment link", "https://www.gov.uk/payment", "https://www.gov.uk/payment_cy"], + ["privacy_policy_url", "Link to privacy information for this form", "https://www.gov.uk/privacy", ""], + ["support_email", "Contact details for support - email address", "support@example.gov.uk", "support@example.gov.uk"], + ["support_phone", "Contact details for support - phone number and opening times", "English support phone", "Welsh support phone"], + ["support_url", "Contact details for support - online contact link", "https://www.gov.uk/support", "https://www.gov.uk/support_cy"], + ["support_url_text", "Contact details for support - online contact link text", "Support URL text", "Welsh Support URL text"], + ] expect(csv).to eq(expected_csv) end end diff --git a/spec/views/forms/welsh_translation/new.html.erb_spec.rb b/spec/views/forms/welsh_translation/new.html.erb_spec.rb index 235a3dea69..8f48d79e7c 100644 --- a/spec/views/forms/welsh_translation/new.html.erb_spec.rb +++ b/spec/views/forms/welsh_translation/new.html.erb_spec.rb @@ -232,7 +232,7 @@ def build_form(attributes = {}) let(:welsh_translation_input) { Forms::WelshTranslationInput.new(form:, page_translations: []).assign_form_values } it "does not render any page translation content" do - expect(rendered).not_to have_field(id: "forms_welsh_page_translation_input_#{page.id}_page_translations_question_text_cy", type: "text") + expect(rendered).not_to have_field(id: "page_#{page.id}_question_text_cy", type: "text") end it "renders message for no pages" do @@ -242,8 +242,8 @@ def build_form(attributes = {}) context "when the form has pages" do it "has a field for each page's Welsh question text" do - expect(rendered).to have_field("Enter Welsh question text for question #{page.position}", type: "text", id: "forms_welsh_page_translation_input_#{page.id}_page_translations_question_text_cy") - expect(rendered).to have_field("Enter Welsh question text for question #{another_page.position}", type: "text", id: "forms_welsh_page_translation_input_#{another_page.id}_page_translations_question_text_cy") + expect(rendered).to have_field("Enter Welsh question text for question #{page.position}", type: "text", id: "page_#{page.id}_question_text_cy") + expect(rendered).to have_field("Enter Welsh question text for question #{another_page.position}", type: "text", id: "page_#{another_page.id}_question_text_cy") end context "when a page has hint text" do @@ -252,7 +252,7 @@ def build_form(attributes = {}) it "shows the English text and Welsh field for pages with English hint text" do expect(rendered).to have_css("td", text: page.hint_text) - expect(rendered).to have_field("Enter Welsh hint text for question #{page.position}", type: "textarea", id: "forms_welsh_page_translation_input_#{page.id}_page_translations_hint_text_cy") + expect(rendered).to have_field("Enter Welsh hint text for question #{page.position}", type: "textarea", id: "page_#{page.id}_hint_text_cy") end it "does not show the Welsh field for pages without English hint text" do @@ -266,9 +266,9 @@ def build_form(attributes = {}) it "shows the English text and Welsh fields for pages with English page heading and guidance markdown" do expect(rendered).to have_css("td", text: another_page.page_heading) - expect(rendered).to have_field("Enter Welsh page heading for question #{another_page.position}", id: "forms_welsh_page_translation_input_#{another_page.id}_page_translations_page_heading_cy") + expect(rendered).to have_field("Enter Welsh page heading for question #{another_page.position}", id: "page_#{another_page.id}_page_heading_cy") expect(rendered).to have_css("td", text: another_page.guidance_markdown) - expect(rendered).to have_field("Enter Welsh guidance text for question #{another_page.position}", id: "forms_welsh_page_translation_input_#{another_page.id}_page_translations_guidance_markdown_cy") + expect(rendered).to have_field("Enter Welsh guidance text for question #{another_page.position}", id: "page_#{another_page.id}_guidance_markdown_cy") end it "does not show the Welsh field for pages without English page heading and guidance markdown" do @@ -368,7 +368,7 @@ def build_form(attributes = {}) it "links the error summary to the invalid field" do error_message = I18n.t("activemodel.errors.models.forms/welsh_page_translation_input.attributes.question_text_cy.blank", question_number: page.position) - expect(rendered).to have_link(error_message, href: "#forms_welsh_page_translation_input_#{page.id}_page_translations_question_text_cy") + expect(rendered).to have_link(error_message, href: "#page_#{page.id}_question_text_cy") end it "adds an inline error message to the invalid field" do @@ -396,7 +396,7 @@ def build_form(attributes = {}) it "links the error summary to the invalid field" do error_message = I18n.t("activemodel.errors.models.forms/welsh_condition_translation_input.attributes.exit_page_heading_cy.blank", question_number: page.position) - expect(rendered).to have_link(error_message, href: "#forms_welsh_condition_translation_input_#{condition.id}_condition_translations_exit_page_heading_cy") + expect(rendered).to have_link(error_message, href: "#condition_#{condition.id}_exit_page_heading_cy") end it "adds an inline error message to the invalid field" do From 7b18f4809f209a24bb6631ef4561b0810cd1b7f4 Mon Sep 17 00:00:00 2001 From: Stephen Daly Date: Wed, 1 Jul 2026 12:16:08 +0100 Subject: [PATCH 2/3] Support XLSX and ODS files --- Gemfile | 3 + Gemfile.lock | 8 + .../forms/welsh_translation_controller.rb | 6 +- .../welsh_condition_translation_input.rb | 6 +- .../forms/welsh_page_translation_input.rb | 12 +- ...elsh_selection_option_translation_input.rb | 4 +- .../forms/welsh_translation_input.rb | 8 +- .../forms/welsh_translation_upload_input.rb | 18 ++- app/services/welsh_csv_import_service.rb | 36 ----- app/services/welsh_csv_service.rb | 2 +- .../welsh_spreadsheet_import_service.rb | 77 ++++++++++ .../welsh_translation/show_upload.html.erb | 2 +- .../welsh_translation_upload.yml | 8 +- .../welsh_translation_controller_spec.rb | 2 +- .../services/welsh_csv_import_service_spec.rb | 141 ------------------ 15 files changed, 130 insertions(+), 203 deletions(-) delete mode 100644 app/services/welsh_csv_import_service.rb create mode 100644 app/services/welsh_spreadsheet_import_service.rb delete mode 100644 spec/services/welsh_csv_import_service_spec.rb diff --git a/Gemfile b/Gemfile index 6c5f041fb4..37bef5e0c9 100644 --- a/Gemfile +++ b/Gemfile @@ -96,6 +96,9 @@ gem "uri-idna" # For converting HTML to Markdown gem "reverse_markdown" +# For reading translations from a spreadsheet +gem "roo" + group :development, :test do # See https://guides.rubyonrails.org/debugging_rails_applications.html#debugging-with-the-debug-gem gem "debug", platforms: %i[mri windows], require: "debug/prelude" diff --git a/Gemfile.lock b/Gemfile.lock index 0bff8782c3..84d44a1d2b 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -653,6 +653,12 @@ GEM reverse_markdown (3.0.2) nokogiri rexml (3.4.4) + roo (3.0.0) + base64 (~> 0.2) + csv (~> 3) + logger (~> 1) + nokogiri (~> 1) + rubyzip (>= 3.0.0, < 4.0.0) rspec (3.13.2) rspec-core (~> 3.13.0) rspec-expectations (~> 3.13.0) @@ -851,6 +857,7 @@ DEPENDENCIES rails (~> 8.1.3) rails-controller-testing reverse_markdown + roo rspec-rails (>= 3.9.0) rubocop-govuk selenium-webdriver @@ -1091,6 +1098,7 @@ CHECKSUMS request_store (1.7.0) sha256=e1b75d5346a315f452242a68c937ef8e48b215b9453a77a6c0acdca2934c88cb reverse_markdown (3.0.2) sha256=818ebb92ce39dbb1a291690dd1ec9a6d62530d4725296b17e9c8f668f9a5b8af rexml (3.4.4) sha256=19e0a2c3425dfbf2d4fc1189747bdb2f849b6c5e74180401b15734bc97b5d142 + roo (3.0.0) sha256=6fdd7a9158d657c69768b4168754ff2110cc21fdc01a1bec1010820cb05c91b1 rspec (3.13.2) sha256=206284a08ad798e61f86d7ca3e376718d52c0bc944626b2349266f239f820587 rspec-core (3.13.6) sha256=a8823c6411667b60a8bca135364351dda34cd55e44ff94c4be4633b37d828b2d rspec-expectations (3.13.5) sha256=33a4d3a1d95060aea4c94e9f237030a8f9eae5615e9bd85718fe3a09e4b58836 diff --git a/app/controllers/forms/welsh_translation_controller.rb b/app/controllers/forms/welsh_translation_controller.rb index a95118e8ff..1d17ee3006 100644 --- a/app/controllers/forms/welsh_translation_controller.rb +++ b/app/controllers/forms/welsh_translation_controller.rb @@ -80,13 +80,13 @@ def upload welsh_translation_upload_input = WelshTranslationUploadInput.new(**welsh_translation_upload_params) - csv_data = welsh_translation_upload_input.read_file - unless csv_data + 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 = WelshTranslationInput.new(form: form_with_pages_and_conditions).assign_form_values - @welsh_translation_input.assign_from_csv_values(csv_data) + @welsh_translation_input.assign_from_spreadsheet(data) @table_presenter = Forms::TranslationTablePresenter.new render :new diff --git a/app/input_objects/forms/welsh_condition_translation_input.rb b/app/input_objects/forms/welsh_condition_translation_input.rb index 2755b94b3f..8b4fe3e0b1 100644 --- a/app/input_objects/forms/welsh_condition_translation_input.rb +++ b/app/input_objects/forms/welsh_condition_translation_input.rb @@ -40,10 +40,10 @@ def assign_condition_values self end - def assign_from_csv_values(csv_values) + def assign_from_spreadsheet(data) %i[exit_page_heading exit_page_markdown].each do |attr| - csv_id = condition_content_id(condition.id, attr) - send(:"#{attr}_cy=", csv_values[csv_id]) if csv_values.key?(csv_id) + spreadsheet_id = condition_content_id(condition.id, attr) + send(:"#{attr}_cy=", data[spreadsheet_id]) if data.key?(spreadsheet_id) end self diff --git a/app/input_objects/forms/welsh_page_translation_input.rb b/app/input_objects/forms/welsh_page_translation_input.rb index b45c4d3799..25fd806dc1 100644 --- a/app/input_objects/forms/welsh_page_translation_input.rb +++ b/app/input_objects/forms/welsh_page_translation_input.rb @@ -83,19 +83,19 @@ def assign_page_values self end - def assign_from_csv_values(csv_values) + def assign_from_spreadsheet(data) %i[question_text hint_text page_heading guidance_markdown none_of_the_above_question].each do |attr| - csv_id = page_content_id(page.id, attr) - send(:"#{attr}_cy=", csv_values[csv_id]) if csv_values.key?(csv_id) + spreadsheet_id = page_content_id(page.id, attr) + send(:"#{attr}_cy=", data[spreadsheet_id]) if data.key?(spreadsheet_id) end selection_options_cy&.each_with_index do |option, index| - csv_id = page_content_id(page.id, "option_#{index}") - option.name_cy = csv_values[csv_id] if csv_values.key?(csv_id) + spreadsheet_id = page_content_id(page.id, "option_#{index}") + option.name_cy = data[spreadsheet_id] if data.key?(spreadsheet_id) end condition_translations.each do |condition_translation| - condition_translation.assign_from_csv_values(csv_values) + condition_translation.assign_from_spreadsheet(data) end self 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 59ddfadfbc..00c9bad28f 100644 --- a/app/input_objects/forms/welsh_selection_option_translation_input.rb +++ b/app/input_objects/forms/welsh_selection_option_translation_input.rb @@ -29,8 +29,8 @@ def assign_selection_option_values def assign_selection_option_values_from_csv_values(csv_values) return self unless selection_option - csv_id = page_content_id(page.id, "option_#{index}") - self.name_cy = csv_values[csv_id] if csv_values.key?(csv_id) + spreadsheet_id = page_content_id(page.id, "option_#{index}") + self.name_cy = csv_values[spreadsheet_id] if csv_values.key?(spreadsheet_id) self end diff --git a/app/input_objects/forms/welsh_translation_input.rb b/app/input_objects/forms/welsh_translation_input.rb index bd523ce14b..7c3b2efa9c 100644 --- a/app/input_objects/forms/welsh_translation_input.rb +++ b/app/input_objects/forms/welsh_translation_input.rb @@ -122,14 +122,14 @@ def assign_form_values self end - def assign_from_csv_values(csv_values) - WelshCsvImportService::FORM_FIELD_IDS.each do |field_id| + def assign_from_spreadsheet(data) + WelshSpreadsheetImportService::FORM_FIELD_IDS.each do |field_id| attr = :"#{field_id}_cy" - send(:"#{attr}=", csv_values[field_id]) if csv_values.key?(field_id) + send(:"#{attr}=", data[field_id]) if data.key?(field_id) end page_translations.each do |page_translation| - page_translation.assign_from_csv_values(csv_values) + page_translation.assign_from_spreadsheet(data) end self diff --git a/app/input_objects/forms/welsh_translation_upload_input.rb b/app/input_objects/forms/welsh_translation_upload_input.rb index 528ac59c83..6a8338f1e7 100644 --- a/app/input_objects/forms/welsh_translation_upload_input.rb +++ b/app/input_objects/forms/welsh_translation_upload_input.rb @@ -2,14 +2,26 @@ class Forms::WelshTranslationUploadInput < BaseInput attr_accessor :file validates :file, presence: true + validate :validate_file_type + + FILE_TYPES = %w[ + text/csv + application/vnd.openxmlformats-officedocument.spreadsheetml.sheet + application/vnd.oasis.opendocument.spreadsheet + ].freeze def read_file return false if invalid? - csv_data = file.read.force_encoding("UTF-8") - WelshCsvImportService.new(csv_data).read - rescue CSV::MalformedCSVError, WelshCsvImportService::InvalidHeadersError + WelshSpreadsheetImportService.new(file).read + rescue CSV::MalformedCSVError, WelshSpreadsheetImportService::InvalidHeadersError errors.add(:file, :invalid_csv) false end + + def validate_file_type + if file.present? && FILE_TYPES.exclude?(file.content_type) + errors.add(:file, :disallowed_type) + end + end end diff --git a/app/services/welsh_csv_import_service.rb b/app/services/welsh_csv_import_service.rb deleted file mode 100644 index 672a569c09..0000000000 --- a/app/services/welsh_csv_import_service.rb +++ /dev/null @@ -1,36 +0,0 @@ -class WelshCsvImportService - class InvalidHeadersError < StandardError; end - - FORM_FIELD_IDS = %w[ - name - declaration_markdown - what_happens_next_markdown - payment_url - privacy_policy_url - support_email - support_phone - support_url - support_url_text - ].freeze - - ID_COLUMN = "Identifier (do not change)".freeze - WELSH_CONTENT_COLUMN = "Welsh content".freeze - - attr_reader :csv_data - - def initialize(csv_data) - @csv_data = csv_data - end - - def read - rows = CSV.parse(csv_data, headers: true) - raise InvalidHeadersError unless rows.headers.include?(ID_COLUMN) && rows.headers.include?(WELSH_CONTENT_COLUMN) - - rows.each_with_object({}) do |row, values| - id = row[ID_COLUMN] - next if id.nil? - - values[id] = row[WELSH_CONTENT_COLUMN].to_s - end - end -end diff --git a/app/services/welsh_csv_service.rb b/app/services/welsh_csv_service.rb index 64d86c9f39..2c728fc42f 100644 --- a/app/services/welsh_csv_service.rb +++ b/app/services/welsh_csv_service.rb @@ -31,7 +31,7 @@ def filename private def add_header(csv) - csv << [WelshCsvImportService::ID_COLUMN, "Content", "English content", "Welsh content"] + csv << [WelshSpreadsheetImportService::ID_COLUMN, "Content", "English content", "Welsh content"] end def add_form_name(csv) diff --git a/app/services/welsh_spreadsheet_import_service.rb b/app/services/welsh_spreadsheet_import_service.rb new file mode 100644 index 0000000000..8d4899eda1 --- /dev/null +++ b/app/services/welsh_spreadsheet_import_service.rb @@ -0,0 +1,77 @@ +class WelshSpreadsheetImportService + class InvalidHeadersError < StandardError; end + class InvalidFileTypeError < StandardError; end + + FORM_FIELD_IDS = %w[ + name + declaration_markdown + what_happens_next_markdown + payment_url + privacy_policy_url + support_email + support_phone + support_url + support_url_text + ].freeze + + ID_COLUMN = "Identifier (do not change)".freeze + WELSH_CONTENT_COLUMN = "Welsh content".freeze + + attr_reader :file + + def initialize(file) + @file = file + end + + def read + case @file.content_type + when "text/csv" + read_csv + when "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet" + read_xlsx + when "application/vnd.oasis.opendocument.spreadsheet" + read_ods + else + raise InvalidFileTypeError + end + end + + def read_csv + csv_data = file.read.force_encoding("UTF-8") + rows = CSV.parse(csv_data, headers: true) + raise InvalidHeadersError unless rows.headers.include?(ID_COLUMN) && rows.headers.include?(WELSH_CONTENT_COLUMN) + + rows.each_with_object({}) do |row, values| + id = row[ID_COLUMN] + next if id.nil? + + values[id] = row[WELSH_CONTENT_COLUMN].to_s + end + end + + def read_xlsx + xlsx = Roo::Excelx.new(file.path) + headers = xlsx.row(1).map(&:to_s) + raise InvalidHeadersError unless headers[0] == ID_COLUMN && headers[3] == WELSH_CONTENT_COLUMN + + xlsx.each_row_streaming(offset: 1).with_object({}) do |row, values| + id = row[0]&.value + next if id.blank? + + values[id] = row[3]&.value.to_s + end + end + + def read_ods + ods = Roo::OpenOffice.new(file.path) + rows = ods.sheet(0).parse(headers: true) + raise InvalidHeadersError unless rows[0].include?(ID_COLUMN) && rows[0].include?(WELSH_CONTENT_COLUMN) + + rows.drop(1).each_with_object({}) do |row, values| + id = row[ID_COLUMN] + next if id.nil? + + values[id] = row[WELSH_CONTENT_COLUMN].to_s + 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 index 0c18140a7d..1056262757 100644 --- a/app/views/forms/welsh_translation/show_upload.html.erb +++ b/app/views/forms/welsh_translation/show_upload.html.erb @@ -10,7 +10,7 @@ <%= f.govuk_file_field :file, label: { tag: 'h1', size: 'l' }, - accept: "text/csv", + accept: Forms::WelshTranslationUploadInput::FILE_TYPES.join(", "), javascript: true %> diff --git a/config/locales/input_objects/welsh_translation_upload.yml b/config/locales/input_objects/welsh_translation_upload.yml index 010cbb708f..1fbf202eae 100644 --- a/config/locales/input_objects/welsh_translation_upload.yml +++ b/config/locales/input_objects/welsh_translation_upload.yml @@ -7,8 +7,12 @@ en: attributes: file: blank: Choose a file - invalid_csv: Invalid CSV file. Download the content as a CSV file, fill in the translations, and upload it again. + disallowed_type: The selected file must be a CSV, XLSX, or ODS + invalid_csv: The spreadsheet is not in the correct format. Download the content as a CSV file, fill in the translations, and upload it again. helpers: + hint: + forms_welsh_translation_upload_input: + file: Supported file types are CSV, XLSX, and ODS label: forms_welsh_translation_upload_input: - file: Upload a CSV file with your Welsh translations + file: Upload a spreadsheet with your Welsh translations diff --git a/spec/requests/forms/welsh_translation_controller_spec.rb b/spec/requests/forms/welsh_translation_controller_spec.rb index 40bb1f8d9f..d904e59d9d 100644 --- a/spec/requests/forms/welsh_translation_controller_spec.rb +++ b/spec/requests/forms/welsh_translation_controller_spec.rb @@ -303,7 +303,7 @@ let(:csv_data) do CSV.generate do |csv| - csv << [WelshCsvImportService::ID_COLUMN, "Content", "English content", "Welsh content"] + csv << [WelshSpreadsheetImportService::ID_COLUMN, "Content", "English content", "Welsh content"] csv << ["name", "Form name", form.name, "Fy Ffurflen"] end end diff --git a/spec/services/welsh_csv_import_service_spec.rb b/spec/services/welsh_csv_import_service_spec.rb deleted file mode 100644 index c9fa92df6f..0000000000 --- a/spec/services/welsh_csv_import_service_spec.rb +++ /dev/null @@ -1,141 +0,0 @@ -require "rails_helper" - -RSpec.describe WelshCsvImportService do - describe "#import" do - subject(:import_values) { described_class.new(csv_data).read } - - context "with a valid CSV containing all form-level fields" do - let(:csv_data) do - CSV.generate do |csv| - csv << ["Identifier (do not change)", "Content", "English content", "Welsh content"] - csv << ["name", "Form name", "My Form", "Fy Ffurflen"] - csv << ["declaration_markdown", "Declaration", "Declaration text", "Welsh declaration"] - csv << ["what_happens_next_markdown", "What happens next", "Next steps", "Welsh next steps"] - csv << ["payment_url", "Payment link", "https://pay.gov.uk/en", "https://pay.gov.uk/cy"] - csv << ["privacy_policy_url", "Privacy policy", "https://privacy.gov.uk", "https://privacy.gov.uk/cy"] - csv << ["support_email", "Support email", "help@example.gov.uk", "help@example.gov.uk"] - csv << ["support_phone", "Support phone", "0800 123 456", "0800 123 456 cy"] - csv << ["support_url", "Support URL", "https://contact.gov.uk", "https://contact.gov.uk/cy"] - csv << ["support_url_text", "Support URL text", "Contact us", "Cysylltu â ni"] - end - end - - it "maps name" do - expect(import_values["name"]).to eq("Fy Ffurflen") - end - - it "maps declaration_markdown" do - expect(import_values["declaration_markdown"]).to eq("Welsh declaration") - end - - it "maps what_happens_next_markdown" do - expect(import_values["what_happens_next_markdown"]).to eq("Welsh next steps") - end - - it "maps all form-level IDs" do - expect(import_values.keys).to include( - "name", - "declaration_markdown", - "what_happens_next_markdown", - "payment_url", - "privacy_policy_url", - "support_email", - "support_phone", - "support_url", - "support_url_text", - ) - end - end - - context "with a valid CSV containing page-level fields" do - let(:page_id) { 42 } - let(:condition_id) { 99 } - let(:csv_data) do - CSV.generate do |csv| - csv << ["Identifier (do not change)", "Content", "English content", "Welsh content"] - csv << ["page_#{page_id}_question_text", "Question 1 - question text", "What is your name?", "Beth yw eich enw?"] - csv << ["page_#{page_id}_hint_text", "Question 1 - hint text", "Enter your full name", "Rhowch eich enw llawn"] - csv << ["page_#{page_id}_page_heading", "Question 1 - page heading", "Page heading", "Pennawd tudalen"] - csv << ["page_#{page_id}_guidance_markdown", "Question 1 - guidance text", "Some guidance", "Rhywfaint o arweiniad"] - csv << ["page_#{page_id}_option_0", "Question 1 - option 1", "Yes", "Ydy"] - csv << ["page_#{page_id}_option_1", "Question 1 - option 2", "No", "Nac ydy"] - csv << ["page_#{page_id}_none_of_the_above_question", "NoneOfAbove", "None of the above?", "Dim un o'r uchod?"] - csv << ["condition_#{condition_id}_exit_page_heading", "Exit heading", "You cannot continue", "Ni allwch barhau"] - csv << ["condition_#{condition_id}_exit_page_markdown", "Exit content", "Sorry", "Mae'n ddrwg"] - end - end - - it "maps page question text" do - expect(import_values["page_#{page_id}_question_text"]).to eq("Beth yw eich enw?") - end - - it "maps page hint text" do - expect(import_values["page_#{page_id}_hint_text"]).to eq("Rhowch eich enw llawn") - end - - it "maps selection option by index" do - expect(import_values["page_#{page_id}_option_0"]).to eq("Ydy") - expect(import_values["page_#{page_id}_option_1"]).to eq("Nac ydy") - end - - it "maps condition exit page fields" do - expect(import_values["condition_#{condition_id}_exit_page_heading"]).to eq("Ni allwch barhau") - expect(import_values["condition_#{condition_id}_exit_page_markdown"]).to eq("Mae'n ddrwg") - end - end - - context "with empty Welsh content" do - let(:csv_data) do - CSV.generate do |csv| - csv << ["Identifier (do not change)", "Content", "English content", "Welsh content"] - csv << ["form_name", "Form name", "My Form", ""] - end - end - - it "returns an empty string for the Welsh value" do - expect(import_values["form_name"]).to eq("") - end - end - - context "with rows missing an identifier" do - let(:csv_data) do - CSV.generate do |csv| - csv << ["Identifier (do not change)", "Content", "English content", "Welsh content"] - csv << [nil, "Some label", "English value", "Welsh value"] - csv << ["name", "Form name", "My Form", "Fy Ffurflen"] - end - end - - it "skips rows without an identifier" do - expect(import_values.keys).not_to include(nil) - expect(import_values["name"]).to eq("Fy Ffurflen") - end - end - - context "when the CSV does not have the identifier column header" do - let(:csv_data) do - CSV.generate do |csv| - csv << ["", "English content", "Welsh content"] - csv << ["Form name", "My Form", "Fy Ffurflen"] - end - end - - it "raises an error" do - expect { import_values }.to raise_error(WelshCsvImportService::InvalidHeadersError) - end - end - - context "when the CSV does not have the Welsh Content column header" do - let(:csv_data) do - CSV.generate do |csv| - csv << [WelshCsvImportService::ID_COLUMN, "English content", ""] - csv << ["Form name", "My Form", "Fy Ffurflen"] - end - end - - it "raises an error" do - expect { import_values }.to raise_error(WelshCsvImportService::InvalidHeadersError) - end - end - end -end From a91b4edff23f7eafda11f38e3139cf4a471c283d Mon Sep 17 00:00:00 2001 From: Stephen Daly Date: Wed, 1 Jul 2026 12:25:20 +0100 Subject: [PATCH 3/3] Show validation error when no file selected --- app/controllers/forms/welsh_translation_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/forms/welsh_translation_controller.rb b/app/controllers/forms/welsh_translation_controller.rb index 1d17ee3006..4fe733b794 100644 --- a/app/controllers/forms/welsh_translation_controller.rb +++ b/app/controllers/forms/welsh_translation_controller.rb @@ -120,7 +120,7 @@ def form_with_pages_and_conditions end def welsh_translation_upload_params - params.require(:forms_welsh_translation_upload_input).permit(:file) + params.fetch(:forms_welsh_translation_upload_input, ActionController::Parameters.new).permit(:file) end end end