Skip to content

Commit

Permalink
Revert "grading period filtering respects overrides"
Browse files Browse the repository at this point in the history 
This reverts commit d9e77b0.

Change-Id: Ifd68ba161d4b6b9e9ece0d91ac9b64ceb85c5598
Reviewed-on: https://gerrit.instructure.com/65114
Reviewed-by: Cody Cutrer <[email protected]>
Product-Review: Cody Cutrer <[email protected]>
QA-Review: Cody Cutrer <[email protected]>
Tested-by: Cody Cutrer <[email protected]>
  • Loading branch information
@evizitei
evizitei committed Oct 13, 2015
1 parent 8c9612a commit 4817ad8
Show file tree
Hide file tree
Showing 14 changed files with 605 additions and 1,515 deletions.
92 changes: 14 additions & 78 deletions app/coffeescripts/gradebook2/Gradebook.coffee
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,9 @@ define [
@gotAllAssignmentGroupsPromise.then(()=> @gotAllStudents())
)

# this method should be removed after a month in production
@alignCoursePreferencesWithLocalStorage()

@assignmentGroupsParams = {exclude_descriptions: true}
if @mgpEnabled && @gradingPeriodToShow && @gradingPeriodToShow != '0' && @gradingPeriodToShow != ''
$.extend(@assignmentGroupsParams, {grading_period_id: @gradingPeriodToShow})
Expand Down Expand Up @@ -288,6 +291,7 @@ define [
assignment.assignment_group = group
assignment.due_at = tz.parse(assignment.due_at)
@assignments[assignment.id] = assignment

@postGradesStore.setGradeBookAssignments @assignments


Expand Down Expand Up @@ -353,9 +357,6 @@ define [
@withAllStudents (students) =>
#empty the array so this is idempotent, you can end up with too many rows
@rows.length = 0
if @mgpEnabled
gradingPeriods = @indexedGradingPeriods()
overrides = @indexedOverrides()
for student_id, student of students
student.computed_current_score ||= 0
student.computed_final_score ||= 0
Expand All @@ -374,10 +375,8 @@ define [
# fill in dummy submissions, so there's something there even if the
# student didn't submit anything for that assignment
for assignment_id, assignment of @assignments
student["assignment_#{assignment_id}"] ?= { assignment_id: assignment_id, user_id: student_id }
submission = student["assignment_#{assignment_id}"]
if @submissionOutsideOfGradingPeriod(submission, student, gradingPeriods, overrides)
submission.hidden = true
student["assignment_#{assignment_id}"] ||= { assignment_id: assignment_id, user_id: student_id }

@rows.push(student)

defaultSortType: 'assignment_group'
Expand Down Expand Up @@ -646,9 +645,6 @@ define [
activeCell = @grid.getActiveCell()
editing = $(@grid.getActiveCellNode()).hasClass('editable')
columns = @grid.getColumns()
if @mgpEnabled
gradingPeriods = @indexedGradingPeriods()
overrides = @indexedOverrides()
for submission in submissions
student = @student(submission.user_id)
idToMatch = "assignment_#{submission.assignment_id}"
Expand All @@ -659,7 +655,6 @@ define [
activeCell.cell is cell
#check for DA visible
submission["hidden"] = !submission.assignment_visible if submission.assignment_visible?
submission.hidden = true if @submissionOutsideOfGradingPeriod(submission, student, gradingPeriods, overrides)
@updateAssignmentVisibilities(submission) if submission["hidden"]
@updateSubmission(submission)
@calculateStudentGrade(student)
Expand Down Expand Up @@ -690,73 +685,6 @@ define [
else
(SubmissionCell[assignment.grading_type] || SubmissionCell).formatter(row, col, submission, assignment, @grid)


indexedOverrides: ->
indexed = { studentOverrides: {}, sectionOverrides: {} }
_.each @assignments, (assignment) ->
if assignment.has_overrides
_.each assignment.overrides, (override) ->
if override.student_ids
indexed.studentOverrides[assignment.id] ?= {}
_.each override.student_ids, (studentId) ->
indexed.studentOverrides[assignment.id][studentId] = override
else if sectionId = override.course_section_id
indexed.sectionOverrides[assignment.id] ?= {}
indexed.sectionOverrides[assignment.id][sectionId] = override
indexed

indexedGradingPeriods: ->
_.indexBy @gradingPeriods, 'id'

submissionOutsideOfGradingPeriod: (submission, student, gradingPeriods, overrides) ->
return false unless @mgpEnabled
selectedPeriodId = @gradingPeriodToShow
return false if @isAllGradingPeriods(selectedPeriodId)

assignment = @assignments[submission.assignment_id]
gradingPeriod = gradingPeriods[selectedPeriodId]
effectiveDueAt = assignment.due_at

if assignment.has_overrides
# we'll eventually need to consider group overrides here
# (group overrides are not yet a feature but are planned)
sectionOverrides = []
sectionOverridesOnAssignment = overrides.sectionOverrides[assignment.id]
if sectionOverridesOnAssignment
_.each student.sections, (sectionId) ->
sectionOverride = sectionOverridesOnAssignment[sectionId]
sectionOverrides.push sectionOverride if sectionOverride

studentOverrides = []
studentOverridesOnAssignment = overrides.studentOverrides[assignment.id]
if studentOverridesOnAssignment
studentOverride = studentOverridesOnAssignment[student.id]
studentOverrides.push studentOverride if studentOverride

allOverridesForSubmission = sectionOverrides.concat studentOverrides
overrideDates = _.chain(allOverridesForSubmission)
.pluck('due_at')
.map((dateString) -> tz.parse(dateString))
.value()

if overrideDates.length > 0
nullDueAtsExist = _.any(overrideDates, (date) -> _.isNull(date))
effectiveDueAt = if nullDueAtsExist then null else _.max(overrideDates)
else
return true if assignment.only_visible_to_overrides

showSubmission = @lastGradingPeriodAndDueAtNull(gradingPeriod, effectiveDueAt) || @dateIsInGradingPeriod(gradingPeriod, effectiveDueAt)
!showSubmission

lastGradingPeriodAndDueAtNull: (gradingPeriod, dueAt) ->
gradingPeriod.is_last && _.isNull(dueAt)

dateIsInGradingPeriod: (gradingPeriod, date) ->
return false if _.isNull(date)
startDate = tz.parse(gradingPeriod.start_date)
endDate = tz.parse(gradingPeriod.end_date)
startDate < date && date <= endDate

staticCellFormatter: (row, col, val) ->
"<div class='cell-content gradebook-cell'>#{htmlEscape(val)}</div>"

Expand Down Expand Up @@ -1664,3 +1592,11 @@ define [

isAllGradingPeriods: (currentPeriodId) ->
currentPeriodId == "0"

# this method should be removed after a month in production
alignCoursePreferencesWithLocalStorage: () ->
local_storage_show_point_totals = userSettings.contextGet('show_point_totals')
if local_storage_show_point_totals and local_storage_show_point_totals != @options.show_total_grade_as_points
@options.show_total_grade_as_points = local_storage_show_point_totals
userSettings.contextRemove('show_point_totals')
$.ajaxJSON @options.setting_update_url, "PUT", show_total_grade_as_points: @options.show_total_grade_as_points
85 changes: 38 additions & 47 deletions app/controllers/assignment_groups_controller.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -234,26 +234,10 @@ def assignment_includes
includes = [:context, :external_tool_tag, {:quiz => :context}]
includes += [:rubric, :rubric_association] unless params[:exclude_rubrics]
includes << :discussion_topic if include_params.include?("discussion_topic")
includes << :assignment_overrides if include_overrides?
includes << :assignment_overrides if override_dates? || include_params.include?('all_dates') || include_params.include?('overrides')
includes
end

def filter_by_grading_period?
return false if all_grading_periods_selected?
params[:grading_period_id].present? && multiple_grading_periods?
end

def all_grading_periods_selected?
params[:grading_period_id] == '0'
end

def include_overrides?
override_dates? ||
include_params.include?('all_dates') ||
include_params.include?('overrides') ||
filter_by_grading_period?
end

def assignment_visibilities(course, assignments)
if include_visibility? && differentiated_assignments?
AssignmentStudentVisibility.users_with_visibility_by_assignment(
Expand Down Expand Up @@ -321,41 +305,48 @@ def user_content_attachments(assignments, context)
end

def visible_assignments(context, current_user, groups)
return Assignment.none unless include_params.include?('assignments')
# TODO: possible keyword arguments refactor
assignments = AssignmentGroup.visible_assignments(
current_user,
context,
groups,
assignment_includes
).with_student_submission_count

if params[:grading_period_id].present? && multiple_grading_periods?
grading_period = GradingPeriod.context_find(
if include_params.include?('assignments')
# TODO: possible keyword arguments refactor
assignments = AssignmentGroup.visible_assignments(
current_user,
context,
params.fetch(:grading_period_id)
)
groups,
assignment_includes
).with_student_submission_count

if params[:grading_period_id].present? && multiple_grading_periods?
grading_period = GradingPeriod.context_find(
context,
params.fetch(:grading_period_id)
)

assignments = Assignment::FilterWithOverridesByDueAt.new(
assignments: assignments,
grading_period: grading_period,
differentiated_assignments: differentiated_assignments?
).filter_assignments
end

assignments = grading_period.assignments(assignments) if grading_period
end
# because of a bug with including content_tags, we are preloading
# here rather than in assignments with multiple associations
# referencing content_tags table and therefore aliased table names
# the conditions on has_many :context_module_tags will break
if include_params.include?("module_ids") || !context.grants_right?(@current_user, session, :read_as_admin)
# loading the context module information here will improve performance for `locked_json` immensely
Assignment.preload_context_module_tags(assignments)
end

# because of a bug with including content_tags, we are preloading
# here rather than in assignments with multiple associations
# referencing content_tags table and therefore aliased table names
# the conditions on has_many :context_module_tags will break
if include_params.include?("module_ids") || !context.grants_right?(@current_user, session, :read_as_admin)
# loading the context module information here will improve performance for `locked_json` immensely
Assignment.preload_context_module_tags(assignments)
end
if AssignmentOverrideApplicator.should_preload_override_students?(assignments, @current_user, "assignment_groups_api")
AssignmentOverrideApplicator.preload_assignment_override_students(assignments, @current_user)
end

if AssignmentOverrideApplicator.should_preload_override_students?(assignments, @current_user, "assignment_groups_api")
AssignmentOverrideApplicator.preload_assignment_override_students(assignments, @current_user)
end
if assignment_includes.include?(:assignment_overrides)
assignments.each { |a| a.has_no_overrides = true if a.assignment_overrides.size == 0 }
end

if assignment_includes.include?(:assignment_overrides)
assignments.each { |a| a.has_no_overrides = true if a.assignment_overrides.size == 0 }
assignments
else
Assignment.none
end

assignments
end
end
16 changes: 4 additions & 12 deletions app/controllers/gradebooks_controller.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,27 +89,19 @@ def grade_summary

def light_weight_ags_json(assignment_groups, opts={})
assignment_groups.map do |ag|
visible_assignments = ag.visible_assignments(opts[:student] || @current_user)

if multiple_grading_periods? && @current_grading_period_id && !view_all_grading_periods?
current_period = GradingPeriod.context_find(@context, @current_grading_period_id)
visible_assignments = current_period.assignments_for_student(visible_assignments, opts[:student])
end

visible_assignments.map! do |a|
assignments = ag.visible_assignments(opts[:student] || @current_user).map do |a|
{
:id => a.id,
:submission_types => a.submission_types_array,
:points_possible => a.points_possible,
:due_at => a.due_at
}
end

{
:id => ag.id,
:rules => ag.rules_hash({stringify_json_ids: true}),
:group_weight => ag.group_weight,
:assignments => visible_assignments,
:assignments => assignments,
}
end
end
Expand Down Expand Up @@ -234,8 +226,8 @@ def external_tool_url_for_lti2(launch_definition)
def set_current_grading_period
unless @current_grading_period_id = params[:grading_period_id].presence
return if view_all_grading_periods?
current = GradingPeriod.for(@context).find(&:current?)
@current_grading_period_id = current ? current.id.to_s : '0'
return unless current = GradingPeriod.for(@context).find(&:current?)
@current_grading_period_id = current.id.to_s
end
end

Expand Down
Loading

0 comments on commit 4817ad8

Please sign in to comment.