From 3ade346ff48d8435016e834b446cab7aca308197 Mon Sep 17 00:00:00 2001 From: Adrian Packel Date: Fri, 21 Jun 2019 10:54:01 -0500 Subject: [PATCH] Disable SG link in submission tray if no group If the "require student group for SpeedGrader" course setting is enabled and no student group has been selected in new Gradebook, disable the SpeedGrader link in the submission tray and show an alert. closes GRADE-2238 closes GRADE-2239 Test plan: - Enable the new "filter by student group" course setting - Have a course with some students and student groups - Open new Gradebook - In the submission tray: - If a student group was selected from the filter in Gradebook, the SpeedGrader link should show up as normal - If no student group was selected, the link should appear as disabled, and an alert should indicate that a group must be selected before launching SpeedGrader - Disable the filter-by-student-group setting, and re-open new Gradebook - The SpeedGrader link in the submission tray should always be displayed, regardless of whether a student group is selected Change-Id: I6e1acaf941c44433f234a88d5ee4c7d3f035af9d Reviewed-on: https://gerrit.instructure.com/198915 Tested-by: Jenkins Reviewed-by: Jeremy Neander Reviewed-by: Derek Bender QA-Review: Adrian Packel Product-Review: Jonathan Fenton --- app/coffeescripts/gradezilla/Gradebook.coffee | 4 ++ .../__tests__/GradebookSpecHelper.js | 3 +- .../components/SubmissionTray.js | 36 ++++++++++++--- .../jsx/gradezilla/GradebookSpec.js | 40 +++++++++++++++++ .../components/SubmissionTraySpec.js | 45 +++++++++++++------ .../gradezilla_grade_detail_tray_page.rb | 4 +- .../speedgrader_student_group_filter_spec.rb | 15 ++++--- 7 files changed, 119 insertions(+), 28 deletions(-) diff --git a/app/coffeescripts/gradezilla/Gradebook.coffee b/app/coffeescripts/gradezilla/Gradebook.coffee index b8b686768e7..f56e53d01cb 100644 --- a/app/coffeescripts/gradezilla/Gradebook.coffee +++ b/app/coffeescripts/gradezilla/Gradebook.coffee @@ -2376,6 +2376,7 @@ export default do -> onRequestClose: @closeSubmissionTray pendingGradeInfo: @getPendingGradeInfo({ assignmentId, userId: studentId }) postPoliciesEnabled: @options.post_policies_enabled + requireStudentGroupForSpeedGrader: @requireStudentGroupForSpeedGrader() selectNextAssignment: => @loadTrayAssignment('next') selectPreviousAssignment: => @loadTrayAssignment('previous') selectNextStudent: => @loadTrayStudent('next') @@ -3049,6 +3050,9 @@ export default do -> # React throws an error if we try to unmount while the event is being handled @delayedCall 0, => ReactDOM.unmountComponentAtNode(anonymousSpeedGraderAlertMountPoint()) + requireStudentGroupForSpeedGrader: => + @options.course_settings.filter_speed_grader_by_student_group && @getStudentGroupToShow() == '0' + destroy: => $(window).unbind('resize.fillWindowWithMe') $(document).unbind('gridready') diff --git a/app/jsx/gradezilla/default_gradebook/__tests__/GradebookSpecHelper.js b/app/jsx/gradezilla/default_gradebook/__tests__/GradebookSpecHelper.js index afd080b6388..8b7098b05fb 100644 --- a/app/jsx/gradezilla/default_gradebook/__tests__/GradebookSpecHelper.js +++ b/app/jsx/gradezilla/default_gradebook/__tests__/GradebookSpecHelper.js @@ -30,7 +30,8 @@ export function createGradebook(options = {}) { context_url: '/courses/1/', course_settings: { - allow_final_grade_override: false + allow_final_grade_override: false, + filter_speed_grader_by_student_group: false }, currentUserId: '1', diff --git a/app/jsx/gradezilla/default_gradebook/components/SubmissionTray.js b/app/jsx/gradezilla/default_gradebook/components/SubmissionTray.js index 50fa5898599..469b0f47eff 100644 --- a/app/jsx/gradezilla/default_gradebook/components/SubmissionTray.js +++ b/app/jsx/gradezilla/default_gradebook/components/SubmissionTray.js @@ -19,6 +19,7 @@ import React from 'react' import {arrayOf, bool, func, number, oneOf, shape, string} from 'prop-types' import I18n from 'i18n!gradezilla' +import Alert from '@instructure/ui-alerts/lib/components/Alert' import Avatar from '@instructure/ui-elements/lib/components/Avatar' import Button from '@instructure/ui-buttons/lib/components/Button' import CloseButton from '@instructure/ui-buttons/lib/components/CloseButton' @@ -94,6 +95,7 @@ export default class SubmissionTray extends React.Component { valid: bool.isRequired }), postPoliciesEnabled: bool.isRequired, + requireStudentGroupForSpeedGrader: bool.isRequired, student: shape({ id: string.isRequired, avatarUrl: string, @@ -206,19 +208,40 @@ export default class SubmissionTray extends React.Component { } renderSpeedGraderLink(speedGraderProps) { - const buttonProps = {variant: 'link', href: speedGraderProps.speedGraderUrl} + const buttonProps = { + disabled: speedGraderProps.requireStudentGroup, + href: speedGraderProps.speedGraderUrl, + variant: 'link' + } if (speedGraderProps.anonymizeStudents) { buttonProps.onClick = e => { e.preventDefault() this.props.onAnonymousSpeedGraderClick(speedGraderProps.speedGraderUrl) } } + return ( - - + + {speedGraderProps.requireStudentGroup && ( + + + {I18n.t('Select Student Group')} + + + + {I18n.t(` + Due to the size of your course you must select a student group before launching + SpeedGrader. + `)} + + + )} + + + ) } @@ -257,6 +280,7 @@ export default class SubmissionTray extends React.Component { if (this.props.speedGraderEnabled) { speedGraderProps = { anonymizeStudents: this.props.assignment.anonymizeStudents, + requireStudentGroup: this.props.requireStudentGroupForSpeedGrader, speedGraderUrl } } diff --git a/spec/javascripts/jsx/gradezilla/GradebookSpec.js b/spec/javascripts/jsx/gradezilla/GradebookSpec.js index 03e92e8633b..055bb11bb43 100644 --- a/spec/javascripts/jsx/gradezilla/GradebookSpec.js +++ b/spec/javascripts/jsx/gradezilla/GradebookSpec.js @@ -6768,6 +6768,46 @@ QUnit.module('Gradebook#getSubmissionTrayProps', function(suiteHooks) { const props = gradebook.getSubmissionTrayProps(gradebook.student('1101')) strictEqual(props.postPoliciesEnabled, true) }) + + QUnit.module('requireStudentGroupForSpeedGrader', requireStudentGroupHooks => { + requireStudentGroupHooks.beforeEach(() => { + const studentGroups = [ + { + groups: [{id: '1', name: 'First Group Set 1'}, {id: '2', name: 'First Group Set 2'}], + id: '1', + name: 'First Group Set' + }, + { + groups: [{id: '3', name: 'Second Group Set 1'}, {id: '4', name: 'Second Group Set 2'}], + id: '2', + name: 'Second Group Set' + } + ] + + gradebook.setStudentGroups(studentGroups) + }) + + test('is true when filter_speed_grader_by_student_group is enabled and no group is selected', () => { + gradebook.options.course_settings.filter_speed_grader_by_student_group = true + gradebook.setSubmissionTrayState(true, '1101', '2301') + const props = gradebook.getSubmissionTrayProps(gradebook.student('1101')) + strictEqual(props.requireStudentGroupForSpeedGrader, true) + }) + + test('is false when filter_speed_grader_by_student_group is enabled and a group is selected', () => { + gradebook.options.course_settings.filter_speed_grader_by_student_group = true + gradebook.setFilterRowsBySetting('studentGroupId', '4') + gradebook.setSubmissionTrayState(true, '1101', '2301') + const props = gradebook.getSubmissionTrayProps(gradebook.student('1101')) + strictEqual(props.requireStudentGroupForSpeedGrader, false) + }) + + test('is false when filter_speed_grader_by_student_group is not enabled', () => { + gradebook.setSubmissionTrayState(true, '1101', '2301') + const props = gradebook.getSubmissionTrayProps(gradebook.student('1101')) + strictEqual(props.requireStudentGroupForSpeedGrader, false) + }) + }) }) QUnit.module('Gradebook#renderSubmissionTray', { diff --git a/spec/javascripts/jsx/gradezilla/default_gradebook/components/SubmissionTraySpec.js b/spec/javascripts/jsx/gradezilla/default_gradebook/components/SubmissionTraySpec.js index 711b0672bb2..c696ea60da4 100644 --- a/spec/javascripts/jsx/gradezilla/default_gradebook/components/SubmissionTraySpec.js +++ b/spec/javascripts/jsx/gradezilla/default_gradebook/components/SubmissionTraySpec.js @@ -149,6 +149,16 @@ QUnit.module('SubmissionTray', function(hooks) { strictEqual($elt.getAttribute('disabled'), disabled ? '' : null, message) } + function speedGraderLink() { + return document.querySelector('.SubmissionTray__Container a[href*="speed_grader"]') + } + + function studentGroupRequiredAlert() { + return [...document.querySelectorAll('div')].find($el => + $el.textContent.includes('you must select a student group') + ) + } + QUnit.module('Student Carousel', function() { function assertStudentButtonsDisabled(disabled) { ;['Previous student', 'Next student'].forEach(label => { @@ -234,10 +244,7 @@ QUnit.module('SubmissionTray', function(hooks) { '/courses/1/gradebook/speed_grader?assignment_id=30&student_id=27' ) mountComponent() - const speedGraderLink = document - .querySelector('.SubmissionTray__Container a[href*="speed_grader"]') - .getAttribute('href') - strictEqual(speedGraderLink, speedGraderUrl) + strictEqual(speedGraderLink().getAttribute('href'), speedGraderUrl) }) test('invokes "onAnonymousSpeedGraderClick" when the SpeedGrader link is clicked if the assignment is anonymous', function() { @@ -252,24 +259,36 @@ QUnit.module('SubmissionTray', function(hooks) { onAnonymousSpeedGraderClick: sinon.stub() } mountComponent(props) - document.querySelector('.SubmissionTray__Container a[href*="speed_grader"]').click() + speedGraderLink().click() strictEqual(props.onAnonymousSpeedGraderClick.callCount, 1) }) test('omits student_id from SpeedGrader link if enabled and assignment has anonymized students', function() { mountComponent({assignment: {anonymizeStudents: true}}) - const speedGraderLink = document - .querySelector('.SubmissionTray__Container a[href*="speed_grader"]') - .getAttribute('href') - notOk(speedGraderLink.match(/student_id/)) + notOk( + speedGraderLink() + .getAttribute('href') + .match(/student_id/) + ) }) test('does not show SpeedGrader link if disabled', function() { mountComponent({speedGraderEnabled: false}) - const speedGraderLink = document.querySelector( - '.SubmissionTray__Container a[href*="speed_grader"]' - ) - notOk(speedGraderLink) + notOk(speedGraderLink()) + }) + + QUnit.module('when requireStudentGroupForSpeedGrader is true', requireStudentGroupHooks => { + requireStudentGroupHooks.beforeEach(() => { + mountComponent({requireStudentGroupForSpeedGrader: true}) + }) + + test('disables the SpeedGrader link', () => { + strictEqual(speedGraderLink().getAttribute('disabled'), '') + }) + + test('shows an alert indicating a group must be selected', () => { + ok(studentGroupRequiredAlert()) + }) }) test('shows avatar if avatar is not null', function() { diff --git a/spec/selenium/grades/pages/gradezilla_grade_detail_tray_page.rb b/spec/selenium/grades/pages/gradezilla_grade_detail_tray_page.rb index e718ea15d1f..3acef073ef3 100755 --- a/spec/selenium/grades/pages/gradezilla_grade_detail_tray_page.rb +++ b/spec/selenium/grades/pages/gradezilla_grade_detail_tray_page.rb @@ -66,8 +66,8 @@ module Gradezilla f("#final-grade-value").text end - def group_message - # TODO: locator for the student group message text + def self.group_message + fj("div:contains('Select Student Group')") end def self.speedgrader_link diff --git a/spec/selenium/grades/speedgrader/speedgrader_student_group_filter_spec.rb b/spec/selenium/grades/speedgrader/speedgrader_student_group_filter_spec.rb index 2c15a52d7af..10320d26236 100644 --- a/spec/selenium/grades/speedgrader/speedgrader_student_group_filter_spec.rb +++ b/spec/selenium/grades/speedgrader/speedgrader_student_group_filter_spec.rb @@ -20,10 +20,12 @@ require_relative '../pages/speedgrader_page' require_relative '../pages/gradezilla_page' require_relative '../pages/gradezilla_grade_detail_tray_page' require_relative '../pages/gradezilla_cells_page' +require_relative '../setup/gradebook_setup' require_relative '../../assignments/page_objects/assignment_page' describe 'filter speed grader by student group' do include_context "in-process server selenium tests" + include GradebookSetup before :once do # course with student groups @@ -55,6 +57,8 @@ describe 'filter speed grader by student group' do @group1_students = @students[0,2] @group2_students = @students[2,2] + + @course.update!(filter_speed_grader_by_student_group: true) end context 'on assignments page' do @@ -79,7 +83,8 @@ describe 'filter speed grader by student group' do end it 'speedgrader link from tray has correct href' do - skip('unskip in GRADE-2238') + show_student_groups_filter(@teacher) + Gradezilla.visit(@course) # select group from gradebook Gradezilla.select_student_group(@category.groups.second) @@ -90,7 +95,7 @@ describe 'filter speed grader by student group' do end it 'loads speedgrader when group selected' do - skip('unskip in GRADE-2238') + skip('Unskip in GRADE-2245') # select group from gradebook setting @teacher.preferences[:gradebook_settings] = { @course.id => { @@ -102,17 +107,15 @@ describe 'filter speed grader by student group' do Speedgrader.visit(@course.id, @assignment.id) # verify Speedgrader.click_students_dropdown - expect(Speedgrader.fetch_student_names).to contain_exaclty(@group2_students) + expect(Speedgrader.fetch_student_names).to contain_exactly(@group2_students) end it 'disables speedgrader from tray' do - skip('unskip in GRADE-2239') Gradezilla.visit(@course) # verify link is disabled and message Gradezilla::Cells.open_tray(@group2_students.first, @assignment) - # expect(Gradezilla::GradeDetailTray.group_message).to contain_text("you must select a student group") + expect(Gradezilla::GradeDetailTray.group_message).to include_text("you must select a student group") expect(Gradezilla::GradeDetailTray.speedgrader_link).to be_disabled end end - end