ylqjgm
/
canvas-lms
mirror of https://github.com/instructure/canvas-lms.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

sort assignments on course grades page 

allow sorting by assignment group, due date, module,
and title on the course grades page. also, store
the user's sort preferences and remember that
preference on subsequent page loads.

closes CNVS-21660

test plan:
  - as a student, go to the /courses/:course_id/grades
    page.
  - verify there is a dropdown to sort assignments by
    due date and title. the first time you visit this
    page, the dropdown option should default to
    ordering by due date. verify each option correctly
    sorts the assignments.
  - if the course has any Modules, there should be an
    option to sort by Module. if the course does not
    have any Modules, there should not be a Module sort
    option.
  - if the course has assignments that belong to different
    Assignment Groups, there should be an option to sort
    by Assignment Group. if the course does not have any
    assignments, or if all of the course's assignments
    belong to the same Assignment Group, there should not
    be an Assignment Group sort option.
  - select any option besides 'Due Date'. next, leave the
    page and return back to the page. verify the selected
    dropdown option matches the option you selected before
    leaving the page.
  - repeat the steps above while logged in as a teacher, TA,
    student view student, and admin. when logged in as a teacher,
    TA, or admin, the url will be /courses/:course_id/grades/
    :student_id
  - verify the dropdown is accessible
  - note: if a student is enrolled in multiple courses, the
    dropdown for the sort order should be on the right-hand
    side of the /courses/:course_id/grades page (and the
    dropdown for the course selection should be on the
    left-hand side). if a student is only enrolled in
    one course, the dropdown for sort order should be on
    the left-hand side of the page.

Change-Id: Idbcbea2d25051cb5d933bfb395daebeedc630855
Reviewed-on: https://gerrit.instructure.com/69419
Reviewed-by: Derek Bender <djbender@instructure.com>
Tested-by: Jenkins
QA-Review: KC Naegle <knaegle@instructure.com>
Product-Review: Spencer Olson <solson@instructure.com>
This commit is contained in:
Spencer Olson 2015-12-21 22:35:21 -06:00
parent e8ed984332
commit d1df06cf72
8 changed files with 415 additions and 55 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
app/controllers/gradebooks_controller.rb
View File

@ -35,7 +35,7 @@ class GradebooksController < ApplicationController
MAX_POST_GRADES_TOOLS = 10
def grade_summary
@presenter = GradeSummaryPresenter.new(@context, @current_user, params[:id])
@presenter = GradeSummaryPresenter.new(@context, @current_user, params[:id], presenter_options)
# do this as the very first thing, if the current user is a teacher in the course and they are not trying to view another user's grades, redirect them to the gradebook
if @presenter.user_needs_redirection?
return redirect_to polymorphic_url([@context, 'gradebook'])
@ -60,7 +60,7 @@ class GradebooksController < ApplicationController
@exclude_total = exclude_total?(@context)
Shackles.activate(:slave) do
#run these queries on the slave database for speed
@presenter.assignments(gp_id)
@presenter.assignments(grading_period_id: gp_id)
@presenter.groups_assignments = groups_as_assignments(
@presenter.groups,
:out_of_final => true,
@ -92,6 +92,20 @@ class GradebooksController < ApplicationController
end
end
def save_assignment_order
if authorized_action(@context, @current_user, :read)
whitelisted_orders = {
'due_at' => :due_at, 'title' => :title,
'module' => :module, 'assignment_group' => :assignment_group
}
assignment_order = whitelisted_orders.fetch(params.fetch(:assignment_order), :due_at)
@current_user.preferences[:course_grades_assignment_order] ||= {}
@current_user.preferences[:course_grades_assignment_order][@context.id] = assignment_order
@current_user.save!
redirect_to :back
end
end
def light_weight_ags_json(assignment_groups, opts={})
assignment_groups.map do |ag|
visible_assignments = ag.visible_assignments(opts[:student] || @current_user)
@ -691,4 +705,10 @@ class GradebooksController < ApplicationController
Canvadocs.annotations_supported? &&
@assignment.submission_types.include?('online_upload')
end
def presenter_options
order_preferences = @current_user.preferences[:course_grades_assignment_order]
saved_order = order_preferences && order_preferences[@context.id]
saved_order ? { assignment_order: saved_order } : {}
end
end

11
app/models/assignment.rb
View File

@ -835,9 +835,16 @@ class Assignment < ActiveRecord::Base
read_attribute(:all_day) || (self.new_record? && self.due_at && (self.due_at.strftime("%H:%M") == '23:59' || self.due_at.strftime("%H:%M") == '00:00'))
end
def self.preload_context_module_tags(assignments)
def self.preload_context_module_tags(assignments, include_context_modules: false)
module_tags_include =
if include_context_modules
{ context_module_tags: :context_module }
else
:context_module_tags
end
ActiveRecord::Associations::Preloader.new.preload(assignments, [
:context_module_tags,
module_tags_include,
{ :discussion_topic => :context_module_tags },
{ :quiz => :context_module_tags }
])

104
app/presenters/grade_summary_presenter.rb
View File

@ -18,13 +18,14 @@
class GradeSummaryPresenter
attr_reader :groups_assignments
attr_reader :groups_assignments, :assignment_order
def initialize(context, current_user, id_param)
def initialize(context, current_user, id_param, assignment_order: :due_at)
@context = context
@current_user = current_user
@id_param = id_param
@groups_assignments = []
@assignment_order = assignment_order
end
def user_has_elevated_permissions?
@ -114,23 +115,29 @@ class GradeSummaryPresenter
@groups ||= @context.assignment_groups.active.to_a
end
def assignments(gp_id = nil)
def assignments(grading_period_id: nil)
@assignments ||= begin
visible_assignments = AssignmentGroup.visible_assignments(student, @context, groups, [:assignment_overrides])
if gp_id
visible_assignments = grading_period_assignments(gp_id, visible_assignments)
end
group_index = groups.index_by(&:id)
visible_assignments.select { |a| a.submission_types != 'not_graded'}.map { |a|
# prevent extra loads
a.context = @context
a.assignment_group = group_index[a.assignment_group_id]
a.overridden_for(student)
}.sort
visible_assignments = assignments_visible_to_student(grading_period_id)
overridden_assignments = assignments_overridden_for_student(visible_assignments)
sorted_assignments(overridden_assignments)
end
end
def assignments_visible_to_student(grading_period_id)
includes = [:assignment_overrides]
includes << :assignment_group if @assignment_order == :assignment_group
visible_assignments = AssignmentGroup
.visible_assignments(student, @context, groups, includes)
.where.not(submission_types: 'not_graded')
.except(:order)
if grading_period_id
visible_assignments = grading_period_assignments(grading_period_id, visible_assignments)
end
visible_assignments
end
def grading_period_assignments(grading_period_id, assignments)
grading_period = GradingPeriod.context_find(@context, grading_period_id)
if grading_period
@ -140,6 +147,37 @@ class GradeSummaryPresenter
end
end
def assignments_overridden_for_student(assignments)
group_index = groups.index_by(&:id)
assignments.map do |assignment|
assignment.context = @context
assignment.assignment_group = group_index.fetch(assignment.assignment_group_id)
assignment.overridden_for(student)
end
end
def sorted_assignments(assignments)
case @assignment_order
when :due_at
assignments.sort_by { |a| [a.due_at || CanvasSort::Last, a.title.downcase] }
when :title
assignments.sort_by { |a| a.title.downcase }
when :module
sorted_by_modules(assignments)
when :assignment_group
assignments.sort_by { |a| [a.assignment_group.position, a.position] }
end
end
def sort_options
options = [["Due Date", "due_at"], ["Title", "title"]]
if @context.active_record_types[:assignments] && assignments.uniq(&:assignment_group_id).length > 1
options << ["Assignment Group", "assignment_group"]
end
options << ["Module", "module"] if @context.active_record_types[:modules]
options.map { |option| [I18n.t('%{option_name}', option_name: option.first), option.last] }.sort_by(&:first)
end
def submissions
@submissions ||= begin
ss = @context.submissions
@ -202,9 +240,9 @@ class GradeSummaryPresenter
def assignment_presenters
submission_index = submissions.index_by(&:assignment_id)
assignments.map{ |a|
assignments.map do |a|
GradeSummaryAssignmentPresenter.new(self, @current_user, a, submission_index[a.id])
}
end
end
def has_muted_assignments?
@ -252,4 +290,36 @@ class GradeSummaryPresenter
@groups_assignments = value
assignments.concat(value)
end
private
def sorted_by_modules(assignments)
Assignment.preload_context_module_tags(assignments, include_context_modules: true)
assignments.sort do |a, b|
a_tags = a.context_module_tags
b_tags = b.context_module_tags
# assignments without modules come after assignments with modules
next -1 if a_tags.present? && b_tags.empty?
next 1 if a_tags.empty? && b_tags.present?
# if both assignments do not belong to a module, compare by
# assignment position
next a.position <=> b.position if a_tags.empty? && b_tags.empty?
# if both assignments belong to modules, compare the module
# position of the first module they each belong to
compare_by_module_position(a_tags.first, b_tags.first)
end
end
def compare_by_module_position(module_tag1, module_tag2)
module_position_comparison =
module_tag1.context_module.position <=> module_tag2.context_module.position
# if module position above is the same, compare by assignment
# position within the module
if module_position_comparison.zero?
module_tag1.position <=> module_tag2.position
else
module_position_comparison
end
end
end

44
app/views/gradebooks/grade_summary.html.erb
View File

@ -67,7 +67,7 @@
</div>
<h2>
<% ot('headers.grades', "Grades For %{student}", :student => capture { %>
<% ot('headers.grades', "Grades for %{student}", :student => capture { %>
<% unless @presenter.multiple_observed_students? %>
<%= @presenter.student_name %>
<% else %>
@ -81,25 +81,33 @@
<% }) %>
</h2>
<% if @presenter.has_courses_with_grades? %>
<div id="course-selector-dropdown" style="margin-left: 10px;" class="course_selector">
<%= t(:for_course, "For the course,") %>
<select id="course_url">
<% @presenter.selectable_courses.each do |course| %>
<%
url = if course.grants_any_right?(@current_user, :manage_grades, :view_all_grades)
context_url(course, :context_student_grades_url, @presenter.student_id)
else
context_url(course, :context_grades_url)
end
%>
<option value="<%= url %>" <%= 'selected' if course == @context %>><%= course.nickname_for(@current_user) %></option>
<div class="dropdowns">
<% if @presenter.has_courses_with_grades? %>
<div id="course-selector-dropdown" style="margin-left: 10px; float: left;" class="course_selector">
<label for="course_url"><%= t(:for_course, "For the course") %></label>
<select id="course_url">
<% @presenter.selectable_courses.each do |course| %>
<%
url = if course.grants_any_right?(@current_user, :manage_grades, :view_all_grades)
context_url(course, :context_student_grades_url, @presenter.student_id)
else
context_url(course, :context_grades_url)
end
%>
<option value="<%= url %>" <%= 'selected' if course == @context %>><%= course.nickname_for(@current_user) %></option>
<% end %>
</select>
</div>
<% js_bundle 'legacy/gradebooks_grade_summary' %>
<% end %>
<div class="assignment_order" style=<%= @presenter.has_courses_with_grades? ? "margin-right:10px;float:right;" : "margin-left:10px;float:left;" %>>
<%= form_tag context_url(@context, :context_save_assignment_order_url), style: 'margin-bottom: 0;' do %>
<label for="assignment_order"><%= t("Arrange by") %></label>
<%= select_tag "assignment_order", options_for_select(@presenter.sort_options, selected: @presenter.assignment_order) %>
<% end %>
</select>
</div>
<% js_bundle 'legacy/gradebooks_grade_summary' %>
<% end %>
</h2>
</div>
<% if @context.feature_enabled?(:student_outcome_gradebook) %>
<ul id="navpills">

1
config/routes.rb
View File

@ -228,6 +228,7 @@ CanvasRails::Application.routes.draw do
get 'grades' => 'gradebooks#grade_summary', id: nil
get 'grading_rubrics' => 'gradebooks#grading_rubrics'
get 'grades/:id' => 'gradebooks#grade_summary', as: :student_grades
post 'save_assignment_order' => 'gradebooks#save_assignment_order', as: :save_assignment_order
concerns :announcements
get 'calendar' => 'calendars#show2', as: :old_calendar
get :locks

5
public/javascripts/grade_summary.js
View File

@ -345,6 +345,10 @@ define([
}
});
$("#assignment_order").change(function() {
this.form.submit();
});
$("#show_all_details_link").click(function(event) {
event.preventDefault();
$button = $('#show_all_details_link');
@ -380,7 +384,6 @@ define([
}
}
$(document).on('change', '.grading_periods_selector', function(e){
var newGP = $(this).val();
if (matches = location.href.match(/grading_period_id=\d*/)) {

104
spec/controllers/gradebooks_controller_spec.rb
View File

@ -193,15 +193,94 @@ describe GradebooksController do
]
end
it "sorts assignments by due date (null last), then title" do
user_session(@teacher)
assignment1 = @course.assignments.create(:title => "Assignment 1")
assignment2 = @course.assignments.create(:title => "Assignment 2", :due_at => 3.days.from_now)
assignment3 = @course.assignments.create(:title => "Assignment 3", :due_at => 2.days.from_now)
context "assignment sorting" do
let!(:teacher_session) { user_session(@teacher) }
let!(:assignment1) { @course.assignments.create(title: "Banana", position: 2) }
let!(:assignment2) { @course.assignments.create(title: "Apple", due_at: 3.days.from_now, position: 3) }
let!(:assignment3) do
assignment_group = @course.assignment_groups.create!(position: 2)
@course.assignments.create!(
assignment_group: assignment_group, title: "Carrot", due_at: 2.days.from_now, position: 1
)
end
let(:assignment_ids) { assigns[:presenter].assignments.select{ |a| a.class == Assignment }.map(&:id) }
get 'grade_summary', :course_id => @course.id, :id => @student.id
assignment_ids = assigns[:presenter].assignments.select{|a| a.class == Assignment}.map(&:id)
expect(assignment_ids).to eq [assignment3, assignment2, assignment1].map(&:id)
it "sorts assignments by due date (null last), then title if there is no saved order preference" do
get 'grade_summary', course_id: @course.id, id: @student.id
expect(assignment_ids).to eq [assignment3, assignment2, assignment1].map(&:id)
end
it "sort order of 'due_at' sorts by due date (null last), then title" do
@teacher.preferences[:course_grades_assignment_order] = { @course.id => :due_at }
@teacher.save!
get 'grade_summary', course_id: @course.id, id: @student.id
expect(assignment_ids).to eq [assignment3, assignment2, assignment1].map(&:id)
end
context "sort by: title" do
let!(:teacher_setup) do
@teacher.preferences[:course_grades_assignment_order] = { @course.id => :title }
@teacher.save!
end
it "sorts assignments by title" do
get 'grade_summary', course_id: @course.id, id: @student.id
expect(assignment_ids).to eq [assignment2, assignment1, assignment3].map(&:id)
end
it "ingores case" do
assignment1.title = 'banana'
assignment1.save!
get 'grade_summary', course_id: @course.id, id: @student.id
expect(assignment_ids).to eq [assignment2, assignment1, assignment3].map(&:id)
end
end
it "sort order of 'assignment_group' sorts by assignment group position, then assignment position" do
@teacher.preferences[:course_grades_assignment_order] = { @course.id => :assignment_group }
@teacher.save!
get 'grade_summary', course_id: @course.id, id: @student.id
expect(assignment_ids).to eq [assignment1, assignment2, assignment3].map(&:id)
end
context "sort by: module" do
let!(:first_context_module) { @course.context_modules.create! }
let!(:second_context_module) { @course.context_modules.create! }
let!(:assignment1_tag) do
a1_tag = assignment1.context_module_tags.new(context: @course, position: 1, tag_type: 'context_module')
a1_tag.context_module = second_context_module
a1_tag.save!
end
let!(:assignment2_tag) do
a2_tag = assignment2.context_module_tags.new(context: @course, position: 3, tag_type: 'context_module')
a2_tag.context_module = first_context_module
a2_tag.save!
end
let!(:assignment3_tag) do
a3_tag = assignment3.context_module_tags.new(context: @course, position: 2, tag_type: 'context_module')
a3_tag.context_module = first_context_module
a3_tag.save!
end
let!(:teacher_setup) do
@teacher.preferences[:course_grades_assignment_order] = { @course.id => :module }
@teacher.save!
end
it "sorts by module position, then context module tag position" do
get 'grade_summary', course_id: @course.id, id: @student.id
expect(assignment_ids).to eq [assignment3, assignment2, assignment1].map(&:id)
end
it "sorts by module position, then context module tag position, " \
"with those not belonging to a module sorted last" do
assignment3.context_module_tags.first.destroy!
get 'grade_summary', course_id: @course.id, id: @student.id
expect(assignment_ids).to eq [assignment2, assignment1, assignment3].map(&:id)
end
end
end
context "Multiple Grading Periods" do
@ -643,6 +722,15 @@ describe GradebooksController do
end
end
describe "POST 'save_assignment_order'" do
it "saves the sort order in the user's preferences" do
user_session(@teacher)
post 'save_assignment_order', course_id: @course.id, assignment_order: 'due_at'
saved_order = @teacher.preferences[:course_grades_assignment_order][@course.id]
expect(saved_order).to eq(:due_at)
end
end
describe '#light_weight_ags_json' do
it 'returns the necessary JSON for GradeCalculator' do
ag = @course.assignment_groups.create! group_weight: 100

177
spec/presenters/grade_summary_presenter_spec.rb
View File

@ -153,9 +153,12 @@ describe GradeSummaryPresenter do
end
describe '#submissions' do
it "doesn't return submissions for deleted assignments" do
before(:once) do
teacher_in_course
student_in_course
end
it "doesn't return submissions for deleted assignments" do
a1, a2 = 2.times.map {
@course.assignments.create! points_possible: 10
}
@ -169,8 +172,6 @@ describe GradeSummaryPresenter do
end
it "doesn't error on submissions for assignments not in the pre-loaded assignment list" do
teacher_in_course
student_in_course
assign = @course.assignments.create! points_possible: 10
assign.grade_student @student, grade: 10
assign.update_attribute(:submission_types, "not_graded")
@ -181,15 +182,177 @@ describe GradeSummaryPresenter do
end
describe '#assignments' do
it "filters unpublished assignments" do
before(:once) do
teacher_in_course
student_in_course
published_assignment = @course.assignments.create!
unpublished_assign = @course.assignments.create!
unpublished_assign.update_attribute(:workflow_state, "unpublished")
end
let!(:published_assignment) { @course.assignments.create! }
it "filters unpublished assignments" do
unpublished_assignment = @course.assignments.create!
unpublished_assignment.update_attribute(:workflow_state, "unpublished")
p = GradeSummaryPresenter.new(@course, @teacher, @student.id)
expect(p.assignments).to eq [published_assignment]
end
end
describe '#sort_options' do
before(:once) do
teacher_in_course
student_in_course
end
let(:presenter) { GradeSummaryPresenter.new(@course, @teacher, @student.id) }
let(:assignment_group_option) { ["Assignment Group", "assignment_group"] }
let(:module_option) { ["Module", "module"] }
it "returns the default sort options" do
default_options = [["Due Date", "due_at"], ["Title", "title"]]
expect(presenter.sort_options).to include(*default_options)
end
it "does not return 'Assignment Group' as an option if the course has no assignments" do
expect(presenter.sort_options).to_not include assignment_group_option
end
it "does not return 'Assignment Group' as an option if all of the " \
"assignments belong to the same assignment group" do
@course.assignments.create!(title: "Math Assignment")
@course.assignments.create!(title: "Science Assignment")
expect(presenter.sort_options).to_not include assignment_group_option
end
it "returns 'Assignment Group' as an option if there are " \
"assignments that belong to different assignment groups" do
@course.assignments.create!(title: "Math Assignment")
science_group = @course.assignment_groups.create!(title: "Science Assignments")
@course.assignments.create!(title: "Science Assignment", assignment_group: science_group)
expect(presenter.sort_options).to include assignment_group_option
end
it "does not return 'Module' as an option if the course does not have any modules" do
expect(presenter.sort_options).to_not include module_option
end
it "returns 'Module' as an option if the course has any modules" do
@course.context_modules.create!(name: "I <3 Modules")
expect(presenter.sort_options).to include module_option
end
end
describe '#sorted_assignments' do
before(:once) do
teacher_in_course
student_in_course
end
let!(:assignment1) { @course.assignments.create!(title: 'Apple', due_at: 2.days.ago, position: 1) }
let!(:assignment2) { @course.assignments.create!(title: 'Banana', due_at: 2.days.from_now, position: 2) }
let!(:assignment3) { @course.assignments.create!(title: 'Carrot', due_at: 5.days.ago, position: 3) }
let(:ordered_assignment_ids) { presenter.assignments.map(&:id) }
it "assignment order defaults to due_at" do
presenter = GradeSummaryPresenter.new(@course, @teacher, @student.id)
expect(presenter.assignment_order).to eq(:due_at)
end
context "assignment order: due_at" do
let(:presenter) { GradeSummaryPresenter.new(@course, @teacher, @student.id, assignment_order: :due_at) }
it "sorts by due_at" do
expected_id_order = [assignment3.id, assignment1.id, assignment2.id]
expect(ordered_assignment_ids).to eq(expected_id_order)
end
it "sorts assignments without due_ats after assignments with due_ats" do
assignment1.due_at = nil
assignment1.save!
expected_id_order = [assignment3.id, assignment2.id, assignment1.id]
expect(ordered_assignment_ids).to eq(expected_id_order)
end
it "sorts by assignment title if due_ats are equal" do
assignment1.due_at = assignment3.due_at
assignment1.save!
expected_id_order = [assignment1.id, assignment3.id, assignment2.id]
expect(ordered_assignment_ids).to eq(expected_id_order)
end
it "ignores case when comparing assignment titles" do
assignment1.due_at = assignment3.due_at
assignment1.title = 'apple'
assignment1.save!
expected_id_order = [assignment1.id, assignment3.id, assignment2.id]
expect(ordered_assignment_ids).to eq(expected_id_order)
end
end
context "assignment order: title" do
let(:presenter) { GradeSummaryPresenter.new(@course, @teacher, @student.id, assignment_order: :title) }
it "sorts by title" do
expected_id_order = [assignment1.id, assignment2.id, assignment3.id]
expect(ordered_assignment_ids).to eq(expected_id_order)
end
it "ignores case when sorting by title" do
assignment1.title = 'apple'
assignment1.save!
expected_id_order = [assignment1.id, assignment2.id, assignment3.id]
expect(ordered_assignment_ids).to eq(expected_id_order)
end
end
context "assignment order: module" do
let(:presenter) { GradeSummaryPresenter.new(@course, @teacher, @student.id, assignment_order: :module) }
let!(:first_context_module) { @course.context_modules.create! }
let!(:second_context_module) { @course.context_modules.create! }
let!(:assignment1_tag) do
a1_tag = assignment1.context_module_tags.new(context: @course, position: 1, tag_type: 'context_module')
a1_tag.context_module = second_context_module
a1_tag.save!
end
let!(:assignment2_tag) do
a2_tag = assignment2.context_module_tags.new(context: @course, position: 3, tag_type: 'context_module')
a2_tag.context_module = first_context_module
a2_tag.save!
end
let!(:assignment3_tag) do
a3_tag = assignment3.context_module_tags.new(context: @course, position: 2, tag_type: 'context_module')
a3_tag.context_module = first_context_module
a3_tag.save!
end
it "sorts by module position, then context module tag position" do
expected_id_order = [assignment3.id, assignment2.id, assignment1.id]
expect(ordered_assignment_ids).to eq(expected_id_order)
end
it "sorts by module position, then context module tag position, " \
"with those not belonging to a module sorted last" do
assignment3.context_module_tags.first.destroy!
expected_id_order = [assignment2.id, assignment1.id, assignment3.id]
expect(ordered_assignment_ids).to eq(expected_id_order)
end
end
context "assignment order: assignment_group" do
let(:presenter) do
GradeSummaryPresenter.new(@course, @teacher, @student.id, assignment_order: :assignment_group)
end
it "sorts by assignment group position then assignment position" do
new_assignment_group = @course.assignment_groups.create!(position: 2)
assignment4 = @course.assignments.create!(
assignment_group: new_assignment_group, title: "Dog", position: 1
)
expected_id_order = [assignment1.id, assignment2.id, assignment3.id, assignment4.id]
expect(ordered_assignment_ids).to eq(expected_id_order)
end
end
end
end