From 91f3fa13a3c6efc513817597003e403847f3179a Mon Sep 17 00:00:00 2001 From: CatalinVoineag <11318084+CatalinVoineag@users.noreply.github.com> Date: Wed, 10 Jul 2024 16:42:08 +0100 Subject: [PATCH] Refactor goto methods for about org form redirect There are a few `goto` methods that tell the about your organisation form where to redirect the user after update or when the user clicks the back links. Basically, both back links and update success methods can go to the same path based on the `goto` param. Because of this and the ever growing of these `goto` params I moved some logic that calculates the redirection in the about_your_organisation_form. This way it can be tested properly. --- app/controllers/concerns/goto_preview.rb | 1 - .../publish/providers_controller.rb | 39 +++--- .../publish/about_your_organisation_form.rb | 42 +++++++ app/views/publish/providers/about.html.erb | 14 +-- .../controllers/concerns/goto_preview_spec.rb | 22 ---- .../about_your_organisation_form_spec.rb | 119 ++++++++++++++++++ spec/helpers/find/goto_preview_helper_spec.rb | 38 ------ 7 files changed, 184 insertions(+), 91 deletions(-) create mode 100644 spec/forms/publish/about_your_organisation_form_spec.rb diff --git a/app/controllers/concerns/goto_preview.rb b/app/controllers/concerns/goto_preview.rb index e4e8cd5166..e917b78be8 100644 --- a/app/controllers/concerns/goto_preview.rb +++ b/app/controllers/concerns/goto_preview.rb @@ -8,5 +8,4 @@ def param_form_key end def goto_preview? = params.dig(param_form_key, :goto_preview) == 'true' - def goto_provider? = params.dig(param_form_key, :goto_provider) == 'true' end diff --git a/app/controllers/publish/providers_controller.rb b/app/controllers/publish/providers_controller.rb index f1d8438794..83ec521940 100644 --- a/app/controllers/publish/providers_controller.rb +++ b/app/controllers/publish/providers_controller.rb @@ -54,29 +54,26 @@ def details def about authorize provider, :show? - @about_form = AboutYourOrganisationForm.new(provider) + @about_form = AboutYourOrganisationForm.new( + provider, + redirect_params:, + course_code: params[:course_code] + ) end def update authorize provider, :update? - @about_form = AboutYourOrganisationForm.new(provider, params: provider_params) + @about_form = AboutYourOrganisationForm.new( + provider, + params: provider_params, + redirect_params:, + course_code: params.dig(param_form_key, :course_code) + ) if @about_form.save! - if goto_provider? - redirect_to( - provider_publish_provider_recruitment_cycle_course_path( - provider.provider_code, - provider.recruitment_cycle_year, - params[:course_code] || params.dig(param_form_key, :course_code) - ) - ) - elsif goto_preview? - redirect_to preview_publish_provider_recruitment_cycle_course_path(provider.provider_code, provider.recruitment_cycle_year, (params[:course_code] || params.dig(param_form_key, :course_code))) - else - flash[:success] = I18n.t('success.published') - redirect_to(details_publish_provider_recruitment_cycle_path(provider.provider_code, provider.recruitment_cycle_year)) - end + redirect_to @about_form.update_success_path + flash[:success] = I18n.t('success.published') if redirect_params.all? { |_k, v| v.blank? } else @errors = @about_form.errors.messages render :about @@ -124,7 +121,7 @@ def redirect_to_contact_page_with_ukprn_error def provider_params params .require(param_form_key) - .except(:goto_preview, :course_code, :goto_provider) + .except(:goto_preview, :course_code, :goto_provider, :goto_training_with_disabilities) .permit( *AboutYourOrganisationForm::FIELDS, accredited_bodies: %i[provider_name provider_code description] @@ -132,5 +129,13 @@ def provider_params end def param_form_key = :publish_about_your_organisation_form + + def redirect_params + params.fetch(param_form_key, params).slice( + :goto_preview, + :goto_provider, + :goto_training_with_disabilities + ).permit!.to_h + end end end diff --git a/app/forms/publish/about_your_organisation_form.rb b/app/forms/publish/about_your_organisation_form.rb index b6c2142fb5..b499a0c262 100644 --- a/app/forms/publish/about_your_organisation_form.rb +++ b/app/forms/publish/about_your_organisation_form.rb @@ -2,6 +2,8 @@ module Publish class AboutYourOrganisationForm < BaseProviderForm + include Rails.application.routes.url_helpers + validates :train_with_us, presence: { message: 'Enter details about training with you' }, if: :train_with_us_changed? validates :train_with_disability, presence: { message: 'Enter details about training with a disability' }, if: :train_with_disability_changed? @@ -10,6 +12,12 @@ class AboutYourOrganisationForm < BaseProviderForm validate :add_enrichment_errors + def initialize(model, params: {}, redirect_params: {}, course_code: nil) + super(model, params:) + @redirect_params = redirect_params + @course_code = course_code + end + FIELDS = %i[ train_with_us train_with_disability @@ -17,6 +25,7 @@ class AboutYourOrganisationForm < BaseProviderForm ].freeze attr_accessor(*FIELDS) + attr_reader :redirect_params, :course_code def accredited_bodies @accredited_bodies ||= provider.accredited_bodies.map do |ab| @@ -24,6 +33,35 @@ def accredited_bodies end end + def update_success_path + case redirection_key + when 'goto_preview' + preview_publish_provider_recruitment_cycle_course_path( + provider.provider_code, + provider.recruitment_cycle_year, + course_code + ) + when 'goto_provider' + provider_publish_provider_recruitment_cycle_course_path( + provider.provider_code, + provider.recruitment_cycle_year, + course_code + ) + when 'goto_training_with_disabilities' + training_with_disabilities_publish_provider_recruitment_cycle_course_path( + provider.provider_code, + provider.recruitment_cycle_year, + course_code + ) + else + details_publish_provider_recruitment_cycle_path( + provider.provider_code, + provider.recruitment_cycle_year + ) + end + end + alias back_path update_success_path + private def train_with_us_changed? @@ -81,5 +119,9 @@ class AccreditedProvider attr_accessor :provider_name, :provider_code, :description end + + def redirection_key + redirect_params.select { |_k, v| v == 'true' }&.keys&.first + end end end diff --git a/app/views/publish/providers/about.html.erb b/app/views/publish/providers/about.html.erb index a50533551e..9bda4a59be 100644 --- a/app/views/publish/providers/about.html.erb +++ b/app/views/publish/providers/about.html.erb @@ -10,19 +10,7 @@ ) do |f| %> <% content_for :before_content do %> - <% if goto_provider?(param_form_key: f.object_name.to_sym, params:) %> - <%= govuk_back_link_to( - provider_publish_provider_recruitment_cycle_course_path( - @provider.provider_code, - @provider.recruitment_cycle_year, - params[:course_code] || params.dig(f.object_name.to_sym, :course_code) - ) - ) %> - <% elsif goto_preview?(param_form_key: f.object_name.to_sym, params:) %> - <%= govuk_back_link_to(preview_publish_provider_recruitment_cycle_course_path(@provider.provider_code, @provider.recruitment_cycle_year, (params[:course_code] || params.dig(f.object_name.to_sym, :course_code)))) %> - <% else %> - <%= govuk_back_link_to(details_publish_provider_recruitment_cycle_path(@provider.provider_code, @provider.recruitment_cycle_year)) %> - <% end %> + <%= govuk_back_link_to(@about_form.back_path) %> <% end %> <%= f.govuk_error_summary %> diff --git a/spec/controllers/concerns/goto_preview_spec.rb b/spec/controllers/concerns/goto_preview_spec.rb index 6b5ea0952e..45e25407d6 100644 --- a/spec/controllers/concerns/goto_preview_spec.rb +++ b/spec/controllers/concerns/goto_preview_spec.rb @@ -38,26 +38,4 @@ def param_form_key = :param_form_key end end end - - describe '#goto_provider?' do - subject do - test_class.goto_provider? - end - - context 'params is empty' do - let(:params) { {} } - - it 'returns falsey' do - expect(subject).to be_falsey - end - end - - context 'params has param_form_key with goto_provider set to "true"' do - let(:params) { { param_form_key: { goto_provider: 'true' } } } - - it 'returns truthy' do - expect(subject).to be_truthy - end - end - end end diff --git a/spec/forms/publish/about_your_organisation_form_spec.rb b/spec/forms/publish/about_your_organisation_form_spec.rb new file mode 100644 index 0000000000..8554d91b40 --- /dev/null +++ b/spec/forms/publish/about_your_organisation_form_spec.rb @@ -0,0 +1,119 @@ +# frozen_string_literal: true + +require 'rails_helper' + +module Publish + describe AboutYourOrganisationForm, type: :model do + include Rails.application.routes.url_helpers + let(:params) do + { + train_with_us:, + train_with_disability: + } + end + let(:train_with_us) { 'train_with_us' } + let(:train_with_disability) { 'train_with_disability' } + let(:provider) do + create( + :provider, + accrediting_provider_enrichments: [ + { UcasProviderCode: accredited_provider.provider_code } + ] + ) + end + let(:accredited_provider) { create(:provider, :accredited_provider) } + let(:redirect_params) { { 'goto_preview' => 'true' } } + let(:course_code) { create(:course) } + + subject { described_class.new(provider, params:, redirect_params:, course_code:) } + + context 'validations' do + it { is_expected.to validate_presence_of(:train_with_us).with_message('Enter details about training with you') } + it { is_expected.to validate_presence_of(:train_with_disability).with_message('Enter details about training with a disability') } + + context 'traing_with_us word count is invalid' do + let(:train_with_us) { Faker::Lorem.sentence(word_count: 251) } + + it 'is not valid when traing_with_us is over 250 words' do + expect(subject.valid?).to be_falsey + expect(subject.errors[:train_with_us]).to include('Reduce the word count for training with you') + end + end + + context 'train_with_disability word count is invalid' do + let(:train_with_disability) { Faker::Lorem.sentence(word_count: 251) } + + it 'is not valid when train_with_disability is over 250 words' do + expect(subject.valid?).to be_falsey + expect(subject.errors[:train_with_disability]).to include('Reduce the word count for training with disabilities and other needs') + end + end + end + + describe '#accredited_bodies' do + it 'returns an array of accredited_providers' do + expect(subject.accredited_bodies).to include( + have_attributes( + provider_name: accredited_provider.provider_name, + provider_code: accredited_provider.provider_code + ) + ) + end + end + + describe '#update_success_path' do + context 'when goto_preview is true' do + it 'returns the goto_preview path' do + expect(subject.update_success_path).to eq( + preview_publish_provider_recruitment_cycle_course_path( + provider.provider_code, + provider.recruitment_cycle_year, + course_code + ) + ) + end + end + + context 'when goto_provider is true' do + let(:redirect_params) { { 'goto_provider' => 'true' } } + + it 'returns the goto_provider path' do + expect(subject.update_success_path).to eq( + provider_publish_provider_recruitment_cycle_course_path( + provider.provider_code, + provider.recruitment_cycle_year, + course_code + ) + ) + end + end + + context 'when goto_training_with_disabilities is true' do + let(:redirect_params) { { 'goto_training_with_disabilities' => 'true' } } + + it 'returns the goto_training_with_disabilities path' do + expect(subject.update_success_path).to eq( + training_with_disabilities_publish_provider_recruitment_cycle_course_path( + provider.provider_code, + provider.recruitment_cycle_year, + course_code + ) + ) + end + end + + context 'when there is no redirect_params' do + let(:redirect_params) { {} } + + it 'returns the details path' do + expect(subject.update_success_path).to eq( + details_publish_provider_recruitment_cycle_path( + provider.provider_code, + provider.recruitment_cycle_year + ) + ) + end + end + end + end +end diff --git a/spec/helpers/find/goto_preview_helper_spec.rb b/spec/helpers/find/goto_preview_helper_spec.rb index 58662b3a6a..53a37cdc8e 100644 --- a/spec/helpers/find/goto_preview_helper_spec.rb +++ b/spec/helpers/find/goto_preview_helper_spec.rb @@ -96,44 +96,6 @@ module Find end end - describe '#goto_provider?' do - subject do - goto_provider?(param_form_key:, params:) - end - - context 'params is empty' do - let(:params) { {} } - - it 'returns falsey' do - expect(subject).to be_falsey - end - end - - context 'params has goto_provider set to "true"' do - let(:params) { { goto_provider: 'true' } } - - it 'returns truthy' do - expect(subject).to be_truthy - end - end - - context 'params has param_form_key with goto_provider set to "true"' do - let(:params) { { param_form_key: { goto_provider: 'true' } } } - - it 'returns the value "true"' do - expect(subject).to be_truthy - end - end - - context 'params has param_form_key with goto_provider set to "false"' do - let(:params) { { param_form_key: { goto_provider: 'false' } } } - - it 'returns the value "false"' do - expect(subject).to be_falsey - end - end - end - describe '#back_link_path' do let(:course) { build(:course) }