From 7aa6b4767ebf606033d8d632fcf64176933fb43b Mon Sep 17 00:00:00 2001 From: mishaschwartz Date: Thu, 13 Feb 2020 21:53:03 -0500 Subject: [PATCH 1/3] autotests: Properly hide result data from students (#4379) --- Changelog.md | 5 ++ app/models/grouping.rb | 6 +- spec/factories/test_results.rb | 11 ++++ spec/models/grouping_spec.rb | 102 +++++++++++++++++++++++++++++++++ 4 files changed, 121 insertions(+), 3 deletions(-) create mode 100644 spec/factories/test_results.rb diff --git a/Changelog.md b/Changelog.md index 92a64941ae..205965a71b 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,4 +1,9 @@ # Changelog +## [unreleased] +- Added option to anonymize group membership when viewed by graders (#4331) +- Added option to only display assigned criteria to graders as opposed to showing unassigned criteria but making them + ungradeable (#4331) +- Fixed bug where test output was not being properly hidden from students (#4379) ## [v1.8.3] - Fixed bug where grace credits were not displayed to Graders viewing the submissions table (#4332) diff --git a/app/models/grouping.rb b/app/models/grouping.rb index 1692264ac7..aae2818047 100644 --- a/app/models/grouping.rb +++ b/app/models/grouping.rb @@ -715,8 +715,8 @@ def test_runs_instructors_released(submission) filtered = filter_test_runs(filters: { 'users.type': 'Admin', 'test_runs.submission': submission }) plucked = Grouping.pluck_test_runs(filtered) plucked.map! do |data| - if data['test_groups.display_output'] == 'instructors_and_student_tests' || - data['test_groups.display_output'] == 'instructors' + if data['test_groups.display_output'] == TestGroup.display_outputs[:instructors_and_student_tests] || + data['test_groups.display_output'] == TestGroup.display_outputs[:instructors] data.delete('test_results.output') end data.delete('test_group_results.extra_info') @@ -729,7 +729,7 @@ def test_runs_students filtered = filter_test_runs(filters: { 'test_runs.user': self.accepted_students }) plucked = Grouping.pluck_test_runs(filtered) plucked.map! do |data| - if data['test_groups.display_output'] == 'instructors' + if data['test_groups.display_output'] == TestGroup.display_outputs[:instructors] data.delete('test_results.output') end data.delete('test_group_results.extra_info') diff --git a/spec/factories/test_results.rb b/spec/factories/test_results.rb new file mode 100644 index 0000000000..f97be0270d --- /dev/null +++ b/spec/factories/test_results.rb @@ -0,0 +1,11 @@ +FactoryBot.define do + factory :test_result do + name { Faker::Lorem.word } + status { 'pass' } + marks_earned { 1 } + output { Faker::TvShows::HeyArnold.quote } + marks_total { 1 } + association :test_group_result + time { Faker::Number.number(digits: 4) } + end +end diff --git a/spec/models/grouping_spec.rb b/spec/models/grouping_spec.rb index eadb27cf02..9ef4d4d07e 100644 --- a/spec/models/grouping_spec.rb +++ b/spec/models/grouping_spec.rb @@ -1098,4 +1098,106 @@ def expect_updated_criteria_coverage_count_eq(expected_count) end end end + + shared_examples 'test run data' do |return_data, show_output, show_extra| + if return_data + it 'should return data for the test run' do + expect(data.length).to eq 1 + expect(data[0]['test_runs.id']).to eq test_run.id + end + + it 'should return data for the test result' do + test_result_data = data[0]['test_data'] + expect(test_result_data.length).to eq 1 + expect(test_result_data[0]['test_runs.id']).to eq test_run.id + end + + if show_extra + it 'should show extra info' do + expect(data[0]['test_data'][0]['test_group_results.extra_info']).not_to be_nil + end + else + it 'should not show extra info' do + expect(data[0]['test_data'][0]['test_group_results.extra_info']).to be_nil + end + end + + if show_output + it 'should show output data' do + expect(data[0]['test_data'][0]['test_results.output']).not_to be_nil + end + else + it 'should not show output data' do + expect(data[0]['test_data'][0]['test_results.output']).to be_nil + end + end + else + it 'should not return data' do + expect(data).to be_empty + end + end + end + + context 'getting test run data' do + let(:grouping) { create :grouping_with_inviter } + let(:test_run) { create :test_run, grouping: grouping, user: test_runner, submission: submission } + let(:display_output) { 'instructors' } + let(:test_group) { create :test_group, assignment: grouping.assignment, display_output: display_output } + let(:test_group_result) { create :test_group_result, test_run: test_run, test_group: test_group, extra_info: 'AAA' } + let!(:test_result) { create :test_result, test_group_result: test_group_result } + + context 'tests run by instructors' do + let(:test_runner) { create :admin } + let(:submission) { create :version_used_submission } + describe '#test_runs_instructors' do + let(:data) { grouping.test_runs_instructors(submission) } + it_behaves_like 'test run data', true, true, true + end + describe '#test_runs_instructors_released' do + let(:data) { grouping.test_runs_instructors_released(submission) } + context 'when display_output is instructors' do + it_behaves_like 'test run data', true, false + end + context 'when display_output is instructors_and_student_tests' do + let(:display_output) { 'instructors_and_student_tests' } + it_behaves_like 'test run data', true, false + end + context 'when display_output is instructors_and_students' do + let(:display_output) { 'instructors_and_students' } + it_behaves_like 'test run data', true, true + end + end + describe '#test_runs_students' do + let(:data) { grouping.test_runs_students } + it_behaves_like 'test run data', false + end + end + + context 'tests run by students' do + let(:submission) { nil } + let(:test_runner) { grouping.inviter } + describe '#test_runs_instructors' do + let(:data) { grouping.test_runs_instructors(submission) } + it_behaves_like 'test run data', false + end + describe '#test_runs_instructors_released' do + let(:data) { grouping.test_runs_instructors_released(submission) } + it_behaves_like 'test run data', false + end + describe '#test_runs_students' do + let(:data) { grouping.test_runs_students } + context 'when display_output is instructors' do + it_behaves_like 'test run data', true, false + end + context 'when display_output is instructors_and_student_tests' do + let(:display_output) { 'instructors_and_student_tests' } + it_behaves_like 'test run data', true, true + end + context 'when display_output is instructors_and_students' do + let(:display_output) { 'instructors_and_students' } + it_behaves_like 'test run data', true, true + end + end + end + end end From 349da01ebdb1c40e1f79e1bec3cc9ef0151902c0 Mon Sep 17 00:00:00 2001 From: David Liu Date: Tue, 18 Feb 2020 11:40:31 -0500 Subject: [PATCH 2/3] pdfjs: Switch to canvas rendering and fix annotations. Because pages are loaded lazily, the annotation_holders were added to each page container before the pages were actually rendered, preventing the annotation from appearing on hover. --- app/assets/javascripts/Components/Result/pdf_viewer.jsx | 2 +- app/assets/stylesheets/grader.scss | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/app/assets/javascripts/Components/Result/pdf_viewer.jsx b/app/assets/javascripts/Components/Result/pdf_viewer.jsx index b7bfbe54c8..23137177a7 100644 --- a/app/assets/javascripts/Components/Result/pdf_viewer.jsx +++ b/app/assets/javascripts/Components/Result/pdf_viewer.jsx @@ -11,7 +11,7 @@ export class PDFViewer extends React.Component { if (this.props.url) { this.pdfViewer = new pdfjsViewer.PDFViewer({ container: this.pdfContainer.current, - renderer: 'svg', + // renderer: 'svg', TODO: investigate why some fonts don't render with SVG }); this.loadPDFFile(); window.pdfViewer = this.pdfViewer; // For fixing display when pane width changes diff --git a/app/assets/stylesheets/grader.scss b/app/assets/stylesheets/grader.scss index 21fcdadf18..ca7c510c8e 100644 --- a/app/assets/stylesheets/grader.scss +++ b/app/assets/stylesheets/grader.scss @@ -193,6 +193,7 @@ cursor: crosshair; opacity: 0.2; position: absolute; + z-index: 1; } .code_scroller { From eb08ebebe882c411917c6f5e08efdaf3f00b2317 Mon Sep 17 00:00:00 2001 From: Misha Schwartz Date: Tue, 18 Feb 2020 14:51:32 -0500 Subject: [PATCH 3/3] changelog: update changelog for v1.8.4 --- Changelog.md | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/Changelog.md b/Changelog.md index 205965a71b..ddf04d981e 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,9 +1,7 @@ # Changelog -## [unreleased] -- Added option to anonymize group membership when viewed by graders (#4331) -- Added option to only display assigned criteria to graders as opposed to showing unassigned criteria but making them - ungradeable (#4331) +## [v1.8.4] - Fixed bug where test output was not being properly hidden from students (#4379) +- Fixed bug where certain fonts were not rendered properly using pdfjs (#4382) ## [v1.8.3] - Fixed bug where grace credits were not displayed to Graders viewing the submissions table (#4332)