Skip to content

Commit e80cef8

Browse files
committed
ISSUE-7711: Simplify api response
1 parent 1b7f640 commit e80cef8

4 files changed

Lines changed: 110 additions & 66 deletions

File tree

Changelog.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
- Enable zip downloads of test results (#7733)
2121
- Create rake task to remove orphaned end users (#7741)
2222
- Enable scanned assignments the ability to add inactive students (#7737)
23+
- Enable test results downloads through the API (#7754)
2324

2425
### 🐛 Bug fixes
2526
- Fix name column search in graders table (#7693)

app/controllers/api/groups_controller.rb

Lines changed: 17 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -204,48 +204,30 @@ def annotations
204204
end
205205

206206
def test_results
207-
test_runs = grouping&.test_runs&.includes(test_group_results: [:test_group,
208-
:test_results])&.order(created_at: :desc)
207+
return render_no_grouping_error unless grouping
209208

210-
if test_runs.blank?
211-
return render 'shared/http_status',
212-
locals: { code: '404', message: 'No test results found for this group' },
213-
status: :not_found
214-
end
209+
# Use the existing Assignment#summary_test_results method filtered for this specific group
210+
# This ensures format consistency with the UI download (summary_test_result_json)
211+
group_name = grouping.group.group_name
212+
results = assignment.summary_test_results([group_name])
215213

216-
results_data = test_runs.map do |test_run|
217-
{
218-
id: test_run.id,
219-
status: test_run.status,
220-
created_at: test_run.created_at,
221-
problems: test_run.problems,
222-
test_groups: test_run.test_group_results.map do |test_group_result|
223-
{
224-
name: test_group_result.test_group.name,
225-
marks_earned: test_group_result.marks_earned,
226-
marks_total: test_group_result.marks_total,
227-
time: test_group_result.time,
228-
tests: test_group_result.test_results.order(:position).map do |test|
229-
{
230-
name: test.name,
231-
status: test.status,
232-
marks_earned: test.marks_earned,
233-
marks_total: test.marks_total,
234-
output: test.output,
235-
time: test.time
236-
}
237-
end
238-
}
239-
end
240-
}
241-
end
214+
return render_no_grouping_error if results.blank?
215+
216+
# Group by test_group name to match the summary_test_result_json format
217+
results_by_test_group = results.group_by(&:name)
242218

243219
respond_to do |format|
244-
format.xml { render xml: results_data.to_xml(root: 'test_runs', skip_types: 'true') }
245-
format.json { render json: results_data }
220+
format.xml { render xml: results_by_test_group.to_xml(root: 'test_results', skip_types: 'true') }
221+
format.json { render json: results_by_test_group }
246222
end
247223
end
248224

225+
def render_no_grouping_error
226+
render 'shared/http_status',
227+
locals: { code: '404', message: 'No test results found for this group' },
228+
status: :not_found
229+
end
230+
249231
def add_annotations
250232
result = self.grouping&.current_result
251233
return page_not_found('No submission exists for that group') if result.nil?

app/models/assignment.rb

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -668,7 +668,7 @@ def summary_json(user)
668668
end
669669

670670
# Generates the summary of the most test results associated with an assignment.
671-
def summary_test_results
671+
def summary_test_results(group_names = nil)
672672
latest_test_run_by_grouping = TestRun.group('grouping_id').select('MAX(created_at) as test_runs_created_at',
673673
'grouping_id')
674674
.where.not(submission_id: nil)
@@ -682,17 +682,21 @@ def summary_test_results
682682
.select('id', 'test_runs.grouping_id', 'groups.group_name')
683683
.to_sql
684684

685-
self.test_groups.joins(test_group_results: :test_results)
686-
.joins("INNER JOIN (#{latest_test_runs}) latest_test_runs \
685+
query = self.test_groups.joins(test_group_results: :test_results)
686+
.joins("INNER JOIN (#{latest_test_runs}) latest_test_runs \
687687
ON test_group_results.test_run_id = latest_test_runs.id")
688-
.select('test_groups.name',
689-
'test_groups.id as test_groups_id',
690-
'latest_test_runs.group_name',
691-
'test_results.name as test_result_name',
692-
'test_results.status',
693-
'test_results.marks_earned',
694-
'test_results.marks_total',
695-
:output, :extra_info, :error_type)
688+
689+
# Optionally - filters specific groups if provided
690+
query = query.where('latest_test_runs.group_name': group_names) if group_names.present?
691+
692+
query.select('test_groups.name',
693+
'test_groups.id as test_groups_id',
694+
'latest_test_runs.group_name',
695+
'test_results.name as test_result_name',
696+
'test_results.status',
697+
'test_results.marks_earned',
698+
'test_results.marks_total',
699+
:output, :extra_info, :error_type)
696700
end
697701

698702
# Generate a JSON summary of the most recent test results associated with an assignment.

spec/controllers/api/groups_controller_spec.rb

Lines changed: 77 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1310,9 +1310,12 @@
13101310
context 'GET test_results' do
13111311
let(:grouping) { create(:grouping_with_inviter, assignment: assignment) }
13121312
let(:test_group) { create(:test_group, assignment: assignment) }
1313+
let(:submission) { create(:version_used_submission, grouping: grouping) }
13131314

13141315
context 'when the group has test results' do
1315-
let!(:test_run) { create(:test_run, grouping: grouping, role: instructor, status: :complete) }
1316+
let!(:test_run) do
1317+
create(:test_run, grouping: grouping, role: instructor, status: :complete, submission: submission)
1318+
end
13161319
let!(:test_group_result) do
13171320
create(:test_group_result, test_run: test_run, test_group: test_group,
13181321
marks_earned: 5.0, marks_total: 10.0, time: 1000)
@@ -1333,21 +1336,19 @@
13331336
expect(response).to have_http_status(:ok)
13341337
end
13351338

1336-
it 'should return test run data' do
1337-
expect(response.parsed_body.first['id']).to eq(test_run.id)
1338-
expect(response.parsed_body.first['status']).to eq('complete')
1339+
it 'should return data grouped by test group name' do
1340+
expect(response.parsed_body).to have_key(test_group.name)
13391341
end
13401342

1341-
it 'should return test group data' do
1342-
test_group_data = response.parsed_body.first['test_groups'].first
1343-
expect(test_group_data['name']).to eq(test_group.name)
1344-
expect(test_group_data['marks_earned']).to eq(5.0)
1345-
end
1346-
1347-
it 'should return individual test data' do
1348-
test_data = response.parsed_body.first['test_groups'].first['tests'].first
1349-
expect(test_data['name']).to eq('Test 1')
1350-
expect(test_data['status']).to eq('pass')
1343+
it 'should return test results for the group' do
1344+
test_results = response.parsed_body[test_group.name]
1345+
expect(test_results).to be_an(Array)
1346+
expect(test_results.first).to include(
1347+
'test_result_name' => 'Test 1',
1348+
'status' => 'pass',
1349+
'marks_earned' => 3.0,
1350+
'marks_total' => 5.0
1351+
)
13511352
end
13521353
end
13531354

@@ -1363,7 +1364,34 @@
13631364

13641365
it 'should return xml content' do
13651366
xml_data = Hash.from_xml(response.body)
1366-
expect(xml_data).to have_key('test_runs')
1367+
expect(xml_data).to have_key('test_results')
1368+
end
1369+
end
1370+
1371+
context 'with multiple test groups' do
1372+
let(:test_group_two) { create(:test_group, assignment: assignment, name: 'Group B') }
1373+
let!(:test_group_result_two) do
1374+
create(:test_group_result, test_run: test_run, test_group: test_group_two)
1375+
end
1376+
1377+
before do
1378+
create(:test_result, test_group_result: test_group_result_two, name: 'Test B1',
1379+
status: 'pass', marks_earned: 2.0, marks_total: 5.0, position: 1)
1380+
request.env['HTTP_ACCEPT'] = 'application/json'
1381+
get :test_results, params: { id: grouping.group.id, assignment_id: assignment.id, course_id: course.id }
1382+
end
1383+
1384+
it 'should be successful' do
1385+
expect(response).to have_http_status(:ok)
1386+
end
1387+
1388+
it 'should return results keyed by each test group name' do
1389+
expect(response.parsed_body.keys).to contain_exactly(test_group.name, test_group_two.name)
1390+
end
1391+
1392+
it 'should return correct test results for each group' do
1393+
expect(response.parsed_body[test_group.name].first['test_result_name']).to eq('Test 1')
1394+
expect(response.parsed_body[test_group_two.name].first['test_result_name']).to eq('Test B1')
13671395
end
13681396
end
13691397
end
@@ -1388,18 +1416,47 @@
13881416
end
13891417
end
13901418

1419+
context 'when the group does not exist' do
1420+
before do
1421+
request.env['HTTP_ACCEPT'] = 'application/json'
1422+
get :test_results, params: { id: 999_999, assignment_id: assignment.id, course_id: course.id }
1423+
end
1424+
1425+
it 'should return 404 status' do
1426+
expect(response).to have_http_status(:not_found)
1427+
end
1428+
end
1429+
13911430
context 'when multiple test runs exist' do
1392-
let!(:older_test_run) { create(:test_run, grouping: grouping, role: instructor, created_at: 2.days.ago) }
1393-
let!(:newer_test_run) { create(:test_run, grouping: grouping, role: instructor, created_at: 1.hour.ago) }
1431+
let!(:older_test_run) do
1432+
create(:test_run, grouping: grouping, role: instructor, created_at: 2.days.ago, status: :complete,
1433+
submission: submission)
1434+
end
1435+
let!(:newer_test_run) do
1436+
create(:test_run, grouping: grouping, role: instructor, created_at: 1.hour.ago, status: :complete,
1437+
submission: submission)
1438+
end
1439+
let!(:older_test_group_result) do
1440+
create(:test_group_result, test_run: older_test_run, test_group: test_group)
1441+
end
1442+
let!(:newer_test_group_result) do
1443+
create(:test_group_result, test_run: newer_test_run, test_group: test_group)
1444+
end
13941445

13951446
before do
1447+
create(:test_result, test_group_result: older_test_group_result, name: 'Old Test',
1448+
marks_earned: 1.0, marks_total: 5.0, status: 'pass', position: 1)
1449+
create(:test_result, test_group_result: newer_test_group_result, name: 'New Test',
1450+
marks_earned: 4.0, marks_total: 5.0, status: 'pass', position: 1)
13961451
request.env['HTTP_ACCEPT'] = 'application/json'
13971452
get :test_results, params: { id: grouping.group.id, assignment_id: assignment.id, course_id: course.id }
13981453
end
13991454

1400-
it 'should return newest first' do
1401-
test_run_ids = response.parsed_body.pluck('id')
1402-
expect(test_run_ids).to eq([newer_test_run.id, older_test_run.id])
1455+
it 'should return only the latest test run results' do
1456+
test_results = response.parsed_body[test_group.name]
1457+
expect(test_results.length).to eq(1)
1458+
expect(test_results.first['test_result_name']).to eq('New Test')
1459+
expect(test_results.first['marks_earned']).to eq(4.0)
14031460
end
14041461
end
14051462
end

0 commit comments

Comments
 (0)