Keep section synced between SG/new gradebook
When new gradebook is enabled and the user restricts the display to a
specific section, have the selected section carry over to SpeedGrader,
and similarly in the other direction. (If a section is changed in
SpeedGrader, the change will propagate to both old and new gradebooks;
if different selections are selected in OG and NG, SpeedGrader itself
will choose depending on whether new gradebook is enabled.)
fixes GRADE-989
Test plan:
- Have a course with multiple sections and new gradebook enabled
- At least one section should be empty (i.e., contain no students)
- Test the following:
- When you change the selected section in new gradebook and then
open SpeedGrader, the new section should be shown
- Similarly, section changes in SG should be persisted in NG
- Selecting the empty section in SG (or selecting it in NG and then
opening SG) should display an alert indicating no students could be
found, and reload showing all students
- Revert to old Gradebook and check that syncing sections still works
between SG and OG
(Note that selecting a section in new gradebook will *not* directly
update the selected section in old gradebook, and vice versa. Changing
sections in SpeedGrader, however, will update both gradebooks.)
Change-Id: I42b4558f40f3208a93bd00fd6a2224d0f954c96e
Reviewed-on: https://gerrit.instructure.com/159356
Reviewed-by: Spencer Olson <solson@instructure.com>
Reviewed-by: Gary Mei <gmei@instructure.com>
Reviewed-by: Derek Bender <djbender@instructure.com>
QA-Review: Gary Mei <gmei@instructure.com>
Tested-by: Jenkins
Product-Review: Sidharth Oberoi <soberoi@instructure.com>
This commit is contained in:
Adrian Packel
parent
bd17729f0b
commit
01e2f8d5f1
|
|
@ -629,6 +629,7 @@ class GradebooksController < ApplicationController
|
|||
env = {
|
||||
CONTEXT_ACTION_SOURCE: :speed_grader,
|
||||
settings_url: speed_grader_settings_course_gradebook_path,
|
||||
new_gradebook_enabled: new_gradebook_enabled?,
|
||||
force_anonymous_grading: force_anonymous_grading?(@assignment),
|
||||
grading_role: grading_role,
|
||||
grading_type: @assignment.grading_type,
|
||||
|
|
@ -651,6 +652,10 @@ class GradebooksController < ApplicationController
|
|||
env[:provisional_select_url] = api_v1_select_provisional_grade_path(@context.id, @assignment.id, "{{provisional_grade_id}}")
|
||||
end
|
||||
|
||||
if new_gradebook_enabled?
|
||||
env[:selected_section_id] = gradebook_settings.dig(@context.id, 'filter_rows_by', 'section_id')
|
||||
end
|
||||
|
||||
if @assignment.quiz
|
||||
env[:quiz_history_url] = course_quiz_history_path @context.id,
|
||||
@assignment.quiz.id,
|
||||
|
|
@ -674,9 +679,29 @@ class GradebooksController < ApplicationController
|
|||
end
|
||||
|
||||
def speed_grader_settings
|
||||
grade_by_question = value_to_boolean(params[:enable_speedgrader_grade_by_question])
|
||||
@current_user.preferences[:enable_speedgrader_grade_by_question] = grade_by_question
|
||||
@current_user.save!
|
||||
if params[:enable_speedgrader_grade_by_question]
|
||||
grade_by_question = value_to_boolean(params[:enable_speedgrader_grade_by_question])
|
||||
@current_user.preferences[:enable_speedgrader_grade_by_question] = grade_by_question
|
||||
@current_user.save!
|
||||
end
|
||||
|
||||
if params[:selected_section_id]
|
||||
section_to_show = if params[:selected_section_id] == 'all'
|
||||
nil
|
||||
elsif @context.active_course_sections.exists?(id: params[:selected_section_id])
|
||||
params[:selected_section_id]
|
||||
end
|
||||
|
||||
gradebook_settings.deep_merge!({
|
||||
@context.id => {
|
||||
'filter_rows_by' => {
|
||||
'section_id' => section_to_show
|
||||
}
|
||||
}
|
||||
})
|
||||
@current_user.save!
|
||||
end
|
||||
|
||||
head :ok
|
||||
end
|
||||
|
||||
|
|
|
|||
|
|
@ -286,12 +286,17 @@ function mergeStudentsAndSubmission() {
|
|||
});
|
||||
|
||||
// handle showing students only in a certain section.
|
||||
// the sectionToShow will be remembered for a given user in a given browser across all assignments in this course
|
||||
|
||||
if (!jsonData.GROUP_GRADING_MODE) {
|
||||
sectionToShow = userSettings.contextGet('grading_show_only_section');
|
||||
sectionToShow = sectionToShow && String(sectionToShow);
|
||||
if (ENV.new_gradebook_enabled) {
|
||||
sectionToShow = ENV.selected_section_id
|
||||
} else {
|
||||
sectionToShow = userSettings.contextGet('grading_show_only_section')
|
||||
}
|
||||
}
|
||||
|
||||
if (sectionToShow) {
|
||||
sectionToShow = sectionToShow.toString()
|
||||
var tempArray = $.grep(jsonData.studentsWithSubmissions, function(student, i){
|
||||
return $.inArray(sectionToShow, student.section_ids) != -1;
|
||||
});
|
||||
|
|
@ -299,8 +304,7 @@ function mergeStudentsAndSubmission() {
|
|||
jsonData.studentsWithSubmissions = tempArray;
|
||||
} else {
|
||||
alert(I18n.t('alerts.no_students_in_section', "Could not find any students in that section, falling back to showing all sections."));
|
||||
userSettings.contextRemove('grading_show_only_section');
|
||||
SpeedgraderHelpers.reloadPage();
|
||||
EG.changeToSection('all')
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -359,17 +363,6 @@ function mergeStudentsAndSubmission() {
|
|||
}
|
||||
}
|
||||
|
||||
function changeToSection (sectionId) {
|
||||
if (sectionId === 'all') {
|
||||
// We're removing all filters and resetting to default
|
||||
userSettings.contextRemove('grading_show_only_section');
|
||||
} else {
|
||||
userSettings.contextSet('grading_show_only_section', sectionId);
|
||||
}
|
||||
|
||||
SpeedgraderHelpers.reloadPage();
|
||||
}
|
||||
|
||||
function initDropdown(){
|
||||
var hideStudentNames = utils.shouldHideStudentNames();
|
||||
$("#hide_student_names").attr('checked', hideStudentNames);
|
||||
|
|
@ -397,7 +390,7 @@ function initDropdown(){
|
|||
|
||||
if (newStudentOrSection && newStudentOrSection.match(/^section_(\d+|all)$/)) {
|
||||
const sectionId = newStudentOrSection.replace(/^section_/, '');
|
||||
changeToSection(sectionId);
|
||||
EG.changeToSection(sectionId)
|
||||
} else {
|
||||
EG.handleStudentChanged();
|
||||
}
|
||||
|
|
@ -407,7 +400,6 @@ function initDropdown(){
|
|||
const $selectmenu_list = $selectmenu.data('selectmenu').list;
|
||||
const $menu = $("#section-menu");
|
||||
|
||||
|
||||
$menu.find('ul').append($.raw($.map(jsonData.context.active_course_sections, function(section, i){
|
||||
return '<li><a class="section_' + section.id + '" data-section-id="'+ section.id +'" href="#">'+ htmlEscape(section.name) +'</a></li>';
|
||||
}).join('')));
|
||||
|
|
@ -421,7 +413,7 @@ function initDropdown(){
|
|||
.hide()
|
||||
.menu()
|
||||
.delegate('a', 'click mousedown', function(){
|
||||
changeToSection($(this).data('section-id'));
|
||||
EG.changeToSection($(this).data('section-id'))
|
||||
});
|
||||
|
||||
if (sectionToShow) {
|
||||
|
|
@ -2991,6 +2983,23 @@ EG = {
|
|||
|
||||
const gradeSelector = <SpeedGraderProvisionalGradeSelector {...props} />
|
||||
ReactDOM.render(gradeSelector, mountPoint)
|
||||
},
|
||||
|
||||
changeToSection (sectionId) {
|
||||
// Update the selected section in old gradebook
|
||||
if (sectionId === 'all') {
|
||||
// We're removing all filters and resetting to default
|
||||
userSettings.contextRemove('grading_show_only_section');
|
||||
} else {
|
||||
userSettings.contextSet('grading_show_only_section', sectionId);
|
||||
}
|
||||
|
||||
// ...and in new gradebook
|
||||
if (ENV.settings_url) {
|
||||
$.post(ENV.settings_url, { selected_section_id: sectionId }, () => { SpeedgraderHelpers.reloadPage() })
|
||||
} else {
|
||||
SpeedgraderHelpers.reloadPage();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -1781,6 +1781,18 @@ describe GradebooksController do
|
|||
get 'speed_grader', params: {course_id: @course, assignment_id: @assignment.id}
|
||||
expect(assigns[:disable_unmute_assignment]).to eq true
|
||||
end
|
||||
|
||||
it 'sets new_gradebook_enabled in ENV to true if new gradebook is enabled' do
|
||||
@course.enable_feature!(:new_gradebook)
|
||||
get 'speed_grader', params: {course_id: @course, assignment_id: @assignment.id}
|
||||
expect(assigns[:js_env][:new_gradebook_enabled]).to eq true
|
||||
end
|
||||
|
||||
it 'sets new_gradebook_enabled in ENV to false if new gradebook is not enabled' do
|
||||
@course.disable_feature!(:new_gradebook)
|
||||
get 'speed_grader', params: {course_id: @course, assignment_id: @assignment.id}
|
||||
expect(assigns[:js_env][:new_gradebook_enabled]).to eq false
|
||||
end
|
||||
end
|
||||
|
||||
describe "POST 'speed_grader_settings'" do
|
||||
|
|
@ -1796,6 +1808,53 @@ describe GradebooksController do
|
|||
enable_speedgrader_grade_by_question: "0"}
|
||||
expect(@teacher.reload.preferences[:enable_speedgrader_grade_by_question]).not_to be_truthy
|
||||
end
|
||||
|
||||
describe 'selected_section_id preference' do
|
||||
let(:course_settings) { @teacher.reload.preferences.dig(:gradebook_settings, @course.id) }
|
||||
|
||||
before(:each) do
|
||||
@teacher.preferences[:gradebook_settings] = { @course.id => {} }
|
||||
|
||||
user_session(@teacher)
|
||||
end
|
||||
|
||||
context 'when new gradebook is enabled' do
|
||||
it 'sets the selected section for the course to the passed-in value' do
|
||||
section_id = @course.course_sections.first.id
|
||||
post 'speed_grader_settings', params: {course_id: @course.id, selected_section_id: section_id}
|
||||
|
||||
expect(course_settings.dig('filter_rows_by', 'section_id')).to eq section_id.to_s
|
||||
end
|
||||
|
||||
it 'clears the selected section for the course if passed the value "all"' do
|
||||
post 'speed_grader_settings', params: {course_id: @course.id, selected_section_id: 'all'}
|
||||
|
||||
expect(course_settings.dig('filter_rows_by', 'section_id')).to be nil
|
||||
end
|
||||
|
||||
it 'clears the selected section if passed an invalid value' do
|
||||
post 'speed_grader_settings', params: {course_id: @course.id, selected_section_id: 'hahahaha'}
|
||||
|
||||
expect(course_settings.dig('filter_rows_by', 'section_id')).to be nil
|
||||
end
|
||||
|
||||
it 'clears the selected section if passed a non-active section in the course' do
|
||||
deleted_section = @course.course_sections.create!
|
||||
deleted_section.destroy!
|
||||
|
||||
post 'speed_grader_settings', params: {course_id: @course.id, selected_section_id: deleted_section.id}
|
||||
|
||||
expect(course_settings.dig('filter_rows_by', 'section_id')).to be nil
|
||||
end
|
||||
|
||||
it 'clears the selected section if passed a section ID not in the course' do
|
||||
section_in_other_course = Course.create!.course_sections.create!
|
||||
post 'speed_grader_settings', params: {course_id: @course.id, selected_section_id: section_in_other_course.id}
|
||||
|
||||
expect(course_settings.dig('filter_rows_by', 'section_id')).to be nil
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe "POST 'save_assignment_order'" do
|
||||
|
|
|
|||
|
|
@ -1906,6 +1906,7 @@ QUnit.module('SpeedGrader', function(suiteHooks) {
|
|||
assignment_id: '2',
|
||||
course_id: '7',
|
||||
help_url: 'example.com/foo',
|
||||
settings_url: 'example.com/settings',
|
||||
show_help_menu_item: false
|
||||
})
|
||||
sinon.stub($, 'getJSON')
|
||||
|
|
@ -4278,4 +4279,221 @@ QUnit.module('SpeedGrader', function(suiteHooks) {
|
|||
})
|
||||
})
|
||||
})
|
||||
|
||||
QUnit.module('when a course has multiple sections', (hooks) => {
|
||||
const sectionSelectPath = '#section-menu li a[data-section-id="2"]'
|
||||
const allSectionsSelectPath = '#section-menu li a[data-section-id="all"]'
|
||||
let originalWindowJSONData
|
||||
|
||||
hooks.beforeEach(() => {
|
||||
fixtures.innerHTML = `
|
||||
<span id="speedgrader-settings"></span>
|
||||
<div id="combo_box_container">
|
||||
</div>
|
||||
<ul
|
||||
id="section-menu"
|
||||
class="ui-selectmenu-menu ui-widget ui-widget-content ui-selectmenu-menu-dropdown ui-selectmenu-open"
|
||||
style="display:none;" role="listbox" aria-activedescendant="section-menu-link"
|
||||
>
|
||||
<li role="presentation" class="ui-selectmenu-item">
|
||||
<a href="#" tabindex="-1" role="option" aria-selected="true" id="section-menu-link">
|
||||
<span>Showing: <span id="section_currently_showing">All Sections</span></span>
|
||||
</a>
|
||||
<ul>
|
||||
<li><a class="selected" data-section-id="all" href="#">Show All Sections</a></li>
|
||||
</ul>
|
||||
</li>
|
||||
</ul>
|
||||
`
|
||||
|
||||
originalWindowJSONData = window.jsonData
|
||||
window.jsonData = {
|
||||
context: {
|
||||
students: [
|
||||
{
|
||||
id: 4,
|
||||
name: 'Guy B. Studying'
|
||||
}
|
||||
],
|
||||
enrollments: [
|
||||
{
|
||||
user_id: 4,
|
||||
workflow_state: 'active',
|
||||
course_section_id: 1
|
||||
},
|
||||
{
|
||||
user_id: 4,
|
||||
workflow_state: 'active',
|
||||
course_section_id: 2
|
||||
},
|
||||
{
|
||||
user_id: 4,
|
||||
workflow_state: 'active',
|
||||
course_section_id: 3
|
||||
}
|
||||
],
|
||||
active_course_sections: [
|
||||
{
|
||||
id: 1,
|
||||
name: 'The First Section'
|
||||
},
|
||||
{
|
||||
id: 2,
|
||||
name: 'The Second Section'
|
||||
},
|
||||
{
|
||||
id: 3,
|
||||
name: 'The Third Section'
|
||||
},
|
||||
{
|
||||
id: 4,
|
||||
name: 'The Lost Section'
|
||||
}
|
||||
]
|
||||
},
|
||||
submissions: []
|
||||
}
|
||||
|
||||
// This function gets set by a jQuery extension in a way that doesn't
|
||||
// appear to happen as part of the testing setup. So far as I can tell
|
||||
// it does nothing except return the current jQuery element.
|
||||
$.fn.menu = function() { return $(this) }
|
||||
|
||||
sandbox.stub($, 'post').yields()
|
||||
sandbox.stub(SpeedGraderHelpers, 'reloadPage')
|
||||
sandbox.stub(SpeedGrader.EG, 'handleFragmentChanged')
|
||||
sandbox.stub(userSettings, 'contextSet')
|
||||
sandbox.stub(userSettings, 'contextRemove')
|
||||
sandbox.stub(userSettings, 'contextGet').returns('3')
|
||||
})
|
||||
|
||||
hooks.afterEach(() => {
|
||||
userSettings.contextGet.restore()
|
||||
userSettings.contextRemove.restore()
|
||||
userSettings.contextSet.restore()
|
||||
SpeedGrader.EG.handleFragmentChanged.restore()
|
||||
SpeedGraderHelpers.reloadPage.restore()
|
||||
$.post.restore()
|
||||
delete $.fn.menu
|
||||
|
||||
document.querySelectorAll('.ui-selectmenu-menu').forEach(element => { element.remove() })
|
||||
|
||||
window.location.hash = ''
|
||||
fixtures.innerHTML = ''
|
||||
SpeedGrader.teardown()
|
||||
window.jsonData = originalWindowJSONData
|
||||
})
|
||||
|
||||
test('initially selects the section in userSettings if ENV.new_gradebook_enabled is not true', () => {
|
||||
SpeedGrader.EG.jsonReady()
|
||||
SpeedGrader.setup()
|
||||
|
||||
const currentlyShowing = document.querySelector('#section_currently_showing')
|
||||
strictEqual(currentlyShowing.innerText, 'The Third Section')
|
||||
})
|
||||
|
||||
test('initially selects the section in ENV.selected_section_id if ENV.new_gradebook_enabled is true', () => {
|
||||
window.ENV.new_gradebook_enabled = true
|
||||
window.ENV.selected_section_id = 2
|
||||
|
||||
SpeedGrader.EG.jsonReady()
|
||||
SpeedGrader.setup()
|
||||
|
||||
const currentlyShowing = document.querySelector('#section_currently_showing')
|
||||
strictEqual(currentlyShowing.innerText, 'The Second Section')
|
||||
|
||||
delete window.ENV.selected_section_id
|
||||
delete window.ENV.new_gradebook_enabled
|
||||
})
|
||||
|
||||
test('reloads SpeedGrader when the user selects a new section and ENV.settings_url is set', () => {
|
||||
SpeedGrader.EG.jsonReady()
|
||||
SpeedGrader.setup()
|
||||
|
||||
$(sectionSelectPath).click()
|
||||
strictEqual(SpeedGraderHelpers.reloadPage.callCount, 1)
|
||||
})
|
||||
|
||||
test('reloads SpeedGrader when the user selects a new section and ENV.settings_url is not set', () => {
|
||||
const settingsURL = window.ENV.settings_url
|
||||
delete window.ENV.settings_url
|
||||
|
||||
SpeedGrader.EG.jsonReady()
|
||||
SpeedGrader.setup()
|
||||
|
||||
$(sectionSelectPath).click()
|
||||
strictEqual(SpeedGraderHelpers.reloadPage.callCount, 1)
|
||||
|
||||
window.ENV.settings_url = settingsURL
|
||||
})
|
||||
|
||||
test('saves the selected section in userSettings when one is selected', () => {
|
||||
SpeedGrader.EG.jsonReady()
|
||||
SpeedGrader.setup()
|
||||
|
||||
$(sectionSelectPath).click()
|
||||
|
||||
const params = userSettings.contextSet.firstCall.args
|
||||
deepEqual(params, ['grading_show_only_section', 2])
|
||||
})
|
||||
|
||||
test('annuls the selected section in userSettings when "all sections" is selected', () => {
|
||||
SpeedGrader.EG.jsonReady()
|
||||
SpeedGrader.setup()
|
||||
|
||||
$(allSectionsSelectPath).click()
|
||||
|
||||
const [key] = userSettings.contextRemove.firstCall.args
|
||||
strictEqual(key, 'grading_show_only_section')
|
||||
})
|
||||
|
||||
test('posts the selected section to the settings URL when a specific section is selected', () => {
|
||||
SpeedGrader.EG.jsonReady()
|
||||
SpeedGrader.setup()
|
||||
|
||||
$(sectionSelectPath).click()
|
||||
|
||||
const [, params] = $.post.firstCall.args
|
||||
deepEqual(params, {selected_section_id: 2})
|
||||
})
|
||||
|
||||
test('posts the value "all" to the settings URL when "all sections" is selected', () => {
|
||||
SpeedGrader.EG.jsonReady()
|
||||
SpeedGrader.setup()
|
||||
|
||||
$(allSectionsSelectPath).click()
|
||||
|
||||
const [, params] = $.post.firstCall.args
|
||||
deepEqual(params, {selected_section_id: 'all'})
|
||||
})
|
||||
|
||||
QUnit.module('when a course loads with an empty section selected', emptySectionHooks => {
|
||||
emptySectionHooks.beforeEach(() => {
|
||||
userSettings.contextGet.returns('4')
|
||||
sandbox.stub(SpeedGrader.EG, 'changeToSection')
|
||||
sandbox.stub(window, 'alert')
|
||||
})
|
||||
|
||||
emptySectionHooks.afterEach(() => {
|
||||
window.alert.restore()
|
||||
SpeedGrader.EG.changeToSection.restore()
|
||||
})
|
||||
|
||||
test('displays an alert indicating the section has no students', () => {
|
||||
SpeedGrader.EG.jsonReady()
|
||||
SpeedGrader.setup()
|
||||
|
||||
const [message] = window.alert.firstCall.args
|
||||
strictEqual(message, "Could not find any students in that section, falling back to showing all sections.")
|
||||
})
|
||||
|
||||
test('calls changeToSection with the value "all"', () => {
|
||||
SpeedGrader.EG.jsonReady()
|
||||
SpeedGrader.setup()
|
||||
|
||||
const [sectionId] = SpeedGrader.EG.changeToSection.firstCall.args
|
||||
strictEqual(sectionId, 'all')
|
||||
})
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
|
|||
Loading…
Reference in New Issue