Skip to content

Commit b8a454a

Browse files
ch-ivch-iv
committed
refactor and add UI options
1 parent 3c20a4b commit b8a454a

File tree

5 files changed

+253
-91
lines changed

5 files changed

+253
-91
lines changed

‎app/controllers/assignments_controller.rb‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,13 @@ def download_test_results
288288
data = test_results.as_json
289289

290290
if latest
291+
send_data(
292+
data,
293+
type: "application/json",
294+
disposition: 'attachment',
295+
filename: "#{@assignment.short_identifier}_test_results.json"
296+
)
297+
else
291298
zip_path = "tmp/#{@assignment.short_identifier}_test_results.zip"
292299
Zip::File.open(zip_path, create: true) do |zip_file|
293300
zip_file.get_output_stream('test_results.json') do |f|
@@ -297,16 +304,9 @@ def download_test_results
297304

298305
send_file(
299306
zip_path,
300-
disposition: 'inline',
307+
disposition: 'attachment',
301308
filename: "#{@assignment.short_identifier}_test_results.zip"
302309
)
303-
else
304-
send_data(
305-
data,
306-
type: "text/json",
307-
disposition: 'inline',
308-
filename: "#{@assignment.short_identifier}_test_results.json"
309-
)
310310
end
311311
end
312312

‎app/helpers/summary_test_results_helper.rb‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,8 @@ class << self
44
def fetch(test_groups:, latest:, student_run:, instructor_run:)
55
query = base_query(latest:)
66

7-
query = query.student_run if student_run
8-
query = query.instructor_run if instructor_run
9-
query = query.unscoped if instructor_run && student_run
7+
query = query.student_run if student_run && !instructor_run
8+
query = query.instructor_run if !student_run && instructor_run
109

1110
test_results = fetch_with_query(test_groups:, query:)
1211

‎spec/controllers/assignments_controller_spec.rb‎

Lines changed: 130 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -108,34 +108,70 @@
108108
let(:assignment) { create(:assignment_with_criteria_and_test_results) }
109109

110110
it 'responds with the appropriate status' do
111-
get_as user, :download_test_results, params: { course_id: user.course.id, id: assignment.id }, format: 'json'
111+
get_as user, :download_test_results, params: {
112+
course_id: user.course.id,
113+
id: assignment.id,
114+
latest: true,
115+
student_run: true,
116+
instructor_run: false
117+
}, format: 'json'
112118
expect(response).to have_http_status :success
113119
end
114120

115121
it 'responds with the appropriate header' do
116-
get_as user, :download_test_results, params: { course_id: user.course.id, id: assignment.id }, format: 'json'
122+
get_as user, :download_test_results, params: {
123+
course_id: user.course.id,
124+
id: assignment.id,
125+
latest: true,
126+
student_run: true,
127+
instructor_run: false
128+
}, format: 'json'
117129
expect(response.header['Content-Type']).to eq('application/json')
118130
end
119131

120132
it 'sets disposition as attachment' do
121-
get_as user, :download_test_results, params: { course_id: user.course.id, id: assignment.id }, format: 'json'
133+
get_as user, :download_test_results, params: {
134+
course_id: user.course.id,
135+
id: assignment.id,
136+
latest: true,
137+
student_run: true,
138+
instructor_run: false
139+
}, format: 'json'
122140
d = response.header['Content-Disposition'].split.first
123141
expect(d).to eq 'attachment;'
124142
end
125143

126144
it 'responds with the appropriate filename' do
127-
get_as user, :download_test_results, params: { course_id: user.course.id, id: assignment.id }, format: 'json'
145+
get_as user, :download_test_results, params: {
146+
course_id: user.course.id,
147+
id: assignment.id,
148+
latest: true,
149+
student_run: true,
150+
instructor_run: false
151+
}, format: 'json'
128152
filename = response.header['Content-Disposition'].split[1].split('"').second
129153
expect(filename).to eq("#{assignment.short_identifier}_test_results.json")
130154
end
131155

132156
it 'returns application/json type' do
133-
get_as user, :download_test_results, params: { course_id: user.course.id, id: assignment.id }, format: 'json'
157+
get_as user, :download_test_results, params: {
158+
course_id: user.course.id,
159+
id: assignment.id,
160+
latest: true,
161+
student_run: true,
162+
instructor_run: false
163+
}, format: 'json'
134164
expect(response.media_type).to eq 'application/json'
135165
end
136166

137167
it 'returns the most recent test results' do
138-
get_as user, :download_test_results, params: { course_id: user.course.id, id: assignment.id }, format: 'json'
168+
get_as user, :download_test_results, params: {
169+
course_id: user.course.id,
170+
id: assignment.id,
171+
latest: true,
172+
student_run: true,
173+
instructor_run: false
174+
}, format: 'json'
139175
body = response.parsed_body
140176

141177
# We want to ensure that the test result's group name, test name and status exists
@@ -156,29 +192,59 @@
156192
let(:assignment) { create(:assignment_with_criteria_and_test_results) }
157193

158194
it 'responds with the appropriate status' do
159-
get_as user, :download_test_results, params: { course_id: user.course.id, id: assignment.id }, format: 'csv'
195+
get_as user, :download_test_results, params: {
196+
course_id: user.course.id,
197+
id: assignment.id,
198+
latest: true,
199+
student_run: true,
200+
instructor_run: false
201+
}, format: 'csv'
160202
expect(response).to have_http_status :success
161203
end
162204

163205
it 'sets disposition as attachment' do
164-
get_as user, :download_test_results, params: { course_id: user.course.id, id: assignment.id }, format: 'csv'
206+
get_as user, :download_test_results, params: {
207+
course_id: user.course.id,
208+
id: assignment.id,
209+
latest: true,
210+
student_run: true,
211+
instructor_run: false
212+
}, format: 'csv'
165213
d = response.header['Content-Disposition'].split.first
166214
expect(d).to eq 'attachment;'
167215
end
168216

169217
it 'responds with the appropriate filename' do
170-
get_as user, :download_test_results, params: { course_id: user.course.id, id: assignment.id }, format: 'csv'
218+
get_as user, :download_test_results, params: {
219+
course_id: user.course.id,
220+
id: assignment.id,
221+
latest: true,
222+
student_run: true,
223+
instructor_run: false
224+
}, format: 'csv'
171225
filename = response.header['Content-Disposition'].split[1].split('"').second
172226
expect(filename).to eq("#{assignment.short_identifier}_test_results.csv")
173227
end
174228

175229
it 'returns text/csv type' do
176-
get_as user, :download_test_results, params: { course_id: user.course.id, id: assignment.id }, format: 'csv'
230+
get_as user, :download_test_results, params: {
231+
course_id: user.course.id,
232+
id: assignment.id,
233+
latest: true,
234+
student_run: true,
235+
instructor_run: false
236+
}, format: 'csv'
177237
expect(response.media_type).to eq 'text/csv'
178238
end
179239

180240
it 'returns the most recent test results of the correct size' do
181-
get_as user, :download_test_results, params: { course_id: user.course.id, id: assignment.id }, format: 'csv'
241+
get_as user, :download_test_results, params: {
242+
course_id: user.course.id,
243+
id: assignment.id,
244+
latest: true,
245+
student_run: true,
246+
instructor_run: false
247+
}, format: 'csv'
182248

183249
test_results = CSV.parse(response.body, headers: true)
184250

@@ -187,10 +253,21 @@
187253
end
188254

189255
it 'returns the correct csv headers' do
190-
get_as user, :download_test_results, params: { course_id: user.course.id, id: assignment.id }, format: 'csv'
256+
get_as user, :download_test_results, params: {
257+
course_id: user.course.id,
258+
id: assignment.id,
259+
latest: true,
260+
student_run: true,
261+
instructor_run: false
262+
}, format: 'csv'
191263
test_results = CSV.parse(response.body, headers: true)
192264

193-
assignment_results = assignment.summary_test_results
265+
assignment_results = SummaryTestResultsHelper::SummaryTestResults.fetch(
266+
test_groups: assignment.test_groups,
267+
latest: true,
268+
student_run: true,
269+
instructor_run: false
270+
).instance_variable_get(:@test_results)
194271

195272
headers = Set.new(test_results.headers.drop(1)).sort
196273
assignment_results.each do |result|
@@ -199,7 +276,13 @@
199276
end
200277

201278
it 'returns the correct csv headers in the correct order' do
202-
get_as user, :download_test_results, params: { course_id: user.course.id, id: assignment.id }, format: 'csv'
279+
get_as user, :download_test_results, params: {
280+
course_id: user.course.id,
281+
id: assignment.id,
282+
latest: true,
283+
student_run: true,
284+
instructor_run: false
285+
}, format: 'csv'
203286
test_results = CSV.parse(response.body, headers: true)
204287

205288
headers = test_results.headers.drop(1)
@@ -210,7 +293,13 @@
210293
end
211294

212295
it 'returns the correct amount of passed tests per group' do
213-
get_as user, :download_test_results, params: { course_id: user.course.id, id: assignment.id }, format: 'csv'
296+
get_as user, :download_test_results, params: {
297+
course_id: user.course.id,
298+
id: assignment.id,
299+
latest: true,
300+
student_run: true,
301+
instructor_run: false
302+
}, format: 'csv'
214303
test_results = CSV.parse(response.body, headers: true).to_a.drop(1)
215304
test_results.to_a.each do |row|
216305
count = 0
@@ -230,7 +319,13 @@
230319
let(:csv_results) { CSV.parse(response.body, headers: true) }
231320

232321
before do
233-
get_as user, :download_test_results, params: { course_id: user.course.id, id: assignment.id }, format: 'csv'
322+
get_as user, :download_test_results, params: {
323+
course_id: user.course.id,
324+
id: assignment.id,
325+
latest: true,
326+
student_run: true,
327+
instructor_run: false
328+
}, format: 'csv'
234329
end
235330

236331
it 'returns the correct headers' do
@@ -2351,4 +2446,23 @@
23512446
expect(response).to redirect_to(edit_course_assignment_path(course.id, assignment.id))
23522447
end
23532448
end
2449+
2450+
describe '#download_test_results' do
2451+
let(:course) { assignment.course }
2452+
let(:assignment) { create(:assignment_with_criteria_and_test_results) }
2453+
let(:instructor) { create(:instructor) }
2454+
2455+
context 'when latest is false and format is json' do
2456+
it 'should send a zip file' do
2457+
get_as instructor, :download_test_results,
2458+
params: { course_id: course.id, id: assignment.id, latest: 'false' },
2459+
format: :json
2460+
2461+
expect(response).to have_http_status(:ok)
2462+
expect(response.headers['Content-Type']).to eq('application/zip')
2463+
expect(response.headers['Content-Disposition']).to include('attachment')
2464+
expect(response.headers['Content-Disposition']).to include("#{assignment.short_identifier}_test_results.zip")
2465+
end
2466+
end
2467+
end
23542468
end

‎spec/helpers/summary_test_results_helper_spec.rb‎

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
describe SummaryTestResultsHelper do
2+
describe SummaryTestResultsHelper::SummaryTestResults do
3+
context 'an assignment with no test results' do
4+
let(:assignment) { create(:assignment_with_criteria_and_results) }
5+
6+
it 'should return {} when using json' do
7+
expect(SummaryTestResultsHelper::SummaryTestResults.fetch(
8+
test_groups: assignment.test_groups,
9+
latest: true,
10+
student_run: true,
11+
instructor_run: false
12+
).as_json).to eq('{}')
13+
end
14+
15+
it 'should be empty when using csv' do
16+
expect(SummaryTestResultsHelper::SummaryTestResults.fetch(
17+
test_groups: assignment.test_groups,
18+
latest: true,
19+
student_run: true,
20+
instructor_run: false
21+
).as_csv).to eq("\n")
22+
end
23+
end
24+
25+
context 'an assignment with test results across multiple test groups' do
26+
let(:assignment) { create(:assignment_with_criteria_and_test_results) }
27+
28+
it 'has the correct group and test names' do
29+
expect(assignment.test_groups.size).to be > 1
30+
31+
summary_test_results = JSON.parse(SummaryTestResultsHelper::SummaryTestResults.fetch(
32+
test_groups: assignment.test_groups,
33+
latest: true,
34+
student_run: true,
35+
instructor_run: false
36+
).as_json)
37+
38+
summary_test_results.map do |group_name, group|
39+
group.map do |test_group_name, test_group|
40+
test_group.each do |test_result|
41+
expect(test_result.fetch('name')).to eq test_group_name
42+
expect(test_result.fetch('group_name')).to eq group_name
43+
expect(test_result.key?('status')).to be true
44+
end
45+
end
46+
end
47+
end
48+
49+
it 'has the correct test result keys' do
50+
expect(assignment.test_groups.size).to be > 1
51+
52+
summary_test_results = JSON.parse(SummaryTestResultsHelper::SummaryTestResults.fetch(
53+
test_groups: assignment.test_groups,
54+
latest: true,
55+
student_run: true,
56+
instructor_run: false
57+
).as_json)
58+
59+
expected_keys = %w[marks_earned
60+
marks_total
61+
output
62+
name
63+
test_result_name
64+
test_groups_id
65+
group_name
66+
status
67+
extra_info
68+
error_type
69+
id]
70+
summary_test_results.map do |_, group|
71+
group.map do |_, test_group|
72+
test_group.each do |test_result|
73+
expect(test_result.keys).to match_array expected_keys
74+
end
75+
end
76+
end
77+
end
78+
79+
# despite having multiple test groups, assignment is set up so every test
80+
# run contains results from exactly one test group; so this should also
81+
# return results from only one test group
82+
it 'returns results from only one test group for each group when fetching latest results' do
83+
expect(assignment.test_groups.size).to be > 1
84+
85+
summary_test_results = JSON.parse(SummaryTestResultsHelper::SummaryTestResults.fetch(
86+
test_groups: assignment.test_groups,
87+
latest: true,
88+
student_run: true,
89+
instructor_run: false
90+
).as_json)
91+
92+
summary_test_results.map do |_group_name, group|
93+
expect(group.count).to eq 1
94+
end
95+
end
96+
97+
it 'returns results from more than one test group for each group when not fetching latest results' do
98+
expect(assignment.test_groups.size).to be > 1
99+
100+
summary_test_results = JSON.parse(SummaryTestResultsHelper::SummaryTestResults.fetch(
101+
test_groups: assignment.test_groups,
102+
latest: false,
103+
student_run: true,
104+
instructor_run: false
105+
).as_json)
106+
107+
summary_test_results.map do |_group_name, group|
108+
expect(group.count).to be > 1
109+
end
110+
end
111+
end
112+
end
113+
end

0 commit comments

Comments
 (0)