From ace6a599b0cfd48b32c43f0edd56da00819070bc Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Tue, 22 Oct 2024 10:53:05 +0100 Subject: [PATCH 1/5] CLDC-3664 Extract income and benefits copy (#2709) * Refactor sales household pages and questions * Refactor sales income and benefits soft validations * Update subsection test * Fix income copy keys * Update subsection test * Add missing question to translations * Fix typo * Add prevshared for 2024 --- app/models/form/sales/pages/buyer1_income.rb | 1 + .../pages/buyer1_income_max_value_check.rb | 3 +- .../pages/buyer1_income_min_value_check.rb | 5 +- app/models/form/sales/pages/buyer2_income.rb | 1 + .../pages/buyer2_income_max_value_check.rb | 3 +- .../pages/buyer2_income_min_value_check.rb | 5 +- .../pages/combined_income_max_value_check.rb | 3 +- .../form/sales/pages/deposit_value_check.rb | 5 +- .../form/sales/pages/housing_benefits.rb | 1 + .../form/sales/pages/mortgage_value_check.rb | 6 +- .../form/sales/pages/previous_ownership.rb | 1 + app/models/form/sales/pages/savings.rb | 1 + .../form/sales/pages/savings_value_check.rb | 5 +- .../form/sales/questions/buyer1_income.rb | 4 +- .../sales/questions/buyer1_income_known.rb | 3 +- .../questions/buyer1_income_value_check.rb | 3 +- .../form/sales/questions/buyer1_mortgage.rb | 2 - .../form/sales/questions/buyer2_income.rb | 4 +- .../sales/questions/buyer2_income_known.rb | 3 +- .../questions/buyer2_income_value_check.rb | 3 +- .../form/sales/questions/buyer2_mortgage.rb | 2 - .../questions/combined_income_value_check.rb | 3 +- .../sales/questions/deposit_value_check.rb | 3 +- .../form/sales/questions/housing_benefits.rb | 3 +- .../sales/questions/mortgage_value_check.rb | 3 +- app/models/form/sales/questions/prevown.rb | 3 +- app/models/form/sales/questions/prevshared.rb | 3 - app/models/form/sales/questions/savings.rb | 3 +- app/models/form/sales/questions/savings_nk.rb | 3 +- .../sales/questions/savings_value_check.rb | 3 +- config/locales/en.yml | 26 ------ .../sales/income_benefits_and_savings.en.yml | 90 +++++++++++++++++++ .../forms/2023/sales/soft_validations.en.yml | 60 +++++++++++++ .../sales/income_benefits_and_savings.en.yml | 90 +++++++++++++++++++ .../forms/2024/sales/soft_validations.en.yml | 60 +++++++++++++ .../buyer1_income_max_value_check_spec.rb | 3 +- .../buyer1_income_min_value_check_spec.rb | 3 +- .../buyer2_income_max_value_check_spec.rb | 3 +- .../buyer2_income_min_value_check_spec.rb | 3 +- .../combined_income_max_value_check_spec.rb | 3 +- .../sales/pages/deposit_value_check_spec.rb | 3 +- .../sales/pages/mortgage_value_check_spec.rb | 3 +- .../sales/pages/savings_value_check_spec.rb | 3 +- .../discounted_ownership_scheme_spec.rb | 3 +- .../shared_ownership_scheme_spec.rb | 2 +- 45 files changed, 359 insertions(+), 87 deletions(-) create mode 100644 config/locales/forms/2023/sales/income_benefits_and_savings.en.yml create mode 100644 config/locales/forms/2023/sales/soft_validations.en.yml create mode 100644 config/locales/forms/2024/sales/income_benefits_and_savings.en.yml create mode 100644 config/locales/forms/2024/sales/soft_validations.en.yml diff --git a/app/models/form/sales/pages/buyer1_income.rb b/app/models/form/sales/pages/buyer1_income.rb index 943296d2b..657cf8807 100644 --- a/app/models/form/sales/pages/buyer1_income.rb +++ b/app/models/form/sales/pages/buyer1_income.rb @@ -2,6 +2,7 @@ class Form::Sales::Pages::Buyer1Income < ::Form::Page def initialize(id, hsh, subsection) super @id = "buyer_1_income" + @copy_key = "sales.income_benefits_and_savings.buyer_1_income" end def questions diff --git a/app/models/form/sales/pages/buyer1_income_max_value_check.rb b/app/models/form/sales/pages/buyer1_income_max_value_check.rb index b093baa2a..55599ff26 100644 --- a/app/models/form/sales/pages/buyer1_income_max_value_check.rb +++ b/app/models/form/sales/pages/buyer1_income_max_value_check.rb @@ -6,8 +6,9 @@ class Form::Sales::Pages::Buyer1IncomeMaxValueCheck < ::Form::Page "income1_over_soft_max?" => true, }, ] + @copy_key = "sales.soft_validations.income1_value_check.max" @title_text = { - "translation" => "soft_validations.income.over_soft_max_for_la_buyer_1", + "translation" => "forms.#{form.start_date.year}.#{@copy_key}.title_text", "arguments" => [ { "key" => "field_formatted_as_currency", diff --git a/app/models/form/sales/pages/buyer1_income_min_value_check.rb b/app/models/form/sales/pages/buyer1_income_min_value_check.rb index 2feb4f887..9fc85bb76 100644 --- a/app/models/form/sales/pages/buyer1_income_min_value_check.rb +++ b/app/models/form/sales/pages/buyer1_income_min_value_check.rb @@ -6,8 +6,9 @@ class Form::Sales::Pages::Buyer1IncomeMinValueCheck < ::Form::Page "income1_under_soft_min?" => true, }, ] + @copy_key = "sales.soft_validations.income1_value_check.min" @title_text = { - "translation" => "soft_validations.income.under_soft_min_for_economic_status.title_text", + "translation" => "forms.#{form.start_date.year}.#{@copy_key}.title_text", "arguments" => [ { "key" => "field_formatted_as_currency", @@ -22,7 +23,7 @@ class Form::Sales::Pages::Buyer1IncomeMinValueCheck < ::Form::Page ], } @informative_text = { - "translation" => "soft_validations.income.under_soft_min_for_economic_status.hint_text", + "translation" => "forms.#{form.start_date.year}.#{@copy_key}.informative_text", "arguments" => [], } end diff --git a/app/models/form/sales/pages/buyer2_income.rb b/app/models/form/sales/pages/buyer2_income.rb index f865418a7..fd35d52cc 100644 --- a/app/models/form/sales/pages/buyer2_income.rb +++ b/app/models/form/sales/pages/buyer2_income.rb @@ -2,6 +2,7 @@ class Form::Sales::Pages::Buyer2Income < ::Form::Page def initialize(id, hsh, subsection) super @id = "buyer_2_income" + @copy_key = "sales.income_benefits_and_savings.buyer_2_income" @depends_on = [{ "joint_purchase?" => true, }] diff --git a/app/models/form/sales/pages/buyer2_income_max_value_check.rb b/app/models/form/sales/pages/buyer2_income_max_value_check.rb index 894da8799..deece885e 100644 --- a/app/models/form/sales/pages/buyer2_income_max_value_check.rb +++ b/app/models/form/sales/pages/buyer2_income_max_value_check.rb @@ -6,8 +6,9 @@ class Form::Sales::Pages::Buyer2IncomeMaxValueCheck < ::Form::Page "income2_over_soft_max?" => true, }, ] + @copy_key = "sales.soft_validations.income2_value_check.max" @title_text = { - "translation" => "soft_validations.income.over_soft_max_for_la_buyer_2", + "translation" => "forms.#{form.start_date.year}.#{@copy_key}.title_text", "arguments" => [ { "key" => "field_formatted_as_currency", diff --git a/app/models/form/sales/pages/buyer2_income_min_value_check.rb b/app/models/form/sales/pages/buyer2_income_min_value_check.rb index adef17843..a7b68cd10 100644 --- a/app/models/form/sales/pages/buyer2_income_min_value_check.rb +++ b/app/models/form/sales/pages/buyer2_income_min_value_check.rb @@ -6,8 +6,9 @@ class Form::Sales::Pages::Buyer2IncomeMinValueCheck < ::Form::Page "income2_under_soft_min?" => true, }, ] + @copy_key = "sales.soft_validations.income2_value_check.min" @title_text = { - "translation" => "soft_validations.income.under_soft_min_for_economic_status.title_text", + "translation" => "forms.#{form.start_date.year}.#{@copy_key}.title_text", "arguments" => [ { "key" => "field_formatted_as_currency", @@ -22,7 +23,7 @@ class Form::Sales::Pages::Buyer2IncomeMinValueCheck < ::Form::Page ], } @informative_text = { - "translation" => "soft_validations.income.under_soft_min_for_economic_status.hint_text", + "translation" => "forms.#{form.start_date.year}.#{@copy_key}.informative_text", "arguments" => [], } end diff --git a/app/models/form/sales/pages/combined_income_max_value_check.rb b/app/models/form/sales/pages/combined_income_max_value_check.rb index 82f2e4469..1cd1fd851 100644 --- a/app/models/form/sales/pages/combined_income_max_value_check.rb +++ b/app/models/form/sales/pages/combined_income_max_value_check.rb @@ -6,8 +6,9 @@ class Form::Sales::Pages::CombinedIncomeMaxValueCheck < ::Form::Page "combined_income_over_soft_max?" => true, }, ] + @copy_key = "sales.soft_validations.combined_income_value_check" @title_text = { - "translation" => "soft_validations.income.over_soft_max_for_la_combined", + "translation" => "forms.#{form.start_date.year}.#{@copy_key}.title_text", "arguments" => [ { "key" => "field_formatted_as_currency", diff --git a/app/models/form/sales/pages/deposit_value_check.rb b/app/models/form/sales/pages/deposit_value_check.rb index cca25a43b..cbec4b031 100644 --- a/app/models/form/sales/pages/deposit_value_check.rb +++ b/app/models/form/sales/pages/deposit_value_check.rb @@ -1,12 +1,13 @@ class Form::Sales::Pages::DepositValueCheck < ::Form::Page def initialize(id, hsh, subsection, joint_purchase:) super(id, hsh, subsection) + @copy_key = "sales.soft_validations.deposit_value_check.#{joint_purchase ? 'joint_purchase' : 'not_joint_purchase'}" @informative_text = { - "translation" => "soft_validations.deposit.hint_text", + "translation" => "forms.#{form.start_date.year}.#{@copy_key}.informative_text", "arguments" => [], } @title_text = { - "translation" => "soft_validations.deposit.title_text.#{joint_purchase ? 'two' : 'one'}", + "translation" => "forms.#{form.start_date.year}.#{@copy_key}.title_text", "arguments" => [ { "key" => "field_formatted_as_currency", diff --git a/app/models/form/sales/pages/housing_benefits.rb b/app/models/form/sales/pages/housing_benefits.rb index 90d721ad8..c40dff5f4 100644 --- a/app/models/form/sales/pages/housing_benefits.rb +++ b/app/models/form/sales/pages/housing_benefits.rb @@ -2,6 +2,7 @@ class Form::Sales::Pages::HousingBenefits < ::Form::Page def initialize(id, hsh, subsection, joint_purchase:) super(id, hsh, subsection) @joint_purchase = joint_purchase + @copy_key = "sales.income_benefits_and_savings.housing_benefits.#{joint_purchase ? 'joint_purchase' : 'not_joint_purchase'}" end def questions diff --git a/app/models/form/sales/pages/mortgage_value_check.rb b/app/models/form/sales/pages/mortgage_value_check.rb index 949cea27f..bfab9a014 100644 --- a/app/models/form/sales/pages/mortgage_value_check.rb +++ b/app/models/form/sales/pages/mortgage_value_check.rb @@ -2,10 +2,10 @@ class Form::Sales::Pages::MortgageValueCheck < ::Form::Page def initialize(id, hsh, subsection, person_index = nil) super(id, hsh, subsection) @depends_on = depends_on - @informative_text = {} @person_index = person_index + @copy_key = "sales.soft_validations.mortgage_value_check" @title_text = { - "translation" => "soft_validations.mortgage.title_text", + "translation" => "forms.#{form.start_date.year}.#{@copy_key}.title_text", "arguments" => [ { "key" => "field_formatted_as_currency", @@ -15,7 +15,7 @@ class Form::Sales::Pages::MortgageValueCheck < ::Form::Page ], } @informative_text = { - "translation" => "soft_validations.mortgage.hint_text", + "translation" => "forms.#{form.start_date.year}.#{@copy_key}.informative_text", "arguments" => [], } end diff --git a/app/models/form/sales/pages/previous_ownership.rb b/app/models/form/sales/pages/previous_ownership.rb index 50711baf7..ac7a17787 100644 --- a/app/models/form/sales/pages/previous_ownership.rb +++ b/app/models/form/sales/pages/previous_ownership.rb @@ -3,6 +3,7 @@ class Form::Sales::Pages::PreviousOwnership < ::Form::Page super(id, hsh, subsection) @joint_purchase = joint_purchase @depends_on = [{ "joint_purchase?" => @joint_purchase }] + @copy_key = "sales.income_benefits_and_savings.prevown.#{joint_purchase ? 'joint_purchase' : 'not_joint_purchase'}" end def questions diff --git a/app/models/form/sales/pages/savings.rb b/app/models/form/sales/pages/savings.rb index 7a13dbe34..64a2b1ecf 100644 --- a/app/models/form/sales/pages/savings.rb +++ b/app/models/form/sales/pages/savings.rb @@ -2,6 +2,7 @@ class Form::Sales::Pages::Savings < ::Form::Page def initialize(id, hsh, subsection, joint_purchase:) super(id, hsh, subsection) @joint_purchase = joint_purchase + @copy_key = "sales.income_benefits_and_savings.savings.#{joint_purchase ? 'joint_purchase' : 'not_joint_purchase'}" end def questions diff --git a/app/models/form/sales/pages/savings_value_check.rb b/app/models/form/sales/pages/savings_value_check.rb index bcb95abc9..00be6decd 100644 --- a/app/models/form/sales/pages/savings_value_check.rb +++ b/app/models/form/sales/pages/savings_value_check.rb @@ -1,8 +1,9 @@ class Form::Sales::Pages::SavingsValueCheck < ::Form::Page def initialize(id, hsh, subsection, joint_purchase:) super(id, hsh, subsection) + @copy_key = "sales.soft_validations.savings_value_check.#{joint_purchase ? 'joint_purchase' : 'not_joint_purchase'}" @title_text = { - "translation" => "soft_validations.savings.title_text.#{joint_purchase ? 'two' : 'one'}", + "translation" => "forms.#{form.start_date.year}.#{@copy_key}.title_text", "arguments" => [ { "key" => "field_formatted_as_currency", @@ -12,7 +13,7 @@ class Form::Sales::Pages::SavingsValueCheck < ::Form::Page ], } @informative_text = { - "translation" => "soft_validations.savings.hint_text", + "translation" => "forms.#{form.start_date.year}.#{@copy_key}.informative_text", "arguments" => [], } @joint_purchase = joint_purchase diff --git a/app/models/form/sales/questions/buyer1_income.rb b/app/models/form/sales/questions/buyer1_income.rb index 106f040f8..3f1159a8f 100644 --- a/app/models/form/sales/questions/buyer1_income.rb +++ b/app/models/form/sales/questions/buyer1_income.rb @@ -2,9 +2,7 @@ class Form::Sales::Questions::Buyer1Income < ::Form::Question def initialize(id, hsh, page) super @id = "income1" - @check_answer_label = "Buyer 1’s gross annual income" - @header = "Buyer 1’s gross annual income" - @hint_text = "Provide the gross annual income (i.e. salary before tax) plus the annual amount of benefits, Universal Credit or pensions, and income from investments." + @copy_key = "sales.income_benefits_and_savings.buyer_1_income.income1" @type = "numeric" @min = 0 @max = 999_999 diff --git a/app/models/form/sales/questions/buyer1_income_known.rb b/app/models/form/sales/questions/buyer1_income_known.rb index b00b45475..46c1f4caa 100644 --- a/app/models/form/sales/questions/buyer1_income_known.rb +++ b/app/models/form/sales/questions/buyer1_income_known.rb @@ -2,8 +2,7 @@ class Form::Sales::Questions::Buyer1IncomeKnown < ::Form::Question def initialize(id, hsh, page) super @id = "income1nk" - @check_answer_label = "Buyer 1’s gross annual income known?" - @header = "Do you know buyer 1’s annual income?" + @copy_key = "sales.income_benefits_and_savings.buyer_1_income.income1nk" @type = "radio" @answer_options = ANSWER_OPTIONS @conditional_for = { diff --git a/app/models/form/sales/questions/buyer1_income_value_check.rb b/app/models/form/sales/questions/buyer1_income_value_check.rb index 63e3b6478..410bd53d5 100644 --- a/app/models/form/sales/questions/buyer1_income_value_check.rb +++ b/app/models/form/sales/questions/buyer1_income_value_check.rb @@ -2,8 +2,7 @@ class Form::Sales::Questions::Buyer1IncomeValueCheck < ::Form::Question def initialize(id, hsh, page, check_answers_card_number:) super(id, hsh, page) @id = "income1_value_check" - @check_answer_label = "Buyer 1 income confirmation" - @header = "Are you sure this is correct?" + @copy_key = "sales.soft_validations.income1_value_check" @type = "interruption_screen" @answer_options = { "0" => { "value" => "Yes" }, diff --git a/app/models/form/sales/questions/buyer1_mortgage.rb b/app/models/form/sales/questions/buyer1_mortgage.rb index eaa2a07fa..d2a94f6b5 100644 --- a/app/models/form/sales/questions/buyer1_mortgage.rb +++ b/app/models/form/sales/questions/buyer1_mortgage.rb @@ -2,8 +2,6 @@ class Form::Sales::Questions::Buyer1Mortgage < ::Form::Question def initialize(id, hsh, page) super @id = "inc1mort" - @check_answer_label = "Buyer 1’s income used for mortgage application" - @header = "Was buyer 1’s income used for a mortgage application?" @type = "radio" @answer_options = ANSWER_OPTIONS @check_answers_card_number = 1 diff --git a/app/models/form/sales/questions/buyer2_income.rb b/app/models/form/sales/questions/buyer2_income.rb index cff25b344..39777f870 100644 --- a/app/models/form/sales/questions/buyer2_income.rb +++ b/app/models/form/sales/questions/buyer2_income.rb @@ -2,10 +2,8 @@ class Form::Sales::Questions::Buyer2Income < ::Form::Question def initialize(id, hsh, page) super @id = "income2" - @check_answer_label = "Buyer 2’s gross annual income" - @header = "Buyer 2’s gross annual income" + @copy_key = "sales.income_benefits_and_savings.buyer_2_income.income2" @type = "numeric" - @hint_text = "Provide the gross annual income (i.e. salary before tax) plus the annual amount of benefits, Universal Credit or pensions, and income from investments." @min = 0 @max = 999_999 @step = 1 diff --git a/app/models/form/sales/questions/buyer2_income_known.rb b/app/models/form/sales/questions/buyer2_income_known.rb index b3cc5f33a..0b125f2af 100644 --- a/app/models/form/sales/questions/buyer2_income_known.rb +++ b/app/models/form/sales/questions/buyer2_income_known.rb @@ -2,8 +2,7 @@ class Form::Sales::Questions::Buyer2IncomeKnown < ::Form::Question def initialize(id, hsh, page) super @id = "income2nk" - @check_answer_label = "Buyer 2’s gross annual income known?" - @header = "Do you know buyer 2’s annual income?" + @copy_key = "sales.income_benefits_and_savings.buyer_2_income.income2" @type = "radio" @answer_options = ANSWER_OPTIONS @conditional_for = { diff --git a/app/models/form/sales/questions/buyer2_income_value_check.rb b/app/models/form/sales/questions/buyer2_income_value_check.rb index 37b6cb380..6b0bab5c2 100644 --- a/app/models/form/sales/questions/buyer2_income_value_check.rb +++ b/app/models/form/sales/questions/buyer2_income_value_check.rb @@ -2,8 +2,7 @@ class Form::Sales::Questions::Buyer2IncomeValueCheck < ::Form::Question def initialize(id, hsh, page, check_answers_card_number:) super(id, hsh, page) @id = "income2_value_check" - @check_answer_label = "Buyer 2 income confirmation" - @header = "Are you sure this is correct?" + @copy_key = "sales.soft_validations.income2_value_check" @type = "interruption_screen" @answer_options = { "0" => { "value" => "Yes" }, diff --git a/app/models/form/sales/questions/buyer2_mortgage.rb b/app/models/form/sales/questions/buyer2_mortgage.rb index 42b8cda06..415d5b714 100644 --- a/app/models/form/sales/questions/buyer2_mortgage.rb +++ b/app/models/form/sales/questions/buyer2_mortgage.rb @@ -2,8 +2,6 @@ class Form::Sales::Questions::Buyer2Mortgage < ::Form::Question def initialize(id, hsh, page) super @id = "inc2mort" - @check_answer_label = "Buyer 2’s income used for mortgage application" - @header = "Was buyer 2’s income used for a mortgage application?" @type = "radio" @answer_options = ANSWER_OPTIONS @check_answers_card_number = 2 diff --git a/app/models/form/sales/questions/combined_income_value_check.rb b/app/models/form/sales/questions/combined_income_value_check.rb index 47cafeaa2..c2688ffe5 100644 --- a/app/models/form/sales/questions/combined_income_value_check.rb +++ b/app/models/form/sales/questions/combined_income_value_check.rb @@ -2,8 +2,7 @@ class Form::Sales::Questions::CombinedIncomeValueCheck < ::Form::Question def initialize(id, hsh, page, check_answers_card_number:) super(id, hsh, page) @id = "combined_income_value_check" - @check_answer_label = "Combined income confirmation" - @header = "Are you sure this is correct?" + @copy_key = "sales.soft_validations.combined_income_value_check" @type = "interruption_screen" @answer_options = { "0" => { "value" => "Yes" }, diff --git a/app/models/form/sales/questions/deposit_value_check.rb b/app/models/form/sales/questions/deposit_value_check.rb index f3cd27e3a..f8d403d9c 100644 --- a/app/models/form/sales/questions/deposit_value_check.rb +++ b/app/models/form/sales/questions/deposit_value_check.rb @@ -2,8 +2,7 @@ class Form::Sales::Questions::DepositValueCheck < ::Form::Question def initialize(id, hsh, page) super @id = "deposit_value_check" - @check_answer_label = "Deposit confirmation" - @header = "Are you sure that the deposit is this much higher than the buyer's savings?" + @copy_key = "sales.soft_validations.deposit_value_check" @type = "interruption_screen" @answer_options = { "0" => { "value" => "Yes" }, diff --git a/app/models/form/sales/questions/housing_benefits.rb b/app/models/form/sales/questions/housing_benefits.rb index 70bc4ba2e..d7a670f2e 100644 --- a/app/models/form/sales/questions/housing_benefits.rb +++ b/app/models/form/sales/questions/housing_benefits.rb @@ -2,8 +2,7 @@ class Form::Sales::Questions::HousingBenefits < ::Form::Question def initialize(id, hsh, page, joint_purchase:) super(id, hsh, page) @id = "hb" - @check_answer_label = "Housing-related benefits #{joint_purchase ? 'buyers' : 'buyer'} received before buying this property" - @header = "#{joint_purchase ? 'Were the buyers' : 'Was the buyer'} receiving any of these housing-related benefits immediately before buying this property?" + @copy_key = "sales.income_benefits_and_savings.housing_benefits.#{joint_purchase ? 'joint_purchase' : 'not_joint_purchase'}" @type = "radio" @answer_options = ANSWER_OPTIONS @question_number = QUESTION_NUMBER_FROM_YEAR[form.start_date.year] || QUESTION_NUMBER_FROM_YEAR[QUESTION_NUMBER_FROM_YEAR.keys.max] diff --git a/app/models/form/sales/questions/mortgage_value_check.rb b/app/models/form/sales/questions/mortgage_value_check.rb index 39079d40e..99307ab38 100644 --- a/app/models/form/sales/questions/mortgage_value_check.rb +++ b/app/models/form/sales/questions/mortgage_value_check.rb @@ -2,8 +2,7 @@ class Form::Sales::Questions::MortgageValueCheck < ::Form::Question def initialize(id, hsh, page) super @id = "mortgage_value_check" - @check_answer_label = "Mortgage confirmation" - @header = "Are you sure that the mortgage is more than 5 times the income used for the mortgage application?" + @copy_key = "sales.soft_validations.mortgage_value_check" @type = "interruption_screen" @answer_options = { "0" => { "value" => "Yes" }, diff --git a/app/models/form/sales/questions/prevown.rb b/app/models/form/sales/questions/prevown.rb index f913f950b..ecd6c9071 100644 --- a/app/models/form/sales/questions/prevown.rb +++ b/app/models/form/sales/questions/prevown.rb @@ -2,8 +2,7 @@ class Form::Sales::Questions::Prevown < ::Form::Question def initialize(id, hsh, page, joint_purchase:) super(id, hsh, page) @id = "prevown" - @check_answer_label = I18n.t("check_answer_labels.prevown", count: joint_purchase ? 2 : 1) - @header = I18n.t("questions.prevown", count: joint_purchase ? 2 : 1) + @copy_key = "sales.income_benefits_and_savings.prevown.#{joint_purchase ? 'joint_purchase' : 'not_joint_purchase'}" @type = "radio" @answer_options = ANSWER_OPTIONS @question_number = QUESTION_NUMBER_FROM_YEAR[form.start_date.year] || QUESTION_NUMBER_FROM_YEAR[QUESTION_NUMBER_FROM_YEAR.keys.max] diff --git a/app/models/form/sales/questions/prevshared.rb b/app/models/form/sales/questions/prevshared.rb index 018f57faa..f4ced14b9 100644 --- a/app/models/form/sales/questions/prevshared.rb +++ b/app/models/form/sales/questions/prevshared.rb @@ -2,11 +2,8 @@ class Form::Sales::Questions::Prevshared < ::Form::Question def initialize(id, hsh, page) super @id = "prevshared" - @check_answer_label = "Previous property shared ownership?" - @header = "Was the previous property under shared ownership?" @type = "radio" @answer_options = ANSWER_OPTIONS - @hint_text = "For any buyer" @question_number = QUESTION_NUMBER_FROM_YEAR[form.start_date.year] || QUESTION_NUMBER_FROM_YEAR[QUESTION_NUMBER_FROM_YEAR.keys.max] end diff --git a/app/models/form/sales/questions/savings.rb b/app/models/form/sales/questions/savings.rb index 82b8e0f87..bb1f939a5 100644 --- a/app/models/form/sales/questions/savings.rb +++ b/app/models/form/sales/questions/savings.rb @@ -2,8 +2,7 @@ class Form::Sales::Questions::Savings < ::Form::Question def initialize(id, hsh, page, joint_purchase:) super(id, hsh, page) @id = "savings" - @check_answer_label = "#{joint_purchase ? 'Buyers’' : 'Buyer’s'} total savings before any deposit paid" - @header = "Enter their total savings to the nearest £10" + @copy_key = "sales.income_benefits_and_savings.savings.#{joint_purchase ? 'joint_purchase' : 'not_joint_purchase'}.savings" @type = "numeric" @width = 5 @prefix = "£" diff --git a/app/models/form/sales/questions/savings_nk.rb b/app/models/form/sales/questions/savings_nk.rb index 931ba835a..3c7e115f4 100644 --- a/app/models/form/sales/questions/savings_nk.rb +++ b/app/models/form/sales/questions/savings_nk.rb @@ -2,8 +2,7 @@ class Form::Sales::Questions::SavingsNk < ::Form::Question def initialize(id, hsh, page, joint_purchase:) super(id, hsh, page) @id = "savingsnk" - @check_answer_label = "#{joint_purchase ? 'Buyers’' : 'Buyer’s'} total savings known?" - @header = "Do you know how much the #{joint_purchase ? 'buyers' : 'buyer'} had in savings before they paid any deposit for the property?" + @copy_key = "sales.income_benefits_and_savings.savings.#{joint_purchase ? 'joint_purchase' : 'not_joint_purchase'}.savingsnk" @type = "radio" @answer_options = ANSWER_OPTIONS @conditional_for = { diff --git a/app/models/form/sales/questions/savings_value_check.rb b/app/models/form/sales/questions/savings_value_check.rb index bcd16827a..f547969bf 100644 --- a/app/models/form/sales/questions/savings_value_check.rb +++ b/app/models/form/sales/questions/savings_value_check.rb @@ -2,8 +2,7 @@ class Form::Sales::Questions::SavingsValueCheck < ::Form::Question def initialize(id, hsh, page) super @id = "savings_value_check" - @check_answer_label = "Savings confirmation" - @header = "Are you sure the savings are higher than £100,000?" + @copy_key = "sales.soft_validations.savings_value_check" @type = "interruption_screen" @answer_options = { "0" => { "value" => "Yes" }, diff --git a/config/locales/en.yml b/config/locales/en.yml index 85be0e677..b39183591 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -703,13 +703,6 @@ en: message: "Net income is lower than expected based on the household’s working situation. Are you sure this is correct?" in_soft_max_range: message: "Net income is higher than expected based on the household’s working situation. Are you sure this is correct?" - income: - under_soft_min_for_economic_status: - title_text: "You told us income was %{income}." - hint_text: "This is less than we would expect for someone in this working situation." - over_soft_max_for_la_buyer_1: "You told us the income of buyer 1 is %{income}. This seems high. Are you sure this is correct?" - over_soft_max_for_la_buyer_2: "You told us the income of buyer 2 is %{income}. This seems high. Are you sure this is correct?" - over_soft_max_for_la_combined: "You told us the combined income of this household is %{combined_income}. This seems high. Are you sure this is correct?" rent: outside_range_title: "You told us the rent is %{brent}." informative_text: "This is %{higher_or_lower} than we would expect." @@ -772,24 +765,11 @@ Make sure these answers are correct." percentage_discount_value: title_text: "You told us that the percentage discount is %{discount}." hint_text: "This is higher than we would expect." - savings: - title_text: - one: "You told us the buyer’s savings were %{savings}." - two: "You told us the buyers’ savings were %{savings}." - hint_text: "This is higher than we would expect." - deposit: - title_text: - one: "You told us the buyer’s deposit was %{deposit} and their savings were %{savings}." - two: "You told us the buyers’ deposit was %{deposit} and their savings were %{savings}." - hint_text: "The deposit amount is higher than we would expect for the amount of savings they have." grant: title_text: "You told us that the grant amount is %{grant}." hint_text: "Loans, grants and subsidies are usually between £9,000 and £16,000." wheelchair: title_text: "You told us that someone in the household uses a wheelchair." - mortgage: - title_text: "You told us that the mortgage amount is %{mortgage}." - hint_text: "This is more than 5 times the income, which is higher than we would expect." referral: title_text: "Are you sure?" hint_text: "This is a general needs log, and this referral type is for supported housing." @@ -859,9 +839,6 @@ Make sure these answers are correct." soctenant: one: "Was the buyer a private registered provider, housing association or local authority tenant immediately before this sale?" other: "Were any of the buyers private registered providers, housing association or local authority tenants immediately before this sale?" - prevown: - one: "Has the buyer previously owned a property?" - other: "Have any of the buyers previously owned a property?" stairowned: one: "What percentage of the property does the buyer now own in total?" other: "What percentage of the property do the buyers now own in total?" @@ -884,9 +861,6 @@ Make sure these answers are correct." soctenant: one: "Buyer was a registered provider, housing association or local authority tenant immediately before this sale?" other: "Any buyers were registered providers, housing association or local authority tenants immediately before this sale?" - prevown: - one: "Buyer previously owned a property." - other: "Buyers previously owned a property." stairowned: one: "Percentage the buyer now owns in total." other: "Percentage the buyers now own in total." diff --git a/config/locales/forms/2023/sales/income_benefits_and_savings.en.yml b/config/locales/forms/2023/sales/income_benefits_and_savings.en.yml new file mode 100644 index 000000000..49262d7a1 --- /dev/null +++ b/config/locales/forms/2023/sales/income_benefits_and_savings.en.yml @@ -0,0 +1,90 @@ +en: + forms: + 2023: + sales: + income_benefits_and_savings: + buyer_1_income: + page_header: "" + income1nk: + check_answer_label: "Buyer 1’s gross annual income known?" + hint_text: "" + question_text: "Do you know buyer 1’s annual income?" + income1: + check_answer_label: "Buyer 1’s gross annual income" + hint_text: "Provide the gross annual income (i.e. salary before tax) plus the annual amount of benefits, Universal Credit or pensions, and income from investments." + question_text: "Buyer 1’s gross annual income" + + inc1mort: + page_header: "" + check_answer_label: "Buyer 1’s income used for mortgage application" + hint_text: "" + question_text: "Was buyer 1’s income used for a mortgage application?" + + buyer_2_income: + page_header: "" + income2nk: + check_answer_label: "Buyer 2’s gross annual income known?" + hint_text: "" + question_text: "Do you know buyer 2’s annual income?" + income2: + check_answer_label: "Buyer 2’s gross annual income" + hint_text: "Provide the gross annual income (i.e. salary before tax) plus the annual amount of benefits, Universal Credit or pensions, and income from investments." + question_text: "Buyer 2’s gross annual income" + + inc2mort: + page_header: "" + check_answer_label: "Buyer 2’s income used for mortgage application" + hint_text: "" + question_text: "Was buyer 2’s income used for a mortgage application?" + + housing_benefits: + joint_purchase: + page_header: "" + check_answer_label: "Housing-related benefits buyers received before buying this property" + hint_text: "" + question_text: "Were the buyers receiving any of these housing-related benefits immediately before buying this property?" + not_joint_purchase: + page_header: "" + check_answer_label: "Housing-related benefits buyer received before buying this property" + hint_text: "" + question_text: "Was the buyer receiving any of these housing-related benefits immediately before buying this property?" + + savings: + joint_purchase: + page_header: "" + savingsnk: + check_answer_label: "Buyers’ total savings known?" + hint_text: "" + question_text: "Do you know how much the 'buyers' had in savings before they paid any deposit for the property?" + savings: + check_answer_label: "Buyers’ total savings before any deposit paid" + hint_text: "Include any savings, investments, ISAs, premium bonds, shares, or money held in a bank or building society account." + question_text: "Enter their total savings to the nearest £10" + not_joint_purchase: + page_header: "" + savingsnk: + check_answer_label: "Buyer’s total savings known?" + hint_text: "" + question_text: "Do you know how much the buyer had in savings before they paid any deposit for the property?" + savings: + check_answer_label: "Buyer’s total savings before any deposit paid" + hint_text: "Include any savings, investments, ISAs, premium bonds, shares, or money held in a bank or building society account." + question_text: "Enter their total savings to the nearest £10" + + prevown: + joint_purchase: + page_header: "" + check_answer_label: "Buyers previously owned a property." + hint_text: "" + question_text: "Have any of the buyers previously owned a property?" + not_joint_purchase: + page_header: "" + check_answer_label: "Buyer previously owned a property." + hint_text: "" + question_text: "Has the buyer previously owned a property?" + + prevshared: + page_header: "" + check_answer_label: "Previous property shared ownership?" + hint_text: "For any buyer" + question_text: "Was the previous property under shared ownership?" \ No newline at end of file diff --git a/config/locales/forms/2023/sales/soft_validations.en.yml b/config/locales/forms/2023/sales/soft_validations.en.yml new file mode 100644 index 000000000..de5138208 --- /dev/null +++ b/config/locales/forms/2023/sales/soft_validations.en.yml @@ -0,0 +1,60 @@ +en: + forms: + 2023: + sales: + soft_validations: + income1_value_check: + page_header: "" + check_answer_label: "Buyer 1 income confirmation" + hint_text: "" + question_text: "Are you sure this is correct?" + min: + title_text: "You told us income was %{income}." + informative_text: "This is less than we would expect for someone in this working situation." + max: + title_text: "You told us the income of buyer 1 is %{income}. This seems high. Are you sure this is correct?" + income2_value_check: + page_header: "" + check_answer_label: "Buyer 2 income confirmation" + hint_text: "" + question_text: "Are you sure this is correct?" + min: + title_text: "You told us income was %{income}." + informative_text: "This is less than we would expect for someone in this working situation." + max: + title_text: "You told us the income of buyer 2 is %{income}. This seems high. Are you sure this is correct?" + combined_income_value_check: + page_header: "" + check_answer_label: "Combined income confirmation" + hint_text: "" + question_text: "Are you sure this is correct?" + title_text: "You told us the combined income of this household is %{combined_income}. This seems high. Are you sure this is correct?" + mortgage_value_check: + page_header: "" + check_answer_label: "Mortgage confirmation" + hint_text: "" + question_text: "Are you sure that the mortgage is more than 5 times the income used for the mortgage application?" + title_text: "You told us that the mortgage amount is %{mortgage}." + informative_text: "This is more than 5 times the income, which is higher than we would expect." + savings_value_check: + page_header: "" + check_answer_label: "Savings confirmation" + hint_text: "" + question_text: "Are you sure the savings are higher than £100,000?" + joint_purchase: + title_text: You told us the buyers’ savings were %{savings}." + informative_text: "This is higher than we would expect." + not_joint_purchase: + title_text: "You told us the buyer’s savings were %{savings}." + informative_text: "This is higher than we would expect." + deposit_value_check:: + page_header: "" + check_answer_label: "Deposit confirmation" + hint_text: "" + question_text: "Are you sure that the deposit is this much higher than the buyer's savings?" + joint_purchase: + title_text: "You told us the buyers’ deposit was %{deposit} and their savings were %{savings}." + informative_text: "The deposit amount is higher than we would expect for the amount of savings they have." + not_joint_purchase: + title_text: "You told us the buyer’s deposit was %{deposit} and their savings were %{savings}." + informative_text: "The deposit amount is higher than we would expect for the amount of savings they have." diff --git a/config/locales/forms/2024/sales/income_benefits_and_savings.en.yml b/config/locales/forms/2024/sales/income_benefits_and_savings.en.yml new file mode 100644 index 000000000..0654c9e1f --- /dev/null +++ b/config/locales/forms/2024/sales/income_benefits_and_savings.en.yml @@ -0,0 +1,90 @@ +en: + forms: + 2024: + sales: + income_benefits_and_savings: + buyer_1_income: + page_header: "" + income1nk: + check_answer_label: "Buyer 1’s gross annual income known?" + hint_text: "" + question_text: "Do you know buyer 1’s annual income?" + income1: + check_answer_label: "Buyer 1’s gross annual income" + hint_text: "Provide the gross annual income (i.e. salary before tax) plus the annual amount of benefits, Universal Credit or pensions, and income from investments." + question_text: "Buyer 1’s gross annual income" + + inc1mort: + page_header: "" + check_answer_label: "Buyer 1’s income used for mortgage application" + hint_text: "" + question_text: "Was buyer 1’s income used for a mortgage application?" + + buyer_2_income: + page_header: "" + income2nk: + check_answer_label: "Buyer 2’s gross annual income known?" + hint_text: "" + question_text: "Do you know buyer 2’s annual income?" + income2: + check_answer_label: "Buyer 2’s gross annual income" + hint_text: "Provide the gross annual income (i.e. salary before tax) plus the annual amount of benefits, Universal Credit or pensions, and income from investments." + question_text: "Buyer 2’s gross annual income" + + inc2mort: + page_header: "" + check_answer_label: "Buyer 2’s income used for mortgage application" + hint_text: "" + question_text: "Was buyer 2’s income used for a mortgage application?" + + housing_benefits: + joint_purchase: + page_header: "" + check_answer_label: "Housing-related benefits buyers received before buying this property" + hint_text: "" + question_text: "Were the buyers receiving any of these housing-related benefits immediately before buying this property?" + not_joint_purchase: + page_header: "" + check_answer_label: "Housing-related benefits buyer received before buying this property" + hint_text: "" + question_text: "Was the buyer receiving any of these housing-related benefits immediately before buying this property?" + + savings: + joint_purchase: + page_header: "" + savingsnk: + check_answer_label: "Buyers’ total savings known?" + hint_text: "" + question_text: "Do you know how much the 'buyers' had in savings before they paid any deposit for the property?" + savings: + check_answer_label: "Buyers’ total savings before any deposit paid" + hint_text: "Include any savings, investments, ISAs, premium bonds, shares, or money held in a bank or building society account." + question_text: "Enter their total savings to the nearest £10" + not_joint_purchase: + page_header: "" + savingsnk: + check_answer_label: "Buyer’s total savings known?" + hint_text: "" + question_text: "Do you know how much the buyer had in savings before they paid any deposit for the property?" + savings: + check_answer_label: "Buyer’s total savings before any deposit paid" + hint_text: "Include any savings, investments, ISAs, premium bonds, shares, or money held in a bank or building society account." + question_text: "Enter their total savings to the nearest £10" + + prevown: + joint_purchase: + page_header: "" + check_answer_label: "Buyers previously owned a property." + hint_text: "" + question_text: "Have any of the buyers previously owned a property?" + not_joint_purchase: + page_header: "" + check_answer_label: "Buyer previously owned a property." + hint_text: "" + question_text: "Has the buyer previously owned a property?" + + prevshared: + page_header: "" + check_answer_label: "Previous property shared ownership?" + hint_text: "For any buyer" + question_text: "Was the previous property under shared ownership?" \ No newline at end of file diff --git a/config/locales/forms/2024/sales/soft_validations.en.yml b/config/locales/forms/2024/sales/soft_validations.en.yml new file mode 100644 index 000000000..853d21ad2 --- /dev/null +++ b/config/locales/forms/2024/sales/soft_validations.en.yml @@ -0,0 +1,60 @@ +en: + forms: + 2024: + sales: + soft_validations: + income1_value_check: + page_header: "" + check_answer_label: "Buyer 1 income confirmation" + hint_text: "" + question_text: "Are you sure this is correct?" + min: + title_text: "You told us income was %{income}." + informative_text: "This is less than we would expect for someone in this working situation." + max: + title_text: "You told us the income of buyer 1 is %{income}. This seems high. Are you sure this is correct?" + income2_value_check: + page_header: "" + check_answer_label: "Buyer 2 income confirmation" + hint_text: "" + question_text: "Are you sure this is correct?" + min: + title_text: "You told us income was %{income}." + informative_text: "This is less than we would expect for someone in this working situation." + max: + title_text: "You told us the income of buyer 2 is %{income}. This seems high. Are you sure this is correct?" + combined_income_value_check: + page_header: "" + check_answer_label: "Combined income confirmation" + hint_text: "" + question_text: "Are you sure this is correct?" + title_text: "You told us the combined income of this household is %{combined_income}. This seems high. Are you sure this is correct?" + mortgage_value_check: + page_header: "" + check_answer_label: "Mortgage confirmation" + hint_text: "" + question_text: "Are you sure that the mortgage is more than 5 times the income used for the mortgage application?" + title_text: "You told us that the mortgage amount is %{mortgage}." + informative_text: "This is more than 5 times the income, which is higher than we would expect." + savings_value_check: + page_header: "" + check_answer_label: "Savings confirmation" + hint_text: "" + question_text: "Are you sure the savings are higher than £100,000?" + joint_purchase: + title_text: You told us the buyers’ savings were %{savings}." + informative_text: "This is higher than we would expect." + not_joint_purchase: + title_text: "You told us the buyer’s savings were %{savings}." + informative_text: "This is higher than we would expect." + deposit_value_check: + page_header: "" + check_answer_label: "Deposit confirmation" + hint_text: "" + question_text: "Are you sure that the deposit is this much higher than the buyer's savings?" + joint_purchase: + title_text: "You told us the buyers’ deposit was %{deposit} and their savings were %{savings}." + informative_text: "The deposit amount is higher than we would expect for the amount of savings they have." + not_joint_purchase: + title_text: "You told us the buyer’s deposit was %{deposit} and their savings were %{savings}." + informative_text: "The deposit amount is higher than we would expect for the amount of savings they have." diff --git a/spec/models/form/sales/pages/buyer1_income_max_value_check_spec.rb b/spec/models/form/sales/pages/buyer1_income_max_value_check_spec.rb index 48e921468..fa5cf1e7c 100644 --- a/spec/models/form/sales/pages/buyer1_income_max_value_check_spec.rb +++ b/spec/models/form/sales/pages/buyer1_income_max_value_check_spec.rb @@ -5,7 +5,8 @@ RSpec.describe Form::Sales::Pages::Buyer1IncomeMaxValueCheck, type: :model do let(:page_id) { "prefix_buyer_1_income_max_value_check" } let(:page_definition) { nil } - let(:subsection) { instance_double(Form::Subsection) } + let(:form) { instance_double(Form, start_date: Time.zone.local(2024, 4, 1)) } + let(:subsection) { instance_double(Form::Subsection, form:) } it "has correct subsection" do expect(page.subsection).to eq(subsection) diff --git a/spec/models/form/sales/pages/buyer1_income_min_value_check_spec.rb b/spec/models/form/sales/pages/buyer1_income_min_value_check_spec.rb index 1736daef7..79d61ce06 100644 --- a/spec/models/form/sales/pages/buyer1_income_min_value_check_spec.rb +++ b/spec/models/form/sales/pages/buyer1_income_min_value_check_spec.rb @@ -5,7 +5,8 @@ RSpec.describe Form::Sales::Pages::Buyer1IncomeMinValueCheck, type: :model do let(:page_id) { "prefix_buyer_1_income_min_value_check" } let(:page_definition) { nil } - let(:subsection) { instance_double(Form::Subsection) } + let(:form) { instance_double(Form, start_date: Time.zone.local(2024, 4, 1)) } + let(:subsection) { instance_double(Form::Subsection, form:) } it "has correct subsection" do expect(page.subsection).to eq(subsection) diff --git a/spec/models/form/sales/pages/buyer2_income_max_value_check_spec.rb b/spec/models/form/sales/pages/buyer2_income_max_value_check_spec.rb index 440a6bb13..f467db18d 100644 --- a/spec/models/form/sales/pages/buyer2_income_max_value_check_spec.rb +++ b/spec/models/form/sales/pages/buyer2_income_max_value_check_spec.rb @@ -5,7 +5,8 @@ RSpec.describe Form::Sales::Pages::Buyer2IncomeMaxValueCheck, type: :model do let(:page_id) { "prefix_buyer_2_income_max_value_check" } let(:page_definition) { nil } - let(:subsection) { instance_double(Form::Subsection) } + let(:form) { instance_double(Form, start_date: Time.zone.local(2024, 4, 1)) } + let(:subsection) { instance_double(Form::Subsection, form:) } it "has correct subsection" do expect(page.subsection).to eq(subsection) diff --git a/spec/models/form/sales/pages/buyer2_income_min_value_check_spec.rb b/spec/models/form/sales/pages/buyer2_income_min_value_check_spec.rb index 887dcab29..44b85ef9e 100644 --- a/spec/models/form/sales/pages/buyer2_income_min_value_check_spec.rb +++ b/spec/models/form/sales/pages/buyer2_income_min_value_check_spec.rb @@ -5,7 +5,8 @@ RSpec.describe Form::Sales::Pages::Buyer2IncomeMinValueCheck, type: :model do let(:page_id) { "prefix_buyer_2_income_min_value_check" } let(:page_definition) { nil } - let(:subsection) { instance_double(Form::Subsection) } + let(:form) { instance_double(Form, start_date: Time.zone.local(2024, 4, 1)) } + let(:subsection) { instance_double(Form::Subsection, form:) } it "has correct subsection" do expect(page.subsection).to eq(subsection) diff --git a/spec/models/form/sales/pages/combined_income_max_value_check_spec.rb b/spec/models/form/sales/pages/combined_income_max_value_check_spec.rb index c79cb38da..f9b9954d9 100644 --- a/spec/models/form/sales/pages/combined_income_max_value_check_spec.rb +++ b/spec/models/form/sales/pages/combined_income_max_value_check_spec.rb @@ -5,7 +5,8 @@ RSpec.describe Form::Sales::Pages::CombinedIncomeMaxValueCheck, type: :model do let(:page_id) { "prefix_combined_income_max_value_check" } let(:page_definition) { nil } - let(:subsection) { instance_double(Form::Subsection) } + let(:form) { instance_double(Form, start_date: Time.zone.local(2024, 4, 1)) } + let(:subsection) { instance_double(Form::Subsection, form:) } it "has correct subsection" do expect(page.subsection).to eq(subsection) diff --git a/spec/models/form/sales/pages/deposit_value_check_spec.rb b/spec/models/form/sales/pages/deposit_value_check_spec.rb index 5c4cd3608..dcd12449c 100644 --- a/spec/models/form/sales/pages/deposit_value_check_spec.rb +++ b/spec/models/form/sales/pages/deposit_value_check_spec.rb @@ -5,7 +5,8 @@ RSpec.describe Form::Sales::Pages::DepositValueCheck, type: :model do let(:page_id) { "deposit_value_check" } let(:page_definition) { nil } - let(:subsection) { instance_double(Form::Subsection) } + let(:form) { instance_double(Form, start_date: Time.zone.local(2024, 4, 1)) } + let(:subsection) { instance_double(Form::Subsection, form:) } it "has correct subsection" do expect(page.subsection).to eq(subsection) diff --git a/spec/models/form/sales/pages/mortgage_value_check_spec.rb b/spec/models/form/sales/pages/mortgage_value_check_spec.rb index e4dc11bbc..be5aa5a77 100644 --- a/spec/models/form/sales/pages/mortgage_value_check_spec.rb +++ b/spec/models/form/sales/pages/mortgage_value_check_spec.rb @@ -6,7 +6,8 @@ RSpec.describe Form::Sales::Pages::MortgageValueCheck, type: :model do let(:page_id) { "buyer_1_income_mortgage_value_check" } let(:page_definition) { nil } let(:index) { 1 } - let(:subsection) { instance_double(Form::Subsection) } + let(:form) { instance_double(Form, start_date: Time.zone.local(2024, 4, 1)) } + let(:subsection) { instance_double(Form::Subsection, form:) } it "has correct subsection" do expect(page.subsection).to eq(subsection) diff --git a/spec/models/form/sales/pages/savings_value_check_spec.rb b/spec/models/form/sales/pages/savings_value_check_spec.rb index 81672977d..e680ff5d1 100644 --- a/spec/models/form/sales/pages/savings_value_check_spec.rb +++ b/spec/models/form/sales/pages/savings_value_check_spec.rb @@ -5,7 +5,8 @@ RSpec.describe Form::Sales::Pages::SavingsValueCheck, type: :model do let(:page_id) { "savings_value_check" } let(:page_definition) { nil } - let(:subsection) { instance_double(Form::Subsection) } + let(:form) { instance_double(Form, start_date: Time.zone.local(2024, 4, 1)) } + let(:subsection) { instance_double(Form::Subsection, form:) } it "has correct subsection" do expect(page.subsection).to eq(subsection) diff --git a/spec/models/form/sales/subsections/discounted_ownership_scheme_spec.rb b/spec/models/form/sales/subsections/discounted_ownership_scheme_spec.rb index e703e713a..bfd5313fb 100644 --- a/spec/models/form/sales/subsections/discounted_ownership_scheme_spec.rb +++ b/spec/models/form/sales/subsections/discounted_ownership_scheme_spec.rb @@ -5,7 +5,8 @@ RSpec.describe Form::Sales::Subsections::DiscountedOwnershipScheme, type: :model let(:subsection_id) { nil } let(:subsection_definition) { nil } - let(:section) { instance_double(Form::Sales::Sections::SaleInformation) } + let(:form) { instance_double(Form, start_date: Time.zone.local(2024, 4, 1)) } + let(:section) { instance_double(Form::Sales::Sections::SaleInformation, form:) } it "has correct section" do expect(discounted_ownership_scheme.section).to eq(section) diff --git a/spec/models/form/sales/subsections/shared_ownership_scheme_spec.rb b/spec/models/form/sales/subsections/shared_ownership_scheme_spec.rb index ea3fcd40e..922281fd0 100644 --- a/spec/models/form/sales/subsections/shared_ownership_scheme_spec.rb +++ b/spec/models/form/sales/subsections/shared_ownership_scheme_spec.rb @@ -8,7 +8,7 @@ RSpec.describe Form::Sales::Subsections::SharedOwnershipScheme, type: :model do let(:section) { instance_double(Form::Sales::Sections::SaleInformation) } before do - allow(section).to receive(:form).and_return(instance_double(Form, start_year_after_2024?: false)) + allow(section).to receive(:form).and_return(instance_double(Form, start_year_after_2024?: false, start_date: Time.zone.local(2023, 4, 1))) end it "has correct section" do From a65ee5676f5188a9cbde7d2ad86907f8a190b504 Mon Sep 17 00:00:00 2001 From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com> Date: Tue, 22 Oct 2024 14:14:53 +0100 Subject: [PATCH 2/5] CLDC-2399: Bulk upload table styling (#2714) --- .../bulk_upload_error_row_component.html.erb | 30 +++++++++---------- .../bulk_upload_error_row_component.rb | 13 ++++++++ app/frontend/styles/_bulk-uploads.scss | 5 ++++ 3 files changed, 33 insertions(+), 15 deletions(-) diff --git a/app/components/bulk_upload_error_row_component.html.erb b/app/components/bulk_upload_error_row_component.html.erb index 65e38613f..4bd303957 100644 --- a/app/components/bulk_upload_error_row_component.html.erb +++ b/app/components/bulk_upload_error_row_component.html.erb @@ -13,7 +13,7 @@ <% if critical_errors.any? %>

Critical errors

These errors must be fixed to complete your logs.

- <%= govuk_table do |table| %> + <%= govuk_table(html_attributes: { class: potential_errors.any? ? "" : "no-bottom-border" }) do |table| %> <%= table.with_head do |head| %> <% head.with_row do |row| %> <% row.with_cell(header: true, text: "Cell") %> @@ -39,7 +39,7 @@ <% if potential_errors.any? %>

Potential errors

The following groups of cells might have conflicting data. Check the answers and fix any incorrect data.

If the answers are correct, fix the critical errors and reupload the file. You'll need to confirm that the following data is correct when the file only contains potential errors.

- <%= govuk_table do |table| %> + <%= govuk_table(html_attributes: { class: "no-bottom-border" }) do |table| %> <%= table.with_head do |head| %> <% head.with_row do |row| %> <% row.with_cell(header: true, text: "Cell") %> @@ -49,24 +49,24 @@ <% end %> <% end %> - <%= table.with_body do |body| %> - <% potential_errors.group_by(&:error).each do |error_message, errors| %> - <% errors.each_with_index do |error, index| %> - <% row_class = "grouped-rows" %> - <% row_class += " first-row" if index.zero? %> - <% row_class += " last-row" if index == errors.size - 1 %> - <% body.with_row(html_attributes: { class: row_class }) do |row| %> - <% row.with_cell(text: error.cell) %> - <% row.with_cell(text: question_for_field(error.field), html_attributes: { class: "govuk-!-width-one-half" }) %> - <% if index == 0 %> - <% row.with_cell(text: error_message.html_safe, rowspan: errors.size, html_attributes: { class: "govuk-!-font-weight-bold govuk-!-width-one-half grouped-multirow-cell" }) %> - <% end %> - <% row.with_cell(text: error.field.humanize) %> + <%= table.with_body do |body| %> + <% potential_errors.group_by(&:error).each_with_index do |(error_message, errors), group_index| %> + <% total_groups = potential_errors.group_by(&:error).size %> + <% errors.each_with_index do |error, index| %> + <% row_class = row_classes(index, errors.size) %> + <% body.with_row(html_attributes: { class: row_class }) do |row| %> + <% row.with_cell(text: error.cell) %> + <% row.with_cell(text: question_for_field(error.field), html_attributes: { class: "govuk-!-width-one-half" }) %> + <% if index == 0 %> + <% cell_class = cell_classes(group_index, total_groups) %> + <% row.with_cell(text: error_message.html_safe, rowspan: errors.size, html_attributes: { class: cell_class }) %> <% end %> + <% row.with_cell(text: error.field.humanize) %> <% end %> <% end %> <% end %> <% end %> + <% end %> <% end %> diff --git a/app/components/bulk_upload_error_row_component.rb b/app/components/bulk_upload_error_row_component.rb index 887eef10c..1cb4de9d8 100644 --- a/app/components/bulk_upload_error_row_component.rb +++ b/app/components/bulk_upload_error_row_component.rb @@ -62,4 +62,17 @@ class BulkUploadErrorRowComponent < ViewComponent::Base def sales? bulk_upload.log_type == "sales" end + + def row_classes(index, errors_size) + row_class = "grouped-rows" + row_class += " first-row" if index.zero? + row_class += " last-row" if index == errors_size - 1 + row_class + end + + def cell_classes(group_index, total_groups) + cell_class = "govuk-!-font-weight-bold govuk-!-width-one-half" + cell_class += " grouped-multirow-cell" unless group_index == total_groups - 1 + cell_class + end end diff --git a/app/frontend/styles/_bulk-uploads.scss b/app/frontend/styles/_bulk-uploads.scss index eceae6565..65213ee82 100644 --- a/app/frontend/styles/_bulk-uploads.scss +++ b/app/frontend/styles/_bulk-uploads.scss @@ -12,6 +12,11 @@ border-bottom: 1px solid #b1b4b6; } +.no-bottom-border, +.no-bottom-border > tbody > tr:last-of-type td { + border-bottom: none; +} + .text-normal-break { white-space: normal; word-break: break-all; From 443a19f42d5fe5243982e9c385b153e9a326df42 Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Tue, 22 Oct 2024 16:08:44 +0100 Subject: [PATCH 3/5] Update household validations (#2708) --- .../sales/household_validations.rb | 48 +++++++++--------- app/models/validations/shared_validations.rb | 6 ++- .../bulk_upload/sales/year2024/row_parser.rb | 12 ++--- config/locales/en.yml | 11 ---- .../validations/sales/household.en.yml | 47 +++++++++++++++++ .../sales/household_validations_spec.rb | 50 +++++++++---------- 6 files changed, 107 insertions(+), 67 deletions(-) create mode 100644 config/locales/validations/sales/household.en.yml diff --git a/app/models/validations/sales/household_validations.rb b/app/models/validations/sales/household_validations.rb index e006374aa..fb1ea65c2 100644 --- a/app/models/validations/sales/household_validations.rb +++ b/app/models/validations/sales/household_validations.rb @@ -9,9 +9,9 @@ module Validations::Sales::HouseholdValidations return unless record.form.start_date.year >= 2023 if record.buyers_will_live_in? && record.buyer_one_will_not_live_in_property? && record.buyer_two_will_not_live_in_property? - record.errors.add :buylivein, I18n.t("validations.household.buylivein.buyers_will_live_in_property_values_inconsistent_setup") - record.errors.add :buy1livein, I18n.t("validations.household.buylivein.buyers_will_live_in_property_values_inconsistent") - record.errors.add :buy2livein, I18n.t("validations.household.buylivein.buyers_will_live_in_property_values_inconsistent") + record.errors.add :buylivein, I18n.t("validations.sales.household.buylivein.buyers_will_live_in_property_values_inconsistent") + record.errors.add :buy1livein, I18n.t("validations.sales.household.buy1livein.buyers_will_live_in_property_values_inconsistent") + record.errors.add :buy2livein, I18n.t("validations.sales.household.buy2livein.buyers_will_live_in_property_values_inconsistent") end end @@ -20,8 +20,8 @@ module Validations::Sales::HouseholdValidations return unless record.discounted_ownership_sale? && record.prevten if [3, 4, 5, 6, 7, 9, 0].include?(record.prevten) - record.errors.add :prevten, I18n.t("validations.household.prevten.invalid_for_discounted_sale") - record.errors.add :ownershipsch, I18n.t("validations.household.prevten.invalid_for_discounted_sale") + record.errors.add :prevten, I18n.t("validations.sales.household.prevten.prevten_invalid_for_discounted_sale") + record.errors.add :ownershipsch, I18n.t("validations.sales.household.ownershipsch.prevten_invalid_for_discounted_sale") end end @@ -34,11 +34,11 @@ module Validations::Sales::HouseholdValidations next unless age && relationship if age < 16 && !relationship_is_child_other_or_refused?(relationship) - record.errors.add "age#{person_num}", I18n.t("validations.household.age.child_under_16_relat_sales", person_num:) - record.errors.add "relat#{person_num}", I18n.t("validations.household.relat.child_under_16_sales", person_num:) + record.errors.add "age#{person_num}", I18n.t("validations.sales.household.age.child_under_16", person_num:) + record.errors.add "relat#{person_num}", I18n.t("validations.sales.household.relat.child_under_16", person_num:) elsif age >= 20 && person_is_child?(relationship) - record.errors.add "age#{person_num}", I18n.t("validations.household.age.child_over_20") - record.errors.add "relat#{person_num}", I18n.t("validations.household.relat.child_over_20") + record.errors.add "age#{person_num}", I18n.t("validations.sales.household.age.child_over_20") + record.errors.add "relat#{person_num}", I18n.t("validations.sales.household.relat.child_over_20") end end end @@ -58,16 +58,16 @@ module Validations::Sales::HouseholdValidations child = person_is_child?(relationship) if age_between_16_19 && !(student || economic_status_refused) && child - record.errors.add "ecstat#{person_num}", I18n.t("validations.household.ecstat.student_16_19.must_be_student") - record.errors.add "age#{person_num}", I18n.t("validations.household.age.student_16_19.cannot_be_16_19.child_not_student") - record.errors.add "relat#{person_num}", I18n.t("validations.household.relat.student_16_19.cannot_be_child.16_19_not_student") + record.errors.add "ecstat#{person_num}", I18n.t("validations.sales.household.ecstat.student_16_19.must_be_student") + record.errors.add "age#{person_num}", I18n.t("validations.sales.household.age.student_16_19.cannot_be_16_19.child_not_student") + record.errors.add "relat#{person_num}", I18n.t("validations.sales.household.relat.student_16_19.cannot_be_child.16_19_not_student") end next unless !age_between_16_19 && student && child - record.errors.add "age#{person_num}", I18n.t("validations.household.age.student_16_19.must_be_16_19") - record.errors.add "ecstat#{person_num}", I18n.t("validations.household.ecstat.student_16_19.cannot_be_student.child_not_16_19") - record.errors.add "relat#{person_num}", I18n.t("validations.household.relat.student_16_19.cannot_be_child.student_not_16_19") + record.errors.add "age#{person_num}", I18n.t("validations.sales.household.age.student_16_19.must_be_16_19") + record.errors.add "ecstat#{person_num}", I18n.t("validations.sales.household.ecstat.student_16_19.cannot_be_student.child_not_16_19") + record.errors.add "relat#{person_num}", I18n.t("validations.sales.household.relat.student_16_19.cannot_be_child.student_not_16_19") end end @@ -78,12 +78,12 @@ module Validations::Sales::HouseholdValidations next unless age && economic_status if age < 16 && !economic_status_is_child_other_or_refused?(economic_status) && !record.form.start_year_after_2024? - record.errors.add "ecstat#{person_num}", I18n.t("validations.household.ecstat.child_under_16", person_num:) - record.errors.add "age#{person_num}", I18n.t("validations.household.age.child_under_16_ecstat", person_num:) + record.errors.add "ecstat#{person_num}", I18n.t("validations.sales.household.ecstat.child_under_16", person_num:) + record.errors.add "age#{person_num}", I18n.t("validations.sales.household.age.child_under_16_ecstat", person_num:) end if person_is_economic_child?(economic_status) && age > 16 - record.errors.add "ecstat#{person_num}", I18n.t("validations.household.ecstat.child_over_16", person_num:) - record.errors.add "age#{person_num}", I18n.t("validations.household.age.child_over_16", person_num:) + record.errors.add "ecstat#{person_num}", I18n.t("validations.sales.household.ecstat.child_over_16", person_num:) + record.errors.add "age#{person_num}", I18n.t("validations.sales.household.age.child_over_16", person_num:) end end end @@ -99,17 +99,17 @@ module Validations::Sales::HouseholdValidations next unless person_age > buyer_1_age - 12 && person_is_child?(relationship) - record.errors.add "age1", I18n.t("validations.household.age.child_12_years_younger") - record.errors.add "age#{person_num}", I18n.t("validations.household.age.child_12_years_younger") - record.errors.add "relat#{person_num}", I18n.t("validations.household.age.child_12_years_younger") + record.errors.add "age1", I18n.t("validations.sales.household.age1.child_12_years_younger") + record.errors.add "age#{person_num}", I18n.t("validations.sales.household.age.child_12_years_younger") + record.errors.add "relat#{person_num}", I18n.t("validations.sales.household.relat.child_12_years_younger") end end def validate_buyer_not_child(record) return unless record.saledate && record.form.start_year_after_2024? - record.errors.add "ecstat1", I18n.t("validations.household.ecstat.buyer_cannot_be_child", buyer_index: "1") if person_is_economic_child?(record.ecstat1) - record.errors.add "ecstat2", I18n.t("validations.household.ecstat.buyer_cannot_be_child", buyer_index: "2") if person_is_economic_child?(record.ecstat2) && record.joint_purchase? + record.errors.add "ecstat1", I18n.t("validations.sales.household.ecstat1.buyer_cannot_be_child") if person_is_economic_child?(record.ecstat1) + record.errors.add "ecstat2", I18n.t("validations.sales.household.ecstat2.buyer_cannot_be_child") if person_is_economic_child?(record.ecstat2) && record.joint_purchase? end private diff --git a/app/models/validations/shared_validations.rb b/app/models/validations/shared_validations.rb index 217b2c170..a2ae916c1 100644 --- a/app/models/validations/shared_validations.rb +++ b/app/models/validations/shared_validations.rb @@ -131,7 +131,11 @@ module Validations::SharedValidations partner_numbers = (2..max_people).select { |n| person_is_partner?(record["relat#{n}"]) } if partner_numbers.count > 1 partner_numbers.each do |n| - record.errors.add "relat#{n}", I18n.t("validations.household.relat.one_partner") + if record.sales? + record.errors.add "relat#{n}", I18n.t("validations.sales.household.relat.one_partner") + else + record.errors.add "relat#{n}", I18n.t("validations.household.relat.one_partner") + end end end end diff --git a/app/services/bulk_upload/sales/year2024/row_parser.rb b/app/services/bulk_upload/sales/year2024/row_parser.rb index fbc99ba02..35984a045 100644 --- a/app/services/bulk_upload/sales/year2024/row_parser.rb +++ b/app/services/bulk_upload/sales/year2024/row_parser.rb @@ -1469,10 +1469,10 @@ private def validate_buyer1_economic_status if field_35 == 9 if field_31.present? && field_31.to_i >= 16 - errors.add(:field_35, I18n.t("validations.household.ecstat.buyer_cannot_be_over_16_and_child", buyer_index: "1")) - errors.add(:field_31, I18n.t("validations.household.ecstat.buyer_cannot_be_over_16_and_child", buyer_index: "1")) + errors.add(:field_35, I18n.t("validations.sales.household.ecstat.buyer_cannot_be_over_16_and_child", buyer_index: "1")) + errors.add(:field_31, I18n.t("validations.sales.household.ecstat.buyer_cannot_be_over_16_and_child", buyer_index: "1")) else - errors.add(:field_35, I18n.t("validations.household.ecstat.buyer_cannot_be_child", buyer_index: "1")) + errors.add(:field_35, I18n.t("validations.sales.household.ecstat1.buyer_cannot_be_child")) end end end @@ -1482,10 +1482,10 @@ private if field_42 == 9 if field_38.present? && field_38.to_i >= 16 - errors.add(:field_42, I18n.t("validations.household.ecstat.buyer_cannot_be_over_16_and_child", buyer_index: "2")) - errors.add(:field_38, I18n.t("validations.household.ecstat.buyer_cannot_be_over_16_and_child", buyer_index: "2")) + errors.add(:field_42, I18n.t("validations.sales.household.ecstat.buyer_cannot_be_over_16_and_child", buyer_index: "2")) + errors.add(:field_38, I18n.t("validations.sales.household.ecstat.buyer_cannot_be_over_16_and_child", buyer_index: "2")) else - errors.add(:field_42, I18n.t("validations.household.ecstat.buyer_cannot_be_child", buyer_index: "2")) + errors.add(:field_42, I18n.t("validations.sales.household.ecstat2.buyer_cannot_be_child")) end end end diff --git a/config/locales/en.yml b/config/locales/en.yml index b39183591..7cff7aae7 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -501,11 +501,8 @@ en: retired_female: "A female tenant who is retired must be 60 or over." retired_over_70: "Answer cannot be over 70 as person %{person_num} has economic status that is not ‘retired’." child_under_16_relat_lettings: "Answer cannot be under 16 as person %{person_num}'s relationship to the lead tenant is ‘partner’." - child_under_16_relat_sales: "Answer cannot be under 16 as person %{person_num}'s relationship to buyer 1 is ‘partner’." child_under_16_ecstat: "Answer cannot be under 16 as person %{person_num}’s working situation is not ‘child under 16’, ‘other’ or ‘prefers not to say’." child_over_16: "Answer cannot be over 16 as person’s %{person_num} working situation is ‘child under 16‘." - child_over_20: "Answer cannot be 20 or over as the relationship is ‘child’." - child_12_years_younger: "A child must be at least 12 years younger than their parent." not_student_16_19: "Answer cannot be between 16 and 19 as person %{person_num} is a child of the lead tenant but is not a full-time student." student_16_19: cannot_be_16_19: @@ -528,12 +525,8 @@ en: retired_female: "Answer cannot be ‘retired’ as the female tenant is under 60." not_child_16_19: cannot_be_student: "Person cannot be a student if they are aged 16-19 but are not a child." - buyer_cannot_be_child: "Buyer %{buyer_index} cannot have a working situation of child under 16." - buyer_cannot_be_over_16_and_child: "Buyer %{buyer_index}'s age cannot be 16 or over if their working situation is child under 16." relat: - child_under_16_sales: "Answer cannot be ‘partner’ as you told us person %{person_num}'s age is under 16." child_under_16_lettings: "Answer cannot be ‘partner’ as you told us person %{person_num}'s age is under 16." - child_over_20: "Answer cannot be ‘child’ if the person's age is 20 or over." one_partner: "Number of partners cannot be greater than 1." not_student_16_19: "Answer cannot be ‘child’ as you told us the person %{person_num} is between 16 and 19 and is not a full-time student." student_16_19: @@ -556,7 +549,6 @@ en: internal_transfer: "Answer cannot be %{prevten} as this tenancy is an internal transfer." la_general_needs: internal_transfer: "Answer cannot be a fixed-term or lifetime local authority general needs tenancy as it’s an internal transfer and a private registered provider is on the tenancy agreement." - invalid_for_discounted_sale: "Buyer 1’s previous tenure should be “local authority tenant” or “private registered provider or housing association tenant” for discounted sales." referral: secure_tenancy: "Answer must be internal transfer as this is a secure tenancy." rsnvac_non_temp: "Answer cannot be this source of referral as this is a re-let to tenant who occupied the same property as temporary accommodation." @@ -596,9 +588,6 @@ en: no_choices: "You cannot answer this question as you told us nobody in the household has a physical or mental health condition (or other illness) expected to last 12 months or more." postcode: discounted_ownership: "Last settled accommodation and discounted ownership property postcodes must match." - buylivein: - buyers_will_live_in_property_values_inconsistent_setup: "You have already told us that both buyer 1 and buyer 2 will not live in the property." - buyers_will_live_in_property_values_inconsistent: "You have already told us that the buyers will live in the property. Either buyer 1 or buyer 2 must live in the property." nationality: "Select a valid nationality." tenancy: diff --git a/config/locales/validations/sales/household.en.yml b/config/locales/validations/sales/household.en.yml new file mode 100644 index 000000000..c1f90971c --- /dev/null +++ b/config/locales/validations/sales/household.en.yml @@ -0,0 +1,47 @@ +en: + validations: + sales: + household: + buylivein: + buyers_will_live_in_property_values_inconsistent: "You have already told us that both buyer 1 and buyer 2 will not live in the property." + buy1livein: + buyers_will_live_in_property_values_inconsistent: "You have already told us that the buyers will live in the property. Either buyer 1 or buyer 2 must live in the property." + buy2livein: + buyers_will_live_in_property_values_inconsistent: "You have already told us that the buyers will live in the property. Either buyer 1 or buyer 2 must live in the property." + ownershipsch: + prevten_invalid_for_discounted_sale: "Buyer 1’s previous tenure should be “local authority tenant” or “private registered provider or housing association tenant” for discounted sales." + prevten: + prevten_invalid_for_discounted_sale: "Buyer 1’s previous tenure should be “local authority tenant” or “private registered provider or housing association tenant” for discounted sales." + age1: + child_12_years_younger: "A child must be at least 12 years younger than their parent." + ecstat1: + buyer_cannot_be_child: "Buyer 1 cannot have a working situation of child under 16." + ecstat2: + buyer_cannot_be_child: "Buyer 2 cannot have a working situation of child under 16." + age: + child_12_years_younger: "A child must be at least 12 years younger than their parent." + child_under_16: "Answer cannot be under 16 as person %{person_num}'s relationship to buyer 1 is ‘partner’." + child_under_16_ecstat: "Answer cannot be under 16 as person %{person_num}’s working situation is not ‘child under 16’, ‘other’ or ‘prefers not to say’." + child_over_16: "Answer cannot be over 16 as person’s %{person_num} working situation is ‘child under 16‘." + child_over_20: "Answer cannot be 20 or over as the relationship is ‘child’." + student_16_19: + cannot_be_16_19: + child_not_student: "Person cannot be aged 16-19 if they have relationship ‘child’ but are not a student." + must_be_16_19: "Person must be aged 16-19 if they are a student and have relationship ‘child’." + relat: + one_partner: "Number of partners cannot be greater than 1." + child_12_years_younger: "A child must be at least 12 years younger than their parent." + child_under_16: "Answer cannot be ‘partner’ as you told us person %{person_num}'s age is under 16." + child_over_20: "Answer cannot be ‘child’ if the person's age is 20 or over." + student_16_19: + cannot_be_child: + student_not_16_19: "Answer cannot be ‘child’ if the person is a student but not aged 16-19." + 16_19_not_student: "Answer cannot be ‘child’ if the person is aged 16-19 but not a student." + ecstat: + child_under_16: "Person %{person_num}’s working situation must be ‘child under 16’, ‘other’ or ‘prefers not to say’ as you told us they’re under 16." + child_over_16: "Answer cannot be ‘child under 16’ as you told us the person %{person_num} is older than 16." + student_16_19: + must_be_student: "Person must be a student if they are aged 16-19 and have relationship ‘child’." + cannot_be_student: + child_not_16_19: "Person cannot be a student if they are not aged 16-19 but have relationship ‘child’." + buyer_cannot_be_over_16_and_child: "Buyer %{buyer_index}'s age cannot be 16 or over if their working situation is child under 16." diff --git a/spec/models/validations/sales/household_validations_spec.rb b/spec/models/validations/sales/household_validations_spec.rb index 510681420..cffb42ab1 100644 --- a/spec/models/validations/sales/household_validations_spec.rb +++ b/spec/models/validations/sales/household_validations_spec.rb @@ -15,11 +15,11 @@ RSpec.describe Validations::Sales::HouseholdValidations do record.relat3 = "P" household_validator.validate_partner_count(record) expect(record.errors["relat2"]) - .to include(match I18n.t("validations.household.relat.one_partner")) + .to include(match I18n.t("validations.sales.household.relat.one_partner")) expect(record.errors["relat3"]) - .to include(match I18n.t("validations.household.relat.one_partner")) + .to include(match I18n.t("validations.sales.household.relat.one_partner")) expect(record.errors["relat4"]) - .not_to include(match I18n.t("validations.household.relat.one_partner")) + .not_to include(match I18n.t("validations.sales.household.relat.one_partner")) end it "expects that a tenant can have a partner" do @@ -47,9 +47,9 @@ RSpec.describe Validations::Sales::HouseholdValidations do record.relat2 = "P" household_validator.validate_person_age_matches_relationship(record) expect(record.errors["relat2"]) - .to include(match I18n.t("validations.household.relat.child_under_16_sales", person_num: 2)) + .to include(match I18n.t("validations.sales.household.relat.child_under_16", person_num: 2)) expect(record.errors["age2"]) - .to include(match I18n.t("validations.household.age.child_under_16_relat_sales", person_num: 2)) + .to include(match I18n.t("validations.sales.household.age.child_under_16", person_num: 2)) end end @@ -58,9 +58,9 @@ RSpec.describe Validations::Sales::HouseholdValidations do record.relat2 = "C" household_validator.validate_person_age_matches_relationship(record) expect(record.errors["relat2"]) - .to include(match I18n.t("validations.household.relat.child_over_20")) + .to include(match I18n.t("validations.sales.household.relat.child_over_20")) expect(record.errors["age2"]) - .to include(match I18n.t("validations.household.age.child_over_20")) + .to include(match I18n.t("validations.sales.household.age.child_over_20")) end end @@ -94,9 +94,9 @@ RSpec.describe Validations::Sales::HouseholdValidations do record.ecstat2 = 1 household_validator.validate_person_age_matches_economic_status(record) expect(record.errors["ecstat2"]) - .to include(match I18n.t("validations.household.ecstat.child_under_16", person_num: 2)) + .to include(match I18n.t("validations.sales.household.ecstat.child_under_16", person_num: 2)) expect(record.errors["age2"]) - .to include(match I18n.t("validations.household.age.child_under_16_ecstat", person_num: 2)) + .to include(match I18n.t("validations.sales.household.age.child_under_16_ecstat", person_num: 2)) end it "expects that person's economic status is Child" do @@ -112,9 +112,9 @@ RSpec.describe Validations::Sales::HouseholdValidations do record.ecstat2 = 9 household_validator.validate_person_age_matches_economic_status(record) expect(record.errors["ecstat2"]) - .to include(match I18n.t("validations.household.ecstat.child_over_16", person_num: 2)) + .to include(match I18n.t("validations.sales.household.ecstat.child_over_16", person_num: 2)) expect(record.errors["age2"]) - .to include(match I18n.t("validations.household.age.child_over_16", person_num: 2)) + .to include(match I18n.t("validations.sales.household.age.child_over_16", person_num: 2)) end end @@ -126,9 +126,9 @@ RSpec.describe Validations::Sales::HouseholdValidations do record.ecstat2 = 1 household_validator.validate_person_age_matches_economic_status(record) expect(record.errors["ecstat2"]) - .not_to include(match I18n.t("validations.household.ecstat.child_under_16", person_num: 2)) + .not_to include(match I18n.t("validations.sales.household.ecstat.child_under_16", person_num: 2)) expect(record.errors["age2"]) - .not_to include(match I18n.t("validations.household.age.child_under_16_ecstat", person_num: 2)) + .not_to include(match I18n.t("validations.sales.household.age.child_under_16_ecstat", person_num: 2)) end end end @@ -143,11 +143,11 @@ RSpec.describe Validations::Sales::HouseholdValidations do record.relat2 = "C" household_validator.validate_child_12_years_younger(record) expect(record.errors["age1"]) - .to include(match I18n.t("validations.household.age.child_12_years_younger", person_num: 2)) + .to include(match I18n.t("validations.sales.household.age.child_12_years_younger", person_num: 2)) expect(record.errors["age2"]) - .to include(match I18n.t("validations.household.age.child_12_years_younger", person_num: 2)) + .to include(match I18n.t("validations.sales.household.age.child_12_years_younger", person_num: 2)) expect(record.errors["relat2"]) - .to include(match I18n.t("validations.household.age.child_12_years_younger", person_num: 2)) + .to include(match I18n.t("validations.sales.household.age.child_12_years_younger", person_num: 2)) end it "expects the child is at least 12 years younger than buyer 1" do @@ -206,11 +206,11 @@ RSpec.describe Validations::Sales::HouseholdValidations do record.relat2 = "C" household_validator.validate_person_age_and_relationship_matches_economic_status(record) expect(record.errors["relat2"]) - .to include(match I18n.t("validations.household.relat.student_16_19.cannot_be_child.16_19_not_student")) + .to include(match I18n.t("validations.sales.household.relat.student_16_19.cannot_be_child.16_19_not_student")) expect(record.errors["age2"]) - .to include(match I18n.t("validations.household.age.student_16_19.cannot_be_16_19.child_not_student")) + .to include(match I18n.t("validations.sales.household.age.student_16_19.cannot_be_16_19.child_not_student")) expect(record.errors["ecstat2"]) - .to include(match I18n.t("validations.household.ecstat.student_16_19.must_be_student")) + .to include(match I18n.t("validations.sales.household.ecstat.student_16_19.must_be_student")) end it "adds errors for a person who is a child of the buyer and a student but not aged 16-19" do @@ -219,11 +219,11 @@ RSpec.describe Validations::Sales::HouseholdValidations do record.relat2 = "C" household_validator.validate_person_age_and_relationship_matches_economic_status(record) expect(record.errors["relat2"]) - .to include(match I18n.t("validations.household.relat.student_16_19.cannot_be_child.student_not_16_19")) + .to include(match I18n.t("validations.sales.household.relat.student_16_19.cannot_be_child.student_not_16_19")) expect(record.errors["age2"]) - .to include(match I18n.t("validations.household.age.student_16_19.must_be_16_19")) + .to include(match I18n.t("validations.sales.household.age.student_16_19.must_be_16_19")) expect(record.errors["ecstat2"]) - .to include(match I18n.t("validations.household.ecstat.student_16_19.cannot_be_student.child_not_16_19")) + .to include(match I18n.t("validations.sales.household.ecstat.student_16_19.cannot_be_student.child_not_16_19")) end end @@ -316,9 +316,9 @@ RSpec.describe Validations::Sales::HouseholdValidations do it "triggers a validation if buyer two will also not live in the property" do sales_log.buy2livein = 2 household_validator.validate_buyers_living_in_property(sales_log) - expect(sales_log.errors[:buylivein]).to include I18n.t("validations.household.buylivein.buyers_will_live_in_property_values_inconsistent_setup") - expect(sales_log.errors[:buy2livein]).to include I18n.t("validations.household.buylivein.buyers_will_live_in_property_values_inconsistent") - expect(sales_log.errors[:buy1livein]).to include I18n.t("validations.household.buylivein.buyers_will_live_in_property_values_inconsistent") + expect(sales_log.errors[:buylivein]).to include I18n.t("validations.sales.household.buylivein.buyers_will_live_in_property_values_inconsistent") + expect(sales_log.errors[:buy2livein]).to include I18n.t("validations.sales.household.buy2livein.buyers_will_live_in_property_values_inconsistent") + expect(sales_log.errors[:buy1livein]).to include I18n.t("validations.sales.household.buy1livein.buyers_will_live_in_property_values_inconsistent") end end end From 9bd3389c846243bf9ba1bb81334661f2c5ff53ba Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Tue, 22 Oct 2024 16:27:51 +0100 Subject: [PATCH 4/5] CLDC-3655 Extract property information validations (#2700) * Extract property information validations * Refactor possesive variable from errors --- app/models/form/sales/questions/uprn.rb | 2 +- app/models/form/sales/questions/uprn_known.rb | 2 +- .../validations/sales/property_validations.rb | 15 ++++++----- config/locales/en.yml | 6 ----- .../sales/property_information.en.yml | 27 +++++++++++++++++++ 5 files changed, 37 insertions(+), 15 deletions(-) create mode 100644 config/locales/validations/sales/property_information.en.yml diff --git a/app/models/form/sales/questions/uprn.rb b/app/models/form/sales/questions/uprn.rb index 75a41a0ec..94aaccb43 100644 --- a/app/models/form/sales/questions/uprn.rb +++ b/app/models/form/sales/questions/uprn.rb @@ -16,7 +16,7 @@ class Form::Sales::Questions::Uprn < ::Form::Question end def unanswered_error_message - I18n.t("validations.property.uprn.invalid") + I18n.t("validations.sales.property_information.uprn.invalid") end def get_extra_check_answer_value(log) diff --git a/app/models/form/sales/questions/uprn_known.rb b/app/models/form/sales/questions/uprn_known.rb index 44ce03cad..fdd0c9e9a 100644 --- a/app/models/form/sales/questions/uprn_known.rb +++ b/app/models/form/sales/questions/uprn_known.rb @@ -31,7 +31,7 @@ class Form::Sales::Questions::UprnKnown < ::Form::Question }.freeze def unanswered_error_message - I18n.t("validations.property.uprn_known.invalid") + I18n.t("validations.sales.property_information.uprn_known.invalid") end QUESTION_NUMBER_FROM_YEAR = { 2023 => 14, 2024 => 15 }.freeze diff --git a/app/models/validations/sales/property_validations.rb b/app/models/validations/sales/property_validations.rb index 50818c422..59d616c9a 100644 --- a/app/models/validations/sales/property_validations.rb +++ b/app/models/validations/sales/property_validations.rb @@ -4,10 +4,11 @@ module Validations::Sales::PropertyValidations return unless record.ppostcode_full.present? && record.postcode_full.present? if record.discounted_ownership_sale? && record.ppostcode_full != record.postcode_full - record.errors.add :postcode_full, I18n.t("validations.property.postcode.must_match_previous", buyer_possessive: record.joint_purchase? ? "Buyers’" : "Buyer’s") - record.errors.add :ppostcode_full, I18n.t("validations.property.postcode.must_match_previous", buyer_possessive: record.joint_purchase? ? "Buyers’" : "Buyer’s") - record.errors.add :ownershipsch, I18n.t("validations.property.postcode.must_match_previous", buyer_possessive: record.joint_purchase? ? "Buyers’" : "Buyer’s") - record.errors.add :uprn, I18n.t("validations.property.postcode.must_match_previous", buyer_possessive: record.joint_purchase? ? "Buyers’" : "Buyer’s") + joint_purchase_id = record.joint_purchase? ? "joint_purchase" : "not_joint_purchase" + record.errors.add :postcode_full, I18n.t("validations.sales.property_information.postcode_full.postcode_must_match_previous.#{joint_purchase_id}") + record.errors.add :ppostcode_full, I18n.t("validations.sales.property_information.ppostcode_full.postcode_must_match_previous.#{joint_purchase_id}") + record.errors.add :ownershipsch, I18n.t("validations.sales.property_information.ownershipsch.postcode_must_match_previous.#{joint_purchase_id}") + record.errors.add :uprn, I18n.t("validations.sales.property_information.uprn.postcode_must_match_previous.#{joint_purchase_id}") end end @@ -15,8 +16,8 @@ module Validations::Sales::PropertyValidations return unless record.proptype.present? && record.beds.present? if record.is_bedsit? && record.beds > 1 - record.errors.add :proptype, I18n.t("validations.property.proptype.bedsits_have_max_one_bedroom") - record.errors.add :beds, I18n.t("validations.property.beds.bedsits_have_max_one_bedroom") + record.errors.add :proptype, I18n.t("validations.sales.property_information.proptype.bedsits_have_max_one_bedroom") + record.errors.add :beds, I18n.t("validations.sales.property_information.beds.bedsits_have_max_one_bedroom") end end @@ -25,6 +26,6 @@ module Validations::Sales::PropertyValidations return if record.uprn.match?(/^[0-9]{1,12}$/) - record.errors.add :uprn, I18n.t("validations.property.uprn.invalid") + record.errors.add :uprn, I18n.t("validations.sales.property_information.uprn.invalid") end end diff --git a/config/locales/en.yml b/config/locales/en.yml index 7cff7aae7..ff73bfdcb 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -382,12 +382,6 @@ en: one_bedroom_bedsit: "A bedsit can only have one bedroom." one_seven_bedroom_shared: "A shared house must have 1 to 7 bedrooms." one_three_bedroom_single_tenant_shared: "A shared house with fewer than two tenants must have 1 to 3 bedrooms." - beds: - bedsits_have_max_one_bedroom: "Number of bedrooms must be 1 if the property is a bedsit." - proptype: - bedsits_have_max_one_bedroom: "Answer cannot be 'Bedsit' if the property has 2 or more bedrooms." - postcode: - must_match_previous: "%{buyer_possessive} last accommodation and discounted ownership postcodes must match." financial: tshortfall: diff --git a/config/locales/validations/sales/property_information.en.yml b/config/locales/validations/sales/property_information.en.yml new file mode 100644 index 000000000..e69f90edf --- /dev/null +++ b/config/locales/validations/sales/property_information.en.yml @@ -0,0 +1,27 @@ +en: + validations: + sales: + property_information: + postcode_full: + postcode_must_match_previous: + joint_purchase: "Buyers’ last accommodation and discounted ownership postcodes must match." + not_joint_purchase: "Buyer’s last accommodation and discounted ownership postcodes must match." + ppostcode_full: + postcode_must_match_previous: + joint_purchase: "Buyers’ last accommodation and discounted ownership postcodes must match." + not_joint_purchase: "Buyer’s last accommodation and discounted ownership postcodes must match." + ownershipsch: + postcode_must_match_previous: + joint_purchase: "Buyers’ last accommodation and discounted ownership postcodes must match." + not_joint_purchase: "Buyer’s last accommodation and discounted ownership postcodes must match." + uprn: + postcode_must_match_previous: + joint_purchase: "Buyers’ last accommodation and discounted ownership postcodes must match." + not_joint_purchase: "Buyer’s last accommodation and discounted ownership postcodes must match." + invalid: "UPRN must be 12 digits or less." + beds: + bedsits_have_max_one_bedroom: "Number of bedrooms must be 1 if the property is a bedsit." + proptype: + bedsits_have_max_one_bedroom: "Answer cannot be 'Bedsit' if the property has 2 or more bedrooms." + uprn_known: + invalid: "You must answer UPRN known?" From e71b2aeb9f687c9ac41d2b3e5993d5867a6f4690 Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Wed, 23 Oct 2024 10:21:41 +0100 Subject: [PATCH 5/5] CLDC-3638 Allow adding custom resources (#2690) * Stub some collection resource requests * Rebase changes * Add upload new resource page * Allow adding custom collection resources * Display and validate additional resources * Allow downloading additional collection resources * Allow updating additional resources * Display additional resources on the homepage * Add delete link to additional resources * Update hint text * Update tests * Allow unauthenticated users to donwload additional collection resources * Add some changes to copy and styling * Add content type to the files * Validate short display name at the same time as file --- .../collection_resources_controller.rb | 122 +++++-- app/controllers/start_controller.rb | 2 + app/helpers/collection_resources_helper.rb | 2 +- app/models/collection_resource.rb | 31 +- app/services/collection_resources_service.rb | 3 +- .../mandatory_collection_resources_service.rb | 1 + app/services/storage/local_disk_service.rb | 2 +- app/services/storage/s3_service.rb | 21 +- .../_collection_resource_summary_list.erb | 19 +- app/views/collection_resources/edit.html.erb | 21 +- app/views/collection_resources/index.html.erb | 4 +- app/views/collection_resources/new.html.erb | 36 ++ .../layouts/_collection_resources.html.erb | 4 +- config/locales/en.yml | 7 +- config/routes.rb | 8 +- spec/factories/collection_resource.rb | 10 + spec/features/collection_resources_spec.rb | 73 ++++ .../collection_resources_helper_spec.rb | 12 +- .../collection_resources_controller_spec.rb | 342 +++++++++++++++++- spec/requests/start_controller_spec.rb | 2 + .../collection_resources_service_spec.rb | 2 +- 21 files changed, 668 insertions(+), 56 deletions(-) create mode 100644 app/views/collection_resources/new.html.erb diff --git a/app/controllers/collection_resources_controller.rb b/app/controllers/collection_resources_controller.rb index ed080c749..5203ceea1 100644 --- a/app/controllers/collection_resources_controller.rb +++ b/app/controllers/collection_resources_controller.rb @@ -1,13 +1,15 @@ class CollectionResourcesController < ApplicationController include CollectionResourcesHelper - before_action :authenticate_user!, except: %i[download_mandatory_collection_resource] + before_action :authenticate_user!, except: %i[download_mandatory_collection_resource download_additional_collection_resource] def index render_not_found unless current_user.support? @mandatory_lettings_collection_resources_per_year = MandatoryCollectionResourcesService.generate_resources("lettings", editable_collection_resource_years) @mandatory_sales_collection_resources_per_year = MandatoryCollectionResourcesService.generate_resources("sales", editable_collection_resource_years) + @additional_lettings_collection_resources_per_year = CollectionResource.where(log_type: "lettings", mandatory: false).group_by(&:year) + @additional_sales_collection_resources_per_year = CollectionResource.where(log_type: "sales", mandatory: false).group_by(&:year) end def download_mandatory_collection_resource @@ -23,7 +25,16 @@ class CollectionResourcesController < ApplicationController download_resource(resource.download_filename) end - def edit + def download_additional_collection_resource + resource = CollectionResource.find_by(id: params[:collection_resource_id]) + + return render_not_found unless resource + return render_not_found unless resource_for_year_can_be_downloaded?(resource.year) + + download_resource(resource.download_filename) + end + + def edit_mandatory_collection_resource return render_not_found unless current_user.support? year = params[:year].to_i @@ -39,7 +50,18 @@ class CollectionResourcesController < ApplicationController render "collection_resources/edit" end - def update + def edit_additional_collection_resource + return render_not_found unless current_user.support? + + @collection_resource = CollectionResource.find_by(id: params[:collection_resource_id]) + + return render_not_found unless @collection_resource + return render_not_found unless resource_for_year_can_be_updated?(@collection_resource.year) + + render "collection_resources/edit" + end + + def update_mandatory_collection_resource return render_not_found unless current_user.support? year = resource_params[:year].to_i @@ -52,7 +74,8 @@ class CollectionResourcesController < ApplicationController @collection_resource = MandatoryCollectionResourcesService.generate_resource(log_type, year, resource_type) render_not_found unless @collection_resource - validate_file(file) + @collection_resource.file = file + @collection_resource.validate_attached_file return render "collection_resources/edit" if @collection_resource.errors.any? @@ -68,6 +91,36 @@ class CollectionResourcesController < ApplicationController redirect_to collection_resources_path end + def update_additional_collection_resource + return render_not_found unless current_user.support? + + @collection_resource = CollectionResource.find_by(id: params[:collection_resource_id]) + + return render_not_found unless @collection_resource + return render_not_found unless resource_for_year_can_be_updated?(@collection_resource.year) + + @collection_resource.file = resource_params[:file] + @collection_resource.validate_attached_file + @collection_resource.validate_short_display_name + return render "collection_resources/edit" if @collection_resource.errors.any? + + @collection_resource.short_display_name = resource_params[:short_display_name] + @collection_resource.download_filename = @collection_resource.file&.original_filename + @collection_resource.display_name = "#{@collection_resource.log_type} #{@collection_resource.short_display_name} (#{text_year_range_format(@collection_resource.year)})" + if @collection_resource.save + begin + CollectionResourcesService.new.upload_collection_resource(@collection_resource.download_filename, @collection_resource.file) + flash[:notice] = "The #{@collection_resource.log_type} #{text_year_range_format(@collection_resource.year)} #{@collection_resource.short_display_name.downcase} has been updated." + redirect_to collection_resources_path + rescue StandardError + @collection_resource.errors.add(:file, :error_uploading) + render "collection_resources/edit" + end + else + render "collection_resources/edit" + end + end + def confirm_mandatory_collection_resources_release return render_not_found unless current_user.support? @@ -91,10 +144,50 @@ class CollectionResourcesController < ApplicationController redirect_to collection_resources_path end + def new + return render_not_found unless current_user.support? + + year = params[:year].to_i + log_type = params[:log_type] + + return render_not_found unless editable_collection_resource_years.include?(year) + + @collection_resource = CollectionResource.new(year:, log_type:) + end + + def create + return render_not_found unless current_user.support? && editable_collection_resource_years.include?(resource_params[:year].to_i) + + @collection_resource = CollectionResource.new(resource_params) + @collection_resource.download_filename ||= @collection_resource.file&.original_filename + @collection_resource.display_name = "#{@collection_resource.log_type} #{@collection_resource.short_display_name} (#{text_year_range_format(@collection_resource.year)})" + + @collection_resource.validate_attached_file + @collection_resource.validate_short_display_name + return render "collection_resources/new" if @collection_resource.errors.any? + + if @collection_resource.save + begin + CollectionResourcesService.new.upload_collection_resource(@collection_resource.download_filename, @collection_resource.file) + flash[:notice] = if displayed_collection_resource_years.include?(@collection_resource.year) + "The #{@collection_resource.log_type} #{text_year_range_format(@collection_resource.year)} #{@collection_resource.short_display_name} is now available to users." + else + "The #{@collection_resource.log_type} #{text_year_range_format(@collection_resource.year)} #{@collection_resource.short_display_name} has been uploaded." + end + redirect_to collection_resources_path + rescue StandardError + @collection_resource.errors.add(:file, :error_uploading) + render "collection_resources/new" + end + else + render "collection_resources/new" + end + end + private def resource_params - params.require(:collection_resource).permit(:year, :log_type, :resource_type, :file) + params.require(:collection_resource).permit(:year, :log_type, :resource_type, :file, :mandatory, :short_display_name) end def download_resource(filename) @@ -113,23 +206,4 @@ private def resource_for_year_can_be_updated?(year) editable_collection_resource_years.include?(year) end - - def validate_file(file) - return @collection_resource.errors.add(:file, :blank) unless file - return @collection_resource.errors.add(:file, :above_100_mb) if file.size > 100.megabytes - - argv = %W[file --brief --mime-type -- #{file.path}] - output = `#{argv.shelljoin}` - - case @collection_resource.resource_type - when "paper_form" - unless output.match?(/application\/pdf/) - @collection_resource.errors.add(:file, :must_be_pdf) - end - when "bulk_upload_template", "bulk_upload_specification" - unless output.match?(/application\/vnd\.ms-excel|application\/vnd\.openxmlformats-officedocument\.spreadsheetml\.sheet/) - @collection_resource.errors.add(:file, :must_be_xlsx, resource: @collection_resource.short_display_name.downcase) - end - end - end end diff --git a/app/controllers/start_controller.rb b/app/controllers/start_controller.rb index 5bd49df3f..b65da4d44 100644 --- a/app/controllers/start_controller.rb +++ b/app/controllers/start_controller.rb @@ -4,6 +4,8 @@ class StartController < ApplicationController def index @mandatory_lettings_collection_resources_per_year = MandatoryCollectionResourcesService.generate_resources("lettings", displayed_collection_resource_years) @mandatory_sales_collection_resources_per_year = MandatoryCollectionResourcesService.generate_resources("sales", displayed_collection_resource_years) + @additional_lettings_collection_resources_per_year = CollectionResource.where(log_type: "lettings", mandatory: false, year: displayed_collection_resource_years).group_by(&:year) + @additional_sales_collection_resources_per_year = CollectionResource.where(log_type: "sales", mandatory: false, year: displayed_collection_resource_years).group_by(&:year) if current_user @homepage_presenter = HomepagePresenter.new(current_user) render "home/index" diff --git a/app/helpers/collection_resources_helper.rb b/app/helpers/collection_resources_helper.rb index 9deb13887..1a5887744 100644 --- a/app/helpers/collection_resources_helper.rb +++ b/app/helpers/collection_resources_helper.rb @@ -49,7 +49,7 @@ module CollectionResourcesHelper def document_list_component_items(resources) resources.map do |resource| { - name: "Download the #{resource.display_name}", + name: "Download the #{resource.display_name.downcase}", href: resource.download_path, metadata: file_type_size_and_pages(resource.download_filename), } diff --git a/app/models/collection_resource.rb b/app/models/collection_resource.rb index 81665ccf6..cc217b8cd 100644 --- a/app/models/collection_resource.rb +++ b/app/models/collection_resource.rb @@ -3,7 +3,36 @@ class CollectionResource < ApplicationRecord attr_accessor :file + validates :short_display_name, presence: true + def download_path - download_mandatory_collection_resource_path(log_type:, year:, resource_type:) + if mandatory + download_mandatory_collection_resource_path(log_type:, year:, resource_type:) + else + collection_resource_download_path(self) + end + end + + def validate_attached_file + return errors.add(:file, :blank) unless file + return errors.add(:file, :above_100_mb) if file.size > 100.megabytes + + argv = %W[file --brief --mime-type -- #{file.path}] + output = `#{argv.shelljoin}` + + case resource_type + when "paper_form" + unless output.match?(/application\/pdf/) + errors.add(:file, :must_be_pdf) + end + when "bulk_upload_template", "bulk_upload_specification" + unless output.match?(/application\/vnd\.ms-excel|application\/vnd\.openxmlformats-officedocument\.spreadsheetml\.sheet/) + errors.add(:file, :must_be_xlsx, resource: short_display_name.downcase) + end + end + end + + def validate_short_display_name + errors.add(:short_display_name, :blank) if short_display_name.blank? end end diff --git a/app/services/collection_resources_service.rb b/app/services/collection_resources_service.rb index 81ab08254..82247f7a0 100644 --- a/app/services/collection_resources_service.rb +++ b/app/services/collection_resources_service.rb @@ -24,6 +24,7 @@ class CollectionResourcesService end def upload_collection_resource(filename, file) - @storage_service.write_file(filename, file) + content_type = MiniMime.lookup_by_filename(filename)&.content_type + @storage_service.write_file(filename, file, content_type:) end end diff --git a/app/services/mandatory_collection_resources_service.rb b/app/services/mandatory_collection_resources_service.rb index 8b34153e9..397e4b5d0 100644 --- a/app/services/mandatory_collection_resources_service.rb +++ b/app/services/mandatory_collection_resources_service.rb @@ -27,6 +27,7 @@ class MandatoryCollectionResourcesService year:, log_type:, download_filename: download_filename(resource_type, year, log_type), + mandatory: true, ) end diff --git a/app/services/storage/local_disk_service.rb b/app/services/storage/local_disk_service.rb index bb5825340..ad3cc9608 100644 --- a/app/services/storage/local_disk_service.rb +++ b/app/services/storage/local_disk_service.rb @@ -19,7 +19,7 @@ module Storage File.open(path, "r") end - def write_file(filename, data) + def write_file(filename, data, _content_type: nil) path = Rails.root.join("tmp/storage", filename) FileUtils.mkdir_p(path.dirname) diff --git a/app/services/storage/s3_service.rb b/app/services/storage/s3_service.rb index 88199c0a0..2e8daa719 100644 --- a/app/services/storage/s3_service.rb +++ b/app/services/storage/s3_service.rb @@ -36,12 +36,21 @@ module Storage .body.read end - def write_file(file_name, data) - @client.put_object( - body: data, - bucket: @configuration.bucket_name, - key: file_name, - ) + def write_file(file_name, data, content_type: nil) + if content_type.nil? + @client.put_object( + body: data, + bucket: @configuration.bucket_name, + key: file_name, + ) + else + @client.put_object( + body: data, + bucket: @configuration.bucket_name, + key: file_name, + content_type:, + ) + end end def get_file_metadata(file_name) diff --git a/app/views/collection_resources/_collection_resource_summary_list.erb b/app/views/collection_resources/_collection_resource_summary_list.erb index 8ef588fd5..0e630f0aa 100644 --- a/app/views/collection_resources/_collection_resource_summary_list.erb +++ b/app/views/collection_resources/_collection_resource_summary_list.erb @@ -23,9 +23,26 @@ <% end %> <% end %> <% end %> + <% additional_resources&.each do |resource| %> + <% summary_list.with_row do |row| %> + <% row.with_key { resource.short_display_name } %> + <% row.with_value do %> + <%= render DocumentListComponent.new(items: document_list_edit_component_items([resource]), label: "") %> + <% end %> + <% row.with_action( + text: "Change", + href: collection_resource_edit_path(resource), + ) %> + <% row.with_action( + text: "Delete", + href: "/", + classes: "app-!-colour-red" + ) %> + <% end %> + <% end %> <% end %>
- <%= govuk_link_to "Add new #{mandatory_resources.first.log_type} #{text_year_range_format(mandatory_resources.first.year)} resource", href: "/" %> + <%= govuk_link_to "Add new #{mandatory_resources.first.log_type} #{text_year_range_format(mandatory_resources.first.year)} resource", href: new_collection_resource_path(year: mandatory_resources.first.year, log_type: mandatory_resources.first.log_type) %>
diff --git a/app/views/collection_resources/edit.html.erb b/app/views/collection_resources/edit.html.erb index e5b2a6d33..6100256c8 100644 --- a/app/views/collection_resources/edit.html.erb +++ b/app/views/collection_resources/edit.html.erb @@ -5,7 +5,7 @@
<% resource_exists = file_exists_on_s3?(@collection_resource.download_filename) %> - <%= form_with model: @collection_resource, url: update_mandatory_collection_resource_path, method: :patch do |f| %> + <%= form_with model: @collection_resource, url: @collection_resource.mandatory ? update_mandatory_collection_resource_path : collection_resource_update_path(@collection_resource), method: :patch do |f| %> <%= f.hidden_field :year %> <%= f.hidden_field :log_type %> <%= f.hidden_field :resource_type %> @@ -13,7 +13,7 @@ <%= f.govuk_error_summary %> <%= "#{@collection_resource.log_type.humanize} #{text_year_range_format(@collection_resource.year)}" %> -

<%= resource_exists ? "Change" : "Upload" %> the <%= @collection_resource.resource_type.humanize.downcase %>

+

<%= resource_exists ? "Change" : "Upload" %> the <%= @collection_resource.short_display_name.downcase %>

This file will be available for all users to download. @@ -21,9 +21,22 @@ <%= f.govuk_file_field :file, label: { text: "Upload file", size: "m" } %> + <% if resource_exists %> +

Current file: <%= govuk_link_to @collection_resource.download_filename, href: @collection_resource.download_path %>

+ <% end %> - <%= f.govuk_submit resource_exists ? "Save changes" : "Upload" %> - <%= govuk_button_link_to "Cancel", collection_resources_path, secondary: true %> + <% unless @collection_resource.mandatory %> + <%= f.govuk_text_field :short_display_name, + label: { text: "Resource type", size: "m" }, + hint: { text: safe_join(["This will be used in the download link on the homepage. Do not include the log type or collection year.", + content_tag(:br), + "For example, if you enter “bulk upload change log”, the download link will say “Download the #{@collection_resource.log_type} bulk upload change log (#{text_year_range_format(@collection_resource.year)})”."]) } %> + <% end %> + +
+ <%= f.govuk_submit resource_exists ? "Save changes" : "Upload" %> + <%= govuk_button_link_to "Cancel", collection_resources_path, secondary: true %> +
<% end %>
diff --git a/app/views/collection_resources/index.html.erb b/app/views/collection_resources/index.html.erb index f6785eb46..207a63991 100644 --- a/app/views/collection_resources/index.html.erb +++ b/app/views/collection_resources/index.html.erb @@ -18,12 +18,12 @@

Lettings <%= text_year_range_format(year) %>

- <%= render partial: "collection_resource_summary_list", locals: { mandatory_resources: } %> + <%= render partial: "collection_resource_summary_list", locals: { mandatory_resources:, additional_resources: @additional_lettings_collection_resources_per_year[year] } %> <% end %> <% @mandatory_sales_collection_resources_per_year.each do |year, mandatory_resources| %>

Sales <%= text_year_range_format(year) %>

- <%= render partial: "collection_resource_summary_list", locals: { mandatory_resources: } %> + <%= render partial: "collection_resource_summary_list", locals: { mandatory_resources:, additional_resources: @additional_sales_collection_resources_per_year[year] } %> <% end %> diff --git a/app/views/collection_resources/new.html.erb b/app/views/collection_resources/new.html.erb new file mode 100644 index 000000000..f66c56b94 --- /dev/null +++ b/app/views/collection_resources/new.html.erb @@ -0,0 +1,36 @@ +<% content_for :before_content do %> + <%= govuk_back_link href: collection_resources_path %> +<% end %> + +
+
+ <%= form_with model: @collection_resource, url: collection_resources_path, method: :post do |f| %> + <%= f.hidden_field :year %> + <%= f.hidden_field :log_type %> + <%= f.hidden_field :mandatory, value: false %> + + <%= f.govuk_error_summary %> + + <%= "#{@collection_resource.log_type.humanize} #{text_year_range_format(@collection_resource.year)}" %> +

Add a new collection resource

+ +

+ This file will be available for all users to download. +

+ + <%= f.govuk_file_field :file, + label: { text: "Upload file", size: "m" } %> + + <%= f.govuk_text_field :short_display_name, + label: { text: "Resource type", size: "m" }, + hint: { text: safe_join(["This will be used in the download link on the homepage. Do not include the log type or collection year.", + content_tag(:br), + "For example, if you enter “bulk upload change log”, the download link will say “Download the #{@collection_resource.log_type} bulk upload change log (#{text_year_range_format(@collection_resource.year)})”."]) } %> + +
+ <%= f.govuk_submit "Add resource" %> + <%= govuk_button_link_to "Cancel", collection_resources_path, secondary: true %> +
+ <% end %> +
+
diff --git a/app/views/layouts/_collection_resources.html.erb b/app/views/layouts/_collection_resources.html.erb index 935f1806c..07648e5c0 100644 --- a/app/views/layouts/_collection_resources.html.erb +++ b/app/views/layouts/_collection_resources.html.erb @@ -12,12 +12,12 @@ <%= govuk_tabs(title: "Collection resources", classes: %w[app-tab__small-headers]) do |c| %> <% @mandatory_lettings_collection_resources_per_year.each do |year, resources| %> <% c.with_tab(label: "Lettings #{year_range_format(year)}") do %> - <%= render DocumentListComponent.new(items: document_list_component_items(resources), label: "Lettings #{text_year_range_format(year)}") %> + <%= render DocumentListComponent.new(items: document_list_component_items(resources.concat(@additional_lettings_collection_resources_per_year[year] || [])), label: "Lettings #{text_year_range_format(year)}") %> <% end %> <% end %> <% @mandatory_sales_collection_resources_per_year.each do |year, resources| %> <% c.with_tab(label: "Sales #{year_range_format(year)}") do %> - <%= render DocumentListComponent.new(items: document_list_component_items(resources), label: "Sales #{text_year_range_format(year)}") %> + <%= render DocumentListComponent.new(items: document_list_component_items(resources.concat(@additional_sales_collection_resources_per_year[year] || [])), label: "Sales #{text_year_range_format(year)}") %> <% end %> <% end %> <% end %> diff --git a/config/locales/en.yml b/config/locales/en.yml index ff73bfdcb..d490fd8ae 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -122,7 +122,7 @@ en: file: error_uploading: There was an error uploading this file. blank: Select which file to upload. - above_100_mb: The file is above 100MB. + above_100_mb: File must be 100MB or less. must_be_pdf: The paper form must be a PDF. must_be_xlsx: The %{resource} must be a Microsoft Excel file. @@ -212,9 +212,12 @@ en: file: error_uploading: There was an error uploading this file. blank: Select which file to upload. - above_100_mb: The file is above 100MB. + above_100_mb: File must be 100MB or less. must_be_pdf: The paper form must be a PDF. must_be_xlsx: The %{resource} must be a Microsoft Excel file. + short_display_name: + blank: "You must answer resource type." + notification: logs_deleted: one: "%{count} log has been deleted." diff --git a/config/routes.rb b/config/routes.rb index db4b2a8a2..cbac4894a 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -42,13 +42,15 @@ Rails.application.routes.draw do get "collection-resources", to: "collection_resources#index" get "/collection-resources/:log_type/:year/:resource_type/download", to: "collection_resources#download_mandatory_collection_resource", as: :download_mandatory_collection_resource - get "/collection-resources/:log_type/:year/:resource_type/edit", to: "collection_resources#edit", as: :edit_mandatory_collection_resource - patch "/collection-resources", to: "collection_resources#update", as: :update_mandatory_collection_resource + get "/collection-resources/:log_type/:year/:resource_type/edit", to: "collection_resources#edit_mandatory_collection_resource", as: :edit_mandatory_collection_resource + patch "/collection-resources", to: "collection_resources#update_mandatory_collection_resource", as: :update_mandatory_collection_resource get "/collection-resources/:year/release", to: "collection_resources#confirm_mandatory_collection_resources_release", as: :confirm_mandatory_collection_resources_release patch "/collection-resources/:year/release", to: "collection_resources#release_mandatory_collection_resources", as: :release_mandatory_collection_resources resources :collection_resources, path: "/collection-resources" do - get "/download", to: "collection_resources#download_additional_collection_resource" # when we get to adding them + get "/download", to: "collection_resources#download_additional_collection_resource" + get "/edit", to: "collection_resources#edit_additional_collection_resource" + patch "/update", to: "collection_resources#update_additional_collection_resource" end get "clear-filters", to: "sessions#clear_filters" diff --git a/spec/factories/collection_resource.rb b/spec/factories/collection_resource.rb index c352b70a9..0282e33e3 100644 --- a/spec/factories/collection_resource.rb +++ b/spec/factories/collection_resource.rb @@ -6,5 +6,15 @@ FactoryBot.define do year { 2024 } log_type { "lettings" } download_filename { "24_25_lettings_paper_form.pdf" } + mandatory { true } + trait(:additional) do + resource_type { nil } + display_name { "lettings additional resource (2024 to 2025)" } + short_display_name { "additional resource" } + year { 2024 } + log_type { "lettings" } + download_filename { "additional.pdf" } + mandatory { false } + end end end diff --git a/spec/features/collection_resources_spec.rb b/spec/features/collection_resources_spec.rb index 643f3537e..ad9a370b6 100644 --- a/spec/features/collection_resources_spec.rb +++ b/spec/features/collection_resources_spec.rb @@ -5,6 +5,9 @@ RSpec.describe "Collection resources" do let(:collection_resources_service) { instance_double(CollectionResourcesService, file_exists_on_s3?: true) } before do + # rubocop:disable RSpec/AnyInstance + allow_any_instance_of(CollectionResourcesHelper).to receive(:editable_collection_resource_years).and_return([2024, 2025]) + # rubocop:enable RSpec/AnyInstance allow(CollectionResourcesService).to receive(:new).and_return(collection_resources_service) allow(collection_resources_service).to receive(:upload_collection_resource) allow(collection_resources_service).to receive(:get_file_metadata).and_return({ "content_type" => "application/pdf", "content_length" => 1000 }) @@ -165,4 +168,74 @@ RSpec.describe "Collection resources" do expect(page).to have_content("There was an error uploading this file.") end end + + context "when uploading an additional resource" do + it "allows valid files" do + expect(CollectionResource.count).to eq(0) + + visit(new_collection_resource_path(year: 2025, log_type: "sales")) + fill_in("collection_resource[short_display_name]", with: "some file") + attach_file "file", file_fixture("pdf_file.pdf") + + click_button("Add resource") + expect(collection_resources_service).to have_received(:upload_collection_resource).with("pdf_file.pdf", anything) + expect(CollectionResource.count).to eq(1) + expect(CollectionResource.first.year).to eq(2025) + expect(CollectionResource.first.log_type).to eq("sales") + expect(CollectionResource.first.resource_type).to be_nil + expect(CollectionResource.first.mandatory).to be_falsey + expect(CollectionResource.first.released_to_user).to be_nil + expect(CollectionResource.first.display_name).to eq("sales some file (2025 to 2026)") + expect(CollectionResource.first.short_display_name).to eq("some file") + expect(page).to have_content("The sales 2025 to 2026 some file has been uploaded.") + end + + it "validates file is attached" do + visit(new_collection_resource_path(year: 2025, log_type: "sales")) + + fill_in("collection_resource[short_display_name]", with: "some file") + click_button("Add resource") + expect(page).to have_content("Select which file to upload.") + end + + it "validates resource type is given" do + visit(new_collection_resource_path(year: 2025, log_type: "sales")) + + attach_file "file", file_fixture("pdf_file.pdf") + click_button("Add resource") + expect(page).to have_content("You must answer resource type.") + end + end + + context "when updating an additional resource" do + let!(:collection_resource) { create(:collection_resource, :additional, year: 2025, log_type: "sales") } + + it "only allows valid files" do + expect(CollectionResource.count).to eq(1) + + visit(collection_resource_edit_path(collection_resource)) + fill_in("collection_resource[short_display_name]", with: "some updated file") + attach_file "file", file_fixture("pdf_file.pdf") + + click_button("Save changes") + expect(collection_resources_service).to have_received(:upload_collection_resource).with("pdf_file.pdf", anything) + expect(CollectionResource.count).to eq(1) + expect(CollectionResource.first.year).to eq(2025) + expect(CollectionResource.first.log_type).to eq("sales") + expect(CollectionResource.first.resource_type).to be_nil + expect(CollectionResource.first.mandatory).to be_falsey + expect(CollectionResource.first.released_to_user).to be_nil + expect(CollectionResource.first.display_name).to eq("sales some updated file (2025 to 2026)") + expect(CollectionResource.first.short_display_name).to eq("some updated file") + expect(page).to have_content("The sales 2025 to 2026 some updated file has been updated.") + end + + it "validates file is attached" do + visit(collection_resource_edit_path(collection_resource)) + + fill_in("collection_resource[short_display_name]", with: "some file") + click_button("Save changes") + expect(page).to have_content("Select which file to upload.") + end + end end diff --git a/spec/helpers/collection_resources_helper_spec.rb b/spec/helpers/collection_resources_helper_spec.rb index 859d14cd1..9fc77a4c9 100644 --- a/spec/helpers/collection_resources_helper_spec.rb +++ b/spec/helpers/collection_resources_helper_spec.rb @@ -94,9 +94,9 @@ RSpec.describe CollectionResourcesHelper do context "and next year resources were manually released" do before do - CollectionResource.create!(year: 2025, resource_type: "paper_form", display_name: "lettings log for tenants (2025 to 2026)", download_filename: "file.pdf", mandatory: true, released_to_user: true) - CollectionResource.create!(year: 2025, resource_type: "bulk_upload_template", display_name: "bulk upload template (2025 to 2026)", download_filename: "file.xlsx", mandatory: true, released_to_user: true) - CollectionResource.create!(year: 2025, resource_type: "bulk_upload_specification", display_name: "sales log for tenants (2025 to 2026)", download_filename: "file.xlsx", mandatory: true, released_to_user: true) + create(:collection_resource, year: 2025, resource_type: "paper_form", display_name: "lettings log for tenants (2025 to 2026)", download_filename: "file.pdf", mandatory: true, released_to_user: true) + create(:collection_resource, year: 2025, resource_type: "bulk_upload_template", display_name: "bulk upload template (2025 to 2026)", download_filename: "file.xlsx", mandatory: true, released_to_user: true) + create(:collection_resource, year: 2025, resource_type: "bulk_upload_specification", display_name: "sales log for tenants (2025 to 2026)", download_filename: "file.xlsx", mandatory: true, released_to_user: true) end it "reutrns current and next years" do @@ -199,9 +199,9 @@ RSpec.describe CollectionResourcesHelper do context "and the resources have been manually released" do before do - CollectionResource.create!(year: 2025, resource_type: "paper_form", display_name: "lettings log for tenants (2025 to 2026)", download_filename: "file.pdf", mandatory: true, released_to_user: true) - CollectionResource.create!(year: 2025, resource_type: "bulk_upload_template", display_name: "bulk upload template (2025 to 2026)", download_filename: "file.xlsx", mandatory: true, released_to_user: true) - CollectionResource.create!(year: 2025, resource_type: "bulk_upload_specification", display_name: "sales log for tenants (2025 to 2026)", download_filename: "file.xlsx", mandatory: true, released_to_user: true) + create(:collection_resource, year: 2025, resource_type: "paper_form", display_name: "lettings log for tenants (2025 to 2026)", download_filename: "file.pdf", mandatory: true, released_to_user: true) + create(:collection_resource, year: 2025, resource_type: "bulk_upload_template", display_name: "bulk upload template (2025 to 2026)", download_filename: "file.xlsx", mandatory: true, released_to_user: true) + create(:collection_resource, year: 2025, resource_type: "bulk_upload_specification", display_name: "sales log for tenants (2025 to 2026)", download_filename: "file.xlsx", mandatory: true, released_to_user: true) end it "returns false" do diff --git a/spec/requests/collection_resources_controller_spec.rb b/spec/requests/collection_resources_controller_spec.rb index 7c9bb0f05..a6d88f911 100644 --- a/spec/requests/collection_resources_controller_spec.rb +++ b/spec/requests/collection_resources_controller_spec.rb @@ -62,7 +62,7 @@ RSpec.describe CollectionResourcesController, type: :request do expect(page).to have_content("Sales 2025 to 2026") end - it "displays mandatory filed" do + it "displays mandatory files" do get collection_resources_path expect(page).to have_content("Paper form") @@ -70,6 +70,15 @@ RSpec.describe CollectionResourcesController, type: :request do expect(page).to have_content("Bulk upload specification") end + it "allows uploading new resources" do + get collection_resources_path + + expect(page).to have_link("Add new sales 2024 to 2025 resource", href: new_collection_resource_path(year: 2024, log_type: "sales")) + expect(page).to have_link("Add new lettings 2024 to 2025 resource", href: new_collection_resource_path(year: 2024, log_type: "lettings")) + expect(page).to have_link("Add new sales 2025 to 2026 resource", href: new_collection_resource_path(year: 2025, log_type: "sales")) + expect(page).to have_link("Add new lettings 2025 to 2026 resource", href: new_collection_resource_path(year: 2025, log_type: "lettings")) + end + context "when files are on S3" do before do allow(storage_service).to receive(:file_exists?).and_return(true) @@ -113,6 +122,16 @@ RSpec.describe CollectionResourcesController, type: :request do expect(page).to have_content("The 2025 to 2026 collection resources are not yet available to users.") expect(page).to have_link("Release the 2025 to 2026 collection resources to users", href: confirm_mandatory_collection_resources_release_path(year: 2025)) end + + context "when there are additional resources" do + let!(:collection_resource) { create(:collection_resource, :additional, year: 2025, short_display_name: "additional resource", download_filename: "additional.pdf") } + + it "displays change links for additional resources" do + get collection_resources_path + + expect(page).to have_link("Change", href: collection_resource_edit_path(collection_resource)) + end + end end context "when files are not on S3" do @@ -147,6 +166,26 @@ RSpec.describe CollectionResourcesController, type: :request do expect(page).to have_content("Once you have uploaded all the required 2025 to 2026 collection resources, you will be able to release them to users.") end end + + context "when there are additional resources" do + let!(:collection_resource) { create(:collection_resource, :additional, year: 2025, short_display_name: "additional resource", download_filename: "additional.pdf") } + + before do + # rubocop:disable RSpec/AnyInstance + allow_any_instance_of(CollectionResourcesHelper).to receive(:editable_collection_resource_years).and_return([2025]) + # rubocop:enable RSpec/AnyInstance + create(:collection_resource, :additional, year: 2026, short_display_name: "additional resource 2") + end + + it "displays additional resources for editable years" do + get collection_resources_path + + expect(page).to have_content("additional resource") + expect(page).not_to have_content("additional resource 2") + expect(page).to have_link("additional.pdf", href: collection_resource_download_path(collection_resource)) + expect(page).to have_link("Delete") + end + end end end @@ -473,4 +512,305 @@ RSpec.describe CollectionResourcesController, type: :request do end end end + + describe "GET #new_collection_resource" do + context "when user is not signed in" do + it "redirects to the sign in page" do + get new_collection_resource_path(year: 2025, log_type: "sales") + expect(response).to redirect_to(new_user_session_path) + end + end + + context "when user is signed in as a data coordinator" do + let(:user) { create(:user, :data_coordinator) } + + before do + sign_in user + end + + it "returns page not found" do + get new_collection_resource_path(year: 2025, log_type: "sales") + expect(response).to have_http_status(:not_found) + end + end + + context "when user is signed in as a data provider" do + let(:user) { create(:user, :data_provider) } + + before do + sign_in user + end + + it "returns page not found" do + get new_collection_resource_path(year: 2025, log_type: "sales") + expect(response).to have_http_status(:not_found) + end + end + + context "when user is signed in as a support user" do + let(:user) { create(:user, :support) } + + before do + # rubocop:disable RSpec/AnyInstance + allow_any_instance_of(CollectionResourcesHelper).to receive(:editable_collection_resource_years).and_return([2025, 2026]) + # rubocop:enable RSpec/AnyInstance + allow(user).to receive(:need_two_factor_authentication?).and_return(false) + sign_in user + end + + it "displays new collection resource page content" do + get new_collection_resource_path(year: 2025, log_type: "sales") + + expect(page).to have_content("Sales 2025 to 2026") + expect(page).to have_content("Add a new collection resource") + expect(page).to have_content("Upload file") + expect(page).to have_button("Add resource") + expect(page).to have_link("Back", href: collection_resources_path) + expect(page).to have_link("Cancel", href: collection_resources_path) + end + end + end + + describe "POST #collection_resources" do + let(:some_file) { File.open(file_fixture("blank_bulk_upload_sales.csv")) } + let(:params) { { collection_resource: { year: 2025, log_type: "sales", file: some_file, display_name: "some file" } } } + + context "when user is not signed in" do + it "redirects to the sign in page" do + post collection_resources_path, params: params + expect(response).to redirect_to(new_user_session_path) + end + end + + context "when user is signed in as a data coordinator" do + let(:user) { create(:user, :data_coordinator) } + + before do + sign_in user + end + + it "returns page not found" do + post collection_resources_path, params: params + expect(response).to have_http_status(:not_found) + end + end + + context "when user is signed in as a data provider" do + let(:user) { create(:user, :data_provider) } + + before do + sign_in user + end + + it "returns page not found" do + post collection_resources_path, params: params + expect(response).to have_http_status(:not_found) + end + end + end + + describe "GET #download_additional_collection_resource" do + let(:collection_resource) { create(:collection_resource, :additional, year: 2025, short_display_name: "additional resource") } + + before do + # rubocop:disable RSpec/AnyInstance + allow_any_instance_of(CollectionResourcesHelper).to receive(:editable_collection_resource_years).and_return([2025, 2026]) + allow_any_instance_of(CollectionResourcesHelper).to receive(:displayed_collection_resource_years).and_return([2025]) + # rubocop:enable RSpec/AnyInstance + end + + context "when the user is not signed in" do + context "when the file exists on S3" do + before do + allow(storage_service).to receive(:get_file).and_return("file") + get collection_resource_download_path(collection_resource) + end + + it "downloads the file" do + expect(response.body).to eq("file") + end + end + end + + context "when user is signed in as a data coordinator" do + let(:user) { create(:user, :data_coordinator) } + + context "when the file exists on S3" do + before do + sign_in user + allow(storage_service).to receive(:get_file).and_return("file") + get collection_resource_download_path(collection_resource) + end + + it "downloads the file" do + expect(response.body).to eq("file") + end + end + + context "when the file does not exist on S3" do + before do + sign_in user + allow(storage_service).to receive(:get_file).and_return(nil) + get collection_resource_download_path(collection_resource) + end + + it "returns page not found" do + expect(response).to have_http_status(:not_found) + end + end + + context "when resource id is invalid" do + before do + sign_in user + allow(storage_service).to receive(:get_file).and_return(nil) + get collection_resource_download_path(collection_resource_id: "invalid") + end + + it "returns page not found" do + expect(response).to have_http_status(:not_found) + end + end + + context "when year not in displayed_collection_resource_years" do + let(:collection_resource) { create(:collection_resource, :additional, year: 2026, short_display_name: "additional resource") } + + before do + sign_in user + get collection_resource_download_path(collection_resource) + end + + it "returns page not found" do + expect(response).to have_http_status(:not_found) + end + end + end + + context "when user is signed in as a support user" do + let(:collection_resource) { create(:collection_resource, :additional, year: 2026, short_display_name: "additional resource") } + let(:user) { create(:user, :support) } + + context "when year is in editable_collection_resource_years but not in displayed_collection_resource_years" do + before do + allow(user).to receive(:need_two_factor_authentication?).and_return(false) + sign_in user + allow(storage_service).to receive(:get_file).and_return("file") + get collection_resource_download_path(collection_resource) + end + + it "downloads the file" do + expect(response.status).to eq(200) + expect(response.body).to eq("file") + end + end + end + end + + describe "GET #edit_additional_collection_resource" do + let(:collection_resource) { create(:collection_resource, :additional, year: 2025, log_type: "sales", short_display_name: "additional resource", download_filename: "additional.pdf") } + + context "when user is not signed in" do + it "redirects to the sign in page" do + get collection_resource_edit_path(collection_resource) + expect(response).to redirect_to(new_user_session_path) + end + end + + context "when user is signed in as a data coordinator" do + let(:user) { create(:user, :data_coordinator) } + + before do + sign_in user + end + + it "returns page not found" do + get collection_resource_edit_path(collection_resource) + expect(response).to have_http_status(:not_found) + end + end + + context "when user is signed in as a data provider" do + let(:user) { create(:user, :data_provider) } + + before do + sign_in user + end + + it "returns page not found" do + get collection_resource_edit_path(collection_resource) + expect(response).to have_http_status(:not_found) + end + end + + context "when user is signed in as a support user" do + let(:user) { create(:user, :support) } + + before do + allow(Time.zone).to receive(:today).and_return(Time.zone.local(2025, 1, 8)) + allow(user).to receive(:need_two_factor_authentication?).and_return(false) + sign_in user + end + + context "and the file exists on S3" do + before do + allow(storage_service).to receive(:file_exists?).and_return(true) + end + + it "displays update collection resources page content" do + get collection_resource_edit_path(collection_resource) + + expect(page).to have_content("Sales 2025 to 2026") + expect(page).to have_content("Change the additional resource") + expect(page).to have_content("This file will be available for all users to download.") + expect(page).to have_content("Upload file") + expect(page).to have_button("Save changes") + expect(page).to have_link("Back", href: collection_resources_path) + expect(page).to have_link("Cancel", href: collection_resources_path) + end + end + end + end + + describe "PATCH #update_additional_collection_resource" do + let(:some_file) { File.open(file_fixture("blank_bulk_upload_sales.csv")) } + let(:params) { { collection_resource: { short_display_name: "short name", file: some_file } } } + let(:collection_resource_service) { instance_double(CollectionResourcesService) } + let(:collection_resource) { create(:collection_resource, :additional, year: 2025, log_type: "sales", short_display_name: "additional resource", download_filename: "additional.pdf") } + + before do + allow(CollectionResourcesService).to receive(:new).and_return(collection_resource_service) + end + + context "when user is not signed in" do + it "redirects to the sign in page" do + patch collection_resource_update_path(collection_resource), params: params + expect(response).to redirect_to(new_user_session_path) + end + end + + context "when user is signed in as a data coordinator" do + let(:user) { create(:user, :data_coordinator) } + + before do + sign_in user + end + + it "returns page not found" do + patch collection_resource_update_path(collection_resource), params: params + expect(response).to have_http_status(:not_found) + end + end + + context "when user is signed in as a data provider" do + let(:user) { create(:user, :data_provider) } + + before do + sign_in user + end + + it "returns page not found" do + patch collection_resource_update_path(collection_resource), params: params + expect(response).to have_http_status(:not_found) + end + end + end end diff --git a/spec/requests/start_controller_spec.rb b/spec/requests/start_controller_spec.rb index db884d09f..b3846809c 100644 --- a/spec/requests/start_controller_spec.rb +++ b/spec/requests/start_controller_spec.rb @@ -324,6 +324,7 @@ RSpec.describe StartController, type: :request do context "and 2023 collection window is open for editing" do before do + create(:collection_resource, :additional, year: 2023, log_type: "sales", display_name: "sales additional resource (2023 to 2024)") allow(Time).to receive(:now).and_return(Time.zone.local(2024, 4, 1)) end @@ -337,6 +338,7 @@ RSpec.describe StartController, type: :request do expect(page).to have_content("Sales 23/24") expect(page).to have_content("Sales 2024 to 2025") expect(page).to have_content("Sales 2023 to 2024") + expect(page).to have_content("Download the sales additional resource (2023 to 2024)") end end diff --git a/spec/services/collection_resources_service_spec.rb b/spec/services/collection_resources_service_spec.rb index 3786b70c9..602e36c38 100644 --- a/spec/services/collection_resources_service_spec.rb +++ b/spec/services/collection_resources_service_spec.rb @@ -12,7 +12,7 @@ describe CollectionResourcesService do end it "calls write_file on S3 service" do - expect(storage_service).to receive(:write_file).with("2025_26_lettings_paper_form.pdf", some_file) + expect(storage_service).to receive(:write_file).with("2025_26_lettings_paper_form.pdf", some_file, content_type: "application/pdf") service.upload_collection_resource("2025_26_lettings_paper_form.pdf", some_file) end end