From 47f7715271367974758daf87eea0f2d6ab1f0488 Mon Sep 17 00:00:00 2001 From: Jamie Date: Tue, 12 Nov 2024 10:13:03 +0000 Subject: [PATCH 1/8] User adding a claim uses the wizard pattern --- .../schools/claims/add_claim_controller.rb | 44 +++++++ app/models/claims/claim.rb | 2 + app/services/claims/claim/calculate_amount.rb | 2 +- app/services/claims/claim/submit.rb | 7 +- .../schools/claims/add_claim/edit.html.erb | 13 +++ .../claims/schools/claims/index.html.erb | 2 +- .../schools/claims/mentors/_form.html.erb | 2 +- .../_check_your_answers_step.html.erb | 110 ++++++++++++++++++ .../add_claim_wizard/_mentor_step.html.erb | 25 ++++ .../_mentor_training_step.html.erb | 36 ++++++ .../_no_mentors_step.html.erb | 13 +++ .../add_claim_wizard/_provider_step.html.erb | 23 ++++ app/wizards/claims/add_claim_wizard.rb | 101 ++++++++++++++++ .../check_your_answers_step.rb | 11 ++ .../claims/add_claim_wizard/mentor_step.rb | 27 +++++ .../add_claim_wizard/mentor_training_step.rb | 55 +++++++++ .../add_claim_wizard/no_mentors_step.rb | 2 + .../claims/add_claim_wizard/provider_step.rb | 13 +++ config/locales/en/activemodel.yml | 22 ++++ .../en/claims/schools/claims/add_claim.yml | 8 ++ .../en/wizards/claims/add_claim_wizard.yml | 47 ++++++++ config/routes/claims.rb | 6 + .../schools/claims/create_claim_spec.rb | 4 +- .../schools/claims/create_claim_spec.rb | 1 - 24 files changed, 568 insertions(+), 8 deletions(-) create mode 100644 app/controllers/claims/schools/claims/add_claim_controller.rb create mode 100644 app/views/claims/schools/claims/add_claim/edit.html.erb create mode 100644 app/views/wizards/claims/add_claim_wizard/_check_your_answers_step.html.erb create mode 100644 app/views/wizards/claims/add_claim_wizard/_mentor_step.html.erb create mode 100644 app/views/wizards/claims/add_claim_wizard/_mentor_training_step.html.erb create mode 100644 app/views/wizards/claims/add_claim_wizard/_no_mentors_step.html.erb create mode 100644 app/views/wizards/claims/add_claim_wizard/_provider_step.html.erb create mode 100644 app/wizards/claims/add_claim_wizard.rb create mode 100644 app/wizards/claims/add_claim_wizard/check_your_answers_step.rb create mode 100644 app/wizards/claims/add_claim_wizard/mentor_step.rb create mode 100644 app/wizards/claims/add_claim_wizard/mentor_training_step.rb create mode 100644 app/wizards/claims/add_claim_wizard/no_mentors_step.rb create mode 100644 app/wizards/claims/add_claim_wizard/provider_step.rb create mode 100644 config/locales/en/claims/schools/claims/add_claim.yml create mode 100644 config/locales/en/wizards/claims/add_claim_wizard.yml diff --git a/app/controllers/claims/schools/claims/add_claim_controller.rb b/app/controllers/claims/schools/claims/add_claim_controller.rb new file mode 100644 index 000000000..00f3a919f --- /dev/null +++ b/app/controllers/claims/schools/claims/add_claim_controller.rb @@ -0,0 +1,44 @@ +class Claims::Schools::Claims::AddClaimController < Claims::ApplicationController + include WizardController + include Claims::BelongsToSchool + + before_action :has_school_accepted_grant_conditions? + before_action :set_wizard + before_action :authorize_claim + + def update + if !@wizard.save_step + render "edit" + elsif @wizard.next_step.present? + redirect_to step_path(@wizard.next_step) + elsif @wizard.valid? + @wizard.create_claim + @wizard.reset_state + redirect_to confirmation_claims_school_claim_path(@school, @wizard.claim) + else + redirect_to rejected_claims_school_claim_path(@school) + end + end + + private + + def set_wizard + state = session[state_key] ||= {} + current_step = params[:step]&.to_sym + @wizard = Claims::AddClaimWizard.new( + school: @school, created_by: current_user, params:, state:, current_step:, + ) + end + + def authorize_claim + authorize Claims::Claim, :create? + end + + def step_path(step) + add_claim_claims_school_claims_path(state_key:, step:) + end + + def index_path + claims_school_claims_path(@school) + end +end diff --git a/app/models/claims/claim.rb b/app/models/claims/claim.rb index 963e8a71e..a5ed7cc85 100644 --- a/app/models/claims/claim.rb +++ b/app/models/claims/claim.rb @@ -49,6 +49,8 @@ class Claims::Claim < ApplicationRecord has_many :mentor_trainings, dependent: :destroy has_many :mentors, through: :mentor_trainings + accepts_nested_attributes_for :mentor_trainings + validates :status, presence: true validates( :reference, diff --git a/app/services/claims/claim/calculate_amount.rb b/app/services/claims/claim/calculate_amount.rb index a80823751..03e52d820 100644 --- a/app/services/claims/claim/calculate_amount.rb +++ b/app/services/claims/claim/calculate_amount.rb @@ -9,7 +9,7 @@ def call region = claim.school.region claims_funding_available_per_hour_pence = region.claims_funding_available_per_hour_pence - total_hours_completed = claim.mentor_trainings.sum(:hours_completed) + total_hours_completed = claim.mentor_trainings.filter_map(&:hours_completed).sum amount_in_pence = claims_funding_available_per_hour_pence * total_hours_completed diff --git a/app/services/claims/claim/submit.rb b/app/services/claims/claim/submit.rb index 56297f992..65906c7f9 100644 --- a/app/services/claims/claim/submit.rb +++ b/app/services/claims/claim/submit.rb @@ -41,7 +41,10 @@ def updated_claim end def generate_reference - reference = SecureRandom.random_number(99_999_999) while Claims::Claim.exists?(reference:) - reference + loop do + reference = SecureRandom.random_number(99_999_999) + + break reference unless Claims::Claim.exists?(reference:) + end end end diff --git a/app/views/claims/schools/claims/add_claim/edit.html.erb b/app/views/claims/schools/claims/add_claim/edit.html.erb new file mode 100644 index 000000000..bef69cedc --- /dev/null +++ b/app/views/claims/schools/claims/add_claim/edit.html.erb @@ -0,0 +1,13 @@ +<% render "claims/schools/primary_navigation", school: @school, current: :claims %> + +<%= content_for(:before_content) do %> + <%= govuk_back_link(href: back_link_path) %> +<% end %> + +
+ <%= render_wizard(@wizard, contextual_text: t(".caption")) %> + +

+ <%= govuk_link_to(t(".cancel"), claims_school_claims_path(@school), no_visited_state: true) %> +

+
diff --git a/app/views/claims/schools/claims/index.html.erb b/app/views/claims/schools/claims/index.html.erb index 5f801240a..ccef17f18 100644 --- a/app/views/claims/schools/claims/index.html.erb +++ b/app/views/claims/schools/claims/index.html.erb @@ -11,7 +11,7 @@

<%= sanitize t(".closing_date_disclaimer", time: l(Claims::ClaimWindow.current.ends_on.end_of_day, format: :time_on_date)) %>

<% end %> - <%= govuk_link_to t(".add_claim"), new_claims_school_claim_path, class: "govuk-button" %> + <%= govuk_button_link_to(t(".add_claim"), new_add_claim_claims_school_claims_path) %> <% else %> <%= govuk_inset_text text: t(".add_mentor_guidance_html", link_to: govuk_link_to(t(".add_a_mentor"), claims_school_mentors_path(@school))) %> <% end %> diff --git a/app/views/claims/support/schools/claims/mentors/_form.html.erb b/app/views/claims/support/schools/claims/mentors/_form.html.erb index 8f2884689..b09e0abbc 100644 --- a/app/views/claims/support/schools/claims/mentors/_form.html.erb +++ b/app/views/claims/support/schools/claims/mentors/_form.html.erb @@ -33,7 +33,7 @@

- <%= govuk_link_to t(".change_provider"), new_claims_school_claim_path(school, id: claim_mentors_form.claim.id) %> + <%= govuk_link_to t(".change_provider"), new_claims_support_school_claim_path(school, id: claim_mentors_form.claim.id, claim_id: claim_mentors_form.claim.id) %>

<% end %> diff --git a/app/views/wizards/claims/add_claim_wizard/_check_your_answers_step.html.erb b/app/views/wizards/claims/add_claim_wizard/_check_your_answers_step.html.erb new file mode 100644 index 000000000..96cc3eb78 --- /dev/null +++ b/app/views/wizards/claims/add_claim_wizard/_check_your_answers_step.html.erb @@ -0,0 +1,110 @@ +<%= form_for(current_step, url: current_step_path, method: :put) do |f| %> + <%= f.govuk_error_summary %> + +
+
+ <%= t(".caption") %> +

<%= t(".page_title") %>

+ + <%= govuk_summary_list do |summary_list| %> + <% summary_list.with_row do |row| %> + <% row.with_key(text: Claims::Claim.human_attribute_name(:school)) %> + <% row.with_value(text: @wizard.school.name) %> + <% end %> + + <% summary_list.with_row do |row| %> + <% row.with_key(text: Claims::ClaimWindow.human_attribute_name(:academic_year)) %> + <% row.with_value(text: @wizard.academic_year.name) %> + <% end %> + + <% summary_list.with_row do |row| %> + <% row.with_key(text: Claims::Claim.human_attribute_name(:accredited_provider)) %> + <% row.with_value(text: @wizard.steps[:provider].provider.name) %> + <% row.with_action(text: t("change"), + href: step_path(:provider), + visually_hidden_text: Claims::Claim.human_attribute_name(:accredited_provider), + html_attributes: { + class: "govuk-link--no-visited-state", + }) %> + <% end %> + + <% summary_list.with_row do |row| %> + <% row.with_key(text: Claims::Claim.human_attribute_name(:mentors)) %> + <% row.with_value do %> +
    + <% @wizard.steps[:mentor].selected_mentors.each do |mentor| %> +
  • <%= mentor.full_name %>
    • + <% end %> +
+ <% end %> + <% row.with_action(text: t("change"), + href: step_path(:mentor), + visually_hidden_text: Claims::Claim.human_attribute_name(:mentors), + html_attributes: { + class: "govuk-link--no-visited-state", + }) %> + <% end %> + <% end %> + +

<%= t(".hours_of_training") %>

+ + <%= govuk_summary_list do |summary_list| %> + <% @wizard.steps[:mentor].selected_mentors.each do |mentor| %> + <% mentor_training = @wizard.steps["mentor_training_#{mentor.id}".to_sym] %> + <% summary_list.with_row do |row| %> + <% row.with_key(text: mentor_training.mentor.full_name) %> + <% row.with_value( + text: pluralize( + mentor_training.total_hours_completed, + t(".hour"), + ), + ) %> + <% row.with_action( + text: t("change"), + href: step_path("mentor_training_#{mentor.id}".to_sym), + visually_hidden_text: "Hours of training for #{mentor.full_name}", + html_attributes: { class: "govuk-link--no-visited-state" }, + ) %> + <% end %> + <% end %> + <% end %> + +

<%= t(".grant_funding") %>

+ + <%= govuk_summary_list(actions: false) do |summary_list| %> + <% summary_list.with_row do |row| %> + <% row.with_key(text: t(".total_hours")) %> + <% row.with_value(text: pluralize(@wizard.total_hours, t(".hour"))) %> + <% end %> + + <% summary_list.with_row do |row| %> + <% row.with_key(text: t(".hourly_rate")) %> + <% row.with_value(text: humanized_money_with_symbol(@wizard.school.region.funding_available_per_hour)) %> + <% end %> + + <% summary_list.with_row do |row| %> + <% row.with_key(text: Claims::Claim.human_attribute_name(:claim_amount)) %> + <% row.with_value(text: humanized_money_with_symbol(@wizard.claim.amount)) %> + <% end %> + <% end %> + + <% if !current_user.support_user? %> +

<%= t(".disclaimer") %>

+ +
    +
  • <%= t(".claim_on_behalf_of_school") %>
    • +
  • <%= t(".read_and_accepted_grant_conditions") %>
    • +
  • <%= t(".accurate_information") %>
    • +
  • <%= t(".provide_evidence") %>
    • +
+ +
+ + <%= t(".warning") %> +
+ <% end %> + + <%= f.govuk_submit t(".submit") %> +
+
+<% end %> diff --git a/app/views/wizards/claims/add_claim_wizard/_mentor_step.html.erb b/app/views/wizards/claims/add_claim_wizard/_mentor_step.html.erb new file mode 100644 index 000000000..5cea2e03a --- /dev/null +++ b/app/views/wizards/claims/add_claim_wizard/_mentor_step.html.erb @@ -0,0 +1,25 @@ +<%= form_for(current_step, url: current_step_path, method: :put) do |f| %> + <%= f.govuk_error_summary %> + +
+
+ <%= contextual_text %> +

<%= t(".heading", provider_name: @wizard.steps[:provider].provider.name) %>

+ + <%= render Claims::Claim::MentorsForm::DisclaimerComponent.new(mentors_form: current_step) %> + + <%= f.govuk_collection_check_boxes( + :mentor_ids, + current_step.mentors_with_claimable_hours, + :id, :full_name, :trn, + legend: { + size: "s", + text: t(".label"), + }, + hint: { text: t(".select_all_that_apply") } + ) %> + + <%= f.govuk_submit t(".continue") %> +
+
+<% end %> diff --git a/app/views/wizards/claims/add_claim_wizard/_mentor_training_step.html.erb b/app/views/wizards/claims/add_claim_wizard/_mentor_training_step.html.erb new file mode 100644 index 000000000..8f8cc0226 --- /dev/null +++ b/app/views/wizards/claims/add_claim_wizard/_mentor_training_step.html.erb @@ -0,0 +1,36 @@ +<%= form_for(current_step, url: current_step_path, method: :put) do |f| %> + <%= f.govuk_error_summary %> + +
+
+ <%= contextual_text %> - <%= @wizard.steps[:provider].provider.name %> +

<%= t(".hours_of_training_for_mentor", mentor: current_step.mentor.full_name) %>

+ + + <%= render Claims::Claim::MentorTrainingForm::DisclaimerComponent.new(mentor_training_form: current_step) %> + + <%= f.govuk_radio_buttons_fieldset( + :hours_completed, + legend: { + size: "m", + text: t(".hours_of_training"), + }, + ) do %> + <%= f.govuk_radio_button :hours_completed, current_step.max_hours, label: { text: t(".hours", count: current_step.max_hours) }, hint: { text: t(".hours_hint.#{current_step.max_hours == 20 ? "full" : "remaining"}", count: current_step.max_hours) } %> + + <%= f.govuk_radio_divider %> + + <%= f.govuk_radio_button(:hours_completed, "custom", label: { text: t(".other_amount") }) do %> + <%= f.govuk_number_field( + :custom_hours_completed, + class: "govuk-input--width-2", + label: { text: t(".number_of_hours"), class: "govuk-!-font-weight-bold" }, + hint: { text: t(".custom_hours_completed_hint", count: current_step.max_hours) }, + ) %> + <% end %> + <% end %> + + <%= f.govuk_submit t("continue") %> +
+
+<% end %> diff --git a/app/views/wizards/claims/add_claim_wizard/_no_mentors_step.html.erb b/app/views/wizards/claims/add_claim_wizard/_no_mentors_step.html.erb new file mode 100644 index 000000000..ce5c5c2ff --- /dev/null +++ b/app/views/wizards/claims/add_claim_wizard/_no_mentors_step.html.erb @@ -0,0 +1,13 @@ +
+
+

<%= t(".heading_empty", provider_name: @wizard.provider.name) %>

+ +

+ <%= t(".no_mentors_with_claimable_hours", provider_name: @wizard.provider.name) %> +

+ +

+ <%= govuk_link_to t(".change_provider"), step_path(:provider) %> +

+
+
diff --git a/app/views/wizards/claims/add_claim_wizard/_provider_step.html.erb b/app/views/wizards/claims/add_claim_wizard/_provider_step.html.erb new file mode 100644 index 000000000..22c61bdfd --- /dev/null +++ b/app/views/wizards/claims/add_claim_wizard/_provider_step.html.erb @@ -0,0 +1,23 @@ +<% content_for :page_title, title_with_error_prefix( + t(".page_title", contextual_text:), + error: current_step.errors.any?, +) %> + +<%= form_for(current_step, url: current_step_path, method: :put) do |f| %> + <%= f.govuk_error_summary %> + +
+
+ <%= contextual_text %> + + <%= f.govuk_collection_radio_buttons( + :id, + current_step.providers_for_selection, + :id, :name, + legend: { size: "l", text: t(".title"), tag: "h1" } + ) %> + + <%= f.govuk_submit t(".continue") %> +
+
+<% end %> diff --git a/app/wizards/claims/add_claim_wizard.rb b/app/wizards/claims/add_claim_wizard.rb new file mode 100644 index 000000000..1009ee0ab --- /dev/null +++ b/app/wizards/claims/add_claim_wizard.rb @@ -0,0 +1,101 @@ +module Claims + class AddClaimWizard < BaseWizard + attr_reader :school, :created_by + + def initialize(school:, created_by:, params:, state:, current_step: nil) + @school = school + @created_by = created_by + super(state:, params:, current_step:) + end + + def define_steps + add_step(ProviderStep) + if mentors_with_claimable_hours.any? || current_step == :check_your_answers + add_step(MentorStep) + # Loop over mentors + steps.fetch(:mentor).selected_mentors.each do |mentor| + add_step(MentorTrainingStep, { mentor_id: mentor.id }) + end + add_step(CheckYourAnswersStep) + else + add_step(NoMentorsStep) + end + end + + def add_step(step_class, preset_attributes = {}) + name = step_name(step_class, preset_attributes[:mentor_id]) + attributes = step_attributes(name, step_class, preset_attributes) + @steps[name] = step_class.new(wizard: self, attributes:) + end + + def academic_year + Claims::ClaimWindow.current.academic_year + end + + def total_hours + mentor_training_steps.values.map(&:total_hours_completed).sum + end + + def claim + @claim ||= Claims::Claim.new( + provider:, + school:, + created_by:, + claim_window: Claims::ClaimWindow.current, + mentor_trainings_attributes: mentor_training_steps.map do |_k, v| + { + mentor_id: v.mentor_id, + hours_completed: v.total_hours_completed, + provider:, + } + end, + ) + end + + def create_claim + raise "Invalid wizard state" unless valid? + return unless claim.valid_mentor_training_hours? + + Claims::Claim::Submit.call(claim:, user: created_by) + end + + def mentors_with_claimable_hours + return Claims::Mentor.none if provider.blank? + + @mentors_with_claimable_hours ||= Claims::MentorsWithRemainingClaimableHoursQuery.call( + params: { + school:, + provider:, + claim: Claims::Claim.new(academic_year:), + }, + ) + end + + def provider + steps.fetch(:provider).provider + end + + private + + def step_name(step_class, id = nil) + # e.g. YearGroupStep becomes :year_group + name = step_class.name.chomp("Step").demodulize.underscore + return name.to_sym if id.blank? + + # e.g. with id it becomes :year_group_#{id} + "#{name}_#{id}".to_sym + end + + def step_attributes(name, step_class, preset_attributes = {}) + attributes = super(name, step_class) + return attributes if preset_attributes.blank? + + attributes = {} if attributes.blank? + attributes.merge(preset_attributes) + end + + def mentor_training_steps + steps.select { |k, _v| k.to_s.include?("mentor_training_") } + end + end +end diff --git a/app/wizards/claims/add_claim_wizard/check_your_answers_step.rb b/app/wizards/claims/add_claim_wizard/check_your_answers_step.rb new file mode 100644 index 000000000..17a4e1ceb --- /dev/null +++ b/app/wizards/claims/add_claim_wizard/check_your_answers_step.rb @@ -0,0 +1,11 @@ +class Claims::AddClaimWizard::CheckYourAnswersStep < BaseStep + validate :mentors_have_claimable_hours + + delegate :claim, to: :wizard + + def mentors_have_claimable_hours + return unless claim.mentor_trainings.empty? || !claim.valid_mentor_training_hours? + + errors.add(:base, :unclaimable) + end +end diff --git a/app/wizards/claims/add_claim_wizard/mentor_step.rb b/app/wizards/claims/add_claim_wizard/mentor_step.rb new file mode 100644 index 000000000..b2ef5aa0c --- /dev/null +++ b/app/wizards/claims/add_claim_wizard/mentor_step.rb @@ -0,0 +1,27 @@ +class Claims::AddClaimWizard::MentorStep < BaseStep + attribute :mentor_ids, default: [] + + validates :mentor_ids, presence: true, inclusion: { in: ->(step) { step.mentors_with_claimable_hours.unscoped.ids } } + + delegate :school, :claim, :mentors_with_claimable_hours, to: :wizard + + def selected_mentors + return Claims::Mentor.none if mentors_with_claimable_hours.nil? + + @selected_mentors ||= mentors_with_claimable_hours.where(id: mentor_ids).order_by_full_name + end + + def all_school_mentors_visible? + @all_school_mentors_visible ||= school.mentors.count == mentors_with_claimable_hours.count + end + + def mentor_ids=(value) + super Array(value).compact_blank + end + + private + + def provider + @wizard.steps.fetch(:provider).provider + end +end diff --git a/app/wizards/claims/add_claim_wizard/mentor_training_step.rb b/app/wizards/claims/add_claim_wizard/mentor_training_step.rb new file mode 100644 index 000000000..8a620f71d --- /dev/null +++ b/app/wizards/claims/add_claim_wizard/mentor_training_step.rb @@ -0,0 +1,55 @@ +class Claims::AddClaimWizard::MentorTrainingStep < BaseStep + attribute :hours_completed + attribute :mentor_id + attribute :custom_hours_completed + + validates :mentor_id, presence: true + validates( + :hours_completed, + presence: true, + numericality: { + only_integer: true, + greater_than_or_equal_to: 1, + less_than_or_equal_to: :max_hours, + }, + unless: :custom_hours_selected?, + ) + + validates( + :custom_hours_completed, + presence: true, + numericality: { only_integer: true }, + between: { min: 1, max: :max_hours }, + if: :custom_hours_selected?, + ) + + def mentor + @mentor ||= @wizard.steps.fetch(:mentor).selected_mentors.find_by(id: mentor_id) + end + + def max_hours + training_allowance.remaining_hours + end + + def training_allowance + @training_allowance ||= Claims::TrainingAllowance.new( + mentor:, + provider:, + academic_year: @wizard.academic_year, + ) + end + + def total_hours_completed + (custom_hours_completed.presence || hours_completed).to_i + end + + delegate :full_name, to: :mentor, prefix: true + delegate :name, to: :provider, prefix: true + delegate :provider, to: :wizard + + private + + def custom_hours_selected? + hours_completed == "custom" + end +end diff --git a/app/wizards/claims/add_claim_wizard/no_mentors_step.rb b/app/wizards/claims/add_claim_wizard/no_mentors_step.rb new file mode 100644 index 000000000..1b10c11f8 --- /dev/null +++ b/app/wizards/claims/add_claim_wizard/no_mentors_step.rb @@ -0,0 +1,2 @@ +class Claims::AddClaimWizard::NoMentorsStep < BaseStep +end diff --git a/app/wizards/claims/add_claim_wizard/provider_step.rb b/app/wizards/claims/add_claim_wizard/provider_step.rb new file mode 100644 index 000000000..07a4b6eb1 --- /dev/null +++ b/app/wizards/claims/add_claim_wizard/provider_step.rb @@ -0,0 +1,13 @@ +class Claims::AddClaimWizard::ProviderStep < BaseStep + attribute :id + + validates :id, presence: true, inclusion: { in: ->(step) { step.providers_for_selection.ids } } + + def providers_for_selection + Claims::Provider.private_beta_providers.select(:id, :name) + end + + def provider + @provider ||= Claims::Provider.private_beta_providers.find_by(id:) + end +end diff --git a/config/locales/en/activemodel.yml b/config/locales/en/activemodel.yml index f7de13d64..e78aba34f 100644 --- a/config/locales/en/activemodel.yml +++ b/config/locales/en/activemodel.yml @@ -135,6 +135,28 @@ en: id: already_added: "%{school_name} has already been added. Try another school" blank: Select a school + claims/add_claim_wizard/provider_step: + attributes: + id: + blank: Select a provider + claims/add_claim_wizard/mentor_step: + attributes: + mentor_ids: + blank: Select a mentor + claims/add_claim_wizard/mentor_training_step: + attributes: + mentor_id: + blank: Select a mentor + custom_hours_completed: + blank: Enter the number of hours + not_an_integer: Enter whole numbers only + between: Enter the number of hours between %{min} and %{max} + hours_completed: + blank: Select the number of hours + claims/add_claim_wizard/check_your_answers_step: + attributes: + base: + unclaimable: You cannot submit the claim claims/claim/mentor_training_form: attributes: custom_hours_completed: diff --git a/config/locales/en/claims/schools/claims/add_claim.yml b/config/locales/en/claims/schools/claims/add_claim.yml new file mode 100644 index 000000000..6139ed36a --- /dev/null +++ b/config/locales/en/claims/schools/claims/add_claim.yml @@ -0,0 +1,8 @@ +en: + claims: + schools: + claims: + add_claim: + edit: + caption: Add claim + cancel: Cancel \ No newline at end of file diff --git a/config/locales/en/wizards/claims/add_claim_wizard.yml b/config/locales/en/wizards/claims/add_claim_wizard.yml new file mode 100644 index 000000000..bccd1d08f --- /dev/null +++ b/config/locales/en/wizards/claims/add_claim_wizard.yml @@ -0,0 +1,47 @@ +en: + wizards: + claims: + add_claim_wizard: + provider_step: + continue: Continue + page_title: Accredited provider - %{contextual_text} + title: Accredited provider + mentor_step: + continue: Continue + label: Mentor + heading: Mentors for %{provider_name} + select_all_that_apply: Select all that apply + mentor_training_step: + hours_of_training_for_mentor: Hours of training for %{mentor} + hours_of_training: Hours of training + hours: + one: "%{count} hour" + other: "%{count} hours" + hours_hint: + full: The full amount of hours for standard training + remaining: The remaining amount of hours for standard training + other_amount: Another amount + custom_hours_completed_hint: + one: Enter whole numbers up to a maximum of %{count} hour + other: Enter whole numbers up to a maximum of %{count} hours + number_of_hours: Number of hours + check_your_answers_step: + page_title: Check your answers + caption: Add claim + warning: You will not be able to change any of the claim details once you have submitted it. + submit: Submit claim + declaration: Declaration + disclaimer: "By submitting this claim, I confirm that:" + claim_on_behalf_of_school: I am authorised to claim on behalf of the school + read_and_accepted_grant_conditions: I have read and accepted the grant terms and conditions + accurate_information: the information detailed above is accurate and the total I am claiming back has been used to support the cost of the mentor training + provide_evidence: I will provide evidence to support this claim if requested by the Department for Education + total_hours: Total hours + hourly_rate: Hourly rate + hour: hour + grant_funding: Grant funding + hours_of_training: Hours of training + no_mentors_step: + heading_empty: No mentors for %{provider_name} + no_mentors_with_claimable_hours: There are no mentors you can include in a claim because they have already had 20 hours of training claimed for with %{provider_name}. + change_provider: Change the accredited provider \ No newline at end of file diff --git a/config/routes/claims.rb b/config/routes/claims.rb index 6933ba0d1..d557ce9d3 100644 --- a/config/routes/claims.rb +++ b/config/routes/claims.rb @@ -16,6 +16,12 @@ resources :schools, only: %i[index show] do scope module: :schools do resources :claims do + collection do + get "new", to: "claims/add_claim#new", as: :new_add_claim + get "new/:state_key/:step", to: "claims/add_claim#edit", as: :add_claim + put "new/:state_key/:step", to: "claims/add_claim#update" + end + resource :mentors, only: %i[new create edit update], module: :claims do member do get :create_revision diff --git a/spec/system/claims/schools/claims/create_claim_spec.rb b/spec/system/claims/schools/claims/create_claim_spec.rb index e1bd73f50..736f8562a 100644 --- a/spec/system/claims/schools/claims/create_claim_spec.rb +++ b/spec/system/claims/schools/claims/create_claim_spec.rb @@ -143,12 +143,12 @@ when_i_click("Change Accredited provider") when_i_choose_a_provider(bpn) when_i_click("Continue") - when_i_click("Change Mentors") then_i_should_see_the_message("There are no mentors you can include in a claim because they have already had 20 hours of training claimed for with Best Practice Network.") when_i_click("Change the accredited provider") when_i_choose_a_provider(niot) when_i_click("Continue") when_i_click("Continue") + when_i_click("Continue") then_i_should_land_on_the_check_page end @@ -311,7 +311,7 @@ def then_i_get_a_claim_reference_and_see_next_steps def then_i_expect_the_training_hours_for(hours, mentor) expect(page).to have_content("Hours of training for #{mentor.full_name}") - find("#claims-claim-mentor-training-form-hours-completed-#{hours}-field").checked? + find("#claims-add-claim-wizard-mentor-training-step-hours-completed-#{hours}-field").checked? end def then_i_see_the_error(message) diff --git a/spec/system/claims/support/schools/claims/create_claim_spec.rb b/spec/system/claims/support/schools/claims/create_claim_spec.rb index 39a53a428..3d25ea60c 100644 --- a/spec/system/claims/support/schools/claims/create_claim_spec.rb +++ b/spec/system/claims/support/schools/claims/create_claim_spec.rb @@ -15,7 +15,6 @@ create( :claims_support_user, :colin, - user_memberships: [create(:user_membership, organisation: school)], ) end let!(:bpn) { create(:claims_provider, :best_practice_network) } From f5c5bebafd141d0e517be5ac6b0eeab37f1d76f8 Mon Sep 17 00:00:00 2001 From: Jamie Date: Fri, 15 Nov 2024 13:22:47 +0000 Subject: [PATCH 2/8] Support user adding a claim uses the wizard pattern --- .../schools/claims/add_claim_controller.rb | 46 +++++++++++++++++++ app/services/claims/claim/create_draft.rb | 7 ++- .../schools/claims/add_claim/edit.html.erb | 13 ++++++ .../support/schools/claims/index.html.erb | 2 +- .../_check_your_answers_step.html.erb | 10 ++-- app/wizards/claims/add_claim_wizard.rb | 6 ++- .../support/schools/claims/add_claim.yml | 11 +++++ .../en/wizards/claims/add_claim_wizard.yml | 1 + config/routes/claims.rb | 6 +++ .../schools/claims/create_claim_spec.rb | 4 +- 10 files changed, 96 insertions(+), 10 deletions(-) create mode 100644 app/controllers/claims/support/schools/claims/add_claim_controller.rb create mode 100644 app/views/claims/support/schools/claims/add_claim/edit.html.erb create mode 100644 config/locales/en/claims/support/schools/claims/add_claim.yml diff --git a/app/controllers/claims/support/schools/claims/add_claim_controller.rb b/app/controllers/claims/support/schools/claims/add_claim_controller.rb new file mode 100644 index 000000000..0af2eec4e --- /dev/null +++ b/app/controllers/claims/support/schools/claims/add_claim_controller.rb @@ -0,0 +1,46 @@ +class Claims::Support::Schools::Claims::AddClaimController < Claims::Support::ApplicationController + include WizardController + include Claims::BelongsToSchool + + before_action :has_school_accepted_grant_conditions? + before_action :set_wizard + before_action :authorize_claim + + def update + if !@wizard.save_step + render "edit" + elsif @wizard.next_step.present? + redirect_to step_path(@wizard.next_step) + elsif @wizard.valid? + @wizard.create_claim + @wizard.reset_state + redirect_to claims_support_school_claims_path(@school), flash: { + heading: t(".success"), + } + else + redirect_to rejected_claims_support_school_claim_path(@school) + end + end + + private + + def set_wizard + state = session[state_key] ||= {} + current_step = params[:step]&.to_sym + @wizard = Claims::AddClaimWizard.new( + school: @school, created_by: current_user, params:, state:, current_step:, + ) + end + + def authorize_claim + authorize Claims::Claim, :create? + end + + def step_path(step) + add_claim_claims_support_school_claims_path(state_key:, step:) + end + + def index_path + claims_support_school_claims_path(@school) + end +end diff --git a/app/services/claims/claim/create_draft.rb b/app/services/claims/claim/create_draft.rb index f3b08c40f..bf4fd0a6a 100644 --- a/app/services/claims/claim/create_draft.rb +++ b/app/services/claims/claim/create_draft.rb @@ -35,7 +35,10 @@ def updated_claim end def generate_reference - reference = SecureRandom.random_number(99_999_999) while Claims::Claim.exists?(reference:) - reference + loop do + reference = SecureRandom.random_number(99_999_999) + + break reference unless Claims::Claim.exists?(reference:) + end end end diff --git a/app/views/claims/support/schools/claims/add_claim/edit.html.erb b/app/views/claims/support/schools/claims/add_claim/edit.html.erb new file mode 100644 index 000000000..3a9105273 --- /dev/null +++ b/app/views/claims/support/schools/claims/add_claim/edit.html.erb @@ -0,0 +1,13 @@ +<% render "claims/support/primary_navigation", current: :organisations %> + +<%= content_for(:before_content) do %> + <%= govuk_back_link(href: back_link_path) %> +<% end %> + +
+ <%= render_wizard(@wizard, contextual_text: t(".caption", school_name: @school.name)) %> + +

+ <%= govuk_link_to(t(".cancel"), claims_support_school_claims_path(@school), no_visited_state: true) %> +

+
diff --git a/app/views/claims/support/schools/claims/index.html.erb b/app/views/claims/support/schools/claims/index.html.erb index e4cb316ef..21be7981b 100644 --- a/app/views/claims/support/schools/claims/index.html.erb +++ b/app/views/claims/support/schools/claims/index.html.erb @@ -10,7 +10,7 @@ <% if @school.mentors.any? %> <%= govuk_inset_text text: t(".guidance", start_year: Claims::ClaimWindow.current.academic_year.starts_on.year, end_year: Claims::ClaimWindow.current.academic_year.ends_on.year) %> - <%= govuk_button_link_to t(".add_claim"), new_claims_support_school_claim_path %> + <%= govuk_button_link_to t(".add_claim"), new_add_claim_claims_support_school_claims_path %> <% else %> <%= govuk_inset_text text: t(".add_mentor_guidance_html", link_to: govuk_link_to(t(".add_a_mentor"), claims_support_school_mentors_path(@school))) %> <% end %> diff --git a/app/views/wizards/claims/add_claim_wizard/_check_your_answers_step.html.erb b/app/views/wizards/claims/add_claim_wizard/_check_your_answers_step.html.erb index 96cc3eb78..721c43a8a 100644 --- a/app/views/wizards/claims/add_claim_wizard/_check_your_answers_step.html.erb +++ b/app/views/wizards/claims/add_claim_wizard/_check_your_answers_step.html.erb @@ -7,9 +7,11 @@

<%= t(".page_title") %>

<%= govuk_summary_list do |summary_list| %> - <% summary_list.with_row do |row| %> - <% row.with_key(text: Claims::Claim.human_attribute_name(:school)) %> - <% row.with_value(text: @wizard.school.name) %> + <% unless current_user.support_user? %> + <% summary_list.with_row do |row| %> + <% row.with_key(text: Claims::Claim.human_attribute_name(:school)) %> + <% row.with_value(text: @wizard.school.name) %> + <% end %> <% end %> <% summary_list.with_row do |row| %> @@ -104,7 +106,7 @@ <% end %> - <%= f.govuk_submit t(".submit") %> + <%= f.govuk_submit current_user.support_user? ? t(".save") : t(".submit") %> <% end %> diff --git a/app/wizards/claims/add_claim_wizard.rb b/app/wizards/claims/add_claim_wizard.rb index 1009ee0ab..449e9e505 100644 --- a/app/wizards/claims/add_claim_wizard.rb +++ b/app/wizards/claims/add_claim_wizard.rb @@ -56,7 +56,11 @@ def create_claim raise "Invalid wizard state" unless valid? return unless claim.valid_mentor_training_hours? - Claims::Claim::Submit.call(claim:, user: created_by) + if created_by.support_user? + Claims::Claim::CreateDraft.call(claim: @claim) + else + Claims::Claim::Submit.call(claim:, user: created_by) + end end def mentors_with_claimable_hours diff --git a/config/locales/en/claims/support/schools/claims/add_claim.yml b/config/locales/en/claims/support/schools/claims/add_claim.yml new file mode 100644 index 000000000..588d6ebfd --- /dev/null +++ b/config/locales/en/claims/support/schools/claims/add_claim.yml @@ -0,0 +1,11 @@ +en: + claims: + support: + schools: + claims: + add_claim: + edit: + caption: Add claim - %{school_name} + cancel: Cancel + update: + success: Claim added \ No newline at end of file diff --git a/config/locales/en/wizards/claims/add_claim_wizard.yml b/config/locales/en/wizards/claims/add_claim_wizard.yml index bccd1d08f..d6e49c2d8 100644 --- a/config/locales/en/wizards/claims/add_claim_wizard.yml +++ b/config/locales/en/wizards/claims/add_claim_wizard.yml @@ -41,6 +41,7 @@ en: hour: hour grant_funding: Grant funding hours_of_training: Hours of training + save: Save claim no_mentors_step: heading_empty: No mentors for %{provider_name} no_mentors_with_claimable_hours: There are no mentors you can include in a claim because they have already had 20 hours of training claimed for with %{provider_name}. diff --git a/config/routes/claims.rb b/config/routes/claims.rb index d557ce9d3..6adf3cc2c 100644 --- a/config/routes/claims.rb +++ b/config/routes/claims.rb @@ -104,6 +104,12 @@ scope module: :schools do resources :claims do + collection do + get "new", to: "claims/add_claim#new", as: :new_add_claim + get "new/:state_key/:step", to: "claims/add_claim#edit", as: :add_claim + put "new/:state_key/:step", to: "claims/add_claim#update" + end + resource :mentors, only: %i[new create edit update], module: :claims do member do get :create_revision diff --git a/spec/system/claims/support/schools/claims/create_claim_spec.rb b/spec/system/claims/support/schools/claims/create_claim_spec.rb index 3d25ea60c..11e93df86 100644 --- a/spec/system/claims/support/schools/claims/create_claim_spec.rb +++ b/spec/system/claims/support/schools/claims/create_claim_spec.rb @@ -145,12 +145,12 @@ when_i_click("Change Accredited provider") when_i_choose_a_provider(bpn) when_i_click("Continue") - when_i_click("Change Mentors") then_i_should_see_the_message("There are no mentors you can include in a claim because they have already had 20 hours of training claimed for with Best Practice Network.") when_i_click("Change the accredited provider") when_i_choose_a_provider(niot) when_i_click("Continue") when_i_click("Continue") + when_i_click("Continue") then_i_should_land_on_the_check_page end @@ -248,7 +248,7 @@ def then_i_check_my_answers def then_i_expect_the_training_hours_for(hours, mentor) expect(page).to have_content("Hours of training for #{mentor.full_name}") - find("#claims-support-claim-mentor-training-form-hours-completed-#{hours}-field").checked? + find("#claims-add-claim-wizard-mentor-training-step-hours-completed-#{hours}-field").checked? end def then_i_am_redirectd_to_index_page(claim) From 816a3608e285d72549bb543c8c39ca51bed972c8 Mon Sep 17 00:00:00 2001 From: Jamie Date: Tue, 19 Nov 2024 09:15:52 +0000 Subject: [PATCH 3/8] Rspec tests for the add claim wizard --- app/wizards/claims/add_claim_wizard.rb | 2 +- .../claims/add_claim_wizard/mentor_step.rb | 2 +- .../add_claim_wizard/mentor_training_step.rb | 9 +- .../claims/add_claim_wizard/provider_step.rb | 2 +- config/locales/en/activemodel.yml | 2 + .../en/claims/schools/claims/add_claim.yml | 2 +- .../support/schools/claims/add_claim.yml | 2 +- .../en/wizards/claims/add_claim_wizard.yml | 2 +- .../check_your_answers_step_spec.rb | 46 +++ .../add_claim_wizard/mentor_step_spec.rb | 93 ++++++ .../mentor_training_step_spec.rb | 145 +++++++++ .../add_claim_wizard/provider_step_spec.rb | 56 ++++ spec/wizards/claims/add_claim_wizard_spec.rb | 284 ++++++++++++++++++ 13 files changed, 637 insertions(+), 10 deletions(-) create mode 100644 spec/wizards/claims/add_claim_wizard/check_your_answers_step_spec.rb create mode 100644 spec/wizards/claims/add_claim_wizard/mentor_step_spec.rb create mode 100644 spec/wizards/claims/add_claim_wizard/mentor_training_step_spec.rb create mode 100644 spec/wizards/claims/add_claim_wizard/provider_step_spec.rb create mode 100644 spec/wizards/claims/add_claim_wizard_spec.rb diff --git a/app/wizards/claims/add_claim_wizard.rb b/app/wizards/claims/add_claim_wizard.rb index 449e9e505..de8b6bd68 100644 --- a/app/wizards/claims/add_claim_wizard.rb +++ b/app/wizards/claims/add_claim_wizard.rb @@ -83,7 +83,7 @@ def provider def step_name(step_class, id = nil) # e.g. YearGroupStep becomes :year_group - name = step_class.name.chomp("Step").demodulize.underscore + name = super(step_class) return name.to_sym if id.blank? # e.g. with id it becomes :year_group_#{id} diff --git a/app/wizards/claims/add_claim_wizard/mentor_step.rb b/app/wizards/claims/add_claim_wizard/mentor_step.rb index b2ef5aa0c..96f3e3541 100644 --- a/app/wizards/claims/add_claim_wizard/mentor_step.rb +++ b/app/wizards/claims/add_claim_wizard/mentor_step.rb @@ -8,7 +8,7 @@ class Claims::AddClaimWizard::MentorStep < BaseStep def selected_mentors return Claims::Mentor.none if mentors_with_claimable_hours.nil? - @selected_mentors ||= mentors_with_claimable_hours.where(id: mentor_ids).order_by_full_name + @selected_mentors ||= Claims::Mentor.where(id: mentor_ids).order_by_full_name end def all_school_mentors_visible? diff --git a/app/wizards/claims/add_claim_wizard/mentor_training_step.rb b/app/wizards/claims/add_claim_wizard/mentor_training_step.rb index 8a620f71d..86fc31277 100644 --- a/app/wizards/claims/add_claim_wizard/mentor_training_step.rb +++ b/app/wizards/claims/add_claim_wizard/mentor_training_step.rb @@ -4,6 +4,7 @@ class Claims::AddClaimWizard::MentorTrainingStep < BaseStep attribute :custom_hours_completed validates :mentor_id, presence: true + validates :hours_completed, presence: true validates( :hours_completed, presence: true, @@ -23,6 +24,10 @@ class Claims::AddClaimWizard::MentorTrainingStep < BaseStep if: :custom_hours_selected?, ) + delegate :full_name, to: :mentor, prefix: true + delegate :name, to: :provider, prefix: true + delegate :provider, to: :wizard + def mentor @mentor ||= @wizard.steps.fetch(:mentor).selected_mentors.find_by(id: mentor_id) end @@ -43,10 +48,6 @@ def total_hours_completed (custom_hours_completed.presence || hours_completed).to_i end - delegate :full_name, to: :mentor, prefix: true - delegate :name, to: :provider, prefix: true - delegate :provider, to: :wizard - private def custom_hours_selected? diff --git a/app/wizards/claims/add_claim_wizard/provider_step.rb b/app/wizards/claims/add_claim_wizard/provider_step.rb index 07a4b6eb1..865ac1d74 100644 --- a/app/wizards/claims/add_claim_wizard/provider_step.rb +++ b/app/wizards/claims/add_claim_wizard/provider_step.rb @@ -4,7 +4,7 @@ class Claims::AddClaimWizard::ProviderStep < BaseStep validates :id, presence: true, inclusion: { in: ->(step) { step.providers_for_selection.ids } } def providers_for_selection - Claims::Provider.private_beta_providers.select(:id, :name) + Claims::Provider.private_beta_providers.order_by_name.select(:id, :name) end def provider diff --git a/config/locales/en/activemodel.yml b/config/locales/en/activemodel.yml index e78aba34f..2f475c7d3 100644 --- a/config/locales/en/activemodel.yml +++ b/config/locales/en/activemodel.yml @@ -153,6 +153,8 @@ en: between: Enter the number of hours between %{min} and %{max} hours_completed: blank: Select the number of hours + not_an_integer: Enter whole numbers only + between: Enter the number of hours between %{min} and %{max} claims/add_claim_wizard/check_your_answers_step: attributes: base: diff --git a/config/locales/en/claims/schools/claims/add_claim.yml b/config/locales/en/claims/schools/claims/add_claim.yml index 6139ed36a..2bf8a1a3e 100644 --- a/config/locales/en/claims/schools/claims/add_claim.yml +++ b/config/locales/en/claims/schools/claims/add_claim.yml @@ -5,4 +5,4 @@ en: add_claim: edit: caption: Add claim - cancel: Cancel \ No newline at end of file + cancel: Cancel diff --git a/config/locales/en/claims/support/schools/claims/add_claim.yml b/config/locales/en/claims/support/schools/claims/add_claim.yml index 588d6ebfd..4d2d4ee48 100644 --- a/config/locales/en/claims/support/schools/claims/add_claim.yml +++ b/config/locales/en/claims/support/schools/claims/add_claim.yml @@ -8,4 +8,4 @@ en: caption: Add claim - %{school_name} cancel: Cancel update: - success: Claim added \ No newline at end of file + success: Claim added diff --git a/config/locales/en/wizards/claims/add_claim_wizard.yml b/config/locales/en/wizards/claims/add_claim_wizard.yml index d6e49c2d8..9bb86bffe 100644 --- a/config/locales/en/wizards/claims/add_claim_wizard.yml +++ b/config/locales/en/wizards/claims/add_claim_wizard.yml @@ -45,4 +45,4 @@ en: no_mentors_step: heading_empty: No mentors for %{provider_name} no_mentors_with_claimable_hours: There are no mentors you can include in a claim because they have already had 20 hours of training claimed for with %{provider_name}. - change_provider: Change the accredited provider \ No newline at end of file + change_provider: Change the accredited provider diff --git a/spec/wizards/claims/add_claim_wizard/check_your_answers_step_spec.rb b/spec/wizards/claims/add_claim_wizard/check_your_answers_step_spec.rb new file mode 100644 index 000000000..b4006eda5 --- /dev/null +++ b/spec/wizards/claims/add_claim_wizard/check_your_answers_step_spec.rb @@ -0,0 +1,46 @@ +require "rails_helper" + +RSpec.describe Claims::AddClaimWizard::CheckYourAnswersStep, type: :model do + subject(:step) { described_class.new(wizard: mock_wizard, attributes:) } + + let(:mock_wizard) do + instance_double(Claims::AddClaimWizard).tap do |mock_wizard| + allow(mock_wizard).to receive(:claim).and_return(claim) + end + end + let(:claim) { build(:claim) } + let(:attributes) { nil } + + describe "validations" do + describe "#mentors_have_claimable_hours" do + context "when the claim has no mentor trainings" do + it "returns an error" do + expect(step.valid?).to be(false) + expect(step.errors.messages[:base]).to include("You cannot submit the claim") + end + end + + context "when the claim has no more claimable training hours" do + it "returns an error" do + allow(claim).to receive(:valid_mentor_training_hours?).and_return(false) + + expect(step.valid?).to be(false) + expect(step.errors.messages[:base]).to include("You cannot submit the claim") + end + end + + context "when the claim has mentor trainings and has claimable training hours" do + it "returns valid" do + allow(claim).to receive(:valid_mentor_training_hours?).and_return(true) + claim.mentor_trainings << build(:mentor_training) + + expect(step.valid?).to be(true) + end + end + end + end + + describe "delegations" do + it { is_expected.to delegate_method(:claim).to(:wizard) } + end +end diff --git a/spec/wizards/claims/add_claim_wizard/mentor_step_spec.rb b/spec/wizards/claims/add_claim_wizard/mentor_step_spec.rb new file mode 100644 index 000000000..ef8ca3346 --- /dev/null +++ b/spec/wizards/claims/add_claim_wizard/mentor_step_spec.rb @@ -0,0 +1,93 @@ +require "rails_helper" + +RSpec.describe Claims::AddClaimWizard::MentorStep, type: :model do + subject(:step) { described_class.new(wizard: mock_wizard, attributes:) } + + let(:mock_wizard) do + instance_double(Claims::AddClaimWizard).tap do |mock_wizard| + allow(mock_wizard).to receive_messages(school:, mentors_with_claimable_hours: claimable_mentors) + end + end + let(:school) { create(:claims_school) } + let!(:mentor_1) { create(:claims_mentor, schools: [school]) } + let!(:mentor_2) { create(:claims_mentor, schools: [school]) } + let(:claimable_mentors) do + Claims::Mentor.where(id: [mentor_1.id, mentor_2.id]) + end + + let(:attributes) { nil } + + describe "attributes" do + it { is_expected.to have_attributes(mentor_ids: []) } + end + + describe "validations" do + it { is_expected.to validate_inclusion_of(:mentor_ids).in_array(claimable_mentors.ids) } + end + + describe "delegations" do + it { is_expected.to delegate_method(:school).to(:wizard) } + it { is_expected.to delegate_method(:claim).to(:wizard) } + it { is_expected.to delegate_method(:mentors_with_claimable_hours).to(:wizard) } + end + + describe "#selected_mentors" do + subject(:selected_mentors) { step.selected_mentors } + + let(:attributes) { { mentor_ids: [mentor_1.id, mentor_2.id] } } + + context "when there are no mentors with claimable hours with the provider" do + let(:claimable_mentors) { nil } + + it "returns no mentors" do + expect(selected_mentors).to eq([]) + end + end + + context "when there are mentors with claimable hours with the provider" do + let(:claimable_mentors) { Claims::Mentor.where(id: mentor_1.id) } + + it "returns all selected mentors" do + expect(selected_mentors).to contain_exactly(mentor_1, mentor_2) + end + end + end + + describe "#all_school_mentors_visible?" do + subject(:all_school_mentors_visible) { step.all_school_mentors_visible? } + + context "when the number of members assigned to a school is the same as the number of mentors with claimable hours" do + it "returns true" do + expect(all_school_mentors_visible).to be(true) + end + end + + context "when the number of members assigned to a school is not the same as the number of mentors with claimable hours" do + let(:claimable_mentors) do + Claims::Mentor.where(id: mentor_1.id) + end + + it "returns true" do + expect(all_school_mentors_visible).to be(false) + end + end + end + + describe "#mentor_ids=" do + context "when the value is blank" do + it "remains blank" do + step.mentor_ids = [] + + expect(step.mentor_ids).to eq([]) + end + end + + context "when the value includes nil" do + it "removes all values except valid mentor ids" do + step.mentor_ids = [nil, mentor_1.id, mentor_2.id] + + expect(step.mentor_ids).to contain_exactly(mentor_1.id, mentor_2.id) + end + end + end +end diff --git a/spec/wizards/claims/add_claim_wizard/mentor_training_step_spec.rb b/spec/wizards/claims/add_claim_wizard/mentor_training_step_spec.rb new file mode 100644 index 000000000..918bbda51 --- /dev/null +++ b/spec/wizards/claims/add_claim_wizard/mentor_training_step_spec.rb @@ -0,0 +1,145 @@ +require "rails_helper" + +RSpec.describe Claims::AddClaimWizard::MentorTrainingStep, type: :model do + subject(:step) { described_class.new(wizard: mock_wizard, attributes:) } + + let(:mock_wizard) do + instance_double(Claims::AddClaimWizard).tap do |mock_wizard| + allow(mock_wizard).to receive_messages( + school:, + provider:, + academic_year: claim_window.academic_year, + steps: { mentor: mentor_step }, + ) + end + end + let(:mentor_step) do + instance_double(Claims::AddClaimWizard::MentorStep).tap do |mentor_step| + allow(mentor_step).to receive(:mentor_ids).and_return([mentor.id]) + allow(mentor_step).to receive_messages( + mentor_ids: [mentor.id], + selected_mentors: Claims::Mentor.where(id: mentor.id), + ) + end + end + let(:school) { create(:claims_school) } + let(:provider) { create(:claims_provider) } + let!(:mentor) { create(:claims_mentor, schools: [school]) } + let(:claim_window) { Claims::ClaimWindow.current || create(:claim_window, :current) } + let(:attributes) { nil } + + describe "attributes" do + it { is_expected.to have_attributes(mentor_id: nil, hours_completed: nil, custom_hours_completed: nil) } + end + + describe "validations" do + it { is_expected.to validate_presence_of(:mentor_id) } + it { is_expected.to validate_presence_of(:hours_completed) } + + context "when custom are selected" do + let(:attributes) { { hours_completed: "custom", mentor_id: mentor.id } } + + it do + expect(step).to validate_numericality_of(:custom_hours_completed) + .only_integer + .is_greater_than_or_equal_to(1) + .is_less_than_or_equal_to(20) + .with_message("Enter the number of hours between 1 and 20") + end + end + + context "when custom are not selected" do + let(:attributes) { { hours_completed: 20, mentor_id: mentor.id } } + + it { is_expected.to be_valid } + it { is_expected.not_to validate_presence_of(:custom_hours_completed) } + + it do + expect(step).to validate_numericality_of(:hours_completed) + .only_integer + .is_greater_than_or_equal_to(1) + .is_less_than_or_equal_to(20) + end + end + + describe "delegations" do + it { is_expected.to delegate_method(:provider).to(:wizard) } + it { is_expected.to delegate_method(:name).to(:provider).with_prefix(true) } + it { is_expected.to delegate_method(:full_name).to(:mentor).with_prefix(true) } + end + end + + describe "#mentor" do + context "when a mentor id is given" do + let(:attributes) { { mentor_id: mentor.id } } + + it "return the mentor associated with the given mentor id" do + expect(step.mentor).to eq(mentor) + end + end + + context "when a mentor id is not given" do + it "return the mentor associated with the given mentor id" do + expect(step.mentor).to be_nil + end + end + end + + describe "#training_allowance" do + subject(:training_allowance) { step.training_allowance } + + let(:attributes) { { mentor_id: mentor.id } } + + it "returns the training allowance for a given mentor" do + expect(training_allowance).to be_a(Claims::TrainingAllowance) + end + end + + describe "#max_hours" do + subject(:max_hours) { step.max_hours } + + let(:attributes) { { mentor_id: mentor.id } } + + context "when the mentor has no previous training hours with the provider" do + it "returns the maximum number of hours" do + expect(max_hours).to eq(20) + end + end + + context "when the mentor has previous training hours with the provider" do + before do + existing_claim = create(:claim, :submitted, provider:, school:, claim_window:) + create(:mentor_training, + claim: existing_claim, + hours_completed: 1, + mentor:, + provider:, + date_completed: claim_window.starts_on) + end + + it "returns the remaining number of claimable hours" do + expect(max_hours).to eq(19) + end + end + end + + describe "#total_hours_completed" do + subject(:total_hours_completed) { step.total_hours_completed } + + context "when custom hours completed is present" do + let(:attributes) { { mentor_id: mentor.id, custom_hours_completed: 20, hours_completed: 18 } } + + it "returns hours completed" do + expect(total_hours_completed).to eq(20) + end + end + + context "when custom hours completed is not present" do + let(:attributes) { { mentor_id: mentor.id, hours_completed: 18 } } + + it "returns hours completed" do + expect(total_hours_completed).to eq(18) + end + end + end +end diff --git a/spec/wizards/claims/add_claim_wizard/provider_step_spec.rb b/spec/wizards/claims/add_claim_wizard/provider_step_spec.rb new file mode 100644 index 000000000..a73bdce51 --- /dev/null +++ b/spec/wizards/claims/add_claim_wizard/provider_step_spec.rb @@ -0,0 +1,56 @@ +require "rails_helper" + +RSpec.describe Claims::AddClaimWizard::ProviderStep, type: :model do + subject(:step) { described_class.new(wizard: mock_wizard, attributes:) } + + let(:attributes) { nil } + let!(:niot_provider) { create(:claims_provider, :niot) } + let!(:bpn_provider) { create(:claims_provider, :best_practice_network) } + + let(:mock_wizard) do + instance_double(Claims::AddClaimWizard) + end + + describe "attributes" do + it { is_expected.to have_attributes(id: nil) } + end + + describe "validations" do + it { is_expected.to validate_inclusion_of(:id).in_array([niot_provider.id, bpn_provider.id]) } + end + + describe "#providers_for_selection" do + subject(:providers_for_selection) { step.providers_for_selection } + + before { create(:claims_provider) } + + it "returns only the providers scoped in the private beta" do + private_beta_providers = Claims::Provider.where(id: [bpn_provider.id, niot_provider.id]) + expect(providers_for_selection).to match_array(private_beta_providers.select(:id, :name)) + end + end + + describe "#provider" do + subject { step.provider } + + context "when id is set" do + context "when the provider is a private beta provider" do + let(:attributes) { { id: niot_provider.id } } + + it { is_expected.to eq(niot_provider) } + end + + context "when the provider is not a private beta provider" do + let(:attributes) { { id: create(:claims_provider).id } } + + it { is_expected.to be_nil } + end + end + + context "when id is nil" do + let(:attributes) { { id: nil } } + + it { is_expected.to be_nil } + end + end +end diff --git a/spec/wizards/claims/add_claim_wizard_spec.rb b/spec/wizards/claims/add_claim_wizard_spec.rb new file mode 100644 index 000000000..4261817e2 --- /dev/null +++ b/spec/wizards/claims/add_claim_wizard_spec.rb @@ -0,0 +1,284 @@ +require "rails_helper" + +RSpec.describe Claims::AddClaimWizard do + subject(:wizard) { described_class.new(school:, created_by:, state:, params:, current_step: nil) } + + let(:state) { {} } + let(:params_data) { {} } + let(:params) { ActionController::Parameters.new(params_data) } + let(:school) { create(:claims_school) } + let(:created_by) { create(:claims_user, schools: [school]) } + let(:provider) { create(:claims_provider, :niot) } + let(:claim_window) { Claims::ClaimWindow.current || create(:claim_window, :current) } + + before { claim_window } + + describe "#steps" do + subject { wizard.steps.keys } + + let(:state) do + { + "provider" => { "id" => provider.id }, + } + end + + context "when the school has no mentors" do + it { is_expected.to eq %i[provider no_mentors] } + end + + context "when the school has mentors" do + let(:mentor) { create(:claims_mentor, schools: [school]) } + + before { mentor } + + context "with claimable hours" do + it { is_expected.to eq %i[provider mentor check_your_answers] } + end + + context "when mentors have been selected" do + let(:state) do + { + "provider" => { "id" => provider.id }, + "mentor" => { "mentor_ids" => [mentor.id] }, + } + end + + it { is_expected.to eq [:provider, :mentor, "mentor_training_#{mentor.id}".to_sym, :check_your_answers] } + end + + context "with no claimable hours" do + before do + create(:mentor_training, + hours_completed: 20, + mentor:, + provider:, + date_completed: claim_window.starts_on + 1.day, + claim: create(:claim, :submitted, school:, provider:)) + end + + it { is_expected.to eq %i[provider no_mentors] } + end + end + end + + describe "#add_step" do + # this methods behaves just as it does in the BaseWizard, + # unless preset attributes are given. + context "when preset attribute 'mentor_id' is given" do + let(:mentor_id) { "abcd" } + + it "adds a step, with the 'mentor_id' step name and attributes" do + wizard.add_step(Claims::AddClaimWizard::MentorTrainingStep, { mentor_id: }) + expect(wizard.steps).to include(:mentor_training_abcd) + expect(wizard.steps[:mentor_training_abcd]).to be_a(Claims::AddClaimWizard::MentorTrainingStep) + expect(wizard.steps[:mentor_training_abcd]).to have_attributes(mentor_id:) + end + end + end + + describe "#academic_year" do + before { claim_window } + + it "returns the academic year of the current claim window" do + expect(wizard.academic_year).to eq(claim_window.academic_year) + end + end + + describe "#total_hours" do + let(:mentor_1) { create(:claims_mentor, schools: [school]) } + let(:mentor_2) { create(:claims_mentor, schools: [school]) } + let(:mentor_3) { create(:claims_mentor, schools: [school]) } + let(:mentor_4) { create(:claims_mentor, schools: [school]) } + let(:state) do + { + "provider" => { "id" => provider.id }, + "mentor" => { "mentor_ids" => [mentor_1.id, mentor_2.id, mentor_3.id, mentor_4.id] }, + "mentor_training_#{mentor_1.id}" => { + "mentor_id" => mentor_1.id, "hours_completed" => 20 + }, + "mentor_training_#{mentor_2.id}" => { + "mentor_id" => mentor_2.id, "hours_completed" => 20 + }, + "mentor_training_#{mentor_3.id}" => { + "mentor_id" => mentor_3.id, "hours_completed" => "custom", "custom_hours_completed" => 16 + }, + "mentor_training_#{mentor_4.id}" => { + "mentor_id" => mentor_4.id, "hours_completed" => "custom", "custom_hours_completed" => 4 + }, + } + end + + it "return the sum of the hours completed and custom hours completed" do + expect(wizard.total_hours).to eq(60) + end + end + + describe "#claim" do + let(:mentor_1) { create(:claims_mentor, schools: [school]) } + let(:mentor_2) { create(:claims_mentor, schools: [school]) } + let(:state) do + { + "provider" => { "id" => provider.id }, + "mentor" => { "mentor_ids" => [mentor_1.id, mentor_2.id] }, + "mentor_training_#{mentor_1.id}" => { + "mentor_id" => mentor_1.id, "hours_completed" => 20 + }, + "mentor_training_#{mentor_2.id}" => { + "mentor_id" => mentor_2.id, "hours_completed" => "custom", "custom_hours_completed" => 16 + }, + } + end + + it "initialises a new claim, build from the step attributes" do + claim = wizard.claim + expect(claim.new_record?).to be(true) + expect(claim).to be_a(Claims::Claim) + expect(claim.provider).to eq(provider) + expect(claim.school).to eq(school) + expect(claim.created_by).to eq(created_by) + end + + it "initialises new mentor trainings for the claim, per mentor set in the step attributes" do + claim = wizard.claim + expect(claim.mentor_trainings.size).to eq(2) + expect(claim.mentor_trainings.map(&:mentor)).to contain_exactly(mentor_1, mentor_2) + expect(claim.mentor_trainings.map(&:hours_completed)).to contain_exactly(20, 16) + end + end + + describe "#create_claim" do + subject(:create_claim) { wizard.create_claim } + + let(:mentor_1) { create(:claims_mentor, schools: [school], first_name: "Alan", last_name: "Anderson") } + let(:mentor_2) { create(:claims_mentor, schools: [school], first_name: "Bob", last_name: "Bletcher") } + + let(:state) do + { + "provider" => { "id" => provider.id }, + "mentor" => { "mentor_ids" => [mentor_1.id, mentor_2.id] }, + "mentor_training_#{mentor_1.id}" => { + "mentor_id" => mentor_1.id, "hours_completed" => 20 + }, + "mentor_training_#{mentor_2.id}" => { + "mentor_id" => mentor_2.id, "hours_completed" => "custom", "custom_hours_completed" => 16 + }, + } + end + + context "when the mentors still have available training hours with the provider" do + context "when the created by user, is not a support user" do + it "creates a submitted claim" do + expect { create_claim }.to change(Claims::Claim, :count).by(1) + .and change(Claims::MentorTraining, :count).by(2) + + claim = wizard.claim + + expect(claim).to be_persisted + expect(claim.school).to eq(school) + expect(claim.provider).to eq(provider) + expect(claim.created_by).to eq(created_by) + expect(claim.status).to eq("submitted") + expect(claim.claim_window).to eq(claim_window) + expect(claim.mentors.order_by_full_name).to contain_exactly(mentor_1, mentor_2) + + mentor_1_training = claim.mentor_trainings.find_by(mentor_id: mentor_1) + expect(mentor_1_training.hours_completed).to eq(20) + expect(mentor_1_training.provider).to eq(provider) + + mentor_2_training = claim.mentor_trainings.find_by(mentor_id: mentor_2) + expect(mentor_2_training.hours_completed).to eq(16) + expect(mentor_2_training.provider).to eq(provider) + end + end + + context "when the created by user, is a support user" do + let(:created_by) { create(:claims_support_user) } + + it "creates a draft claim" do + expect { create_claim }.to change(Claims::Claim, :count).by(1) + .and change(Claims::MentorTraining, :count).by(2) + + claim = wizard.claim + + expect(claim).to be_persisted + expect(claim.school).to eq(school) + expect(claim.provider).to eq(provider) + expect(claim.created_by).to eq(created_by) + expect(claim.status).to eq("draft") + expect(claim.claim_window).to eq(claim_window) + expect(claim.mentors.order_by_full_name).to contain_exactly(mentor_1, mentor_2) + + mentor_1_training = claim.mentor_trainings.find_by(mentor_id: mentor_1) + expect(mentor_1_training.hours_completed).to eq(20) + expect(mentor_1_training.provider).to eq(provider) + + mentor_2_training = claim.mentor_trainings.find_by(mentor_id: mentor_2) + expect(mentor_2_training.hours_completed).to eq(16) + expect(mentor_2_training.provider).to eq(provider) + end + end + end + + context "when the mentors have no available training hours with the provider" do + before do + existing_claim = create(:claim, :submitted, provider:, school:, claim_window:) + create(:mentor_training, + claim: existing_claim, + hours_completed: 20, + mentor: mentor_1, + provider:, + date_completed: claim_window.starts_on) + end + + it "does not create a claim" do + expect { create_claim }.to raise_error("Invalid wizard state") + end + end + end + + describe "#mentors_with_claimable_hours" do + subject(:mentors_with_claimable_hours) { wizard.mentors_with_claimable_hours } + + context "when a provider is not provided" do + it "returns no mentors" do + expect(mentors_with_claimable_hours).to eq([]) + end + end + + context "when a provider is provided" do + let(:mentor_1) { create(:claims_mentor, schools: [school]) } + let!(:mentor_2) { create(:claims_mentor, schools: [school]) } + let(:state) do + { + "provider" => { "id" => provider.id }, + } + end + + before do + existing_claim = create(:claim, :submitted, provider:, school:, claim_window:) + create(:mentor_training, + claim: existing_claim, + hours_completed: 20, + mentor: mentor_1, + provider:, + date_completed: claim_window.starts_on) + end + + it "returns all mentors with available hours with the provider" do + expect(mentors_with_claimable_hours).to contain_exactly(mentor_2) + end + end + end + + describe "#provider" do + let(:state) do + { + "provider" => { "id" => provider.id }, + } + end + + it "returns the provider given by the provider step" do + expect(wizard.provider).to eq(provider) + end + end +end From 0b2880ae35df73209db438c503d853e43594d5da Mon Sep 17 00:00:00 2001 From: Jamie Date: Tue, 19 Nov 2024 11:49:42 +0000 Subject: [PATCH 4/8] Fixes for rspec related to rejecting claims --- .../schools/claims/add_claim_controller.rb | 12 +++-- .../claims/schools/claims_controller.rb | 4 +- .../schools/claims/add_claim_controller.rb | 16 ++++--- .../support/schools/claims_controller.rb | 4 +- app/policies/claims/claim_policy.rb | 2 + app/wizards/claims/add_claim_wizard.rb | 3 +- .../check_your_answers_step.rb | 9 ---- .../claims/add_claim_wizard/mentor_step.rb | 6 --- config/routes/claims.rb | 10 +++- spec/policies/claims/claim_policy_spec.rb | 8 +++- .../check_your_answers_step_spec.rb | 46 ------------------- 11 files changed, 38 insertions(+), 82 deletions(-) delete mode 100644 spec/wizards/claims/add_claim_wizard/check_your_answers_step_spec.rb diff --git a/app/controllers/claims/schools/claims/add_claim_controller.rb b/app/controllers/claims/schools/claims/add_claim_controller.rb index 00f3a919f..5ce0d1e2f 100644 --- a/app/controllers/claims/schools/claims/add_claim_controller.rb +++ b/app/controllers/claims/schools/claims/add_claim_controller.rb @@ -11,12 +11,14 @@ def update render "edit" elsif @wizard.next_step.present? redirect_to step_path(@wizard.next_step) - elsif @wizard.valid? - @wizard.create_claim - @wizard.reset_state - redirect_to confirmation_claims_school_claim_path(@school, @wizard.claim) else - redirect_to rejected_claims_school_claim_path(@school) + begin + @wizard.create_claim + @wizard.reset_state + redirect_to confirmation_claims_school_claim_path(@school, @wizard.claim) + rescue StandardError + redirect_to rejected_claims_school_claims_path(@school) + end end end diff --git a/app/controllers/claims/schools/claims_controller.rb b/app/controllers/claims/schools/claims_controller.rb index 9424b9449..6f92aa72c 100644 --- a/app/controllers/claims/schools/claims_controller.rb +++ b/app/controllers/claims/schools/claims_controller.rb @@ -2,7 +2,7 @@ class Claims::Schools::ClaimsController < Claims::ApplicationController include Claims::BelongsToSchool before_action :has_school_accepted_grant_conditions? - before_action :set_claim, only: %i[show check confirmation submit edit update rejected create_revision remove destroy] + before_action :set_claim, only: %i[show check confirmation submit edit update create_revision remove destroy] before_action :authorize_claim before_action :get_valid_revision, only: :check @@ -103,7 +103,7 @@ def claim_id end def authorize_claim - authorize @claim || Claims::Claim + authorize @claim || Claims::Claim.new end def get_valid_revision diff --git a/app/controllers/claims/support/schools/claims/add_claim_controller.rb b/app/controllers/claims/support/schools/claims/add_claim_controller.rb index 0af2eec4e..97fabd852 100644 --- a/app/controllers/claims/support/schools/claims/add_claim_controller.rb +++ b/app/controllers/claims/support/schools/claims/add_claim_controller.rb @@ -11,14 +11,16 @@ def update render "edit" elsif @wizard.next_step.present? redirect_to step_path(@wizard.next_step) - elsif @wizard.valid? - @wizard.create_claim - @wizard.reset_state - redirect_to claims_support_school_claims_path(@school), flash: { - heading: t(".success"), - } else - redirect_to rejected_claims_support_school_claim_path(@school) + begin + @wizard.create_claim + @wizard.reset_state + redirect_to claims_support_school_claims_path(@school), flash: { + heading: t(".success"), + } + rescue StandardError + redirect_to rejected_claims_support_school_claims_path(@school) + end end end diff --git a/app/controllers/claims/support/schools/claims_controller.rb b/app/controllers/claims/support/schools/claims_controller.rb index 8f35c41b5..161e72874 100644 --- a/app/controllers/claims/support/schools/claims_controller.rb +++ b/app/controllers/claims/support/schools/claims_controller.rb @@ -1,7 +1,7 @@ class Claims::Support::Schools::ClaimsController < Claims::Support::ApplicationController include Claims::BelongsToSchool - before_action :set_claim, only: %i[check draft show edit update remove destroy rejected create_revision] + before_action :set_claim, only: %i[check draft show edit update remove destroy create_revision] before_action :authorize_claim before_action :get_valid_revision, only: :check @@ -112,7 +112,7 @@ def claim_provider_form end def authorize_claim - authorize @claim || Claims::Claim + authorize @claim || Claims::Claim.new end def get_valid_revision diff --git a/app/policies/claims/claim_policy.rb b/app/policies/claims/claim_policy.rb index 492af658e..dfeb030c6 100644 --- a/app/policies/claims/claim_policy.rb +++ b/app/policies/claims/claim_policy.rb @@ -33,6 +33,8 @@ def confirmation? # TODO: Remove record.draft? and not create drafts for existing drafts def draft? + return true if record.new_record? + current_claim_window? && user.support_user? && (record.internal_draft? || record.draft?) end diff --git a/app/wizards/claims/add_claim_wizard.rb b/app/wizards/claims/add_claim_wizard.rb index de8b6bd68..8555c771f 100644 --- a/app/wizards/claims/add_claim_wizard.rb +++ b/app/wizards/claims/add_claim_wizard.rb @@ -54,10 +54,9 @@ def claim def create_claim raise "Invalid wizard state" unless valid? - return unless claim.valid_mentor_training_hours? if created_by.support_user? - Claims::Claim::CreateDraft.call(claim: @claim) + Claims::Claim::CreateDraft.call(claim:) else Claims::Claim::Submit.call(claim:, user: created_by) end diff --git a/app/wizards/claims/add_claim_wizard/check_your_answers_step.rb b/app/wizards/claims/add_claim_wizard/check_your_answers_step.rb index 17a4e1ceb..70f9df247 100644 --- a/app/wizards/claims/add_claim_wizard/check_your_answers_step.rb +++ b/app/wizards/claims/add_claim_wizard/check_your_answers_step.rb @@ -1,11 +1,2 @@ class Claims::AddClaimWizard::CheckYourAnswersStep < BaseStep - validate :mentors_have_claimable_hours - - delegate :claim, to: :wizard - - def mentors_have_claimable_hours - return unless claim.mentor_trainings.empty? || !claim.valid_mentor_training_hours? - - errors.add(:base, :unclaimable) - end end diff --git a/app/wizards/claims/add_claim_wizard/mentor_step.rb b/app/wizards/claims/add_claim_wizard/mentor_step.rb index 96f3e3541..81e2726d2 100644 --- a/app/wizards/claims/add_claim_wizard/mentor_step.rb +++ b/app/wizards/claims/add_claim_wizard/mentor_step.rb @@ -18,10 +18,4 @@ def all_school_mentors_visible? def mentor_ids=(value) super Array(value).compact_blank end - - private - - def provider - @wizard.steps.fetch(:provider).provider - end end diff --git a/config/routes/claims.rb b/config/routes/claims.rb index 6adf3cc2c..67ad1b60d 100644 --- a/config/routes/claims.rb +++ b/config/routes/claims.rb @@ -37,10 +37,13 @@ get :remove get :check get :confirmation - get :rejected get :create_revision post :submit end + + collection do + get :rejected + end end resource :grant_conditions, only: %i[show update] @@ -124,10 +127,13 @@ member do get :remove get :check - get :rejected post :draft get :create_revision end + + collection do + get :rejected + end end resources :mentors, only: %i[index show destroy] do diff --git a/spec/policies/claims/claim_policy_spec.rb b/spec/policies/claims/claim_policy_spec.rb index 117556f8e..19be80c00 100644 --- a/spec/policies/claims/claim_policy_spec.rb +++ b/spec/policies/claims/claim_policy_spec.rb @@ -7,7 +7,7 @@ let(:support_user) { build(:claims_support_user) } let(:internal_draft_claim) { build(:claim) } let(:draft_claim) { build(:claim, :draft) } - let(:submitted_claim) { build(:claim, :submitted) } + let(:submitted_claim) { create(:claim, :submitted) } before do Claims::ClaimWindow::Build.call(claim_window_params: { starts_on: 2.days.ago, ends_on: 2.days.from_now }).save!(validate: false) @@ -80,6 +80,12 @@ end permissions :rejected? do + context "when user has an new claim (unsaved)" do + it "grants access" do + expect(claim_policy).to permit(user, Claims::Claim.new) + end + end + context "when user has an internal draft claim" do it "grants access" do expect(claim_policy).to permit(user, internal_draft_claim) diff --git a/spec/wizards/claims/add_claim_wizard/check_your_answers_step_spec.rb b/spec/wizards/claims/add_claim_wizard/check_your_answers_step_spec.rb deleted file mode 100644 index b4006eda5..000000000 --- a/spec/wizards/claims/add_claim_wizard/check_your_answers_step_spec.rb +++ /dev/null @@ -1,46 +0,0 @@ -require "rails_helper" - -RSpec.describe Claims::AddClaimWizard::CheckYourAnswersStep, type: :model do - subject(:step) { described_class.new(wizard: mock_wizard, attributes:) } - - let(:mock_wizard) do - instance_double(Claims::AddClaimWizard).tap do |mock_wizard| - allow(mock_wizard).to receive(:claim).and_return(claim) - end - end - let(:claim) { build(:claim) } - let(:attributes) { nil } - - describe "validations" do - describe "#mentors_have_claimable_hours" do - context "when the claim has no mentor trainings" do - it "returns an error" do - expect(step.valid?).to be(false) - expect(step.errors.messages[:base]).to include("You cannot submit the claim") - end - end - - context "when the claim has no more claimable training hours" do - it "returns an error" do - allow(claim).to receive(:valid_mentor_training_hours?).and_return(false) - - expect(step.valid?).to be(false) - expect(step.errors.messages[:base]).to include("You cannot submit the claim") - end - end - - context "when the claim has mentor trainings and has claimable training hours" do - it "returns valid" do - allow(claim).to receive(:valid_mentor_training_hours?).and_return(true) - claim.mentor_trainings << build(:mentor_training) - - expect(step.valid?).to be(true) - end - end - end - end - - describe "delegations" do - it { is_expected.to delegate_method(:claim).to(:wizard) } - end -end From aa3b435b44349ed7cd7b4092301a5b168e3f5c29 Mon Sep 17 00:00:00 2001 From: Jamie Date: Tue, 19 Nov 2024 12:04:18 +0000 Subject: [PATCH 5/8] Remove unnecessary claim routes --- app/controllers/claims/schools/claims_controller.rb | 10 ---------- .../claims/support/schools/claims_controller.rb | 10 ---------- config/routes/claims.rb | 4 ++-- 3 files changed, 2 insertions(+), 22 deletions(-) diff --git a/app/controllers/claims/schools/claims_controller.rb b/app/controllers/claims/schools/claims_controller.rb index 6f92aa72c..c4f2877a6 100644 --- a/app/controllers/claims/schools/claims_controller.rb +++ b/app/controllers/claims/schools/claims_controller.rb @@ -12,16 +12,6 @@ def index @pagy, @claims = pagy(@school.claims.active.order_created_at_desc) end - def new; end - - def create - if claim_provider_form.save - redirect_to new_claims_school_claim_mentors_path(@school, claim_provider_form.claim) - else - render :new - end - end - def create_revision revision = Claims::Claim::CreateRevision.call(claim: @claim) redirect_to edit_claims_school_claim_path(@school, revision) diff --git a/app/controllers/claims/support/schools/claims_controller.rb b/app/controllers/claims/support/schools/claims_controller.rb index 161e72874..6f40b984d 100644 --- a/app/controllers/claims/support/schools/claims_controller.rb +++ b/app/controllers/claims/support/schools/claims_controller.rb @@ -11,8 +11,6 @@ def index @pagy, @claims = pagy(@school.claims.active.order_created_at_desc) end - def new; end - def show; end def remove; end @@ -25,14 +23,6 @@ def destroy } end - def create - if claim_provider_form.save - redirect_to new_claims_support_school_claim_mentors_path(@school, claim_provider_form.claim) - else - render :new - end - end - def create_revision revision = Claims::Claim::CreateRevision.call(claim: @claim) redirect_to edit_claims_support_school_claim_path(@school, revision) diff --git a/config/routes/claims.rb b/config/routes/claims.rb index 67ad1b60d..472790144 100644 --- a/config/routes/claims.rb +++ b/config/routes/claims.rb @@ -15,7 +15,7 @@ resources :schools, only: %i[index show] do scope module: :schools do - resources :claims do + resources :claims, except: %i[new create] do collection do get "new", to: "claims/add_claim#new", as: :new_add_claim get "new/:state_key/:step", to: "claims/add_claim#edit", as: :add_claim @@ -106,7 +106,7 @@ end scope module: :schools do - resources :claims do + resources :claims, except: %i[new create] do collection do get "new", to: "claims/add_claim#new", as: :new_add_claim get "new/:state_key/:step", to: "claims/add_claim#edit", as: :add_claim From 07189fbefaff3008e13f1cf415bff9c6c595b34b Mon Sep 17 00:00:00 2001 From: Jamie Date: Tue, 19 Nov 2024 15:44:31 +0000 Subject: [PATCH 6/8] Change valid claim wizard guard check --- .../schools/claims/add_claim_controller.rb | 12 +++++------- app/wizards/claims/add_claim_wizard.rb | 2 -- spec/wizards/claims/add_claim_wizard_spec.rb | 16 ---------------- 3 files changed, 5 insertions(+), 25 deletions(-) diff --git a/app/controllers/claims/schools/claims/add_claim_controller.rb b/app/controllers/claims/schools/claims/add_claim_controller.rb index 5ce0d1e2f..99dda6621 100644 --- a/app/controllers/claims/schools/claims/add_claim_controller.rb +++ b/app/controllers/claims/schools/claims/add_claim_controller.rb @@ -11,14 +11,12 @@ def update render "edit" elsif @wizard.next_step.present? redirect_to step_path(@wizard.next_step) + elsif @wizard.valid? + @wizard.create_claim + @wizard.reset_state + redirect_to confirmation_claims_school_claim_path(@school, @wizard.claim) else - begin - @wizard.create_claim - @wizard.reset_state - redirect_to confirmation_claims_school_claim_path(@school, @wizard.claim) - rescue StandardError - redirect_to rejected_claims_school_claims_path(@school) - end + redirect_to rejected_claims_school_claims_path(@school) end end diff --git a/app/wizards/claims/add_claim_wizard.rb b/app/wizards/claims/add_claim_wizard.rb index 8555c771f..6bc27bb20 100644 --- a/app/wizards/claims/add_claim_wizard.rb +++ b/app/wizards/claims/add_claim_wizard.rb @@ -53,8 +53,6 @@ def claim end def create_claim - raise "Invalid wizard state" unless valid? - if created_by.support_user? Claims::Claim::CreateDraft.call(claim:) else diff --git a/spec/wizards/claims/add_claim_wizard_spec.rb b/spec/wizards/claims/add_claim_wizard_spec.rb index 4261817e2..6bb7acad0 100644 --- a/spec/wizards/claims/add_claim_wizard_spec.rb +++ b/spec/wizards/claims/add_claim_wizard_spec.rb @@ -218,22 +218,6 @@ end end end - - context "when the mentors have no available training hours with the provider" do - before do - existing_claim = create(:claim, :submitted, provider:, school:, claim_window:) - create(:mentor_training, - claim: existing_claim, - hours_completed: 20, - mentor: mentor_1, - provider:, - date_completed: claim_window.starts_on) - end - - it "does not create a claim" do - expect { create_claim }.to raise_error("Invalid wizard state") - end - end end describe "#mentors_with_claimable_hours" do From d4e2de1eed7fe3c14c517d5748fa3491b33497de Mon Sep 17 00:00:00 2001 From: Jamie Date: Thu, 21 Nov 2024 12:02:52 +0000 Subject: [PATCH 7/8] Improve mentor training step --- .../claims/add_claim_wizard/mentor_training_step.rb | 10 +++++++++- .../add_claim_wizard/mentor_training_step_spec.rb | 8 ++++---- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/app/wizards/claims/add_claim_wizard/mentor_training_step.rb b/app/wizards/claims/add_claim_wizard/mentor_training_step.rb index 86fc31277..73fd896cf 100644 --- a/app/wizards/claims/add_claim_wizard/mentor_training_step.rb +++ b/app/wizards/claims/add_claim_wizard/mentor_training_step.rb @@ -28,6 +28,14 @@ class Claims::AddClaimWizard::MentorTrainingStep < BaseStep delegate :name, to: :provider, prefix: true delegate :provider, to: :wizard + def initialize(wizard:, attributes:) + super + + return if custom_hours_selected? + + self.custom_hours_completed = nil + end + def mentor @mentor ||= @wizard.steps.fetch(:mentor).selected_mentors.find_by(id: mentor_id) end @@ -45,7 +53,7 @@ def training_allowance end def total_hours_completed - (custom_hours_completed.presence || hours_completed).to_i + (custom_hours_selected? ? custom_hours_completed : hours_completed).to_i end private diff --git a/spec/wizards/claims/add_claim_wizard/mentor_training_step_spec.rb b/spec/wizards/claims/add_claim_wizard/mentor_training_step_spec.rb index 918bbda51..4183e2b59 100644 --- a/spec/wizards/claims/add_claim_wizard/mentor_training_step_spec.rb +++ b/spec/wizards/claims/add_claim_wizard/mentor_training_step_spec.rb @@ -127,18 +127,18 @@ subject(:total_hours_completed) { step.total_hours_completed } context "when custom hours completed is present" do - let(:attributes) { { mentor_id: mentor.id, custom_hours_completed: 20, hours_completed: 18 } } + let(:attributes) { { mentor_id: mentor.id, custom_hours_completed: 6, hours_completed: "custom" } } it "returns hours completed" do - expect(total_hours_completed).to eq(20) + expect(total_hours_completed).to eq(6) end end context "when custom hours completed is not present" do - let(:attributes) { { mentor_id: mentor.id, hours_completed: 18 } } + let(:attributes) { { mentor_id: mentor.id, hours_completed: 20, custom_hours_completed: 6 } } it "returns hours completed" do - expect(total_hours_completed).to eq(18) + expect(total_hours_completed).to eq(20) end end end From 4e5d6e8bfed2aa233579403705ee4a6ed61df492 Mon Sep 17 00:00:00 2001 From: Jamie Date: Fri, 22 Nov 2024 15:07:31 +0000 Subject: [PATCH 8/8] Ollie suggestions and page titles --- .../schools/claims/add_claim_controller.rb | 2 ++ .../schools/claims/add_claim_controller.rb | 18 +++++------ app/policies/claims/claim_policy.rb | 4 +-- app/services/claims/claim/create_draft.rb | 10 ++---- app/services/claims/claim/submit.rb | 10 ++---- .../concerns/claims/claim/referencable.rb | 13 ++++++++ .../schools/claims/add_claim/edit.html.erb | 2 +- .../schools/claims/add_claim/edit.html.erb | 2 +- .../_check_your_answers_step.html.erb | 22 +++++++------ .../add_claim_wizard/_mentor_step.html.erb | 7 +++- .../_mentor_training_step.html.erb | 16 +++++++--- .../_no_mentors_step.html.erb | 5 +++ app/wizards/claims/add_claim_wizard.rb | 14 +++++--- .../add_claim_wizard/mentor_training_step.rb | 28 ++++++---------- config/locales/en/activemodel.yml | 6 ++-- .../en/wizards/claims/add_claim_wizard.yml | 7 ++-- config/routes/claims.rb | 10 ++---- .../schools/claims/create_claim_spec.rb | 4 +-- .../schools/claims/create_claim_spec.rb | 4 +-- .../mentor_training_step_spec.rb | 32 ++++++++----------- spec/wizards/claims/add_claim_wizard_spec.rb | 28 ++++++++-------- 21 files changed, 124 insertions(+), 120 deletions(-) create mode 100644 app/services/concerns/claims/claim/referencable.rb diff --git a/app/controllers/claims/schools/claims/add_claim_controller.rb b/app/controllers/claims/schools/claims/add_claim_controller.rb index 99dda6621..993963478 100644 --- a/app/controllers/claims/schools/claims/add_claim_controller.rb +++ b/app/controllers/claims/schools/claims/add_claim_controller.rb @@ -6,6 +6,8 @@ class Claims::Schools::Claims::AddClaimController < Claims::ApplicationControlle before_action :set_wizard before_action :authorize_claim + helper_method :index_path + def update if !@wizard.save_step render "edit" diff --git a/app/controllers/claims/support/schools/claims/add_claim_controller.rb b/app/controllers/claims/support/schools/claims/add_claim_controller.rb index 97fabd852..b3cb3f992 100644 --- a/app/controllers/claims/support/schools/claims/add_claim_controller.rb +++ b/app/controllers/claims/support/schools/claims/add_claim_controller.rb @@ -6,21 +6,21 @@ class Claims::Support::Schools::Claims::AddClaimController < Claims::Support::Ap before_action :set_wizard before_action :authorize_claim + helper_method :index_path + def update if !@wizard.save_step render "edit" elsif @wizard.next_step.present? redirect_to step_path(@wizard.next_step) + elsif @wizard.valid? + @wizard.create_claim + @wizard.reset_state + redirect_to claims_support_school_claims_path(@school), flash: { + heading: t(".success"), + } else - begin - @wizard.create_claim - @wizard.reset_state - redirect_to claims_support_school_claims_path(@school), flash: { - heading: t(".success"), - } - rescue StandardError - redirect_to rejected_claims_support_school_claims_path(@school) - end + redirect_to rejected_claims_support_school_claims_path(@school) end end diff --git a/app/policies/claims/claim_policy.rb b/app/policies/claims/claim_policy.rb index dfeb030c6..314214ba9 100644 --- a/app/policies/claims/claim_policy.rb +++ b/app/policies/claims/claim_policy.rb @@ -33,9 +33,7 @@ def confirmation? # TODO: Remove record.draft? and not create drafts for existing drafts def draft? - return true if record.new_record? - - current_claim_window? && user.support_user? && (record.internal_draft? || record.draft?) + current_claim_window? && user.support_user? && (record.internal_draft? || record.draft? || record.new_record?) end def check? diff --git a/app/services/claims/claim/create_draft.rb b/app/services/claims/claim/create_draft.rb index bf4fd0a6a..8ad6876af 100644 --- a/app/services/claims/claim/create_draft.rb +++ b/app/services/claims/claim/create_draft.rb @@ -1,4 +1,6 @@ class Claims::Claim::CreateDraft < ApplicationService + include Claims::Claim::Referencable + def initialize(claim:) @claim = claim end @@ -33,12 +35,4 @@ def updated_claim claim end end - - def generate_reference - loop do - reference = SecureRandom.random_number(99_999_999) - - break reference unless Claims::Claim.exists?(reference:) - end - end end diff --git a/app/services/claims/claim/submit.rb b/app/services/claims/claim/submit.rb index 65906c7f9..586e75e88 100644 --- a/app/services/claims/claim/submit.rb +++ b/app/services/claims/claim/submit.rb @@ -1,4 +1,6 @@ class Claims::Claim::Submit < ApplicationService + include Claims::Claim::Referencable + def initialize(claim:, user:) @claim = claim @user = user @@ -39,12 +41,4 @@ def updated_claim claim end end - - def generate_reference - loop do - reference = SecureRandom.random_number(99_999_999) - - break reference unless Claims::Claim.exists?(reference:) - end - end end diff --git a/app/services/concerns/claims/claim/referencable.rb b/app/services/concerns/claims/claim/referencable.rb new file mode 100644 index 000000000..233111ccf --- /dev/null +++ b/app/services/concerns/claims/claim/referencable.rb @@ -0,0 +1,13 @@ +module Claims::Claim::Referencable + extend ActiveSupport::Concern + + included do + def generate_reference + loop do + reference = SecureRandom.random_number(99_999_999) + + break reference unless Claims::Claim.exists?(reference:) + end + end + end +end diff --git a/app/views/claims/schools/claims/add_claim/edit.html.erb b/app/views/claims/schools/claims/add_claim/edit.html.erb index bef69cedc..259fbe83c 100644 --- a/app/views/claims/schools/claims/add_claim/edit.html.erb +++ b/app/views/claims/schools/claims/add_claim/edit.html.erb @@ -8,6 +8,6 @@ <%= render_wizard(@wizard, contextual_text: t(".caption")) %>

- <%= govuk_link_to(t(".cancel"), claims_school_claims_path(@school), no_visited_state: true) %> + <%= govuk_link_to(t(".cancel"), index_path, no_visited_state: true) %>

diff --git a/app/views/claims/support/schools/claims/add_claim/edit.html.erb b/app/views/claims/support/schools/claims/add_claim/edit.html.erb index 3a9105273..728ff1db2 100644 --- a/app/views/claims/support/schools/claims/add_claim/edit.html.erb +++ b/app/views/claims/support/schools/claims/add_claim/edit.html.erb @@ -8,6 +8,6 @@ <%= render_wizard(@wizard, contextual_text: t(".caption", school_name: @school.name)) %>

- <%= govuk_link_to(t(".cancel"), claims_support_school_claims_path(@school), no_visited_state: true) %> + <%= govuk_link_to(t(".cancel"), index_path, no_visited_state: true) %>

diff --git a/app/views/wizards/claims/add_claim_wizard/_check_your_answers_step.html.erb b/app/views/wizards/claims/add_claim_wizard/_check_your_answers_step.html.erb index 721c43a8a..77b4ce82b 100644 --- a/app/views/wizards/claims/add_claim_wizard/_check_your_answers_step.html.erb +++ b/app/views/wizards/claims/add_claim_wizard/_check_your_answers_step.html.erb @@ -1,10 +1,15 @@ +<% content_for :page_title, title_with_error_prefix( + t(".page_title", contextual_text:), + error: current_step.errors.any?, +) %> + <%= form_for(current_step, url: current_step_path, method: :put) do |f| %> <%= f.govuk_error_summary %>
- <%= t(".caption") %> -

<%= t(".page_title") %>

+ <%= contextual_text %> +

<%= t(".title") %>

<%= govuk_summary_list do |summary_list| %> <% unless current_user.support_user? %> @@ -21,7 +26,7 @@ <% summary_list.with_row do |row| %> <% row.with_key(text: Claims::Claim.human_attribute_name(:accredited_provider)) %> - <% row.with_value(text: @wizard.steps[:provider].provider.name) %> + <% row.with_value(text: @wizard.provider.name) %> <% row.with_action(text: t("change"), href: step_path(:provider), visually_hidden_text: Claims::Claim.human_attribute_name(:accredited_provider), @@ -52,18 +57,18 @@ <%= govuk_summary_list do |summary_list| %> <% @wizard.steps[:mentor].selected_mentors.each do |mentor| %> - <% mentor_training = @wizard.steps["mentor_training_#{mentor.id}".to_sym] %> + <% mentor_training = @wizard.steps[@wizard.step_name_for_mentor(mentor)] %> <% summary_list.with_row do |row| %> <% row.with_key(text: mentor_training.mentor.full_name) %> <% row.with_value( text: pluralize( - mentor_training.total_hours_completed, + mentor_training.hours_completed, t(".hour"), ), ) %> <% row.with_action( text: t("change"), - href: step_path("mentor_training_#{mentor.id}".to_sym), + href: step_path(@wizard.step_name_for_mentor(mentor)), visually_hidden_text: "Hours of training for #{mentor.full_name}", html_attributes: { class: "govuk-link--no-visited-state" }, ) %> @@ -100,10 +105,7 @@
  • <%= t(".provide_evidence") %>
    • -
    - - <%= t(".warning") %> -
    + <%= govuk_warning_text(text: t(".warning")) %> <% end %> <%= f.govuk_submit current_user.support_user? ? t(".save") : t(".submit") %> diff --git a/app/views/wizards/claims/add_claim_wizard/_mentor_step.html.erb b/app/views/wizards/claims/add_claim_wizard/_mentor_step.html.erb index 5cea2e03a..7a2f84281 100644 --- a/app/views/wizards/claims/add_claim_wizard/_mentor_step.html.erb +++ b/app/views/wizards/claims/add_claim_wizard/_mentor_step.html.erb @@ -1,10 +1,15 @@ +<% content_for :page_title, title_with_error_prefix( + t(".page_title", contextual_text:, provider_name: @wizard.provider.name), + error: current_step.errors.any?, +) %> + <%= form_for(current_step, url: current_step_path, method: :put) do |f| %> <%= f.govuk_error_summary %>
    <%= contextual_text %> -

    <%= t(".heading", provider_name: @wizard.steps[:provider].provider.name) %>

    +

    <%= t(".heading", provider_name: @wizard.provider.name) %>

    <%= render Claims::Claim::MentorsForm::DisclaimerComponent.new(mentors_form: current_step) %> diff --git a/app/views/wizards/claims/add_claim_wizard/_mentor_training_step.html.erb b/app/views/wizards/claims/add_claim_wizard/_mentor_training_step.html.erb index 8f8cc0226..ca815bab0 100644 --- a/app/views/wizards/claims/add_claim_wizard/_mentor_training_step.html.erb +++ b/app/views/wizards/claims/add_claim_wizard/_mentor_training_step.html.erb @@ -1,3 +1,8 @@ +<% content_for :page_title, title_with_error_prefix( + t(".page_title", contextual_text:, provider_name: @wizard.provider.name, mentor: current_step.mentor.full_name), + error: current_step.errors.any?, +) %> + <%= form_for(current_step, url: current_step_path, method: :put) do |f| %> <%= f.govuk_error_summary %> @@ -6,24 +11,25 @@ <%= contextual_text %> - <%= @wizard.steps[:provider].provider.name %>

    <%= t(".hours_of_training_for_mentor", mentor: current_step.mentor.full_name) %>

    - <%= render Claims::Claim::MentorTrainingForm::DisclaimerComponent.new(mentor_training_form: current_step) %> <%= f.govuk_radio_buttons_fieldset( - :hours_completed, + :hours_to_claim, legend: { size: "m", text: t(".hours_of_training"), }, ) do %> - <%= f.govuk_radio_button :hours_completed, current_step.max_hours, label: { text: t(".hours", count: current_step.max_hours) }, hint: { text: t(".hours_hint.#{current_step.max_hours == 20 ? "full" : "remaining"}", count: current_step.max_hours) } %> + <%= f.govuk_radio_button :hours_to_claim, "maximum", label: { text: t(".hours", count: current_step.max_hours) }, hint: { text: t(".hours_hint.#{current_step.max_hours == 20 ? "full" : "remaining"}", count: current_step.max_hours) } %> <%= f.govuk_radio_divider %> - <%= f.govuk_radio_button(:hours_completed, "custom", label: { text: t(".other_amount") }) do %> + <%= f.govuk_radio_button(:hours_to_claim, "custom", label: { text: t(".other_amount") }) do %> <%= f.govuk_number_field( - :custom_hours_completed, + :custom_hours, class: "govuk-input--width-2", + min: 1, + max: current_step.max_hours, label: { text: t(".number_of_hours"), class: "govuk-!-font-weight-bold" }, hint: { text: t(".custom_hours_completed_hint", count: current_step.max_hours) }, ) %> diff --git a/app/views/wizards/claims/add_claim_wizard/_no_mentors_step.html.erb b/app/views/wizards/claims/add_claim_wizard/_no_mentors_step.html.erb index ce5c5c2ff..d525b1046 100644 --- a/app/views/wizards/claims/add_claim_wizard/_no_mentors_step.html.erb +++ b/app/views/wizards/claims/add_claim_wizard/_no_mentors_step.html.erb @@ -1,3 +1,8 @@ +<% content_for :page_title, title_with_error_prefix( + t(".page_title", contextual_text:, provider_name: @wizard.provider.name), + error: current_step.errors.any?, +) %> +

    <%= t(".heading_empty", provider_name: @wizard.provider.name) %>

    diff --git a/app/wizards/claims/add_claim_wizard.rb b/app/wizards/claims/add_claim_wizard.rb index 6bc27bb20..1cfb5cb1e 100644 --- a/app/wizards/claims/add_claim_wizard.rb +++ b/app/wizards/claims/add_claim_wizard.rb @@ -33,7 +33,7 @@ def academic_year end def total_hours - mentor_training_steps.values.map(&:total_hours_completed).sum + mentor_training_steps.map(&:hours_completed).sum end def claim @@ -42,10 +42,10 @@ def claim school:, created_by:, claim_window: Claims::ClaimWindow.current, - mentor_trainings_attributes: mentor_training_steps.map do |_k, v| + mentor_trainings_attributes: mentor_training_steps.map do |mentor_training_step| { - mentor_id: v.mentor_id, - hours_completed: v.total_hours_completed, + mentor_id: mentor_training_step.mentor_id, + hours_completed: mentor_training_step.hours_completed, provider:, } end, @@ -76,6 +76,10 @@ def provider steps.fetch(:provider).provider end + def step_name_for_mentor(mentor) + step_name(MentorTrainingStep, mentor.id) + end + private def step_name(step_class, id = nil) @@ -96,7 +100,7 @@ def step_attributes(name, step_class, preset_attributes = {}) end def mentor_training_steps - steps.select { |k, _v| k.to_s.include?("mentor_training_") } + steps.values.select { |step| step.is_a?(MentorTrainingStep) } end end end diff --git a/app/wizards/claims/add_claim_wizard/mentor_training_step.rb b/app/wizards/claims/add_claim_wizard/mentor_training_step.rb index 73fd896cf..e1ac0bb8f 100644 --- a/app/wizards/claims/add_claim_wizard/mentor_training_step.rb +++ b/app/wizards/claims/add_claim_wizard/mentor_training_step.rb @@ -1,23 +1,15 @@ class Claims::AddClaimWizard::MentorTrainingStep < BaseStep - attribute :hours_completed attribute :mentor_id - attribute :custom_hours_completed + attribute :hours_to_claim, :string + attribute :custom_hours + + HOURS_TO_CLAIM = %w[maximum custom].freeze validates :mentor_id, presence: true - validates :hours_completed, presence: true - validates( - :hours_completed, - presence: true, - numericality: { - only_integer: true, - greater_than_or_equal_to: 1, - less_than_or_equal_to: :max_hours, - }, - unless: :custom_hours_selected?, - ) + validates :hours_to_claim, presence: true, inclusion: { in: HOURS_TO_CLAIM } validates( - :custom_hours_completed, + :custom_hours, presence: true, numericality: { only_integer: true }, between: { min: 1, max: :max_hours }, @@ -33,7 +25,7 @@ def initialize(wizard:, attributes:) return if custom_hours_selected? - self.custom_hours_completed = nil + self.custom_hours = nil end def mentor @@ -52,13 +44,13 @@ def training_allowance ) end - def total_hours_completed - (custom_hours_selected? ? custom_hours_completed : hours_completed).to_i + def hours_completed + (hours_to_claim == "maximum" ? max_hours : custom_hours).to_i end private def custom_hours_selected? - hours_completed == "custom" + hours_to_claim == "custom" end end diff --git a/config/locales/en/activemodel.yml b/config/locales/en/activemodel.yml index 2f475c7d3..99a2a70e6 100644 --- a/config/locales/en/activemodel.yml +++ b/config/locales/en/activemodel.yml @@ -147,14 +147,12 @@ en: attributes: mentor_id: blank: Select a mentor - custom_hours_completed: + custom_hours: blank: Enter the number of hours not_an_integer: Enter whole numbers only between: Enter the number of hours between %{min} and %{max} - hours_completed: + hours_to_claim: blank: Select the number of hours - not_an_integer: Enter whole numbers only - between: Enter the number of hours between %{min} and %{max} claims/add_claim_wizard/check_your_answers_step: attributes: base: diff --git a/config/locales/en/wizards/claims/add_claim_wizard.yml b/config/locales/en/wizards/claims/add_claim_wizard.yml index 9bb86bffe..633381858 100644 --- a/config/locales/en/wizards/claims/add_claim_wizard.yml +++ b/config/locales/en/wizards/claims/add_claim_wizard.yml @@ -7,11 +7,13 @@ en: page_title: Accredited provider - %{contextual_text} title: Accredited provider mentor_step: + page_title: Mentors for %{provider_name} - %{contextual_text} continue: Continue label: Mentor heading: Mentors for %{provider_name} select_all_that_apply: Select all that apply mentor_training_step: + page_title: Hours of training for %{mentor} - %{contextual_text} - %{provider_name} hours_of_training_for_mentor: Hours of training for %{mentor} hours_of_training: Hours of training hours: @@ -26,8 +28,8 @@ en: other: Enter whole numbers up to a maximum of %{count} hours number_of_hours: Number of hours check_your_answers_step: - page_title: Check your answers - caption: Add claim + page_title: Check your answers - %{contextual_text} + title: Check your answers warning: You will not be able to change any of the claim details once you have submitted it. submit: Submit claim declaration: Declaration @@ -43,6 +45,7 @@ en: hours_of_training: Hours of training save: Save claim no_mentors_step: + page_title: No mentors for %{provider_name} - %{contextual_text} heading_empty: No mentors for %{provider_name} no_mentors_with_claimable_hours: There are no mentors you can include in a claim because they have already had 20 hours of training claimed for with %{provider_name}. change_provider: Change the accredited provider diff --git a/config/routes/claims.rb b/config/routes/claims.rb index 472790144..f0eaa401a 100644 --- a/config/routes/claims.rb +++ b/config/routes/claims.rb @@ -20,6 +20,7 @@ get "new", to: "claims/add_claim#new", as: :new_add_claim get "new/:state_key/:step", to: "claims/add_claim#edit", as: :add_claim put "new/:state_key/:step", to: "claims/add_claim#update" + get :rejected end resource :mentors, only: %i[new create edit update], module: :claims do @@ -40,10 +41,6 @@ get :create_revision post :submit end - - collection do - get :rejected - end end resource :grant_conditions, only: %i[show update] @@ -111,6 +108,7 @@ get "new", to: "claims/add_claim#new", as: :new_add_claim get "new/:state_key/:step", to: "claims/add_claim#edit", as: :add_claim put "new/:state_key/:step", to: "claims/add_claim#update" + get :rejected end resource :mentors, only: %i[new create edit update], module: :claims do @@ -130,10 +128,6 @@ post :draft get :create_revision end - - collection do - get :rejected - end end resources :mentors, only: %i[index show destroy] do diff --git a/spec/system/claims/schools/claims/create_claim_spec.rb b/spec/system/claims/schools/claims/create_claim_spec.rb index 736f8562a..a9ba04472 100644 --- a/spec/system/claims/schools/claims/create_claim_spec.rb +++ b/spec/system/claims/schools/claims/create_claim_spec.rb @@ -309,9 +309,9 @@ def then_i_get_a_claim_reference_and_see_next_steps expect(page).to have_content("We will process this claim at the end of September 2024 and all payments will be paid from December 2024.") end - def then_i_expect_the_training_hours_for(hours, mentor) + def then_i_expect_the_training_hours_for(_hours, mentor) expect(page).to have_content("Hours of training for #{mentor.full_name}") - find("#claims-add-claim-wizard-mentor-training-step-hours-completed-#{hours}-field").checked? + find("#claims-add-claim-wizard-mentor-training-step-hours-to-claim-maximum-field").checked? end def then_i_see_the_error(message) diff --git a/spec/system/claims/support/schools/claims/create_claim_spec.rb b/spec/system/claims/support/schools/claims/create_claim_spec.rb index 11e93df86..8d9165c62 100644 --- a/spec/system/claims/support/schools/claims/create_claim_spec.rb +++ b/spec/system/claims/support/schools/claims/create_claim_spec.rb @@ -246,9 +246,9 @@ def then_i_check_my_answers end end - def then_i_expect_the_training_hours_for(hours, mentor) + def then_i_expect_the_training_hours_for(_hours, mentor) expect(page).to have_content("Hours of training for #{mentor.full_name}") - find("#claims-add-claim-wizard-mentor-training-step-hours-completed-#{hours}-field").checked? + find("#claims-add-claim-wizard-mentor-training-step-hours-to-claim-maximum-field").checked? end def then_i_am_redirectd_to_index_page(claim) diff --git a/spec/wizards/claims/add_claim_wizard/mentor_training_step_spec.rb b/spec/wizards/claims/add_claim_wizard/mentor_training_step_spec.rb index 4183e2b59..fbbfd0a58 100644 --- a/spec/wizards/claims/add_claim_wizard/mentor_training_step_spec.rb +++ b/spec/wizards/claims/add_claim_wizard/mentor_training_step_spec.rb @@ -29,18 +29,19 @@ let(:attributes) { nil } describe "attributes" do - it { is_expected.to have_attributes(mentor_id: nil, hours_completed: nil, custom_hours_completed: nil) } + it { is_expected.to have_attributes(mentor_id: nil, hours_to_claim: nil, custom_hours: nil) } end describe "validations" do it { is_expected.to validate_presence_of(:mentor_id) } - it { is_expected.to validate_presence_of(:hours_completed) } + it { is_expected.to validate_presence_of(:hours_to_claim) } + it { is_expected.to validate_inclusion_of(:hours_to_claim).in_array(described_class::HOURS_TO_CLAIM) } context "when custom are selected" do - let(:attributes) { { hours_completed: "custom", mentor_id: mentor.id } } + let(:attributes) { { hours_to_claim: "custom", mentor_id: mentor.id } } it do - expect(step).to validate_numericality_of(:custom_hours_completed) + expect(step).to validate_numericality_of(:custom_hours) .only_integer .is_greater_than_or_equal_to(1) .is_less_than_or_equal_to(20) @@ -49,17 +50,10 @@ end context "when custom are not selected" do - let(:attributes) { { hours_completed: 20, mentor_id: mentor.id } } + let(:attributes) { { hours_to_claim: "maximum", mentor_id: mentor.id } } it { is_expected.to be_valid } - it { is_expected.not_to validate_presence_of(:custom_hours_completed) } - - it do - expect(step).to validate_numericality_of(:hours_completed) - .only_integer - .is_greater_than_or_equal_to(1) - .is_less_than_or_equal_to(20) - end + it { is_expected.not_to validate_presence_of(:custom_hours) } end describe "delegations" do @@ -123,22 +117,22 @@ end end - describe "#total_hours_completed" do - subject(:total_hours_completed) { step.total_hours_completed } + describe "#hours_completed" do + subject(:hours_completed) { step.hours_completed } context "when custom hours completed is present" do - let(:attributes) { { mentor_id: mentor.id, custom_hours_completed: 6, hours_completed: "custom" } } + let(:attributes) { { mentor_id: mentor.id, custom_hours: 6, hours_to_claim: "custom" } } it "returns hours completed" do - expect(total_hours_completed).to eq(6) + expect(hours_completed).to eq(6) end end context "when custom hours completed is not present" do - let(:attributes) { { mentor_id: mentor.id, hours_completed: 20, custom_hours_completed: 6 } } + let(:attributes) { { mentor_id: mentor.id, hours_to_claim: "maximum", custom_hours: 6 } } it "returns hours completed" do - expect(total_hours_completed).to eq(20) + expect(hours_completed).to eq(20) end end end diff --git a/spec/wizards/claims/add_claim_wizard_spec.rb b/spec/wizards/claims/add_claim_wizard_spec.rb index 6bb7acad0..2e0abe874 100644 --- a/spec/wizards/claims/add_claim_wizard_spec.rb +++ b/spec/wizards/claims/add_claim_wizard_spec.rb @@ -27,30 +27,30 @@ end context "when the school has mentors" do - let(:mentor) { create(:claims_mentor, schools: [school]) } - - before { mentor } + let!(:mentor_1) { create(:claims_mentor, schools: [school], first_name: "Alan", last_name: "Anderson") } context "with claimable hours" do it { is_expected.to eq %i[provider mentor check_your_answers] } end context "when mentors have been selected" do + let!(:mentor_2) { create(:claims_mentor, schools: [school], first_name: "Bob", last_name: "Bletcher") } + let(:state) do { "provider" => { "id" => provider.id }, - "mentor" => { "mentor_ids" => [mentor.id] }, + "mentor" => { "mentor_ids" => [mentor_1.id, mentor_2.id] }, } end - it { is_expected.to eq [:provider, :mentor, "mentor_training_#{mentor.id}".to_sym, :check_your_answers] } + it { is_expected.to eq [:provider, :mentor, "mentor_training_#{mentor_1.id}".to_sym, "mentor_training_#{mentor_2.id}".to_sym, :check_your_answers] } end context "with no claimable hours" do before do create(:mentor_training, hours_completed: 20, - mentor:, + mentor: mentor_1, provider:, date_completed: claim_window.starts_on + 1.day, claim: create(:claim, :submitted, school:, provider:)) @@ -94,16 +94,16 @@ "provider" => { "id" => provider.id }, "mentor" => { "mentor_ids" => [mentor_1.id, mentor_2.id, mentor_3.id, mentor_4.id] }, "mentor_training_#{mentor_1.id}" => { - "mentor_id" => mentor_1.id, "hours_completed" => 20 + "mentor_id" => mentor_1.id, "hours_to_claim" => "maximum" }, "mentor_training_#{mentor_2.id}" => { - "mentor_id" => mentor_2.id, "hours_completed" => 20 + "mentor_id" => mentor_2.id, "hours_to_claim" => "maximum" }, "mentor_training_#{mentor_3.id}" => { - "mentor_id" => mentor_3.id, "hours_completed" => "custom", "custom_hours_completed" => 16 + "mentor_id" => mentor_3.id, "hours_to_claim" => "custom", "custom_hours" => 16 }, "mentor_training_#{mentor_4.id}" => { - "mentor_id" => mentor_4.id, "hours_completed" => "custom", "custom_hours_completed" => 4 + "mentor_id" => mentor_4.id, "hours_to_claim" => "custom", "custom_hours" => 4 }, } end @@ -121,10 +121,10 @@ "provider" => { "id" => provider.id }, "mentor" => { "mentor_ids" => [mentor_1.id, mentor_2.id] }, "mentor_training_#{mentor_1.id}" => { - "mentor_id" => mentor_1.id, "hours_completed" => 20 + "mentor_id" => mentor_1.id, "hours_to_claim" => "maximum" }, "mentor_training_#{mentor_2.id}" => { - "mentor_id" => mentor_2.id, "hours_completed" => "custom", "custom_hours_completed" => 16 + "mentor_id" => mentor_2.id, "hours_to_claim" => "custom", "custom_hours" => 16 }, } end @@ -157,10 +157,10 @@ "provider" => { "id" => provider.id }, "mentor" => { "mentor_ids" => [mentor_1.id, mentor_2.id] }, "mentor_training_#{mentor_1.id}" => { - "mentor_id" => mentor_1.id, "hours_completed" => 20 + "mentor_id" => mentor_1.id, "hours_to_claim" => "maximum" }, "mentor_training_#{mentor_2.id}" => { - "mentor_id" => mentor_2.id, "hours_completed" => "custom", "custom_hours_completed" => 16 + "mentor_id" => mentor_2.id, "hours_to_claim" => "custom", "custom_hours" => 16 }, } end