add accessible alert when rubric scores automatically change

fixes OUT-2051

test plan:
- go to account settings and ensure the "Allow Outcome Extra
  Credit" flag is set to "off"
- have at least two outcomes in a course
- create a rubric, align two outcomes, and create a regular
  non-outcome criterion
- attach the rubric to an assignment
- as a student, submit to the assignment
- enable voiceover
- as a teacher, go to speedgrader and attempt to give more than
  the maximum possible score on one outcome - a flash warning
  should appear notifying the user that the outcome's score
  has changed, and the alert should be audible to voiceover
- attempt to give all three criterion scores over their maximum
- both outcomes should have alerts that are read out, the normal
  criterion should not be altered
- go to the submission show page
- repeat the above steps and ensure proper alerts are presented,
  and that they're audible to voiceover

Change-Id: I1c8bb5c4a9d097ef5825fb5a27d515175cd64a39
Reviewed-on: https://gerrit.instructure.com/146036
Reviewed-by: Michael Brewer-Davis <mbd@instructure.com>
Tested-by: Jenkins
Reviewed-by: Augusto Callejas <acallejas@instructure.com>
QA-Review: Dariusz Dzien <ddzien@instructure.com>
Product-Review: Nathan Rogowski <nathan@instructure.com>
This commit is contained in:
Matthew Berns 2018-04-05 11:41:15 -05:00 committed by Matt Berns
parent 0fc9946636
commit 0abca3170b
8 changed files with 128 additions and 33 deletions

View File

@ -44,7 +44,7 @@
<i class="learning_outcome_flag icon-outcomes" aria-hidden="true"></i>
<span class="screenreader-only"><%= t("This criterion is linked to a Learning Outcome") %></span>
</span>
<span class="description criterion_description_value"><%= description %></span>
<span class="description description_title"><%= description %></span>
<span class="learning_outcome_id" style="display: none;"><%= criterion.try(:learning_outcome_id) %></span>
<span class="criterion_id" style="display: none;"><%= criterion.try(:id) %></span>
<% if !learning_outcome_criterion && !assessing %>

View File

@ -165,7 +165,7 @@ import 'compiled/jquery/fixDialogButtons'
$criterion.find("textarea.long_description").text(outcome.get('description'));
$criterion.find(".long_description_holder").toggleClass('empty', !outcome.get('description'));
$criterion.find(".criterion_description_value").text(outcome.get('title'));
$criterion.find(".description_title").text(outcome.get('title'));
$criterion.find(".criterion_description").val(outcome.get('title')).focus().select();
$criterion.find(".mastery_points").text(outcome.get('mastery_points'));
@ -727,7 +727,7 @@ import 'compiled/jquery/fixDialogButtons'
description = $rubric_long_description_dialog.find("textarea.description").val(),
$criterion = $rubric_long_description_dialog.data('current_criterion');
if($criterion) {
$criterion.fillTemplateData({data: {long_description: long_description, criterion_description_value: description}});
$criterion.fillTemplateData({data: {long_description: long_description, description_title: description}});
$criterion.find("textarea.long_description").val(long_description);
$criterion.find("textarea.description").val(description);
$criterion.find(".long_description_holder").toggleClass('empty', !long_description);
@ -1049,7 +1049,7 @@ import 'compiled/jquery/fixDialogButtons'
$criterion.hide();
rubricEditing.editCriterion($criterion);
return false;
}).delegate('.criterion_description_value', 'click', function(event) {
}).delegate('.description_title', 'click', function() {
var $criterion = $(this).parents(".criterion")
rubricEditing.editCriterion($criterion);
return false;

View File

@ -26,6 +26,7 @@ import 'jqueryui/dialog'
import './jquery.instructure_misc_plugins' /* showIf */
import './jquery.templateData'
import './vendor/jquery.scrollTo'
import 'compiled/jquery.rails_flash_notifications' // eslint-disable-line
// TODO: stop managing this in the view and get it out of the global scope submissions/show.html.erb
/*global rubricAssessment*/
@ -154,6 +155,16 @@ window.rubricAssessment = {
setInterval(rubricAssessment.sizeRatings, 2500);
},
checkScoreAdjustment: ($criterion, rating, rawData) => {
const rawPoints = rawData[`rubric_assessment[criterion_${rating.criterion_id}][points]`]
const points = rubricAssessment.roundAndFormat(rating.points)
if (rawPoints > points) {
const criterionDescription = htmlEscape($criterion.find('.description_title').text())
$.flashWarning((I18n.t("Extra credit not permitted on outcomes, " +
"score adjusted to maximum possible for %{outcome}", {outcome: criterionDescription})))
}
},
highlightCriterionScore: function($criterion, val){
$criterion.find(".rating").each(function() {
const rating_val = numberHelper.parse($(this).find(".points").text());
@ -247,7 +258,7 @@ window.rubricAssessment = {
}
},
populateRubric: function($rubric, data) {
populateRubric: function($rubric, data, submitted_data = null) {
$rubric = rubricAssessment.findRubric($rubric);
var id = $rubric.attr('id').substring(7);
$rubric.find(".user_id").text(ENV.RUBRIC_ASSESSMENT.assessment_user_id || data.user_id).end()
@ -283,6 +294,9 @@ window.rubricAssessment = {
}
});
}
if (submitted_data && $criterion.hasClass('learning_outcome_criterion')) {
rubricAssessment.checkScoreAdjustment($criterion, rating, submitted_data)
}
$criterion
.find(".custom_rating_field").val(comments).end()
.find(".custom_rating_comments").html(comments_html).end()
@ -311,16 +325,21 @@ window.rubricAssessment = {
}
},
populateRubricSummary: function($rubricSummary, data) {
populateRubricSummary: function($rubricSummary, data, editing_data) {
$rubricSummary.find(".criterion_points").text("").end()
.find(".rating_custom").text("");
if(data) {
var assessment = data;
var total = 0;
let $criterion = null;
for(var idx in assessment.data) {
var rating = assessment.data[idx];
$rubricSummary.find("#criterion_" + rating.criterion_id)
$criterion = $rubricSummary.find("#criterion_" + rating.criterion_id);
if (editing_data && $criterion.hasClass('learning_outcome_criterion')) {
rubricAssessment.checkScoreAdjustment($criterion, rating, editing_data)
}
$criterion
.find(".rating").hide().end()
.find(".rating_" + rating.id).show().end()
.find('.criterion_points')
@ -328,11 +347,11 @@ window.rubricAssessment = {
.end()
.find(".ignore_for_scoring").showIf(rating.ignore_for_scoring);
if(ratingHasScore(rating) && !$rubricSummary.hasClass('free_form')){
$rubricSummary.find("#criterion_" + rating.criterion_id)
.find(".rating.description").show().text(rating.description).end()
$criterion.find(".rating.description").show()
.text(rating.description).end()
}
if(rating.comments_enabled && rating.comments) {
$rubricSummary.find("#criterion_" + rating.criterion_id).find(".rating_custom").show().text(rating.comments);
$criterion.find(".rating_custom").show().text(rating.comments);
}
if(rating.points && !rating.ignore_for_scoring) {
total += rating.points;

View File

@ -770,11 +770,12 @@ function initRubricStuff(){
});
selectors.get('#rubric_assessments_select').change(() => {
const editingData = rubricAssessment.assessmentData($("#rubric_full"))
var selectedAssessment = getSelectedAssessment();
rubricAssessment.populateRubricSummary(
$("#rubric_summary_holder .rubric_summary"),
selectedAssessment,
isAssessmentEditableByMe(selectedAssessment)
editingData
);
});

View File

@ -156,6 +156,10 @@ import './rubric_assessment' /*global rubricAssessment*/
parentsUntil("#application").siblings().not("#aria_alerts").attr('data-hide_from_rubric', true)
$rubric.hide()
}
function toggleRubric ($rubric) {
var ariaSetting = $rubric.is(":visible");
$("#application").find("[data-hide_from_rubric]").attr("aria-hidden", ariaSetting)
}
function closeRubric () {
$("#rubric_holder").fadeOut(function() {
toggleRubric($(this));
@ -168,10 +172,6 @@ import './rubric_assessment' /*global rubricAssessment*/
$(this).find('.hide_rubric_link').focus();
});
}
function toggleRubric ($rubric) {
var ariaSetting = $rubric.is(":visible");
$("#application").find("[data-hide_from_rubric]").attr("aria-hidden", ariaSetting)
}
function windowResize () {
var $frame = $("#preview_frame");
var top = $frame.offset().top;
@ -273,11 +273,11 @@ import './rubric_assessment' /*global rubricAssessment*/
});
$(".save_rubric_button").click(function() {
var $rubric = $(this).parents("#rubric_holder").find(".rubric");
var data = rubricAssessment.assessmentData($rubric);
var submitted_data = rubricAssessment.assessmentData($rubric);
var url = $(".update_rubric_assessment_url").attr('href');
var method = "POST";
$rubric.loadingImage();
$.ajaxJSON(url, method, data, function(data) {
$.ajaxJSON(url, method, submitted_data, function(data) {
$rubric.loadingImage('remove');
var assessment = data;
var found = false;
@ -304,11 +304,16 @@ import './rubric_assessment' /*global rubricAssessment*/
$("#rubric_assessment_option_" + assessment.id).text(assessment.assessor_name);
$("#new_rubric_assessment_option").remove();
$("#rubric_assessments_list").show();
rubricAssessment.populateRubric($rubric, assessment);
var submission = assessment.artifact;
if (submission) {
showGrade(submission);
}
/* the 500 timeout is due to the fadeOut in the closeRubric function, which defaults to 400.
We need to ensure any warning messages are read out after the fadeOut manages the page focus
so that any messages are not interrupted in voiceover utilities */
setTimeout(function () {
rubricAssessment.populateRubric($rubric, assessment, submitted_data);
var submission = assessment.artifact;
if (submission) {
showGrade(submission);
}
}, 500)
closeRubric();
});
});

View File

@ -17,6 +17,7 @@
*/
import $ from 'jquery'
import 'compiled/jquery.rails_flash_notifications' // eslint-disable-line
define(['rubric_assessment', 'i18n!rubric_assessment'], (rubric_assessment, I18n) => {
QUnit.module('RubricAssessment#roundAndFormat');
@ -64,4 +65,39 @@ define(['rubric_assessment', 'i18n!rubric_assessment'], (rubric_assessment, I18n
strictEqual($criterion.find('.selected').length, 1);
strictEqual($criterion.find('.selected').find('.points').text(), "5");
})
QUnit.module('RubricAssessment#checkScoreAdjustment');
test('displays a flash warning when rawPoints has been adjusted', function () {
const flashSpy = sinon.spy($, 'flashWarning')
const $criterion = $(
"<span>" +
"<span class='description_title'>Some Criterion</span>" +
"<span class='rating'><span class='points'>5</span></span>" +
"<span class='rating'><span class='points'>3</span></span>" +
"<span class='rating'><span class='points'>0</span></span>" +
"</span>"
);
const rating = {points: 5, criterion_id: 1}
const rawData = {'rubric_assessment[criterion_1][points]': 15}
rubric_assessment.checkScoreAdjustment($criterion, rating, rawData);
ok(flashSpy.calledWith('Extra credit not permitted on outcomes, score adjusted to maximum possible for Some Criterion'))
flashSpy.restore()
})
test('does not display a flash warning when rawPoints has not been adjusted', function () {
const flashSpy = sinon.spy($, 'flashWarning')
const $criterion = $(
"<span>" +
"<span class='description_title'>Some Criterion</span>" +
"<span class='rating'><span class='points'>5</span></span>" +
"<span class='rating'><span class='points'>3</span></span>" +
"<span class='rating'><span class='points'>0</span></span>" +
"</span>"
);
const rating = {points: 5, criterion_id: 1}
const rawData = {'rubric_assessment[criterion_1][points]': 5}
rubric_assessment.checkScoreAdjustment($criterion, rating, rawData);
equal(flashSpy.callCount, 0)
flashSpy.restore()
})
});

View File

@ -62,14 +62,6 @@ describe "assignment rubrics" do
get "/courses/#{@course.id}/outcomes"
expect_new_page_load{f(' .manage_rubrics').click}
# this was originally added in OUT-465. It will eventually be moved over
# into the below popover menu, so leaving the blow code in place for
# when that happens
# expect_new_page_load do
# f('#popoverMenu button').click
# f('[data-reactid*="manage-rubrics"]').click
# end
expect do
f('.add_rubric_link').click
f('#add_criterion_container a:nth-of-type(1)').click
@ -86,7 +78,7 @@ describe "assignment rubrics" do
submit_form('#edit_rubric_form')
wait_for_ajaximations
end.to change(Rubric, :count).by(1)
expect(f('.rubric_table tbody tr:nth-of-type(3) .criterion_description_value')).
expect(f('.rubric_table tbody tr:nth-of-type(3) .description_title')).
to include_text('criterion 1')
expect(f('.rubric_table tbody tr:nth-of-type(3) .ratings td:nth-of-type(2) .rating_description_value')).
to include_text('rating 1')
@ -569,7 +561,7 @@ describe "assignment rubrics" do
wait_for_ajaximations
expect(ffj('.criterion:visible .criterion_description_value')[2]).to include_text "no outcome row"
expect(ffj('.criterion:visible .description_title')[2]).to include_text "no outcome row"
end
it "should copy an existing learning outcome", priority: "1", test_id: 220343 do
@ -582,7 +574,7 @@ describe "assignment rubrics" do
f('#criterion_duplicate_menu ul li:nth-of-type(1)').click
wait_for_ajaximations
expect(ffj('.criterion:visible .criterion_description_value')[2]).to include_text "Outcome row"
expect(ffj('.criterion:visible .description_title')[2]).to include_text "Outcome row"
end
end
end

View File

@ -241,6 +241,48 @@ describe 'Speedgrader' do
end
end
context 'rubric with outcomes' do
before :once do
init_course_with_students
@teacher = @user
@assignment = @course.assignments.create!(
title: 'Outcome Rubric',
points_possible: 8
)
rubric = outcome_with_rubric
rubric.save!
rubric.associate_with(@assignment, @course, purpose: 'grading')
rubric.reload
end
describe 'flashes a warning when grade changes in' do
before :each do
user_session(@teacher)
end
it 'speedgrader' do
get "/courses/#{@course.id}/gradebook/speed_grader?assignment_id=#{@assignment.id}#"
f('button.toggle_full_rubric').click
replace_content f('.learning_outcome_criterion input.criterion_points'), '5'
f('button.save_rubric_button').click
wait_for_ajax_requests
expect_flash_message :warning
end
it 'submissions page' do
get "/courses/#{@course.id}/assignments/#{@assignment.id}/submissions/#{@students[0].id}"
f('a.assess_submission_link').click
wait_for_animations
replace_content f('.learning_outcome_criterion input.criterion_points'), '5'
scroll_into_view('button.save_rubric_button')
f('button.save_rubric_button').click
wait_for_ajax_requests
expect_flash_message :warning
end
end
end
context 'Using a rubric to grade' do
it 'should display correct grades for student', priority: "1", test_id: 164205 do
course_with_student_logged_in(active_all: true)