ylqjgm
/
canvas-lms
mirror of https://github.com/instructure/canvas-lms.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

don’t lie about importing records 

fixes CORE-1200

test plan
 - import a sis batch that will fail in the middle
 - it should not say nothing was importer

Change-Id: I43cf34041a2535a5697ed6a98d80562bef424b3f
Reviewed-on: https://gerrit.instructure.com/144714
QA-Review: Jeremy Putnam <jeremyp@instructure.com>
Tested-by: Jenkins
Reviewed-by: Cody Cutrer <cody@instructure.com>
Product-Review: Rob Orton <rob@instructure.com>
This commit is contained in:
Rob Orton 2018-03-23 14:52:41 -06:00
parent e80e190e9f
commit 5057d5dd97
6 changed files with 10 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
app/models/sis_batch.rb
View File

@ -353,7 +353,7 @@ class SisBatch < ActiveRecord::Base
finalize_workflow_state(import_finished)
write_errors_to_file
populate_old_warnings_and_errors
self.progress = 100
self.progress = 100 if import_finished
self.ended_at = Time.now.utc
self.save!

4
app/views/accounts/_sis_batch_messages.html.erb
View File

@ -19,7 +19,7 @@
<% batch = sis_batch_messages %>
<ul>
<% if batch.processing_errors && batch.processing_errors.length > 0 %>
<li><%= t("There were a total of %{count} errors.", count: batch.data[:error_count]) %>
<li><%= t("There were a total of %{count} errors.", count: batch&.data.dig(:counts, :error_count))%>
<li><%= t(:sis_batch_errors_title, "Errors that prevent importing") %>
<ul>
<% batch.processing_errors.each do |message| %>
@ -29,7 +29,7 @@
</li>
<% end %>
<% if batch.processing_warnings && batch.processing_warnings.length > 0 %>
<li><%= t("There were a total of %{count} warnings.", count: batch.data[:warning_count]) %>
<li><%= t("There were a total of %{count} warnings.", count: batch&.data.dig(:counts, :warning_count))%>
<li><%= t(:sis_batch_warnings_title, "Warnings")%>
<ul>
<% batch.processing_warnings.each do |message| %>

5
app/views/accounts/sis_import.html.erb
View File

@ -147,9 +147,9 @@
<%= print_messages(@last_batch) %>
<%= print_counts @last_batch %>
<% elsif @last_batch.workflow_state == 'failed' %>
<%= t(:import_failed_message, "There was an error importing your SIS data. No records were imported.") %>
<%= t(:import_failed_message, "There was an error importing your SIS data.") %>
<% elsif @last_batch.workflow_state == 'failed_with_messages' %>
<%= t(:imported_with_messages_message, "The SIS data was imported but with these messages:") %>
<%= t(:import_failed_with_messages_message, "The import failed with these messages:") %>
<% if @last_batch.errors_attachment_id %>
<p><%= t('Download the complete list of errors and warnings here.') %>
<%= link_to "<i class='icon-download'></i>".html_safe,
@ -160,7 +160,6 @@
'aria-label' => t('Download errors_attchment'),
'title' => t('Download errors_attchment') %></p>
<% end %>
<%= t(:import_failed_with_messages_message, "No SIS records were imported. The import failed with these messages:") %>
<%= print_messages(@last_batch) %>
<% elsif @last_batch.workflow_state == 'aborted' %>
<%= t('The previous SIS batch was aborted') %>

1
lib/sis/csv/import_refactored.rb
View File

@ -240,6 +240,7 @@ module SIS
"(Error report %{number})", number: err_id.to_s)
SisBatch.add_error(nil, error_message, sis_batch: @batch, failure: true, backtrace: e.backtrace)
@batch.workflow_state = :failed_with_messages
@batch.finish(false)
@batch.save!
end

4
public/javascripts/sis_import.js
View File

@ -136,13 +136,13 @@ $(document).ready(function(event) {
var code = "sis_batch_" + sis_batch.id;
$(".progress_bar_holder").hide();
$("#sis_importer").hide();
var message = I18n.t('errors.import_failed_code', "There was an error importing your SIS data. No records were imported. Please notify your system administrator and give them the following code: \"%{code}\"", {code: code});
var message = I18n.t('errors.import_failed_code', "There was an error importing your SIS data. Please notify your system administrator and give them the following code: \"%{code}\"", {code: code});
$(".sis_messages .sis_error_message").text(message);
$(".sis_messages").show();
} else if(sis_batch.workflow_state == 'failed_with_messages') {
$(".progress_bar_holder").hide();
$("#sis_importer").hide();
var message = htmlEscape(I18n.t('errors.import_failed_messages', "No SIS records were imported. The import failed with these messages:"));
var message = htmlEscape(I18n.t('errors.import_failed_messages', "The import failed with these messages:"));
message += createMessageHtml(sis_batch);
$(".sis_messages .sis_error_message").html($.raw(message));
$(".sis_messages").show();

4
spec/selenium/admin/account_admin_sis_imports_spec.rb
View File

@ -136,7 +136,7 @@ describe "sis imports ui" do
submit_form('#sis_importer')
expect(f('.progress_bar_holder .progress_message')).to be_displayed
SisBatch.last.process_without_send_later
expect(f(".sis_messages .sis_error_message")).to include_text "No SIS records were imported. The import failed with these messages:"
expect(f(".sis_messages .sis_error_message")).to include_text "The import failed with these messages:"
expect(SisBatch.last.batch_mode).to eq true
expect(SisBatch.last.options).to eq({:override_sis_stickiness => true,
:add_sis_stickiness => true})
@ -146,7 +146,7 @@ describe "sis imports ui" do
submit_form('#sis_importer')
expect(f('.progress_bar_holder .progress_message')).to be_displayed
SisBatch.last.process_without_send_later
expect(f(".sis_messages .sis_error_message")).to include_text "No SIS records were imported. The import failed with these messages:"
expect(f(".sis_messages .sis_error_message")).to include_text "The import failed with these messages:"
expect(!!SisBatch.last.batch_mode).to be_falsey
expect(SisBatch.last.options).to eq({:override_sis_stickiness => true})
end