Skip to content

Commit

Permalink
Add "Mastery Paths" AssignmentOverride
Browse files Browse the repository at this point in the history
refs CYOE-310

Implement Noop AssignmentOverride set_type to persist a setless "tag"
Add MasteryPaths noop override to assignment, discussion, quiz edit
Add only_visible_to_overrides to Assignment copied attributes

Test Plan
1. Edit an assignment. From the "Assign to" dropdown select "Mastery
   Paths."
2. Ensure "Everyone Else" override is removed from Assign to.
3. After saving, ensure the assignment is no longer visible to students.
4. Ensure adding other overrides (section, student) alongside MP allow
   visibility as expected.
5. Ensure MP override is available and works as expected for other
   "assignment types" including quiz and discussion.
6. Ensure when an assignment with the MP override is copied or imported,
   its MP designation remains intact. (But any individual or section
   overrides do not)
6. With the Mastery Paths feature flag disabled, the option should not
   appear in the dropdown. If the flag is turned off, the MP override
   should disappear but the assignment should stay invisible to students
   until "Everyone" is added back again. If the flag is turned back on,
   MP should reappear.

Change-Id: I17ac8345f872c111d073886c23bd7a3dd9b5dbce
Reviewed-on: https://gerrit.instructure.com/91833
Tested-by: Jenkins
Reviewed-by: Michael Brewer-Davis <[email protected]>
QA-Review: Jahnavi Yetukuri <[email protected]>
Product-Review: Matt Goodwin <[email protected]>
  • Loading branch information
Christian Prescott committed Oct 17, 2016
1 parent 08c623a commit a978231
Show file tree
Hide file tree
Showing 32 changed files with 395 additions and 124 deletions.
7 changes: 6 additions & 1 deletion app/coffeescripts/models/AssignmentOverride.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ define [
'underscore'
'jquery'
'compiled/models/Section'
], (Backbone, _, $, Section) ->
'i18n!assignments',
], (Backbone, _, $, Section, I18n) ->

class AssignmentOverride extends Backbone.Model

Expand All @@ -19,6 +20,10 @@ define [
lock_at_overridden: true
lock_at: null

@conditionalRelease:
name: I18n.t("Mastery Paths")
noop_id: '1'

initialize: ->
super
@on 'change:course_section_id', @clearID, this
Expand Down
1 change: 1 addition & 0 deletions app/controllers/assignment_overrides_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,7 @@ def require_section
def require_course
@course ||= api_find(Course.active, params[:course_id])
raise ActiveRecord::RecordNotFound if @course.deleted?
@context = @course
authorized_action(@course, @current_user, :read)
end

Expand Down
17 changes: 15 additions & 2 deletions app/jsx/due_dates/DueDateRow.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,14 @@ define([
// 1 group override => 1 token for the group

tokenizedOverrides(){
var {sectionOverrides, groupOverrides, adhocOverrides} = _.groupBy(this.props.overrides,
var {sectionOverrides, groupOverrides, adhocOverrides, noopOverrides} = _.groupBy(this.props.overrides,
(ov) => {
if (ov.get("course_section_id")) {
return "sectionOverrides"
} else if (ov.get("group_id")) {
return "groupOverrides"
} else if (ov.get("noop_id")) {
return "noopOverrides"
} else {
return "adhocOverrides"
}
Expand All @@ -54,7 +56,8 @@ define([
return _.union(
this.tokenizedSections(sectionOverrides),
this.tokenizedGroups(groupOverrides),
this.tokenizedAdhoc(adhocOverrides)
this.tokenizedAdhoc(adhocOverrides),
this.tokenizedNoop(noopOverrides)
)
},

Expand Down Expand Up @@ -88,6 +91,16 @@ define([
}, [])
},

tokenizedNoop(noopOverrides){
var noopOverrides = noopOverrides || []
return _.map(noopOverrides, (override) => {
return {
noop_id: override.get("noop_id"),
name: override.get("title")
}
})
},

tokenFromStudentId(studentId){
return {
type: "student",
Expand Down
48 changes: 30 additions & 18 deletions app/jsx/due_dates/DueDateTokenWrapper.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,12 @@ define([
'react',
'react-modal',
'jsx/due_dates/OverrideStudentStore',
'compiled/models/AssignmentOverride',
'bower/react-tokeninput/dist/react-tokeninput',
'i18n!assignments',
'jquery',
'jsx/shared/helpers/searchHelpers'
], (_ ,React, ReactModal, OverrideStudentStore, TokenInput, I18n, $, SearchHelpers) => {
], (_ ,React, ReactModal, OverrideStudentStore, Override, TokenInput, I18n, $, SearchHelpers) => {

var ComboboxOption = TokenInput.Option;
TokenInput = TokenInput.default;
Expand Down Expand Up @@ -87,14 +88,13 @@ define([
}
},

handleTokenAdd(value) {
var token = this.findMatchingOption(value)
handleTokenAdd(name, option) {
var token = this.findMatchingOption(name, option)
this.props.handleTokenAdd(token)
this.clearUserInput()
},

handleTokenRemove(value) {
var token = this.findMatchingOption(value)
handleTokenRemove(token) {
this.props.handleTokenRemove(token)
},

Expand All @@ -113,9 +113,14 @@ define([
// Helpers
// -------------------

findMatchingOption(userInput){
if(typeof userInput !== 'string') { return userInput }
return this.findBestMatch(userInput)
findMatchingOption(name, option){
if(option){
// Selection was made from dropdown, find by unique attributes
return _.findWhere(this.props.potentialOptions, option.props.set_props)
} else {
// Search for best matching name
return this.sortedMatches(name)[0]
}
},

sortedMatches(userInput){
Expand All @@ -129,12 +134,6 @@ define([
);
},

findBestMatch(userInput){
return _.find(this.props.potentialOptions, (item) => SearchHelpers.exactMatchRegex(userInput).test(item.name)) ||
_.find(this.props.potentialOptions, (item) => SearchHelpers.startOfStringRegex(userInput).test(item.name)) ||
_.find(this.props.potentialOptions, (item) => SearchHelpers.substringMatchRegex(userInput).test(item.name))
},

filteredTags() {
if (this.state.userInput === '') return this.props.potentialOptions
return this.sortedMatches(this.state.userInput)
Expand All @@ -149,8 +148,12 @@ define([
return _.groupBy(options, (opt) => {
if (opt["course_section_id"]) {
return "course_section"
} else if (opt["group_id"]) {
return "group"
} else if (opt["noop_id"]){
return "noop"
} else {
return !!opt["group_id"] ? "group" : "student"
return "student"
}
})
},
Expand Down Expand Up @@ -187,6 +190,7 @@ define([

optionsForAllTypes(){
return _.union(
this.conditionalReleaseOptions(),
this.sectionOptions(),
this.groupOptions(),
this.studentOptions()
Expand All @@ -205,19 +209,27 @@ define([
return this.optionsForType("course_section")
},

conditionalReleaseOptions(){
if (!ENV.CONDITIONAL_RELEASE_SERVICE_ENABLED) return []

var selectable = _.contains(this.filteredTagsForType('noop'), Override.conditionalRelease)
return selectable ? [this.headerOption("conditional_release", Override.conditionalRelease)] : []
},

optionsForType(optionType){
var header = this.headerOption(optionType)
var options = this.selectableOptions(optionType)
return _.any(options) ? _.union([header], options) : []
},

headerOption(heading){
headerOption(heading, set){
var headerText = {
"student": I18n.t("Student"),
"course_section": I18n.t("Course Section"),
"group": I18n.t("Group"),
"conditional_release": I18n.t("Mastery Paths"),
}[heading]
return <ComboboxOption className="ic-tokeninput-header" value={heading} key={heading}>
return <ComboboxOption className="ic-tokeninput-header" value={heading} key={heading} set_props={set}>
{headerText}
</ComboboxOption>
},
Expand All @@ -237,7 +249,7 @@ define([

selectableOption(set){
var displayName = set.name || this.props.defaultSectionNamer(set.course_section_id)
return <ComboboxOption key={set.key} value={set.name}>
return <ComboboxOption key={set.key} value={set.name} set_props={set}>
{displayName}
</ComboboxOption>
},
Expand Down
11 changes: 9 additions & 2 deletions app/jsx/due_dates/DueDates.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,11 @@ define([
'jsx/due_dates/OverrideStudentStore',
'jsx/due_dates/StudentGroupStore',
'jsx/due_dates/TokenActions',
'compiled/models/AssignmentOverride',
'i18n!assignments',
'jquery',
'compiled/jquery.rails_flash_notifications'
], (_ ,React, DueDateRow, DueDateAddRowButton, OverrideStudentStore, StudentGroupStore, TokenActions, I18n, $) => {
], (_ ,React, DueDateRow, DueDateAddRowButton, OverrideStudentStore, StudentGroupStore, TokenActions, Override, I18n, $) => {

var DueDates = React.createClass({

Expand All @@ -28,6 +29,7 @@ define([
return {
students: {},
sections: {},
noops: {[Override.conditionalRelease.noop_id]: Override.conditionalRelease},
rows: {},
addedRowCount: 0,
defaultSectionId: null,
Expand Down Expand Up @@ -301,7 +303,8 @@ define([
var validStudents = this.valuesWithOmission({object: this.state.students, keysToOmit: this.chosenStudentIds()})
var validGroups = this.valuesWithOmission({object: this.groupsForSelectedSet(), keysToOmit: this.chosenGroupIds()})
var validSections = this.valuesWithOmission({object: this.state.sections, keysToOmit: this.chosenSectionIds()})
return _.union(validStudents, validSections, validGroups)
var validNoops = this.valuesWithOmission({object: this.state.noops, keysToOmit: this.chosenNoops()})
return _.union(validStudents, validSections, validGroups, validNoops)
},

chosenIds(idType){
Expand All @@ -323,6 +326,10 @@ define([
return this.chosenIds("group_id")
},

chosenNoops(){
return this.chosenIds("noop_id")
},

valuesWithOmission(args){
return _.chain(args["object"]).
omit(args["keysToOmit"]).
Expand Down
35 changes: 32 additions & 3 deletions app/jsx/due_dates/TokenActions.jsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
define([
'underscore',
'compiled/models/AssignmentOverride'
], (_, AssignmentOverride) => {
'compiled/models/AssignmentOverride',
'compiled/models/Section'
], (_, AssignmentOverride, Section) => {

var TokenActions = {

Expand All @@ -18,7 +19,9 @@ define([
else if (newToken.group_id) {
return this.handleGroupTokenAdd(newToken, overridesFromRow)
}
else {
else if (newToken.noop_id) {
return this.handleNoopTokenAdd(newToken, overridesFromRow)
} else {
return this.handleStudentTokenAdd(newToken, overridesFromRow)
}
},
Expand Down Expand Up @@ -71,6 +74,21 @@ define([
return this.addStudentToExistingAdhocOverride(newToken, freshOverride, overridesFromRow)
},

// -- Adding Noop --

handleNoopTokenAdd(token, overridesFromRow){
var newOverride = this.newOverrideForRow({
noop_id: token.noop_id,
title: token.name
})

if(token == AssignmentOverride.conditionalRelease) {
overridesFromRow = this.removeDefaultSection(overridesFromRow)
}

return _.union(overridesFromRow, [newOverride])
},

// -------------------
// Removing Tokens
// -------------------
Expand All @@ -82,6 +100,9 @@ define([
else if (tokenToRemove.group_id) {
return this.handleGroupTokenRemove(tokenToRemove, overridesFromRow)
}
else if (tokenToRemove.noop_id) {
return this.handleNoopTokenRemove(tokenToRemove, overridesFromRow)
}
else {
return this.handleStudentTokenRemove(tokenToRemove, overridesFromRow)
}
Expand All @@ -95,6 +116,10 @@ define([
return this.removeForType("group_id", tokenToRemove, overridesFromRow)
},

handleNoopTokenRemove(tokenToRemove, overridesFromRow){
return this.removeForType("noop_id", tokenToRemove, overridesFromRow)
},

removeForType(selector, tokenToRemove, overridesFromRow){
var overrideToRemove = _.find(overridesFromRow, function(override){
return override.get(selector) == tokenToRemove[selector]
Expand All @@ -103,6 +128,10 @@ define([
return _.difference(overridesFromRow, [overrideToRemove])
},

removeDefaultSection(overridesFromRow){
return this.handleTokenRemove({ course_section_id: Section.defaultDueDateSectionID}, overridesFromRow)
},

handleStudentTokenRemove(tokenToRemove, overridesFromRow){
var adhocOverride = this.findAdhoc(overridesFromRow, tokenToRemove.student_id)
var newStudentIds = _.difference(adhocOverride.get("student_ids"), [tokenToRemove.student_id])
Expand Down
10 changes: 7 additions & 3 deletions app/models/assignment_override.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ class AssignmentOverride < ActiveRecord::Base
has_many :assignment_override_students, :dependent => :destroy, :validate => false
validates_presence_of :assignment_version, :if => :assignment
validates_presence_of :title, :workflow_state
validates_inclusion_of :set_type, :in => %w(CourseSection Group ADHOC)
validates :set_type, inclusion: %w(CourseSection Group ADHOC Noop)
validates_length_of :title, :maximum => maximum_string_length, :allow_nil => true

concrete_set = lambda{ |override| ['CourseSection', 'Group'].include?(override.set_type) }
Expand Down Expand Up @@ -82,7 +82,7 @@ class AssignmentOverride < ActiveRecord::Base

def set_not_empty?
overridable = assignment? ? assignment : quiz
['CourseSection', 'Group'].include?(self.set_type) ||
['CourseSection', 'Group', 'Noop'].include?(self.set_type) ||
(set.any? && overridable.context.current_enrollments.where(user_id: set).exists?)
end

Expand Down Expand Up @@ -164,13 +164,15 @@ def set_id
def set
if self.set_type == 'ADHOC'
assignment_override_students.preload(:user).map(&:user)
elsif self.set_type == 'Noop'
nil
else
super
end
end

def set_id=(id)
if self.set_type == 'ADHOC'
if %w(ADHOC Noop).include? self.set_type
write_attribute(:set_id, id)
else
super
Expand Down Expand Up @@ -259,6 +261,8 @@ def applies_to_students
set.participating_students
when 'Group'
set.participants
else
[]
end
end

Expand Down
14 changes: 13 additions & 1 deletion app/models/importers/assignment_importer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,17 @@ def self.import_from_migration(hash, context, migration, item=nil, quiz=nil)

item.points_possible ||= rubric.points_possible if item.infer_grading_type == "points"
end

if hash[:assignment_overrides]
hash[:assignment_overrides].each do |o|
override = item.assignment_overrides.build
override.set_type = o[:set_type]
override.title = o[:title]
override.set_id = o[:set_id]
override.save!
end
end

if hash[:grading_standard_migration_id]
gs = context.grading_standards.where(migration_id: hash[:grading_standard_migration_id]).first
item.grading_standard = gs if gs
Expand Down Expand Up @@ -171,7 +182,8 @@ def self.import_from_migration(hash, context, migration, item=nil, quiz=nil)
:automatic_peer_reviews, :anonymous_peer_reviews,
:grade_group_students_individually, :allowed_extensions,
:position, :peer_review_count, :muted, :moderated_grading,
:omit_from_final_grade, :intra_group_peer_reviews
:omit_from_final_grade, :intra_group_peer_reviews,
:only_visible_to_overrides
].each do |prop|
item.send("#{prop}=", hash[prop]) unless hash[prop].nil?
end
Expand Down
Loading

0 comments on commit a978231

Please sign in to comment.