diff --git a/app/controllers/claims/support/schools/claims/mentor_trainings_controller.rb b/app/controllers/claims/support/schools/claims/mentor_trainings_controller.rb index 1ec9560149..2c3067ce37 100644 --- a/app/controllers/claims/support/schools/claims/mentor_trainings_controller.rb +++ b/app/controllers/claims/support/schools/claims/mentor_trainings_controller.rb @@ -3,7 +3,11 @@ class Claims::Support::Schools::Claims::MentorTrainingsController < Claims::Appl before_action :authorize_claim helper_method :mentor_training_form - def edit; end + def edit + if create_revision? + @claim_revision = claim.create_revision! + end + end def update if mentor_training_form.save @@ -32,11 +36,20 @@ def mentor_training_params end def default_params - { claim:, mentor_training: } + { claim: @claim_revision || claim, mentor_training: } end def mentor_training - @mentor_training ||= @claim.mentor_trainings.find(params.require(:id)) + current_claim = @claim_revision || claim + mentor_training = current_claim.mentor_trainings.find_by_id(params.require(:id)) + + @mentor_training ||= if mentor_training.nil? + current_claim.mentor_trainings.find_by( + mentor_id: current_claim.previous_revision.mentor_trainings.find(params.require(:id)).mentor_id, + ) + else + mentor_training + end end def claim @@ -46,4 +59,8 @@ def claim def authorize_claim authorize claim end + + def create_revision? + params[:revision] == "true" + end end diff --git a/app/controllers/claims/support/schools/claims/mentors_controller.rb b/app/controllers/claims/support/schools/claims/mentors_controller.rb index f5acafeead..c34b454b38 100644 --- a/app/controllers/claims/support/schools/claims/mentors_controller.rb +++ b/app/controllers/claims/support/schools/claims/mentors_controller.rb @@ -2,9 +2,9 @@ class Claims::Support::Schools::Claims::MentorsController < Claims::ApplicationC include Claims::BelongsToSchool before_action :authorize_claim - helper_method :claim_mentors_form - - def new; end + def new + render locals: { claim_mentors_form: } + end def create if claim_mentors_form.save @@ -16,17 +16,25 @@ def create ), ) else - render :new + render :new, locals: { claim_mentors_form: } end end - def edit; end + def edit + if create_revision? + revision = claim.create_revision! + + render locals: { claim_mentors_form: claim_mentors_form(revision) } + else + render locals: { claim_mentors_form: } + end + end def update if claim_mentors_form.save redirect_to claim_mentors_form.update_success_path else - render :edit + render :edit, locals: { claim_mentors_form: } end end @@ -40,16 +48,25 @@ def claim @claim ||= @school.claims.find(params.require(:claim_id)) end - def claim_mentors_form + def claim_mentors_form(claim_revision = nil) + claim_param = claim_revision || claim + @claim_mentors_form ||= if params[:claims_claim].present? - Claims::Support::Claim::MentorsForm.new(claim:, mentor_ids: claim_params[:mentor_ids]) + Claims::Support::Claim::MentorsForm.new( + claim: claim_param, + mentor_ids: claim_params[:mentor_ids], + ) else - Claims::Support::Claim::MentorsForm.new(claim:) + Claims::Support::Claim::MentorsForm.new(claim: claim_param) end end def authorize_claim authorize claim end + + def create_revision? + params[:revision] == "true" + end end diff --git a/app/controllers/claims/support/schools/claims_controller.rb b/app/controllers/claims/support/schools/claims_controller.rb index 8b0afbdcaa..272655d4ea 100644 --- a/app/controllers/claims/support/schools/claims_controller.rb +++ b/app/controllers/claims/support/schools/claims_controller.rb @@ -30,20 +30,24 @@ def create end def check - last_mentor_training = @claim.mentor_trainings.order_by_mentor_full_name.last + @valid_claim = @claim.get_valid_revision + last_mentor_training = @valid_claim.mentor_trainings.order_by_mentor_full_name.last @back_path = edit_claims_support_school_claim_mentor_training_path( @school, - @claim, + @valid_claim, last_mentor_training, params: { claims_support_claim_mentor_training_form: { hours_completed: last_mentor_training.hours_completed }, }, ) - Claims::Claim::RemoveEmptyMentorTrainingHours.call(claim: @claim) end - def edit; end + def edit + if create_revision? + @claim_revision = @claim.create_revision! + end + end def update if claim_provider_form.save @@ -54,8 +58,14 @@ def update end def draft - success_message = @claim.draft? ? t(".update_success") : t(".add_success") - Claims::Claim::CreateDraft.call(claim: @claim) + claim_was_draft = @claim.was_draft? + success_message = claim_was_draft ? t(".update_success") : t(".add_success") + + if claim_was_draft + Claims::Claim::UpdateDraft.call(claim: @claim) + else + Claims::Claim::CreateDraft.call(claim: @claim) + end redirect_to claims_support_school_claims_path(@school), flash: { success: success_message } end @@ -73,7 +83,7 @@ def default_params end def claim_id - params[:claim_id] || params[:id] + @claim_revision&.id || params[:claim_id] || params[:id] end def set_claim @@ -92,4 +102,8 @@ def claim_provider_form def authorize_claim authorize @claim || Claims::Claim end + + def create_revision? + params[:revision] == "true" + end end diff --git a/app/services/claims/claim/update_draft.rb b/app/services/claims/claim/update_draft.rb new file mode 100644 index 0000000000..a3e3c4c7cf --- /dev/null +++ b/app/services/claims/claim/update_draft.rb @@ -0,0 +1,28 @@ +class Claims::Claim::UpdateDraft < Claims::Claim::CreateDraft + include ServicePattern + + def call + ActiveRecord::Base.transaction do + updated_claim.save! + update_previous_revisions_to_internal_draft + end + end + + private + + def update_previous_revisions_to_internal_draft + claim_record = updated_claim.previous_revision + + while claim_record.present? + claim_record.update!(status: :internal_draft) if claim_record.draft? + claim_record = claim_record.previous_revision + end + end + + def updated_claim + @updated_claim ||= begin + claim.status = :draft + claim + end + end +end diff --git a/app/views/claims/support/claims/_details.html.erb b/app/views/claims/support/claims/_details.html.erb index b8f37a7c50..98bc917585 100644 --- a/app/views/claims/support/claims/_details.html.erb +++ b/app/views/claims/support/claims/_details.html.erb @@ -10,7 +10,11 @@ <% row.with_value(text: claim.provider.name) %> <% if policy(claim).edit? %> <% row.with_action(text: t("change"), - href: edit_claims_support_school_claim_path(claim.school, claim), + href: edit_claims_support_school_claim_path( + claim.school, + claim, + params: { revision: true }, + ), html_attributes: { class: "govuk-link--no-visited-state", }) %> @@ -34,7 +38,11 @@ <% end %> <% if policy(claim).edit? %> <% row.with_action(text: t("change"), - href: edit_claims_support_school_claim_mentor_path(claim.school, claim), + href: edit_claims_support_school_claim_mentor_path( + claim.school, + claim, + params: { revision: true }, + ), html_attributes: { class: "govuk-link--no-visited-state", }) %> @@ -56,6 +64,7 @@ mentor_training, params: { claims_support_claim_mentor_training_form: { hours_completed: mentor_training.hours_completed }, + revision: true, }, ), html_attributes: { diff --git a/app/views/claims/support/schools/claims/check.html.erb b/app/views/claims/support/schools/claims/check.html.erb index 4f453ba2a2..3e2187c55a 100644 --- a/app/views/claims/support/schools/claims/check.html.erb +++ b/app/views/claims/support/schools/claims/check.html.erb @@ -16,11 +16,12 @@ <%= govuk_summary_list do |summary_list| %> <% summary_list.with_row do |row| %> <% row.with_key(text: Claims::Claim.human_attribute_name(:accredited_provider)) %> - <% row.with_value(text: @claim.provider_name) %> + <% row.with_value(text: @valid_claim.provider_name) %> <% row.with_action(text: t("change"), href: edit_claims_support_school_claim_path( @school, - @claim, + @valid_claim, + params: { revision: true }, ), html_attributes: { class: "govuk-link--no-visited-state", @@ -31,13 +32,16 @@ <% row.with_key(text: Claims::Claim.human_attribute_name(:mentors)) %> <% row.with_value do %>
<%= govuk_link_to t("cancel"), claims_support_school_claims_path(@school), no_visited_state: true %> diff --git a/spec/services/claims/claim/update_draft_spec.rb b/spec/services/claims/claim/update_draft_spec.rb new file mode 100644 index 0000000000..be677595b5 --- /dev/null +++ b/spec/services/claims/claim/update_draft_spec.rb @@ -0,0 +1,19 @@ +require "rails_helper" + +describe Claims::Claim::UpdateDraft do + subject(:draft_service) { described_class.call(claim:) } + + let!(:previous_revision) { create(:claim, :draft) } + let!(:claim) { create(:claim, :internal_draft, previous_revision:) } + + it_behaves_like "a service object" do + let(:params) { { claim: } } + end + + describe "#call" do + it "updates draft claim and sets the previous revisions to internal_draft status" do + expect { draft_service }.to change(claim, :status).from("internal_draft").to("draft") + expect(claim.previous_revision.status).to eq("internal_draft") + end + end +end diff --git a/spec/system/claims/support/schools/claims/change_claim_on_check_page_spec.rb b/spec/system/claims/support/schools/claims/change_claim_on_check_page_spec.rb index b0670a24f4..00d4eb5ed0 100644 --- a/spec/system/claims/support/schools/claims/change_claim_on_check_page_spec.rb +++ b/spec/system/claims/support/schools/claims/change_claim_on_check_page_spec.rb @@ -24,28 +24,21 @@ create(:mentor_training, mentor: mentor2, provider: provider1, claim:, hours_completed: 12) end - scenario "Colin changes the provider on claim on check page" do + scenario "Colin changes the provider on claim on check page and doesn't need to add the mentor hours again" do given_i_visit_claim_support_check_page when_i_click_change_provider then_i_expect_the_provider_to_be_checked(provider1) when_i_change_the_provider then_i_expect_the_provider_to_be_checked(provider2) when_i_click("Continue") - then_i_check_my_answers(provider2, [mentor1, mentor2], [20, 12]) + then_i_check_my_answers(claim, provider2, [mentor1, mentor2], [20, 12]) + when_i_click_change_mentors + when_i_click("Continue") + then_i_check_my_answers(claim, provider2, [mentor1, mentor2], [20, 12]) when_i_click("Add claim") then_i_am_redirected_to_index_page(claim) end - scenario "Colin does not have a provider selected when editing a claim from check page" do - given_i_visit_claim_support_check_page - when_i_click_change_provider - then_i_expect_the_provider_to_be_checked(provider1) - when_i_remove_the_provider_from_the_claim - then_i_reload_the_page - when_i_click("Continue") - then_i_see_the_error("Select a provider") - end - scenario "Colin changes the mentors on claim on check page" do given_i_visit_claim_support_check_page when_i_click_change_mentors @@ -59,7 +52,7 @@ then_i_see_the_error("Select the number of hours") when_i_add_training_hours("20 hours") when_i_click("Continue") - then_i_check_my_answers(provider1, [mentor2], [20]) + then_i_check_my_answers(claim.reload.next_revision, provider1, [mentor2], [20]) when_i_click("Add claim") then_i_am_redirected_to_index_page(claim) end @@ -69,7 +62,7 @@ when_i_click_change_mentors then_i_expect_the_mentors_to_be_checked([mentor1, mentor2]) when_i_click("Continue") - then_i_check_my_answers(provider1, [mentor1, mentor2], [20, 12]) + then_i_check_my_answers(claim, provider1, [mentor1, mentor2], [20, 12]) end scenario "Colin changes the mentors on claim without inputting hours" do @@ -85,7 +78,21 @@ when_i_click("Back") then_i_expect_the_mentors_to_be_checked([mentor1, mentor2, mentor3]) when_i_click("Back") - then_i_check_my_answers(provider1, [mentor1, mentor2], [20, 12]) + then_i_check_my_answers(claim, provider1, [mentor1, mentor2], [20, 12]) + then_i_cant_see_the_mentor(mentor3) + end + + scenario "Colin changes the mentors on claim without inputting hours for any mentor" do + given_i_visit_claim_support_check_page + when_i_click_change_mentors + then_i_expect_the_mentors_to_be_checked([mentor1, mentor2]) + when_i_uncheck_the_mentors([mentor1, mentor2]) + when_i_check_the_mentor(mentor3) + when_i_click("Continue") + when_i_click("Back") + then_i_expect_the_mentors_to_be_checked([mentor3]) + when_i_click("Back") + then_i_check_my_answers(claim, provider1, [mentor1, mentor2], [20, 12]) then_i_cant_see_the_mentor(mentor3) end @@ -98,7 +105,7 @@ then_i_see_the_error("Enter the number of hours") when_i_choose_other_amount_and_input_hours(6, with_error: true) when_i_click("Continue") - then_i_check_my_answers(provider1, [mentor1, mentor2], [6, 12]) + then_i_check_my_answers(claim, provider1, [mentor1, mentor2], [6, 12]) end scenario "Collin intends to change the training hours but clicks back link" do @@ -211,7 +218,7 @@ def then_i_expect_the_training_hours_for(hours, mentor) find("#claims-support-claim-mentor-training-form-hours-completed-#{hours}-field").checked? end - def then_i_check_my_answers(provider, mentors, mentor_hours) + def then_i_check_my_answers(claim_record, provider, mentors, mentor_hours) expect(page).to have_content("Check your answers") within("dl.govuk-summary-list:nth(1)") do @@ -229,7 +236,7 @@ def then_i_check_my_answers(provider, mentors, mentor_hours) end within("dl.govuk-summary-list:nth(2)") do - claim.mentor_trainings.each_with_index do |mentor_training, index| + claim_record.mentor_trainings.each_with_index do |mentor_training, index| expect(page).to have_content(mentor_training.mentor.full_name) expect(page).to have_content(mentor_hours[index]) end @@ -257,13 +264,4 @@ def then_i_am_redirected_to_index_page(claim) expect(page).to have_content(claim.reload.reference) end - - def when_i_remove_the_provider_from_the_claim - claim.provider_id = nil - claim.save!(validate: false) - end - - def then_i_reload_the_page - refresh - end end 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 53e20d4a8e..257df7aee0 100644 --- a/spec/system/claims/support/schools/claims/create_claim_spec.rb +++ b/spec/system/claims/support/schools/claims/create_claim_spec.rb @@ -70,6 +70,16 @@ then_i_see_the_error("Enter whole numbers only") end + scenario "Colin tries to create claim but backs off" do + when_i_click(school.name) + when_i_click_on_claims + when_i_click("Add claim") + when_i_choose_a_provider + when_i_click("Continue") + visit edit_claims_support_school_claim_path(school, Claims::Claim.last) + ### This will be fixed in a new PR + end + private def given_i_sign_in diff --git a/spec/system/claims/support/schools/claims/edit_draft_claim_spec.rb b/spec/system/claims/support/schools/claims/edit_draft_claim_spec.rb index cd9a424e64..e858ca0412 100644 --- a/spec/system/claims/support/schools/claims/edit_draft_claim_spec.rb +++ b/spec/system/claims/support/schools/claims/edit_draft_claim_spec.rb @@ -42,10 +42,39 @@ when_i_select_a_school when_i_click_on_claims when_i_visit_the_draft_claim_show_page - then_i_edit_the_provider + then_i_edit_the_provider( + current_provider: best_practice_network_provider, + new_provider: niot_provider, + ) then_i_edit_the_mentors then_i_edit_the_hours_of_training - then_i_update_the_claim + then_i_expect_the_current_draft_claims_to_not_have_my_changes + then_i_update_the_claim(another_claims_mentor) + end + + scenario "A support user I finish editing a claim and go back to the check page to edit it again" do + user_exists_in_dfe_sign_in(user: colin) + given_i_sign_in + when_i_select_a_school + when_i_click_on_claims + when_i_visit_the_draft_claim_show_page + then_i_edit_the_provider( + current_provider: best_practice_network_provider, + new_provider: niot_provider, + ) + then_i_update_the_claim(claims_mentor) + when_i_go_back_to_the_check_page + then_i_edit_the_provider( + current_provider: niot_provider, + new_provider: best_practice_network_provider, + ) + then_i_edit_the_provider( + current_provider: best_practice_network_provider, + new_provider: niot_provider, + ) + then_i_expect_the_current_draft_claims_to_not_have_my_changes + when_i_click("Update claim") + then_i_expect_to_not_have_duplicated_claims end scenario "A support user I can't edit a non draft claim" do @@ -58,9 +87,14 @@ private - def then_i_update_the_claim + def then_i_update_the_claim(mentor) click_on("Update claim") expect(page).to have_content("Claim updated") + expect(page).to have_content(mentor.full_name) + expect(page).to have_content( + "NIoT: National Institute of Teaching, founded by the "\ + "School-Led Development Trust", + ) end def then_i_edit_the_hours_of_training @@ -77,19 +111,17 @@ def then_i_edit_the_mentors click_on("Continue") page.choose("20 hours") click_on("Continue") - page.choose("20 hours") - click_on("Continue") expect(page).to have_content("Laura Clark") expect(page).to have_content("Barry Garlow") end - def then_i_edit_the_provider - expect(page).to have_content("Accredited providerBest Practice NetworkChange") + def then_i_edit_the_provider(current_provider:, new_provider:) + expect(page).to have_content(current_provider.name) first("a", text: "Change").click - page.choose(niot_provider.name) + page.choose(new_provider.name) click_on("Continue") - expect(page).to have_content("Accredited providerNIoT: National Institute of Teaching, founded by the School-Led Development TrustChange") + expect(page).to have_content(new_provider.name) end def when_i_visit_the_draft_claim_show_page @@ -122,4 +154,24 @@ def given_i_sign_in visit sign_in_path click_on "Sign in using DfE Sign In" end + + def then_i_expect_the_current_draft_claims_to_not_have_my_changes + mentor_names = Claims::Claim.visible.flat_map(&:mentors).map(&:full_name) + + expect(mentor_names.include?(another_claims_mentor.full_name)).to eq(false) + end + + def when_i_go_back_to_the_check_page + claim = Claims::Claim.visible.draft.first + visit check_claims_support_school_claim_path(claim.school, claim) + end + + def then_i_expect_to_not_have_duplicated_claims + uniq_references = Claims::Claim.visible.pluck(:reference).uniq + expect(Claims::Claim.visible.count).to eq(uniq_references.count) + end + + def when_i_click(button) + click_on(button) + end end