Skip to content

E
edx-ora2
Commit db29b35f by cahrens Committed by Andy Armstrong
Options

Fix bug with non-unique IDs and names. 

TNL-3958
parent 202e4823
Showing with 150 additions and 108 deletions
openassessment/templates/openassessmentblock/oa_rubric.html
{% spaceless %}
{% load i18n %}
<fieldset class="assessment__fields">
<div class="assessment__fields">
<ol class="list list--fields assessment__rubric">
{% for criterion in rubric_criteria %}
......@@ -15,44 +15,47 @@
</h4>
<div class="ui-toggle-visibility__content">
<ol class="question__answers">
{% for option in criterion.options %}
<li class="answer">
<div class="wrapper--input">
<input type="radio"
name="{{ criterion.name }}"
id="{{ rubric_type }}__assessment__rubric__question--{{ criterion.order_num }}__{{ option.order_num }}"
class="answer__value"
value="{{ option.name }}"
aria-labelledby="{{ rubric_type }}__assessment__rubric__prompt--{{ criterion.order_num }}"/>
<label for="{{ rubric_type }}__assessment__rubric__question--{{ criterion.order_num }}__{{ option.order_num }}"
class="answer__label"
>{{ option.label }}</label>
</div>
<div class="wrapper--metadata">
<span class="answer__tip">{{ option.explanation }}</span>
<span class="answer__points">{{ option.points }} <span class="answer__points__label">{% trans "points" %}</span></span>
<div class="question__answers">
<div role="group" aria-labelledby="{{ rubric_type }}__assessment__rubric__prompt--{{ criterion.order_num }}">
{% for option in criterion.options %}
<div class="answer">
<div class="wrapper--input">
<input type="radio"
data-criterion-name="{{ criterion.name }}"
id="{{ rubric_type }}__assessment__rubric__question--{{ criterion.order_num }}__{{ option.order_num }}"
class="answer__value"
value="{{ option.name }}"
name="{{ rubric_type }}__assessment__rubric__question--{{ criterion.order_num }}"/>
<label for="{{ rubric_type }}__assessment__rubric__question--{{ criterion.order_num }}__{{ option.order_num }}"
class="answer__label"
>{{ option.label }}</label>
</div>
<div class="wrapper--metadata">
<span class="answer__tip">{{ option.explanation }}</span>
<span class="answer__points">{{ option.points }} <span class="answer__points__label">{% trans "points" %}</span></span>
</div>
</div>
</li>
{% endfor %}
{% endfor %}
</div>
{% if criterion.feedback == 'optional' or criterion.feedback == 'required' %}
<li class="answer--feedback">
<div class="answer--feedback">
<div class="wrapper--input">
<label for="{{ rubric_type }}__assessment__rubric__question--{{ criterion.order_num }}__feedback" class="answer__label">{% trans "Comments" %}</label>
<textarea
id="{{ rubric_type }}__assessment__rubric__question--{{ criterion.order_num }}__feedback"
class="answer__value"
value="{{ criterion.name }}"
name="{{ criterion.name }}"
data-criterion-name="{{ criterion.name }}"
aria-describedby="{{ rubric_type }}__assessment__rubric__prompt--{{ criterion.order_num }}"
maxlength="1000"
{% if criterion.feedback == 'required' %}required{% endif %}
>
</textarea>
</div>
</li>
{% endif %}
</ol>
</div>
</div>
{% endif %}
</div>
</li>
{% endfor %}
......@@ -72,5 +75,5 @@
</div>
</li>
</ol>
</fieldset>
</div>
{% endspaceless %}
openassessment/templates/openassessmentblock/staff_area/oa_staff_grade_learners_assessment.html
......@@ -24,7 +24,7 @@
</div>
<form class="staff-assessment__assessment" method="post">
{% include "openassessmentblock/oa_rubric.html" with rubric_type="staff" %}
{% include "openassessmentblock/oa_rubric.html" with rubric_type="staff-full-grade" %}
</form>
</article>
</div>
......
openassessment/templates/openassessmentblock/staff_area/oa_staff_override_assessment.html
......@@ -23,7 +23,7 @@
</div>
<form class="staff-assessment__assessment" method="post">
{% include "openassessmentblock/oa_rubric.html" with rubric_type="staff" %}
{% include "openassessmentblock/oa_rubric.html" with rubric_type="staff-override" %}
</form>
</article>
</div>
......
openassessment/templates/openassessmentblock/student_training/student_training.html
......@@ -77,7 +77,7 @@
</article>
<form id="student-training--001__assessment" class="student-training__assessment" method="post">
<fieldset class="assessment__fields">
<div class="assessment__fields">
<ol class="list list--fields assessment__rubric">
{% for criterion in training_rubric.criteria %}
{% if criterion.options %}
......@@ -105,16 +105,17 @@
<p>{% trans "The option you selected is not the option that the instructor selected." %}</p>
</div>
</div>
<ol class="question__answers">
<div class="question__answers">
<div role="group" aria-labelledby="training__assessment__rubric__prompt--{{ criterion.order_num }}">
{% for option in criterion.options %}
<li class="answer">
<div class="answer">
<div class="wrapper--input">
<input type="radio"
name="{{ criterion.name }}"
data-criterion-name="{{ criterion.name }}"
id="training__assessment__rubric__question--{{ criterion.order_num }}__{{ option.order_num }}"
class="answer__value"
value="{{ option.name }}"
aria-labelledby="training__assessment__rubric__prompt--{{ criterion.order_num }}"/>
name="training__assessment__rubric__question--{{ criterion.order_num }}"/>
<label for="training__assessment__rubric__question--{{ criterion.order_num }}__{{ option.order_num }}"
class="answer__label">{{ option.label }}</label>
</div>
......@@ -122,15 +123,16 @@
<span class="answer__tip">{{ option.explanation }}</span>
<span class="answer__points">{{option.points}} <span class="answer__points__label">{% trans "points" %}</span></span>
</div>
</li>
</div>
{% endfor %}
</ol>
</div>
</div>
</div>
</li>
{% endif %}
{% endfor %}
</ol>
</fieldset>
</div>
</form>
</div>
......
openassessment/xblock/static/js/spec/lms/oa_staff_area.js
......@@ -129,8 +129,8 @@ describe('OpenAssessment.StaffAreaView', function() {
$('.staff__grade__show-form', staffArea.element).click();
};
var fillAssessment = function($assessment) {
$('#staff__assessment__rubric__question--2__feedback', $assessment).val('Text response');
var fillAssessment = function($assessment, type) {
$('#staff-'+ type+ '__assessment__rubric__question--2__feedback', $assessment).val('Text response');
$('.question__answers', $assessment).each(function() {
$('input[type="radio"]', this).first().click();
});
......@@ -196,7 +196,7 @@ describe('OpenAssessment.StaffAreaView', function() {
// Create unsubmitted changes in the "full grade" form.
showInstructorAssessmentForm(fullGradeStaffArea);
$assessment = getAssessment(fullGradeStaffArea, fullGradeTab);
fillAssessment($assessment);
fillAssessment($assessment, 'full-grade');
expect(fullGradeStaffArea.baseView.unsavedWarningEnabled()).toBe(true);
expect(staffOverrideStaffArea.baseView.unsavedWarningEnabled()).toBe(true);
......@@ -204,7 +204,7 @@ describe('OpenAssessment.StaffAreaView', function() {
// Create unsubmitted changes in the "staff grade override" form.
chooseStudent(staffOverrideStaffArea, 'testStudent');
$assessment = getAssessment(staffOverrideStaffArea, staffOverrideTab);
fillAssessment($assessment);
fillAssessment($assessment, 'override');
expect(fullGradeStaffArea.baseView.unsavedWarningEnabled()).toBe(true);
expect(staffOverrideStaffArea.baseView.unsavedWarningEnabled()).toBe(true);
......@@ -442,6 +442,7 @@ describe('OpenAssessment.StaffAreaView', function() {
describe('Staff Grade Override', function() {
var staffAreaTab = "staff-tools";
var gradingType = "override";
afterEach(function() {
// Disable the unsaved page warning (if set)
......@@ -455,7 +456,7 @@ describe('OpenAssessment.StaffAreaView', function() {
$assessment = getAssessment(staffArea, staffAreaTab);
$submitButton = $('.action--submit', $assessment);
expect($submitButton).toHaveClass('is--disabled');
fillAssessment($assessment);
fillAssessment($assessment, gradingType);
expect($submitButton).not.toHaveClass('is--disabled');
});
......@@ -473,7 +474,7 @@ describe('OpenAssessment.StaffAreaView', function() {
// Fill in and submit the assessment
$assessment = getAssessment(staffArea, staffAreaTab);
fillAssessment($assessment);
fillAssessment($assessment, gradingType);
server.studentTemplate = 'oa_staff_graded_submission.html';
submitAssessment(staffArea, staffAreaTab);
......@@ -493,7 +494,7 @@ describe('OpenAssessment.StaffAreaView', function() {
$assessment;
chooseStudent(staffArea, 'testStudent');
$assessment = getAssessment(staffArea, staffAreaTab);
fillAssessment($assessment);
fillAssessment($assessment, gradingType);
// Submit the assessment but return a server error message
server.staffAssess = failWith(server, serverErrorMessage);
......@@ -513,7 +514,7 @@ describe('OpenAssessment.StaffAreaView', function() {
// Fill in and submit the assessment
$assessment = getAssessment(staffArea, staffAreaTab);
fillAssessment($assessment);
fillAssessment($assessment, gradingType);
expect(staffArea.baseView.unsavedWarningEnabled()).toBe(true);
......@@ -545,6 +546,7 @@ describe('OpenAssessment.StaffAreaView', function() {
describe('Grade Available Responses', function() {
var staffAreaTab = "staff-grading";
var gradingType = "full-grade";
beforeEach(function() {
loadFixtures('oa_base_course_staff.html');
......@@ -563,7 +565,7 @@ describe('OpenAssessment.StaffAreaView', function() {
$submitButtons = $('.action--submit', $assessment);
expect($submitButtons.length).toBe(2);
expect($submitButtons).toHaveClass('is--disabled');
fillAssessment($assessment);
fillAssessment($assessment, gradingType);
expect($submitButtons).not.toHaveClass('is--disabled');
});
......@@ -579,7 +581,7 @@ describe('OpenAssessment.StaffAreaView', function() {
);
// Fill in and submit the assessment
fillAssessment($assessment);
fillAssessment($assessment, gradingType);
server.staffGradeFormTemplate = 'oa_staff_grade_learners_assessment_2.html';
submitAssessment(staffArea, staffAreaTab);
verifyAssessType(staffArea, 'full-grade');
......@@ -600,7 +602,7 @@ describe('OpenAssessment.StaffAreaView', function() {
// Fill in and click the button to submit and request another submission
$assessment = getAssessment(staffArea, staffAreaTab);
fillAssessment($assessment);
fillAssessment($assessment, gradingType);
server.staffGradeFormTemplate = 'oa_staff_grade_learners_assessment_2.html';
$('.continue_grading--action', $assessment).click();
......@@ -626,7 +628,7 @@ describe('OpenAssessment.StaffAreaView', function() {
$assessment;
showInstructorAssessmentForm(staffArea);
$assessment = getAssessment(staffArea, staffAreaTab);
fillAssessment($assessment);
fillAssessment($assessment, gradingType);
// Submit the assessment but return a server error message
server.staffAssess = failWith(server, serverErrorMessage);
......@@ -651,7 +653,7 @@ describe('OpenAssessment.StaffAreaView', function() {
// Fill in assessment and make sure the code re-renders the count form.
$assessment = getAssessment(staffArea, staffAreaTab);
fillAssessment($assessment);
fillAssessment($assessment, gradingType);
// Return a different counts template to mimic the counts changing again.
server.staffGradeCountsTemplate = 'oa_staff_grade_learners_count_2.html';
submitAssessment(staffArea, staffAreaTab);
......@@ -669,7 +671,7 @@ describe('OpenAssessment.StaffAreaView', function() {
// Fill in assessment and make sure the code re-renders the count form.
$assessment = getAssessment(staffArea, staffAreaTab);
fillAssessment($assessment);
fillAssessment($assessment, gradingType);
expect(staffArea.baseView.unsavedWarningEnabled()).toBe(true);
submitAssessment(staffArea, staffAreaTab);
......
openassessment/xblock/static/js/src/lms/oa_rubric.js
......@@ -31,14 +31,16 @@ OpenAssessment.Rubric.prototype = {
criterionFeedback: function(criterionFeedback) {
var selector = 'textarea.answer__value';
var feedback = {};
var rubric = this;
$(selector, this.element).each(
function(index, sel) {
var criterionName = rubric.getCriterionName(sel);
if (typeof criterionFeedback !== 'undefined') {
$(sel).val(criterionFeedback[sel.name]);
feedback[sel.name] = criterionFeedback[sel.name];
$(sel).val(criterionFeedback[criterionName]);
feedback[criterionName] = criterionFeedback[criterionName];
}
else {
feedback[sel.name] = $(sel).val();
feedback[criterionName] = $(sel).val();
}
}
);
......@@ -88,11 +90,12 @@ OpenAssessment.Rubric.prototype = {
**/
optionsSelected: function(optionsSelected) {
var selector = "input[type=radio]";
var rubric = this;
if (typeof optionsSelected === 'undefined') {
var options = {};
$(selector + ":checked", this.element).each(
function(index, sel) {
options[sel.name] = sel.value;
options[rubric.getCriterionName(sel)] = sel.value;
}
);
return options;
......@@ -103,8 +106,9 @@ OpenAssessment.Rubric.prototype = {
// Check the selected options
$(selector, this.element).each(function(index, sel) {
if (optionsSelected.hasOwnProperty(sel.name)) {
if (sel.value === optionsSelected[sel.name]) {
var criterionName = rubric.getCriterionName(sel);
if (optionsSelected.hasOwnProperty(criterionName)) {
if (sel.value === optionsSelected[criterionName]) {
$(sel).prop('checked', true);
}
}
......@@ -207,10 +211,11 @@ OpenAssessment.Rubric.prototype = {
showCorrections: function(corrections) {
var selector = "input[type=radio]";
var hasErrors = false;
var rubric = this;
// Display appropriate messages for each selection
$(selector, this.element).each(function(index, sel) {
var listItem = $(sel).parents(".assessment__rubric__question");
if (corrections.hasOwnProperty(sel.name)) {
if (corrections.hasOwnProperty(rubric.getCriterionName(sel))) {
hasErrors = true;
listItem.find('.message--incorrect').removeClass('is--hidden');
listItem.find('.message--correct').addClass('is--hidden');
......@@ -220,5 +225,15 @@ OpenAssessment.Rubric.prototype = {
}
});
return hasErrors;
},
/**
* Gets the criterion name out of the data on the provided DOM element.
*
* @param {object} element
* @returns {String} value stored as data-criterion-name
*/
getCriterionName: function(element) {
return $(element).data('criterion-name');
}
};
test/acceptance/accessibility.py
......@@ -216,6 +216,35 @@ class FullWorkflowA11yTest(OpenAssessmentA11yTest, FullWorkflowMixin):
self._check_a11y(self.staff_area_page)
class FullWorkflowRequiredA11yTest(OpenAssessmentA11yTest, FullWorkflowMixin):
"""
Test accessibility when both staff override and full staff grading rubrics have rendered.
"""
def setUp(self):
super(FullWorkflowRequiredA11yTest, self).setUp('full_workflow_staff_required', staff=True)
self.staff_area_page = StaffAreaPage(self.browser, self.problem_loc)
def test_multiple_rubrics(self):
"""
Test accessibility when both the staff override and the full staff grading
rubric forms have been opened.
"""
# Create a learner with submission, training, and self assessment completed.
learner = self.do_train_self_peer(False)
# Open up the full staff grading form
self.staff_area_page.visit()
self.staff_area_page.click_staff_toolbar_button("staff-grading")
self.staff_area_page.expand_staff_grading_section()
# Open up the override form
self.staff_area_page.show_learner(learner)
self.staff_area_page.expand_learner_report_sections()
self._check_a11y(self.staff_area_page)
if __name__ == "__main__":
# Configure the screenshot directory
......
test/acceptance/pages.py
......@@ -508,7 +508,7 @@ class StaffAreaPage(OpenAssessmentPage, AssessmentMixin):
Clicks the staff grade control to expand staff grading section for use in staff required workflows.
"""
self.q(css=self._bounded_selector(".staff__grade__show-form")).first.click()
self.wait_for_element_visibility("#staff__assessment__rubric__question--0__0", "staff grading is present")
self.wait_for_element_visibility("#staff-full-grade__assessment__rubric__question--0__0", "staff grading is present")
@property
def available_checked_out_numbers(self):
......@@ -599,10 +599,10 @@ class StaffAreaPage(OpenAssessmentPage, AssessmentMixin):
css=self._bounded_selector(".staff-info__student__response .ui-toggle-visibility__content")
).text[0]
def staff_assess(self, options_selected, continue_after=False):
def staff_assess(self, options_selected, grading_type, continue_after=False):
for criterion_num, option_num in enumerate(options_selected):
sel = "#staff__assessment__rubric__question--{criterion_num}__{option_num}".format(
assessment_type="staff",
sel = "#staff-{type}__assessment__rubric__question--{criterion_num}__{option_num}".format(
type=grading_type,
criterion_num=criterion_num,
option_num=option_num
)
......
test/acceptance/tests.py
......@@ -215,7 +215,7 @@ class OpenAssessmentTest(WebAppTest):
self.staff_area_page.visit()
self.staff_area_page.show_learner(username)
self.staff_area_page.expand_learner_report_sections()
self.staff_area_page.staff_assess(self.STAFF_OVERRIDE_OPTIONS_SELECTED)
self.staff_area_page.staff_assess(self.STAFF_OVERRIDE_OPTIONS_SELECTED, "override")
self.staff_area_page.verify_learner_final_score(final_score)
def do_staff_assessment(self, number_to_assess=0, options_selected=OPTIONS_SELECTED):
......@@ -239,7 +239,7 @@ class OpenAssessmentTest(WebAppTest):
assessed = 0
while number_to_assess == 0 or assessed < number_to_assess:
continue_after = False if number_to_assess-1 == assessed else ungraded > 0
self.staff_area_page.staff_assess(options_selected, continue_after)
self.staff_area_page.staff_assess(options_selected, "full-grade", continue_after)
assessed += 1
if not continue_after:
self.staff_area_page.verify_available_checked_out_numbers((ungraded, checked_out-1))
......@@ -578,7 +578,7 @@ class StaffAreaTest(OpenAssessmentTest):
self.staff_area_page.verify_learner_final_score(self.STAFF_AREA_SCORE.format(self.EXPECTED_SCORE))
# Do staff override and wait for final score to change.
self.staff_area_page.assess("staff", self.STAFF_OVERRIDE_OPTIONS_SELECTED)
self.staff_area_page.assess("staff-override", self.STAFF_OVERRIDE_OPTIONS_SELECTED)
# Verify that the new student score is different from the original one.
# Unfortunately there is no indication presently that this was a staff override.
......@@ -784,6 +784,35 @@ class FullWorkflowMixin(object):
return username
def do_train_self_peer(self, peer_to_grade=True):
"""
Common functionality for executing training, self, and peer assessment steps.
Args:
peer_to_grade: boolean, defaults to True. Set to False to have learner complete their required steps,
but no peers to submit a grade for learner in return.
"""
# Create a learner with submission, training, and self assessment completed.
learner = self.do_submission_training_self_assessment(self.LEARNER_EMAIL, self.LEARNER_PASSWORD)
# Now create a second learner so that learner 1 has someone to assess.
# The second learner does all the steps as well (submission, training, self assessment, peer assessment).
self.do_submission_training_self_assessment("learner2@foo.com", None)
if peer_to_grade:
self.do_peer_assessment(options=self.PEER_ASSESSMENT)
# Go back to the first learner to complete her workflow.
self.login_user(learner)
# Learner 1 does peer assessment of learner 2 to complete workflow.
self.do_peer_assessment(options=self.SUBMITTED_ASSESSMENT)
# Continue grading by other students if necessary to ensure learner has a peer grade.
if peer_to_grade:
self.verify_submission_has_peer_grade(learner)
return learner
def verify_staff_area_fields(self, username, peer_assessments, submitted_assessments, self_assessment):
"""
Verifies the expected entries in the staff area for peer assessments,
......@@ -829,16 +858,6 @@ class FullWorkflowMixin(object):
"Learner still not graded after {} additional attempts".format(max_attempts)
)
class FullWorkflowBaseTest(OpenAssessmentTest, FullWorkflowMixin):
"""
Base class for common functionality in full workflow tests.
"""
def setUp(self, problem_type):
super(FullWorkflowBaseTest, self).setUp(problem_type, staff=True)
self.staff_area_page = StaffAreaPage(self.browser, self.problem_loc)
def verify_grade_entries(self, expected_entries):
"""
Verify the grade entries (sources and values) as shown in the
......@@ -853,42 +872,14 @@ class FullWorkflowBaseTest(OpenAssessmentTest, FullWorkflowMixin):
self.assertEqual(expected_entry[0], self.grade_page.grade_entry(0, index))
self.assertEqual(expected_entry[1], self.grade_page.grade_entry(1, index))
def do_train_self_peer(self, peer_to_grade=True):
"""
Common functionality for executing training, self, and peer assessment steps.
Args:
peer_to_grade: boolean, defaults to True. Set to False to have learner complete their required steps,
but no peers to submit a grade for learner in return.
"""
# Create a learner with submission, training, and self assessment completed.
learner = self.do_submission_training_self_assessment(self.LEARNER_EMAIL, self.LEARNER_PASSWORD)
# Now create a second learner so that learner 1 has someone to assess.
# The second learner does all the steps as well (submission, training, self assessment, peer assessment).
self.do_submission_training_self_assessment("learner2@foo.com", None)
if peer_to_grade:
self.do_peer_assessment(options=self.PEER_ASSESSMENT)
# Go back to the first learner to complete her workflow.
self.login_user(learner)
# Learner 1 does peer assessment of learner 2 to complete workflow.
self.do_peer_assessment(options=self.SUBMITTED_ASSESSMENT)
# Continue grading by other students if necessary to ensure learner has a peer grade.
if peer_to_grade:
self.verify_submission_has_peer_grade(learner)
return learner
class FullWorkflowOverrideTest(FullWorkflowBaseTest):
class FullWorkflowOverrideTest(OpenAssessmentTest, FullWorkflowMixin):
"""
Tests of complete workflows, combining multiple required steps together.
"""
def setUp(self):
super(FullWorkflowOverrideTest, self).setUp("full_workflow_staff_override")
super(FullWorkflowOverrideTest, self).setUp("full_workflow_staff_override", staff=True)
self.staff_area_page = StaffAreaPage(self.browser, self.problem_loc)
@retry()
@attr('acceptance')
......@@ -936,7 +927,6 @@ class FullWorkflowOverrideTest(FullWorkflowBaseTest):
[(u"YOUR SELF ASSESSMENT", u"Good"), (u"YOUR SELF ASSESSMENT", u"Excellent")]
])
@retry()
@attr('acceptance')
def test_staff_override_at_beginning(self):
......@@ -1001,12 +991,13 @@ class FullWorkflowOverrideTest(FullWorkflowBaseTest):
@ddt.ddt
class FullWorkflowRequiredTest(FullWorkflowBaseTest):
class FullWorkflowRequiredTest(OpenAssessmentTest, FullWorkflowMixin):
"""
Tests of complete workflows, combining multiple required steps together.
"""
def setUp(self):
super(FullWorkflowRequiredTest, self).setUp("full_workflow_staff_required")
super(FullWorkflowRequiredTest, self).setUp("full_workflow_staff_required", staff=True)
self.staff_area_page = StaffAreaPage(self.browser, self.problem_loc)
@retry()
@attr('acceptance')
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment