From 10f905fe4f5834a5bcaca9b5f21840413fed7cdf Mon Sep 17 00:00:00 2001 From: Stephen Daly Date: Fri, 2 Oct 2026 14:13:23 +0100 Subject: [PATCH 1/4] Stop unnecessarily regenerating draft form document When checking whether the draft form has changed from the live version when calculating the status of the tasks to make only the English or Welsh versions of the form live, use the draft form document from the database rather than regenerating it. The draft form document is updated after any changes to the form are saved, so it will always be up to date. --- app/models/form.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/app/models/form.rb b/app/models/form.rb index c34f238778..0bf4c44adb 100644 --- a/app/models/form.rb +++ b/app/models/form.rb @@ -264,10 +264,11 @@ def changed_from_live_version?(language:) live_document = latest_live_or_archived_form_document(language:) return false if live_document.blank? - ignored_keys = %w[live_at available_languages updated_at tag created_at version] - return false if live_document.content.except(*ignored_keys) == as_form_document(language:).except(*ignored_keys) + draft_document = form_documents.find_by(tag: "draft", language:) + return false if draft_document.blank? - true + ignored_keys = %w[live_at available_languages updated_at tag created_at version] + live_document.content.except(*ignored_keys) != draft_document.content.except(*ignored_keys) end def only_s3_delivery_enabled? From 4c10b91201c77c1f43313098e68ac542d935af73 Mon Sep 17 00:00:00 2001 From: Stephen Daly Date: Fri, 2 Oct 2026 16:37:53 +0100 Subject: [PATCH 2/4] Only calculate the make live task statuses when required Only calculate the `make_only_english_live_status` and `make_only_welsh_live_status` task statuses when we will show the corresponding tasks on the task list. Update the task list service to use the presence of the task statuses to decide whether these tasks are shown. This also fixes the task status count, as it was incorrectly counting all of the make live tasks when only a subset were ever shown to the user. Co-Authored-By: Claude --- app/services/form_task_list_service.rb | 10 ++--- app/services/task_status_service.rb | 18 ++++++-- spec/models/form_spec.rb | 2 - spec/services/task_status_service_spec.rb | 54 ++++++++++++++++------- 4 files changed, 54 insertions(+), 30 deletions(-) diff --git a/app/services/form_task_list_service.rb b/app/services/form_task_list_service.rb index 4e7a3d329d..4804f528a9 100644 --- a/app/services/form_task_list_service.rb +++ b/app/services/form_task_list_service.rb @@ -196,9 +196,9 @@ def group_upgrade_url def make_form_live_section_tasks [ share_preview_task, - (make_live_task unless display_make_languages_live_tasks?), - (make_only_english_live_task if display_make_languages_live_tasks?), - (make_only_welsh_live_task if display_make_languages_live_tasks?), + (make_live_task if @task_statuses.key?(:make_live_status)), + (make_only_english_live_task if @task_statuses.key?(:make_only_english_live_status)), + (make_only_welsh_live_task if @task_statuses.key?(:make_only_welsh_live_status)), ].compact end @@ -250,10 +250,6 @@ def make_only_welsh_live_task } end - def display_make_languages_live_tasks? - !@form.has_live_welsh_translation? && @form.has_welsh_translation? - end - def live_title_name return I18n.t("forms.task_list_create.make_form_live_section.title") if @form.is_archived? diff --git a/app/services/task_status_service.rb b/app/services/task_status_service.rb index df9a9dcb7f..2d2fc91f38 100644 --- a/app/services/task_status_service.rb +++ b/app/services/task_status_service.rb @@ -22,7 +22,7 @@ def all_incomplete_tasks(ignore_missing_welsh: false) end def all_task_statuses - { + statuses = { name_status:, pages_status:, declaration_status:, @@ -35,13 +35,19 @@ def all_task_statuses submission_attachments_status:, batch_submissions_status:, share_preview_status:, - make_live_status:, - make_only_english_live_status:, - make_only_welsh_live_status:, welsh_language_status:, submission_email_status:, confirm_submission_email_status:, } + + if display_make_languages_live_tasks? + statuses[:make_only_english_live_status] = make_only_english_live_status + statuses[:make_only_welsh_live_status] = make_only_welsh_live_status + else + statuses[:make_live_status] = make_live_status + end + + statuses end def incomplete_email_tasks @@ -59,6 +65,10 @@ def email_task_statuses private + def display_make_languages_live_tasks? + !@form.has_live_welsh_translation? && @form.has_welsh_translation? + end + def name_status :completed end diff --git a/spec/models/form_spec.rb b/spec/models/form_spec.rb index 5719f8b67d..9d2b2f914e 100644 --- a/spec/models/form_spec.rb +++ b/spec/models/form_spec.rb @@ -960,8 +960,6 @@ welsh_language_status: :optional, share_preview_status: :completed, make_live_status: :completed, - make_only_english_live_status: :completed, - make_only_welsh_live_status: :cannot_start, } expect(form.all_task_statuses).to eq expected_hash end diff --git a/spec/services/task_status_service_spec.rb b/spec/services/task_status_service_spec.rb index 4be32b334e..4966b80cbe 100644 --- a/spec/services/task_status_service_spec.rb +++ b/spec/services/task_status_service_spec.rb @@ -429,6 +429,7 @@ let(:form) { build(:form, :with_group, group:) } before do + allow(form).to receive(:has_welsh_translation?).and_return(true) allow(form).to receive(:can_make_language_live?).with(language: "en").and_return(can_make_english_live) allow(form).to receive(:changed_from_live_version?).with(language: "en").and_return(changed_from_live_version) allow(form).to receive(:can_make_language_live?).with(language: "cy").and_return(can_make_welsh_live) @@ -514,6 +515,7 @@ let(:form) { build(:form, :with_group, group:) } before do + allow(form).to receive(:has_welsh_translation?).and_return(true) allow(form).to receive(:can_make_language_live?).with(language: "en").and_return(can_make_english_live) allow(form).to receive(:can_make_language_live?).with(language: "cy").and_return(can_make_welsh_live) end @@ -543,8 +545,8 @@ end context "when the form already has a live Welsh version" do - it "returns completed" do - expect(task_status_service.all_task_statuses[:make_only_welsh_live_status]).to eq :completed + it "does not include the language live statuses" do + expect(task_status_service.all_task_statuses).not_to include(:make_only_english_live_status, :make_only_welsh_live_status) end end @@ -704,38 +706,56 @@ share_preview_status: :completed, submission_email_status: :completed, confirm_submission_email_status: :completed, - make_only_english_live_status: :completed, - make_only_welsh_live_status: :cannot_start, } expect(task_status_service.all_task_statuses).to eq expected_hash end - context "when the form has a live Welsh version" do - let(:form) { create(:form, :live_with_draft, :with_welsh_translation, welsh_completed: true) } + context "when the form has no Welsh translation" do + it "includes the make live status and not the language live statuses" do + statuses = task_status_service.all_task_statuses - context "and an incomplete welsh translation" do - before do - # Add new content requiring Welsh translation - form.update!(declaration_markdown: "I declare this is correct", share_preview_completed: true) - end + expect(statuses).to include(:make_live_status) + expect(statuses).not_to include(:make_only_english_live_status, :make_only_welsh_live_status) + end - it "make live_status is not_started" do - expect(task_status_service.all_task_statuses).to include(make_live_status: :not_started) - end + it "does not calculate the language live statuses" do + allow(form).to receive(:can_make_language_live?).and_call_original + + task_status_service.all_task_statuses + + expect(form).not_to have_received(:can_make_language_live?) end end - context "when the form does not have a live Welsh version" do + context "when the form has a Welsh translation but no live Welsh version" do let(:form) { create(:form, :with_welsh_translation, welsh_completed: true) } + it "includes the language live statuses and not the make live status" do + statuses = task_status_service.all_task_statuses + + expect(statuses).to include(:make_only_english_live_status, :make_only_welsh_live_status) + expect(statuses).not_to include(:make_live_status) + end + end + + context "when the form has a live Welsh version" do + let(:form) { create(:form, :live_with_draft, :with_welsh_translation, welsh_completed: true) } + + it "includes the make live status and not the language live statuses" do + statuses = task_status_service.all_task_statuses + + expect(statuses).to include(:make_live_status) + expect(statuses).not_to include(:make_only_english_live_status, :make_only_welsh_live_status) + end + context "and an incomplete welsh translation" do before do # Add new content requiring Welsh translation form.update!(declaration_markdown: "I declare this is correct", share_preview_completed: true) end - it "make live_status is cannot_start" do - expect(task_status_service.all_task_statuses).to include(make_live_status: :cannot_start) + it "make live_status is not_started" do + expect(task_status_service.all_task_statuses).to include(make_live_status: :not_started) end end end From 605ce6012a0d5583a806443fa1bc9104a67eff0d Mon Sep 17 00:00:00 2001 From: Stephen Daly Date: Wed, 14 Oct 2026 10:58:32 +0100 Subject: [PATCH 3/4] Fix task status counts for users who cannot make forms live If the tasks to make the English and Welsh versions of the form live separately would be shown, but the current user does not have the permission to make the form live the task status count was incorrectly including these tasks. Co-Authored-By: Claude --- app/services/form_task_list_service.rb | 2 + spec/services/form_task_list_service_spec.rb | 42 ++++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/app/services/form_task_list_service.rb b/app/services/form_task_list_service.rb index 4804f528a9..3f8e945c9f 100644 --- a/app/services/form_task_list_service.rb +++ b/app/services/form_task_list_service.rb @@ -272,6 +272,8 @@ def statuses_by_user unless Pundit.policy(@current_user, @form).can_make_form_live? statuses.delete(:share_preview_status) statuses.delete(:make_live_status) + statuses.delete(:make_only_english_live_status) + statuses.delete(:make_only_welsh_live_status) end statuses diff --git a/spec/services/form_task_list_service_spec.rb b/spec/services/form_task_list_service_spec.rb index a8aecfedc4..309c701f38 100644 --- a/spec/services/form_task_list_service_spec.rb +++ b/spec/services/form_task_list_service_spec.rb @@ -76,6 +76,48 @@ expect(result.task_counts).to eq expected_hash end end + + context "when the language live statuses are included instead of the make live status" do + let(:statuses) do + { + declaration_status: :completed, + make_only_english_live_status: :not_started, + make_only_welsh_live_status: :completed, + name_status: :completed, + pages_status: :completed, + privacy_policy_status: :not_started, + support_contact_details_status: :not_started, + what_happens_next_status: :completed, + payment_link_status: :optional, + copy_of_answers_status: :optional, + submission_attachments_status: :optional, + batch_submissions_status: :optional, + share_preview_status: :completed, + submission_email_status: :completed, + confirm_submission_email_status: :not_started, + } + end + + context "when the user can make the form live" do + let(:can_make_form_live) { true } + + it "counts the language live statuses" do + result = described_class.new(form:, current_user:) + + expect(result.task_counts).to eq({ completed: 7, total: 11 }) + end + end + + context "when the user cannot make the form live" do + let(:can_make_form_live) { false } + + it "does not count the language live statuses" do + result = described_class.new(form:, current_user:) + + expect(result.task_counts).to eq({ completed: 5, total: 8 }) + end + end + end end describe "#all_sections" do From 8b01ca7c972be5f023e50cd7eaa1b881824b7204 Mon Sep 17 00:00:00 2001 From: Stephen Daly Date: Wed, 14 Oct 2026 11:44:57 +0100 Subject: [PATCH 4/4] Update completed task count to exclude Welsh task This task is optional, so should be excluded from the task count as we do for other optional tasks. --- app/services/form_task_list_service.rb | 1 + spec/services/form_task_list_service_spec.rb | 2 ++ 2 files changed, 3 insertions(+) diff --git a/app/services/form_task_list_service.rb b/app/services/form_task_list_service.rb index 3f8e945c9f..a489646904 100644 --- a/app/services/form_task_list_service.rb +++ b/app/services/form_task_list_service.rb @@ -293,6 +293,7 @@ def remove_optional_statuses(statuses) statuses.delete(:copy_of_answers_status) statuses.delete(:submission_attachments_status) statuses.delete(:batch_submissions_status) + statuses.delete(:welsh_language_status) end def can_enter_submission_email_code diff --git a/spec/services/form_task_list_service_spec.rb b/spec/services/form_task_list_service_spec.rb index 309c701f38..d40234185f 100644 --- a/spec/services/form_task_list_service_spec.rb +++ b/spec/services/form_task_list_service_spec.rb @@ -44,6 +44,7 @@ share_preview_status: :completed, submission_email_status: :completed, confirm_submission_email_status: :not_started, + welsh_language_status: :in_progress, } end let(:form) { create(:form) } @@ -95,6 +96,7 @@ share_preview_status: :completed, submission_email_status: :completed, confirm_submission_email_status: :not_started, + welsh_language_status: :in_progress, } end