+ You cannot add new trainees if there’s an error in their row in the CSV file. + You need to fix the errors if you want to add them. +
+ ++ <%= govuk_link_to( + "Cancel bulk updates to records", + new_bulk_update_trainees_upload_path, + ) %> +
+ <% end %> +You need to review the errors before you can add new trainees
- <%= govuk_link_to("Review errors", class: "govuk-button") %> + <%= render GovukButtonLinkTo::View.new( + body: "Review errors", + url: bulk_update_trainees_review_error_path(id: @bulk_update_trainee_upload.id), + class_option: "govuk-button" + ) %> <% else %> <%= register_form_with( model: @bulk_update_trainee_upload, @@ -78,7 +81,7 @@ method: :post ) do |f| %> <%= f.govuk_submit("Submit") %> - <% end %> + <% end %> <% end %><%= register_form_with( diff --git a/config/analytics.yml b/config/analytics.yml index 6f56161b3a..5826952efb 100644 --- a/config/analytics.yml +++ b/config/analytics.yml @@ -308,6 +308,7 @@ shared: - errored_on_type - error_type - message + - error_type :placements: - id - trainee_id diff --git a/config/routes.rb b/config/routes.rb index 35642ad6fe..91dfa1c2cc 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -93,6 +93,7 @@ resources :submissions, only: %i[show create] end end + resources :review_errors, path: "review_errors", only: %i[show], as: :review_errors end end diff --git a/db/schema.rb b/db/schema.rb index 3f671a4489..4d0357da8e 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -669,10 +669,10 @@ t.string "surname16" t.string "ttcid" t.string "hesa_committed_at" + t.string "previous_hesa_id" t.string "application_choice_id" t.string "itt_start_date" t.string "trainee_start_date" - t.string "previous_hesa_id" t.string "provider_trainee_id" t.string "lead_partner_urn" t.index ["hesa_id", "rec_id"], name: "index_hesa_students_on_hesa_id_and_rec_id", unique: true diff --git a/spec/factories/bulk_update/trainee_uploads.rb b/spec/factories/bulk_update/trainee_uploads.rb index d4bf926c8b..9e082328c6 100644 --- a/spec/factories/bulk_update/trainee_uploads.rb +++ b/spec/factories/bulk_update/trainee_uploads.rb @@ -1,14 +1,24 @@ # frozen_string_literal: true +UPLOAD_ERROR_MESSAGES = [ + "Invalid email address", + "Invalid date of birth", + "Invalid degree grade", + "Missing placement data", + "Missing degree data", +].freeze + FactoryBot.define do factory :bulk_update_trainee_upload, class: "BulkUpdate::TraineeUpload" do provider status { nil } number_of_trainees { 5 } - after(:build) do |bulk_update_trainee_upload| - bulk_update_trainee_upload.attach( - io: Rails.root.join("spec/fixtures/files/bulk_update/trainee_uploads/five_trainees.csv").open, + after(:build) do |upload| + upload.file.attach( + io: Rails.root.join( + "spec/fixtures/files/bulk_update/trainee_uploads/five_trainees.csv", + ).open, filename: "five_trainees.csv", ) end diff --git a/spec/features/bulk_upload/bulk_add_trainees_spec.rb b/spec/features/bulk_upload/bulk_add_trainees_spec.rb index 0a6657aa25..fd0e07352b 100644 --- a/spec/features/bulk_upload/bulk_add_trainees_spec.rb +++ b/spec/features/bulk_upload/bulk_add_trainees_spec.rb @@ -10,6 +10,8 @@ end context "when the feature flag is off", feature_bulk_add_trainees: false do + let(:user) { create(:user, :hei) } + before do given_i_am_authenticated end @@ -89,7 +91,7 @@ when_the_background_job_is_run and_i_refresh_the_page - then_i_see_the_review_page + then_i_see_the_review_page_without_validation_errors and_i_dont_see_the_review_errors_link and_i_dont_see_the_back_to_bulk_updates_link @@ -150,7 +152,7 @@ when_the_upload_has_failed_with_duplicate_errors and_i_dont_see_that_the_upload_is_processing and_i_visit_the_summary_page(upload: @failed_upload) - then_i_see_the_review_page + then_i_see_the_review_page_without_validation_errors and_i_see_the_number_of_trainees_that_can_be_added(number: 3) and_i_dont_see_any_validation_errors and_i_see_the_duplicate_errors(number: 2) @@ -163,7 +165,7 @@ when_the_upload_has_failed_with_validation_and_duplicate_errors and_i_dont_see_that_the_upload_is_processing and_i_visit_the_summary_page(upload: @failed_upload) - then_i_see_the_review_page + then_i_see_the_review_page_without_validation_errors and_i_dont_the_number_of_trainees_that_can_be_added and_i_see_the_validation_errors(number: 2) and_i_see_the_duplicate_errors(number: 3) @@ -171,6 +173,33 @@ and_i_see_the_review_errors_link and_i_dont_see_the_submit_button end + + scenario "when I try to upload a file with errors then upload a corrected file" do + when_i_visit_the_bulk_update_index_page + and_i_click_the_bulk_add_trainees_page + then_i_see_how_instructions_on_how_to_bulk_add_trainees + + when_i_attach_a_file_with_invalid_rows + and_i_click_the_upload_button + when_the_background_job_is_run + and_i_refresh_the_page + then_i_see_the_review_page_without_validation_errors + + when_the_background_job_is_run + and_i_refresh_the_page + then_i_see_the_review_page_with_validation_errors + + when_i_click_the_review_errors_link + then_i_see_the_review_errors_page + + when_i_click_on_the_download_link + then_i_receive_the_file + + when_i_return_to_the_review_errors_page + and_i_attach_a_valid_file + and_i_click_the_upload_button + then_i_see_that_the_upload_is_processing + end end end @@ -267,6 +296,11 @@ def when_i_attach_a_valid_file and_i_attach_a_file(csv) end + def when_i_attach_a_file_with_invalid_rows + csv = Rails.root.join("spec/fixtures/files/bulk_update/trainee_uploads/five_trainees_with_two_errors.csv").read + and_i_attach_a_file(csv) + end + def and_i_attach_a_file(content) tempfile = Tempfile.new("csv") tempfile.write(content) @@ -288,6 +322,10 @@ def then_i_see_the_upload_page_with_errors(empty:) def then_i_see_the_review_page expect(page).to have_content("You uploaded a CSV file with details of 5 trainees.") + end + + def then_i_see_the_review_page_without_validation_errors + expect(page).to have_content("You uploaded a CSV file with details of 5 trainees.") expect(page).to have_content("It included:") end @@ -423,4 +461,30 @@ def and_i_visit_the_summary_page(upload:) alias_method :and_i_attach_a_valid_file, :when_i_attach_a_valid_file alias_method :and_i_click_the_submit_button, :when_i_click_the_submit_button alias_method :when_i_click_the_upload_button, :and_i_click_the_upload_button + + def then_i_see_the_review_page_with_validation_errors + expect(page).to have_content("2 trainees with errors in their details") + end + + def when_i_click_the_review_errors_link + click_on "Review errors" + end + + def then_i_see_the_review_errors_page + expect(page).to have_current_path(bulk_update_trainees_review_error_path(id: BulkUpdate::TraineeUpload.last.id)) + expect(page).to have_content("Review errors for 2 trainees in the CSV that you uploaded") + end + + def when_i_click_on_the_download_link + click_on "Download your CSV file with errors indicated" + end + + def then_i_receive_the_file + expect(page.response_headers["Content-Type"]).to eq("text/csv") + expect(page.response_headers["Content-Disposition"]).to include("attachment; filename=\"trainee-upload-errors-#{BulkUpdate::TraineeUpload.last.id}.csv\"") + end + + def when_i_return_to_the_review_errors_page + visit bulk_update_trainees_review_error_path(id: BulkUpdate::TraineeUpload.last.id) + end end diff --git a/spec/models/reports/bulk_trainee_upload_report_spec.rb b/spec/models/reports/bulk_trainee_upload_report_spec.rb new file mode 100644 index 0000000000..bea17c7c5d --- /dev/null +++ b/spec/models/reports/bulk_trainee_upload_report_spec.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +require "rails_helper" + +describe Reports::BulkTraineeUploadReport do + context "given an empty trainee upload" do + let(:trainee_upload) { create(:bulk_update_trainee_upload) } + + it "generates a CSV with the correct headers and no rows" do + generated_csv = CSV.generate do |csv| + described_class.new(csv, scope: trainee_upload).generate_report + end + + data = CSV.parse(generated_csv) + expect(data.size).to eq(1) + expect(data[0]).to eq(BulkUpdate::AddTrainees::ImportRows::TRAINEE_HEADERS.keys + ["Errors"]) + end + end + + context "given a valid trainee upload without errors" do + let(:trainee_upload) { create(:bulk_update_trainee_upload, :with_rows) } + + it "generates a CSV with an extra _Errors_ column with empty values" do + generated_csv = CSV.generate do |csv| + described_class.new(csv, scope: trainee_upload).generate_report + end + + data = CSV.parse(generated_csv, headers: true) + expect(data.size).to eq(5) + data.each do |row| + expect(row.key?("Errors")).to be(true) + expect(row["Errors"]).to be_blank + end + end + end + + context "given a valid trainee upload with some errors" do + let(:trainee_upload) { create(:bulk_update_trainee_upload, :failed_with_validation_errors) } + + it "generates a CSV with an extra _Errors_ column" do + generated_csv = CSV.generate do |csv| + described_class.new(csv, scope: trainee_upload).generate_report + end + + data = CSV.parse(generated_csv, headers: true) + expect(data.size).to eq(5) + expect(data[0]["Errors"]).to be_blank + expect(data[1]["Errors"]).to be_blank + expect(data[2]["Errors"]).to be_blank + expect(data[3]["Errors"]).to be_present + expect(data[3]["Errors"]).to eq( + trainee_upload.trainee_upload_rows[3].row_errors.pluck(:message).join(", "), + ) + expect(data[4]["Errors"]).to be_present + expect(data[4]["Errors"]).to eq( + trainee_upload.trainee_upload_rows[4].row_errors.pluck(:message).join(", "), + ) + end + end +end diff --git a/spec/services/bulk_update/add_trainees/import_rows_spec.rb b/spec/services/bulk_update/add_trainees/import_rows_spec.rb index e935804ab8..25dba7e22f 100644 --- a/spec/services/bulk_update/add_trainees/import_rows_spec.rb +++ b/spec/services/bulk_update/add_trainees/import_rows_spec.rb @@ -24,6 +24,12 @@ module AddTrainees context "when the upload status is pending" do let(:trainee_upload) { create(:bulk_update_trainee_upload, :pending) } + before do + allow(ImportRow).to receive(:call).and_return( + BulkUpdate::AddTrainees::ImportRow::Result.new(true, []), + ) + end + it "does not create any trainee records" do expect(ImportRow).to receive(:call).exactly(5).times.and_call_original expect(described_class.call(trainee_upload)).to be(true) @@ -53,6 +59,12 @@ module AddTrainees context "when the upload status is in progress" do let(:trainee_upload) { create(:bulk_update_trainee_upload, :with_rows, status: :in_progress) } + before do + allow(ImportRow).to receive(:call).and_return( + BulkUpdate::AddTrainees::ImportRow::Result.new(true, []), + ) + end + it "creates 5 trainee records" do expect(ImportRow).to receive(:call).exactly(5).times.and_call_original expect(described_class.call(trainee_upload)).to be(true) @@ -77,6 +89,22 @@ module AddTrainees context "when some rows are valid and can be imported whilst others are not" do let(:trainee_upload) { create(:bulk_update_trainee_upload, :with_errors) } + before do + allow(ImportRow).to receive(:call).and_return( + BulkUpdate::AddTrainees::ImportRow::Result.new(true, []), + BulkUpdate::AddTrainees::ImportRow::Result.new(true, []), + BulkUpdate::AddTrainees::ImportRow::Result.new( + false, + { errors: { funding: ["Funding type is required"], email: "Enter an email address" } }, + ), + BulkUpdate::AddTrainees::ImportRow::Result.new(true, []), + BulkUpdate::AddTrainees::ImportRow::Result.new( + false, + ["Add at least one degree"], + ), + ) + end + it "does not create any trainee records" do expect(ImportRow).to receive(:call).exactly(5).times.and_call_original expect(described_class.call(trainee_upload)).to be(false) @@ -87,6 +115,18 @@ module AddTrainees described_class.call(trainee_upload) expect(trainee_upload.reload).to be_failed end + + it "creates an error record for the failing row" do + expect { described_class.call(trainee_upload) }.to( + change { BulkUpdate::RowError.count }.by(3), + ) + expect( + trainee_upload.trainee_upload_rows[2].row_errors.pluck(:message), + ).to eq(["Funding type is required", "Enter an email address"]) + expect( + trainee_upload.trainee_upload_rows[4].row_errors.pluck(:message), + ).to eq(["Add at least one degree"]) + end end end end