<%= govuk_link_to t("cancel"), claims_support_school_claims_path(@school), no_visited_state: true %> diff --git a/app/views/claims/support/schools/claims/mentors/edit.html.erb b/app/views/claims/support/schools/claims/mentors/edit.html.erb index 78f08c4b15..8b031d262c 100644 --- a/app/views/claims/support/schools/claims/mentors/edit.html.erb +++ b/app/views/claims/support/schools/claims/mentors/edit.html.erb @@ -10,7 +10,7 @@ partial: "form", locals: { claim_mentors_form:, - form_url: claims_support_school_claim_mentor_path(@school, claim_mentors_form.claim), + form_url: claims_support_school_claim_mentors_path(@school, claim_mentors_form.claim), method: :patch, school: @school, }, diff --git a/config/analytics.yml b/config/analytics.yml index 14a035903f..4d111c94c2 100644 --- a/config/analytics.yml +++ b/config/analytics.yml @@ -81,18 +81,19 @@ shared: - created_at - updated_at :claims: - - id - - school_id - - created_at - - updated_at - - provider_id - - reference - - submitted_at - - created_by_type - - created_by_id - - status - - submitted_by_type - - submitted_by_id + - id + - school_id + - created_at + - updated_at + - provider_id + - reference + - submitted_at + - created_by_type + - created_by_id + - status + - submitted_by_type + - submitted_by_id + - previous_revision_id :schools: - id - urn diff --git a/config/routes/claims.rb b/config/routes/claims.rb index 0b574404d0..4db95f56fe 100644 --- a/config/routes/claims.rb +++ b/config/routes/claims.rb @@ -14,13 +14,23 @@ resources :schools, only: %i[index show] do scope module: :schools do resources :claims, except: %i[destroy] do - resources :mentors, only: %i[new create edit update], module: :claims - resources :mentor_trainings, only: %i[edit update], module: :claims + resource :mentors, only: %i[edit update], module: :claims + resources :mentors, only: %i[new create], module: :claims do + member do + post :create_revision + end + end + resources :mentor_trainings, only: %i[edit update], module: :claims do + member do + post :create_revision + end + end member do get :check get :confirmation get :rejected + post :create_revision post :submit end end @@ -60,14 +70,24 @@ scope module: :schools do resources :claims do - resources :mentors, only: %i[new create edit update], module: :claims - resources :mentor_trainings, only: %i[edit update], module: :claims + resource :mentors, only: %i[edit update], module: :claims + resources :mentors, only: %i[new create], module: :claims do + member do + post :create_revision + end + end + resources :mentor_trainings, only: %i[edit update], module: :claims do + member do + post :create_revision + end + end member do get :remove get :check get :rejected post :draft + post :create_revision end end diff --git a/db/migrate/20240419100125_add_revisions_to_claims.rb b/db/migrate/20240419100125_add_revisions_to_claims.rb new file mode 100644 index 0000000000..6a9e247746 --- /dev/null +++ b/db/migrate/20240419100125_add_revisions_to_claims.rb @@ -0,0 +1,6 @@ +class AddRevisionsToClaims < ActiveRecord::Migration[7.1] + def change + add_column :claims, :previous_revision_id, :uuid, null: true + add_index :claims, :previous_revision_id + end +end diff --git a/db/migrate/20240422135851_remove_claim_reference_unique_constraint.rb b/db/migrate/20240422135851_remove_claim_reference_unique_constraint.rb new file mode 100644 index 0000000000..a718df5e0c --- /dev/null +++ b/db/migrate/20240422135851_remove_claim_reference_unique_constraint.rb @@ -0,0 +1,6 @@ +class RemoveClaimReferenceUniqueConstraint < ActiveRecord::Migration[7.1] + def change + remove_index(:claims, :reference, unique: true) + add_index(:claims, :reference) + end +end diff --git a/db/schema.rb b/db/schema.rb index d702c1ec46..33a0eb242a 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -58,9 +58,11 @@ t.enum "status", enum_type: "claim_status" t.string "submitted_by_type" t.uuid "submitted_by_id" + t.uuid "previous_revision_id" t.index ["created_by_type", "created_by_id"], name: "index_claims_on_created_by" + t.index ["previous_revision_id"], name: "index_claims_on_previous_revision_id" t.index ["provider_id"], name: "index_claims_on_provider_id" - t.index ["reference"], name: "index_claims_on_reference", unique: true + t.index ["reference"], name: "index_claims_on_reference" t.index ["school_id"], name: "index_claims_on_school_id" t.index ["submitted_by_type", "submitted_by_id"], name: "index_claims_on_submitted_by" end diff --git a/spec/factories/claims.rb b/spec/factories/claims.rb index e1bdb2e239..41ef08a1fb 100644 --- a/spec/factories/claims.rb +++ b/spec/factories/claims.rb @@ -2,26 +2,28 @@ # # Table name: claims # -# id :uuid not null, primary key -# created_by_type :string -# reference :string -# status :enum -# submitted_at :datetime -# submitted_by_type :string -# created_at :datetime not null -# updated_at :datetime not null -# created_by_id :uuid -# provider_id :uuid -# school_id :uuid not null -# submitted_by_id :uuid +# id :uuid not null, primary key +# created_by_type :string +# reference :string +# status :enum +# submitted_at :datetime +# submitted_by_type :string +# created_at :datetime not null +# updated_at :datetime not null +# created_by_id :uuid +# previous_revision_id :uuid +# provider_id :uuid +# school_id :uuid not null +# submitted_by_id :uuid # # Indexes # -# index_claims_on_created_by (created_by_type,created_by_id) -# index_claims_on_provider_id (provider_id) -# index_claims_on_reference (reference) UNIQUE -# index_claims_on_school_id (school_id) -# index_claims_on_submitted_by (submitted_by_type,submitted_by_id) +# index_claims_on_created_by (created_by_type,created_by_id) +# index_claims_on_previous_revision_id (previous_revision_id) +# index_claims_on_provider_id (provider_id) +# index_claims_on_reference (reference) +# index_claims_on_school_id (school_id) +# index_claims_on_submitted_by (submitted_by_type,submitted_by_id) # # Foreign Keys # @@ -33,6 +35,7 @@ association :school, factory: :claims_school association :provider association :created_by, factory: :claims_user + association :submitted_by, factory: :claims_user status { :internal_draft } diff --git a/spec/forms/claims/claim/mentor_training_form_spec.rb b/spec/forms/claims/claim/mentor_training_form_spec.rb index 89887f6db3..14026870c9 100644 --- a/spec/forms/claims/claim/mentor_training_form_spec.rb +++ b/spec/forms/claims/claim/mentor_training_form_spec.rb @@ -113,7 +113,7 @@ context "when we are on the alphabetically-first mentor training hours page" do it "returns the path to the mentors check list" do expect(mentor_training_form.back_path).to eq( - "/schools/#{mentor_training.claim.school_id}/claims/#{mentor_training.claim.id}/mentors/#{mentor_training.mentor_id}/edit", + "/schools/#{mentor_training.claim.school_id}/claims/#{mentor_training.claim.id}/mentors/edit", ) end end diff --git a/spec/forms/claims/support/claim/mentor_training_form_spec.rb b/spec/forms/claims/support/claim/mentor_training_form_spec.rb index 99edf2990b..fbea57b055 100644 --- a/spec/forms/claims/support/claim/mentor_training_form_spec.rb +++ b/spec/forms/claims/support/claim/mentor_training_form_spec.rb @@ -113,7 +113,7 @@ context "when we are on the alphabetically-first mentor training hours page" do it "returns the path to the mentors check list" do expect(mentor_training_form.back_path).to eq( - "/support/schools/#{mentor_training.claim.school_id}/claims/#{mentor_training.claim.id}/mentors/#{mentor_training.mentor_id}/edit", + "/support/schools/#{mentor_training.claim.school_id}/claims/#{mentor_training.claim.id}/mentors/edit", ) end end diff --git a/spec/models/claims/claim_spec.rb b/spec/models/claims/claim_spec.rb index c3fbcda678..4bbcaba2da 100644 --- a/spec/models/claims/claim_spec.rb +++ b/spec/models/claims/claim_spec.rb @@ -2,26 +2,28 @@ # # Table name: claims # -# id :uuid not null, primary key -# created_by_type :string -# reference :string -# status :enum -# submitted_at :datetime -# submitted_by_type :string -# created_at :datetime not null -# updated_at :datetime not null -# created_by_id :uuid -# provider_id :uuid -# school_id :uuid not null -# submitted_by_id :uuid +# id :uuid not null, primary key +# created_by_type :string +# reference :string +# status :enum +# submitted_at :datetime +# submitted_by_type :string +# created_at :datetime not null +# updated_at :datetime not null +# created_by_id :uuid +# previous_revision_id :uuid +# provider_id :uuid +# school_id :uuid not null +# submitted_by_id :uuid # # Indexes # -# index_claims_on_created_by (created_by_type,created_by_id) -# index_claims_on_provider_id (provider_id) -# index_claims_on_reference (reference) UNIQUE -# index_claims_on_school_id (school_id) -# index_claims_on_submitted_by (submitted_by_type,submitted_by_id) +# index_claims_on_created_by (created_by_type,created_by_id) +# index_claims_on_previous_revision_id (previous_revision_id) +# index_claims_on_provider_id (provider_id) +# index_claims_on_reference (reference) +# index_claims_on_school_id (school_id) +# index_claims_on_submitted_by (submitted_by_type,submitted_by_id) # # Foreign Keys # @@ -35,13 +37,14 @@ it { is_expected.to belong_to(:school).class_name("Claims::School") } it { is_expected.to belong_to(:provider) } it { is_expected.to belong_to(:created_by) } + it { is_expected.to belong_to(:previous_revision).class_name("Claims::Claim").optional } it { is_expected.to belong_to(:submitted_by).optional } it { is_expected.to have_many(:mentor_trainings).dependent(:destroy) } it { is_expected.to have_many(:mentors).through(:mentor_trainings) } end context "with validations" do - subject { build(:claim) } + subject { create(:claim) } it { is_expected.to validate_presence_of(:status) } it { is_expected.to validate_uniqueness_of(:reference).case_insensitive.allow_nil } @@ -130,4 +133,68 @@ end end end + + describe "#ready_to_be_checked?" do + it "returns true if the claim's mentors all have their hours recorded" do + claim = create(:claim) + create(:mentor_training, hours_completed: 20, claim:) + + expect(claim.ready_to_be_checked?).to eq(true) + end + + it "returns false if the claim does have mentors" do + claim = build(:claim) + + expect(claim.ready_to_be_checked?).to eq(false) + end + + it "returns false if the claim does have mentor training hours" do + claim = create(:claim) + create(:mentor_training, hours_completed: nil, claim:) + + expect(claim.ready_to_be_checked?).to eq(false) + end + end + + describe "#get_valid_revision" do + it "gets the last valid revision" do + claim = create(:claim, :draft) + invalid_revision = create( + :claim, + :draft, + previous_revision: claim, + ) + claim.update!(previous_revision: invalid_revision) + create(:mentor_training, claim: invalid_revision, hours_completed: nil) + create(:mentor_training, claim:, hours_completed: 20) + + expect(claim.get_valid_revision).to eq(claim) + end + end + + describe "#was_draft?" do + it "returns true if any previous revision was draft" do + draft_revision = create(:claim, :draft) + revision = create( + :claim, + :internal_draft, + previous_revision: draft_revision, + ) + claim = create(:claim, :submitted, previous_revision: revision) + + expect(claim.was_draft?).to eq(true) + end + + it "returns false if no previous revision was draft" do + second_revision = create(:claim, :internal_draft) + revision = create( + :claim, + :internal_draft, + previous_revision: second_revision, + ) + claim = create(:claim, :submitted, previous_revision: revision) + + expect(claim.was_draft?).to eq(false) + end + end end diff --git a/spec/services/claims/claim/create_revision_spec.rb b/spec/services/claims/claim/create_revision_spec.rb new file mode 100644 index 0000000000..d35168a565 --- /dev/null +++ b/spec/services/claims/claim/create_revision_spec.rb @@ -0,0 +1,35 @@ +require "rails_helper" + +describe Claims::Claim::CreateRevision do + subject(:service) { described_class.call(claim:) } + + let!(:claim) { create(:claim, reference: nil, status: :internal_draft, school:) } + let(:school) { create(:claims_school, urn: "1234") } + + it_behaves_like "a service object" do + let(:params) { { claim: } } + end + + describe "#call" do + it "creates a revision of the claim, with associations" do + anne = create(:claims_user, :anne) + claim = create(:claim, :draft, submitted_by: anne, created_by: anne) + attributes = claim.attributes.except( + "id", + "created_at", + "updated_at", + "status", + "previous_revision_id", + ).keys + revision = described_class.call(claim:) + + expect(revision.mentor_trainings).to eq(claim.mentor_trainings) + expect(revision.previous_revision_id).to eq(claim.id) + expect(revision.status).to eq("internal_draft") + + attributes.each do |attribute| + expect(revision.public_send(attribute)).to eq(claim.public_send(attribute)) + end + end + end +end 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/schools/claims/change_claim_on_check_page_spec.rb b/spec/system/claims/schools/claims/change_claim_on_check_page_spec.rb index fd8440d352..b0242d3129 100644 --- a/spec/system/claims/schools/claims/change_claim_on_check_page_spec.rb +++ b/spec/system/claims/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 "Anne changes the provider on claim on check page" do + scenario "Anne changes the provider on claim on check page and doesn't need to add the mentor hours again" do given_i_visit_claim_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("Submit claim") then_i_get_a_claim_reference(claim) end - scenario "Anne does not have a provider selected when editing a claim from check page" do - given_i_visit_claim_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 "Anne changes the mentors on claim on check page" do given_i_visit_claim_check_page when_i_click_change_mentors @@ -55,7 +48,12 @@ then_i_see_the_error("Select a mentor") when_i_check_the_mentor(mentor2) when_i_click("Continue") - then_i_check_my_answers(provider1, [mentor2], [12]) + then_i_check_my_answers( + Claims::Claim.find_by(previous_revision_id: claim.id), + provider1, + [mentor2], + [12], + ) when_i_click("Submit claim") then_i_get_a_claim_reference(claim) end @@ -73,7 +71,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 "Anne changes the mentors on claim without inputting hours for any mentor" do + given_i_visit_claim_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 @@ -86,7 +98,12 @@ 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( + Claims::Claim.find_by(previous_revision_id: claim.id), + provider1, + [mentor1, mentor2], + [6, 12], + ) end scenario "Anne intends to change the training hours but clicks back link" do @@ -123,7 +140,7 @@ def given_i_visit_claim_check_page def when_i_click_change_provider within("dl.govuk-summary-list:nth(1)") do within(".govuk-summary-list__row:nth(2)") do - click_on("Change") + click_button("Change") end end end @@ -131,7 +148,7 @@ def when_i_click_change_provider def when_i_click_change_mentors within("dl.govuk-summary-list:nth(1)") do within(".govuk-summary-list__row:nth(3)") do - click_on("Change") + click_button("Change") end end end @@ -139,7 +156,7 @@ def when_i_click_change_mentors def when_i_click_change_training_hours_for_mentor within("dl.govuk-summary-list:nth(2)") do within(".govuk-summary-list__row:nth(1)") do - click_on("Change") + click_button("Change") end end end @@ -199,7 +216,7 @@ def then_i_expect_the_training_hours_for(hours, mentor) find("#claims-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") expect(page).to have_content("Hours of training") expect(page).to have_content("Grant funding") @@ -219,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 @@ -227,7 +244,7 @@ def then_i_check_my_answers(provider, mentors, mentor_hours) within("dl.govuk-summary-list:nth(3)") do within(".govuk-summary-list__row:nth(1)") do - expect(page).to have_content("Total hours#{claim.mentor_trainings.sum(:hours_completed)} hours") + expect(page).to have_content("Total hours#{claim_record.mentor_trainings.sum(:hours_completed)} hours") end within(".govuk-summary-list__row:nth(2)") do @@ -236,7 +253,7 @@ def then_i_check_my_answers(provider, mentors, mentor_hours) within(".govuk-summary-list__row:nth(3)") do expect(page).to have_content("Claim amount") - expect(page).to have_content(claim.amount.format(symbol: true, decimal_mark: ".", no_cents: true)) + expect(page).to have_content(claim_record.amount.format(symbol: true, decimal_mark: ".", no_cents: true)) end end end @@ -260,13 +277,4 @@ def then_i_get_a_claim_reference(claim) expect(page).to have_content("Claim submitted\nYour reference number\n#{claim.reference}") end 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/schools/claims/create_claim_spec.rb b/spec/system/claims/schools/claims/create_claim_spec.rb index 159b1ad7fa..f01c217634 100644 --- a/spec/system/claims/schools/claims/create_claim_spec.rb +++ b/spec/system/claims/schools/claims/create_claim_spec.rb @@ -117,10 +117,10 @@ then_i_expect_to_be_able_to_add_training_hours_to_mentor(mentor1) when_i_add_training_hours("20 hours") when_i_click("Continue") - when_i_click("Change Accredited provider") + when_i_click_change_provider when_i_choose_a_provider(bpn) when_i_click("Continue") - when_i_click("Change Mentors") + 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) @@ -280,4 +280,20 @@ def then_i_should_see_the_message(message) def then_i_should_land_on_the_check_page expect(page).to have_content "Check your answers" end + + def when_i_click_change_provider + within("dl.govuk-summary-list:nth(1)") do + within(".govuk-summary-list__row:nth(2)") do + click_button("Change") + end + end + end + + def when_i_click_change_mentors + within("dl.govuk-summary-list:nth(1)") do + within(".govuk-summary-list__row:nth(3)") do + click_button("Change") + end + 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 aa7fdcfe83..ac44295d6e 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 @@ -55,7 +48,12 @@ then_i_see_the_error("Select a mentor") when_i_check_the_mentor(mentor2) when_i_click("Continue") - then_i_check_my_answers(provider1, [mentor2], [12]) + then_i_check_my_answers( + Claims::Claim.find_by(previous_revision_id: claim.id), + provider1, + [mentor2], + [12], + ) when_i_click("Add claim") then_i_am_redirected_to_index_page(claim) end @@ -65,7 +63,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 @@ -81,7 +79,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 @@ -94,7 +106,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 @@ -131,7 +143,7 @@ def given_i_visit_claim_support_check_page def when_i_click_change_provider within("dl.govuk-summary-list:nth(1)") do within(".govuk-summary-list__row:nth(1)") do - click_on("Change") + click_button("Change") end end end @@ -139,7 +151,7 @@ def when_i_click_change_provider def when_i_click_change_mentors within("dl.govuk-summary-list:nth(1)") do within(".govuk-summary-list__row:nth(2)") do - click_on("Change") + click_button("Change") end end end @@ -147,7 +159,7 @@ def when_i_click_change_mentors def when_i_click_change_training_hours_for_mentor within("dl.govuk-summary-list:nth(2)") do within(".govuk-summary-list__row:nth(1)") do - click_on("Change") + click_button("Change") end end end @@ -207,7 +219,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 @@ -225,7 +237,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 @@ -253,13 +265,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 f56eac94c5..0824983925 100644 --- a/spec/system/claims/support/schools/claims/create_claim_spec.rb +++ b/spec/system/claims/support/schools/claims/create_claim_spec.rb @@ -113,10 +113,10 @@ then_i_expect_to_be_able_to_add_training_hours_to_mentor(mentor1) when_i_add_training_hours("20 hours") when_i_click("Continue") - when_i_click("Change Accredited provider") + when_i_click_change_provider when_i_choose_a_provider(bpn) when_i_click("Continue") - when_i_click("Change Mentors") + 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) @@ -249,4 +249,20 @@ def then_i_should_see_the_message(message) def then_i_should_land_on_the_check_page expect(page).to have_content "Check your answers" end + + def when_i_click_change_provider + within("dl.govuk-summary-list:nth(1)") do + within(".govuk-summary-list__row:nth(1)") do + click_button("Change") + end + end + end + + def when_i_click_change_mentors + within("dl.govuk-summary-list:nth(1)") do + within(".govuk-summary-list__row:nth(2)") do + click_button("Change") + end + end + end end 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 439d02e303..b7e7af317a 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 @@ -34,7 +34,6 @@ end let!(:draft_mentor_training) { create(:mentor_training, claim: draft_claim, mentor: claims_mentor, hours_completed: 6) } - let!(:submitted_mentor_training) { create(:mentor_training, claim: submitted_claim, mentor: claims_mentor, hours_completed: 6) } scenario "A support user I can edit a draft claim" do user_exists_in_dfe_sign_in(user: colin) @@ -42,10 +41,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,21 +86,26 @@ 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 - all("a", text: "Change")[2].click + all("button", text: "Change")[2].click page.choose("Another amount") fill_in("Number of hours", with: 11) click_on("Continue") - expect(page).to have_content("Barry Garlow11 hoursChange") + expect(page).to have_content("Barry Garlow11 hours") end def then_i_edit_the_mentors - all("a", text: "Change")[1].click + all("button", text: "Change")[1].click page.check(another_claims_mentor.full_name) click_on("Continue") page.choose("20 hours") @@ -82,28 +115,27 @@ def then_i_edit_the_mentors expect(page).to have_content("Barry Garlow") end - def then_i_edit_the_provider - expect(page).to have_content("Accredited providerBest Practice NetworkChange") - first("a", text: "Change").click - page.choose(niot_provider.name) + def then_i_edit_the_provider(current_provider:, new_provider:) + expect(page).to have_content(current_provider.name) + first("button", text: "Change").click + 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 click_on draft_claim.reference - expect(page).to have_content("Best Practice NetworkChange") - expect(page).to have_content("Barry Garlow\nChange") - expect(page).to have_content("Barry Garlow#{draft_mentor_training.hours_completed} hoursChange") + expect(page).to have_content("Best Practice Network") + expect(page).to have_content("Barry Garlow") + expect(page).to have_content("Barry Garlow#{draft_mentor_training.hours_completed} hours") end def then_i_cant_edit_the_submitted_claim click_on submitted_claim.reference + change_buttons = all("button", text: "Change") - expect(page).not_to have_content("Best Practice NetworkChange") - expect(page).not_to have_content("Barry Garlow\nChange") - expect(page).not_to have_content("Barry Garlow#{submitted_mentor_training.hours_completed} hoursChange") + expect(change_buttons).to be_empty end def when_i_select_a_school @@ -120,4 +152,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.active.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.active.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.active.pluck(:reference).uniq + expect(Claims::Claim.active.count).to eq(uniq_references.count) + end + + def when_i_click(button) + click_on(button) + end end