Fix user XXX didn’t exist for user enrollment

In order to facilitate better and more efficient debugging, this commit
improves the quality of the error messages reported by SIS imports. When
a user does not exist in users.csv, and enrollments in enrollments.csv
reference that user, the message "User XXX didn't exist for enrollment"
is not particularly useful. This commit adds course and section IDs to
the message to make it easier to search for broken rows.

Test plan:
  * Create or change an existing enrollments.csv file so that it
    references a user that doesn't exist.
  * Run the CSV through the SIS importer.
  * Check the error messages for the last import; the new error message
    should show up instead of the old one.

Change-Id: Ie3ea4d95c6877ef8c5b27cee9d530a6fdf0e6e6e
Fixes: SIS-1733
Reviewed-on: https://gerrit.instructure.com/71116
Tested-by: Jenkins
Reviewed-by: Ken Romney <kromney@instructure.com>
Reviewed-by: Tyler Pickett <tpickett@instructure.com>
QA-Review: Steven Shepherd <sshepherd@instructure.com>
Product-Review: Ken Romney <kromney@instructure.com>
This commit is contained in:
Stewie (Nicholas Stewart) 2016-01-28 15:53:23 -07:00 committed by nstewart
parent 7539016828
commit c324f041c2
2 changed files with 30 additions and 14 deletions

View File

@ -161,7 +161,9 @@ module SIS
end end
unless pseudo unless pseudo
@messages << "User #{user_id} didn't exist for user enrollment" err = "User not found for enrollment "
err << "(User ID: #{user_id}, Course ID: #{course_id}, Section ID: #{section_id})"
@messages << err
next next
end end

View File

@ -2,21 +2,35 @@ require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb')
module SIS module SIS
describe EnrollmentImporter do describe EnrollmentImporter do
let(:user_id) { "5235536377654" }
let(:course_id) { "82433211" }
let(:section_id) { "299981672" }
it 'does not break postgres if I give it integers' do let(:enrollment) do
messages = [] SIS::Models::Enrollment.new(
EnrollmentImporter.new(Account.default, {}).process(messages, 2) do |importer| course_id: course_id,
enrollment = SIS::Models::Enrollment.new() section_id: section_id,
enrollment.course_id = 1 user_id: user_id,
enrollment.section_id = 2 role: 'student',
enrollment.user_id = 3 status: 'active',
enrollment.role = 'student' start_date: Time.zone.today,
enrollment.status = 'active' end_date: Time.zone.today
enrollment.start_date = Time.zone.today )
enrollment.end_date = Time.zone.today end
importer.add_enrollment(enrollment)
context 'gives a meaningful error message when a user does not exist for an enrollment' do
let(:messages) { [] }
before do
EnrollmentImporter.new(Account.default, {}).process(messages, 2) do |importer|
importer.add_enrollment(enrollment)
end
end end
expect(messages).not_to be_empty
it { expect(messages.first).to include("User not found for enrollment") }
it { expect(messages.first).to include("User ID: #{user_id}") }
it { expect(messages.first).to include("Course ID: #{course_id}") }
it { expect(messages.first).to include("Section ID: #{section_id}") }
end end
context 'with a valid user ID but invalid course and section IDs' do context 'with a valid user ID but invalid course and section IDs' do