-
Notifications
You must be signed in to change notification settings - Fork 11
Add separate make live confirmations for each language #2717
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
f8e361c
6579e1b
d62f91a
215aacf
71a1d28
16a4558
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,75 @@ | ||
| module Forms | ||
| class MakeLanguageLiveController < FormsController | ||
| def new | ||
| authorize current_form, :can_make_language_live? | ||
| return redirect_to form_path(form_id: current_form.id) unless current_form.can_make_language_live?(language: params[:language]) | ||
|
|
||
| @make_language_live_input = MakeLiveInput.new(form: current_form) | ||
|
|
||
| render_new | ||
| end | ||
|
|
||
| def create | ||
| authorize current_form, :can_make_language_live? | ||
|
|
||
| @make_language_live_input = MakeLiveInput.new(**make_language_live_input_params) | ||
|
|
||
| return redirect_to form_path(@make_language_live_input.form.id) unless @make_language_live_input.confirmed? | ||
| return render_new(status: :unprocessable_content) unless @make_language_live_input.valid? | ||
|
|
||
| return redirect_to form_path(form_id: current_form.id) unless current_form.can_make_language_live?(language: params[:language]) | ||
|
|
||
| @make_form_live_service = MakeFormLiveService.call(current_form:, current_user:, language: params[:language]) | ||
| @make_form_live_service.make_language_live | ||
|
|
||
| if current_form.state_previously_changed? | ||
| OrgAdminAlertsService.new(form: current_form, current_user:).form_made_live | ||
| end | ||
|
|
||
| @go_to_make_welsh_live_input = GoToMakeWelshLiveInput.new | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we need this as we're redirecting? |
||
|
|
||
| redirect_to make_language_live_show_confirmation_path | ||
| end | ||
|
|
||
| def show_confirmation | ||
| authorize current_form, :can_make_language_live? | ||
|
|
||
| @make_form_live_service = MakeFormLiveService.call(current_form:, current_user:, language: params[:language]) | ||
|
|
||
| @go_to_make_welsh_live_input = GoToMakeWelshLiveInput.new | ||
|
|
||
| render "confirmation", locals: { | ||
| current_form:, | ||
| confirmation_page_title: @make_form_live_service.page_title, | ||
| confirmation_page_body: @make_form_live_service.confirmation_page_body, | ||
| language: params[:language], | ||
| } | ||
| end | ||
|
|
||
| def submit_confirmation | ||
| authorize current_form, :can_make_language_live? | ||
|
|
||
| @go_to_make_welsh_live_input = GoToMakeWelshLiveInput.new(**go_to_make_welsh_live_input_params) | ||
|
|
||
| if @go_to_make_welsh_live_input.confirmed? | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we validate the input for this and show an error message if a radio button isn't selected? |
||
| redirect_to make_language_live_path(language: "cy") | ||
| else | ||
| redirect_to form_path | ||
| end | ||
| end | ||
|
|
||
| private | ||
|
|
||
| def make_language_live_input_params | ||
| params.require(:forms_make_live_input).permit(:confirm).merge(form: current_form) | ||
| end | ||
|
|
||
| def render_new(status: :ok) | ||
| render "new", status:, locals: { current_form:, language: params[:language] } | ||
| end | ||
|
|
||
| def go_to_make_welsh_live_input_params | ||
| params.require(:forms_go_to_make_welsh_live_input).permit(:confirm) | ||
| end | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| class Forms::GoToMakeWelshLiveInput < ConfirmActionInput | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -218,6 +218,12 @@ def draft_created?(previous_state) | |
| (previous_state.to_sym == :archived && archived_with_draft?) | ||
| end | ||
|
|
||
| def can_make_language_live?(language:) | ||
| return can_make_english_version_live? if language == "en" | ||
|
|
||
| can_make_welsh_version_live? if language == "cy" | ||
| end | ||
|
|
||
| private | ||
|
|
||
| def set_external_id | ||
|
|
@@ -268,6 +274,30 @@ def after_make_live | |
| FormDocumentSyncService.new(self).synchronize_live_form | ||
| end | ||
|
|
||
| def before_make_english_live | ||
| before_make_live | ||
| end | ||
|
|
||
| def after_make_english_live | ||
| FormDocumentSyncService.new(self).synchronize_live_english_form | ||
| end | ||
|
|
||
| def before_make_welsh_live | ||
| before_make_live | ||
| end | ||
|
|
||
| def after_make_welsh_live | ||
| FormDocumentSyncService.new(self).synchronize_live_welsh_form | ||
| end | ||
|
|
||
| def can_make_english_version_live? | ||
| draft? && ready_for_live && live_form_document.blank? && live_welsh_form_document.blank? | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it correct that we can't make the English version live if the task to add the Welsh translation is in progress? I had to remove the existing Welsh translations for
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, can we not make changes to the English version live after it's already live but the Welsh version hasn't been made live yet? |
||
| end | ||
|
|
||
| def can_make_welsh_version_live? | ||
| has_live_version && ready_for_live && live_form_document.present? && live_welsh_form_document.blank? | ||
| end | ||
|
|
||
| def after_archive | ||
| FormDocumentSyncService.new(self).synchronize_archived_form | ||
| end | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -52,6 +52,44 @@ def synchronize_archived_welsh_form | |||||||||
| end | ||||||||||
| end | ||||||||||
|
|
||||||||||
| def synchronize_live_english_form | ||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we added the word "only", this might make it clearer that this method isn't called when both languages are updated at the same time. I was getting a bit confused about what method would be called when, and if this is always called when we make changes to multilingual forms.
Suggested change
|
||||||||||
| FormDocument.transaction do | ||||||||||
| # Ensure we only make English version live if there is no existing live Welsh version | ||||||||||
| raise ActiveRecord::RecordNotFound, "Cannot make English version live if there is already a live Welsh version." if FormDocument.where(form:, tag: LIVE_TAG, language: "cy").exists? | ||||||||||
|
Comment on lines
+57
to
+58
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The comment and error message here confused me a little as we can make changes to the English version live if there is already a live Welsh version, we just don't use this method.
Suggested change
|
||||||||||
|
|
||||||||||
| content = form_content("en", live_at: form.updated_at) | ||||||||||
| update_or_create_form_document(LIVE_TAG, content, "en") | ||||||||||
|
|
||||||||||
| # Update the content of the live English version to to not include Welsh in available_languages | ||||||||||
| FormDocument.where(form:, tag: [LIVE_TAG], language: "en").find_each do |live_document| | ||||||||||
| live_document.content["available_languages"] = %w[en] | ||||||||||
| live_document.save! | ||||||||||
| end | ||||||||||
|
|
||||||||||
| # A new live version replaces any previous archived version | ||||||||||
| delete_form_documents_by_tag(ARCHIVED_TAG) | ||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If there was previously a Welsh version that was archived, this will delete the Welsh archived form document. Do we want to do that? |
||||||||||
| end | ||||||||||
| end | ||||||||||
|
|
||||||||||
| def synchronize_live_welsh_form | ||||||||||
| FormDocument.transaction do | ||||||||||
| # Ensure we only make Welsh version live if there is already an existing live English version | ||||||||||
| raise ActiveRecord::RecordNotFound, "Cannot make Welsh version live unless there is already a live English version." unless FormDocument.where(form:, tag: LIVE_TAG, language: "en").exists? | ||||||||||
|
|
||||||||||
| content = form_content("cy", live_at: form.updated_at) | ||||||||||
| update_or_create_form_document(LIVE_TAG, content, "cy") | ||||||||||
|
|
||||||||||
| # Update the content of the live English version to show that it now supports Welsh | ||||||||||
| FormDocument.where(form:, tag: [LIVE_TAG], language: "en").find_each do |live_document| | ||||||||||
| live_document.content["available_languages"] = %w[en cy] | ||||||||||
| live_document.save! | ||||||||||
| end | ||||||||||
|
|
||||||||||
| # A new live version replaces any previous archived version | ||||||||||
| delete_form_documents_by_tag(ARCHIVED_TAG) | ||||||||||
| end | ||||||||||
| end | ||||||||||
|
|
||||||||||
| def update_draft_form_document | ||||||||||
| synchronize_documents_for_tag(DRAFT_TAG) | ||||||||||
| end | ||||||||||
|
|
@@ -79,6 +117,7 @@ def update_or_create_form_document(tag, content, language) | |||||||||
| language:, | ||||||||||
| ) | ||||||||||
| form_document.content = content | ||||||||||
|
|
||||||||||
| form_document.save! | ||||||||||
| end | ||||||||||
|
|
||||||||||
|
|
||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| <% set_page_title(confirmation_page_title) %> | ||
|
|
||
| <div class="govuk-grid-row"> | ||
| <div class="govuk-grid-column-two-thirds"> | ||
| <%= govuk_panel(title_text: confirmation_page_title) %> | ||
|
|
||
| <%= confirmation_page_body %> | ||
|
|
||
| <% if language == "en" %> | ||
| <h2 class="govuk-heading-m"><%= t('make_live.confirmation.form_name_en') %></h2> | ||
|
|
||
| <p><%= current_form.name %></p> | ||
| <% end %> | ||
|
|
||
| <% if language == "cy" %> | ||
| <h2 class="govuk-heading-m"><%= t('make_live.confirmation.form_name_cy') %></h2> | ||
|
|
||
| <p><%= current_form.name_cy %></p> | ||
| <% end %> | ||
|
|
||
| <% if language == "en" %> | ||
| <%= render FormUrlComponent::View.new( | ||
| runner_link: link_to_runner(Settings.forms_runner.url, current_form.id, current_form.form_slug, mode: :live), | ||
| heading_text: t('make_live.confirmation.english_form_url'), | ||
| button_text: t('make_live.confirmation.copy_english_url_to_clipboard') | ||
| ) %> | ||
| <% end %> | ||
|
|
||
| <% if language == "cy" %> | ||
| <%= render FormUrlComponent::View.new( | ||
| runner_link: link_to_runner(Settings.forms_runner.url, current_form.id, current_form.form_slug, mode: :live, locale: "cy"), | ||
| heading_text: t('make_live.confirmation.welsh_form_url'), | ||
| button_text: t('make_live.confirmation.copy_welsh_url_to_clipboard') | ||
| ) %> | ||
| <% end %> | ||
|
|
||
| <p> | ||
| <%= govuk_link_to t("make_live.confirmation.continue_link"), live_form_path(current_form.id) %> | ||
| </p> | ||
|
|
||
| <% if language == "en" %> | ||
| <% if current_form.welsh_completed %> | ||
| <%= form_with model: @go_to_make_welsh_live_input, url: make_language_live_submit_confirmation_path(form_id: current_form.id, language:) do |f| %> | ||
| <%= f.govuk_collection_radio_buttons :confirm, @go_to_make_welsh_live_input.values, :itself, legend: { size: 'm', tag: 'h2' }, bold_labels: false %> | ||
| <%= f.govuk_submit t("save_and_continue") %> | ||
| <% end %> | ||
| <% else %> | ||
| <p> | ||
| <%= t("make_live.confirmation.back_to_draft_link_html", url: form_path(current_form.id)) %> | ||
| </p> | ||
| <% end %> | ||
| <% end %> | ||
| </div> | ||
| </div> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| <% set_page_title(title_with_error_prefix(t("page_titles.make_language_live.#{language}"), @make_language_live_input.errors.any?)) %> | ||
| <% content_for :back_link, govuk_back_link_to(form_path, t("back_link.form_create")) %> | ||
|
|
||
| <div class="govuk-grid-row"> | ||
| <div class="govuk-grid-column-two-thirds"> | ||
| <%= form_with(model: @make_language_live_input, url: make_language_live_create_path) do |f| %> | ||
| <% if @make_language_live_input&.errors&.any? %> | ||
| <%= f.govuk_error_summary %> | ||
| <% end %> | ||
|
|
||
| <h1 class="govuk-heading-l"> | ||
| <span class="govuk-caption-l"><%= @make_language_live_input.form.name %></span> | ||
| <%= t("page_titles.make_language_live.#{language}") %> | ||
| </h1> | ||
|
|
||
| <%= t("make_language_live.#{language}.new.body_html", submission_email: @make_language_live_input.form.submission_email) %> | ||
|
|
||
| <%= f.govuk_collection_radio_buttons :confirm, | ||
| @make_language_live_input.values, ->(option) { option }, ->(option) { t("helpers.label.confirm_action_input.options.#{option}") }, | ||
| legend: { text: t("helpers.label.forms_make_language_live_input.#{language}.confirm"), | ||
| size: 'm'}, inline: true %> | ||
| <%= f.govuk_submit t("save_and_continue") %> | ||
| <% end %> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to check the input is valid before we check whether it's confirmed. If I don't select an option, I don't see an error.