Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[7784] Create review errors page #4822

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
37 changes: 37 additions & 0 deletions app/controllers/bulk_update/trainees/review_errors_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
# frozen_string_literal: true

module BulkUpdate
module Trainees
class ReviewErrorsController < ApplicationController
before_action { require_feature_flag(:bulk_add_trainees) }

def show
@bulk_update_trainee_upload = policy_scope(BulkUpdate::TraineeUpload)
.includes(:row_errors).find(params[:id])

@bulk_add_trainee_upload_form = BulkUpdate::BulkAddTraineesUploadForm.new(
provider: current_user.organisation,
)

authorize(@bulk_update_trainee_upload)

respond_to do |format|
format.html { render(:show) }
format.csv do
send_data(
Exports::BulkTraineeUploadExport.call(trainee_upload: @bulk_update_trainee_upload),
filename: "trainee-upload-errors-#{@bulk_update_trainee_upload.id}.csv",
disposition: :attachment,
)
end
end
end

private

def organisation
@organisation ||= current_user.organisation
end
end
end
end
3 changes: 2 additions & 1 deletion app/models/bulk_update/row_error.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@
#
# Indexes
#
# index_bulk_update_row_errors_on_error_type (error_type)
# idx_on_errored_on_id_errored_on_type_492045ed60 (errored_on_id,errored_on_type)
# index_bulk_update_row_errors_on_error_type (error_type)
#
class BulkUpdate::RowError < ApplicationRecord
belongs_to :errored_on, polymorphic: true
Expand Down
32 changes: 32 additions & 0 deletions app/models/reports/bulk_trainee_upload_report.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
# frozen_string_literal: true

module Reports
class BulkTraineeUploadReport < TemplateClassCsv
HEADERS = (BulkUpdate::AddTrainees::ImportRows::TRAINEE_HEADERS.keys + ["Errors"]).freeze

def initialize(csv, scope:)
@csv = csv
@scope = scope
end

alias trainee_upload scope

private

def add_headers
csv << HEADERS
end

def add_report_rows
trainee_upload.trainee_upload_rows.each do |row|
add_row_to_csv(row)
end
end

def add_row_to_csv(row)
csv << row.data.merge(
Errors: row.row_errors.pluck(:message).join(", "),
).values.map { |value| CsvValueSanitiser.new(value).sanitise }
end
end
end
35 changes: 32 additions & 3 deletions app/services/bulk_update/add_trainees/import_rows.rb
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,8 @@ def call
end
end

results = []

ActiveRecord::Base.transaction(requires_new: true) do
results = trainee_upload.trainee_upload_rows.map do |upload_row|
BulkUpdate::AddTrainees::ImportRow.call(row: upload_row.data, current_provider: current_provider)
Expand All @@ -96,15 +98,18 @@ def call
if results.all?(&:success)
trainee_upload.succeeded! unless dry_run
else
# TODO: copy any errors into `trainee_upload`
success = false
end

raise(ActiveRecord::Rollback) if dry_run || !success
end

trainee_upload.validated! if dry_run && success
trainee_upload.failed! unless success
if !success
trainee_upload.failed!
create_row_errors(results)
elsif dry_run
trainee_upload.validated!
end
end

success
Expand All @@ -119,6 +124,30 @@ def call
def current_provider
@current_provider ||= trainee_upload.provider
end

def create_row_errors(results)
trainee_upload.trainee_upload_rows.each_with_index do |upload_row, index|
next if results[index].success

extract_error_messages(errors: results[index].errors).each do |message|
upload_row.row_errors.create!(message:)
end
end
end

def extract_error_messages(messages = [], errors:)
values = errors.respond_to?(:values) ? errors.values : errors

values.each do |value|
if value.is_a?(Hash) || value.is_a?(Array)
extract_error_messages(messages, errors: value)
else
messages << value
end
end

messages
end
end
end
end
12 changes: 12 additions & 0 deletions app/services/exports/bulk_trainee_upload_export.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# frozen_string_literal: true

require "csv"

module Exports
class BulkTraineeUploadExport < ExportServiceBase
def initialize(trainee_upload:)
@report_class = Reports::BulkTraineeUploadReport
@scope = trainee_upload
end
end
end
52 changes: 52 additions & 0 deletions app/views/bulk_update/trainees/review_errors/show.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
<%= render PageTitle::View.new(text: "Upload summary") %>

<%= content_for(:breadcrumbs) do %>
<%= render GovukComponent::BackLinkComponent.new(text: "Home", href: root_path) %>
<% end %>

<div class="govuk-grid-row">
<div class="govuk-grid-column-two-thirds-from-desktop">
<%= register_form_with(model: @bulk_add_trainee_upload_form, url: bulk_update_trainees_uploads_path) do |f| %>
<%= f.govuk_error_summary %>

<h1 class="govuk-heading-l">
<span class="govuk-caption-xl"><%= organisation_name %></span>
Review errors for <%= @bulk_update_trainee_upload.trainee_upload_rows.joins(:row_errors).count("DISTINCT(bulk_update_trainee_upload_rows.id)") %> trainees in the CSV that you uploaded
</h1>

<p class="govuk-body">
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.
</p>

<ol class="govuk-list govuk-list--number">
<li>
<%= govuk_link_to(
"Download your CSV file with errors indicated",
bulk_update_trainees_review_error_path(@bulk_update_trainee_upload, format: :csv),
) %>
</li>

<li>
Fix the errors. If you cannot fix an error,
you can delete the row and the trainee will not be included.
</li>

<li>
Upload the updated CSV file.
<%= f.govuk_file_field(:file, label: { hidden: true }, accept: ".csv") %>
</li>

<%= f.govuk_submit("Upload records") %>
</ol>

<p class="govuk-body">
<%= govuk_link_to(
"Cancel bulk updates to records",
new_bulk_update_trainees_upload_path,
) %>
</p>
<% end %>
</div>
</div>

9 changes: 6 additions & 3 deletions app/views/bulk_update/trainees/uploads/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -66,19 +66,22 @@
</li>
<% end %>
</ul>

<% if @bulk_update_trainee_upload.row_errors.present? %>
<p class="govuk-body">You need to review the errors before you can add new trainees</p>

<%= 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,
url: bulk_update_trainees_submissions_path(@bulk_update_trainee_upload),
method: :post
) do |f| %>
<%= f.govuk_submit("Submit") %>
<% end %>
<% end %>
<% end %>
<p class="govuk-body">
<%= register_form_with(
Expand Down
1 change: 1 addition & 0 deletions config/analytics.yml
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,7 @@ shared:
- errored_on_type
- error_type
- message
- error_type
:placements:
- id
- trainee_id
Expand Down
1 change: 1 addition & 0 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 13 additions & 3 deletions spec/factories/bulk_update/trainee_uploads.rb
Original file line number Diff line number Diff line change
@@ -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
Expand Down
Loading
Loading