Skip to content

Commit 89aed99

Browse files
committed
AcademicBenchmark: fix missing error message
There are several fatal errors that can be encountered in an import. these were previously being reported as warnings, which is not accurate. Additionally, the error message was not being set properly so in the outcomes import API we were getting back a null error message. Fixes CNVS-22517 Test Plan: Most of these conditions are difficult to reproduce because they result from the Academic Benchmarks server. Therefore, there isn't really a practical test plan we can do without building an emulator (which I actually would like to do when we have a little time) Change-Id: I521fc5250a3369497944cda5e44bbbc622f46681 Reviewed-on: https://gerrit.instructure.com/60876 Reviewed-by: John Corrigan <jcorrigan@instructure.com> Product-Review: Benjamin Porter <bporter@instructure.com> QA-Review: Benjamin Porter <bporter@instructure.com> Tested-by: Jenkins
1 parent f30e329 commit 89aed99

2 files changed

Lines changed: 31 additions & 9 deletions

File tree

gems/plugins/academic_benchmark/lib/academic_benchmark/converter.rb

Lines changed: 31 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,14 @@ def importing_common_core?
5555
def convert_file
5656
data = @api.parse_ab_data(@archive_file.read)
5757
process_json_data(data)
58-
rescue APIError
59-
add_warning(I18n.t("academic_benchmark.bad_ab_file", "The provided Academic Benchmark file has an error."), $!)
58+
rescue APIError => e
59+
add_error(
60+
I18n.t("The provided Academic Benchmark file has an error"),
61+
{
62+
exception: e,
63+
error_message: e.message
64+
}
65+
)
6066
end
6167

6268
def convert_authorities(authorities=[])
@@ -74,8 +80,17 @@ def convert_guids(guids=[])
7480
def refresh_outcomes(opts)
7581
res = build_full_auth_hash(opts)
7682
process_json_data(res)
77-
rescue EOFError, APIError
78-
add_warning(I18n.t("academic_benchmark.api_error", "Couldn't update standards for authority %{auth}.", :auth => opts[:authority] || opts[:guid]), $!)
83+
rescue EOFError, APIError => e
84+
add_error(
85+
I18n.t(
86+
"Couldn't update standards for authority %{auth}",
87+
:auth => opts[:authority] || opts[:guid]
88+
),
89+
{
90+
exception: e,
91+
error_message: e.message
92+
}
93+
)
7994
end
8095

8196
# get a shallow tree for the authority then process the leaves
@@ -126,16 +141,25 @@ def refresh_all_outcomes
126141
end
127142

128143
set_progress(95)
129-
rescue APIError
130-
add_warning(I18n.t("academic_benchmark.bad_response_all", "Couldn't update the standards."), $!)
144+
rescue APIError => e
145+
add_error(
146+
I18n.t("Previously unhandled error encountered while refreshing outcomes"),
147+
{
148+
exception: e,
149+
error_message: e.message
150+
}
151+
)
131152
end
132153

133154
def process_json_data(data)
134155
if data = find_by_prop(data, "type", "authority")
135156
outcomes = Standard.new(data).build_outcomes
136157
@course[:learning_outcomes] << outcomes
137158
else
138-
add_warning(I18n.t("academic_benchmark.no_authority", "Couldn't find an authority to update"))
159+
err_msg = I18n.t("Couldn't find an authority to update")
160+
add_error(
161+
err_msg, { exception: nil, error_message: err_msg }
162+
)
139163
end
140164
end
141165

gems/plugins/academic_benchmark/spec_canvas/academic_benchmark_spec.rb

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -224,8 +224,6 @@ def run_and_check
224224
run_jobs
225225
@cm.reload
226226

227-
er = ErrorReport.last
228-
expect(@cm.old_warnings_format).to eq [["Couldn't update standards for authority CC.", "ErrorReport:#{er.id}"]]
229227
expect(@cm.migration_settings[:last_error]).to be_nil
230228
expect(@cm.workflow_state).to eq 'imported'
231229
end

0 commit comments

Comments
 (0)