add grading period group association to enrollment terms

This adds the assocations required by the referenced ticket. Remaining
requirements for this ticket will be resolved in subsequent commits.

refs CNVS-26725

test plan:
1. run all the specs
2. confirm that they all pass

Change-Id: I0832a080582b4b99f63e546b0f56792b559d2692
Reviewed-on: https://gerrit.instructure.com/79113
Tested-by: Jenkins
Reviewed-by: Spencer Olson <solson@instructure.com>
QA-Review: KC Naegle <knaegle@instructure.com>
Product-Review: Cameron Matheson <cameron@instructure.com>
This commit is contained in:
Jeremy Neander 2016-05-09 12:04:04 -05:00
parent 5b5eb0c710
commit e996c877e6
6 changed files with 345 additions and 44 deletions

View File

@ -1,5 +1,5 @@
#
# Copyright (C) 2011 Instructure, Inc.
# Copyright (C) 2011-2016 Instructure, Inc.
#
# This file is part of Canvas.
#
@ -23,6 +23,8 @@ class EnrollmentTerm < ActiveRecord::Base
attr_accessible :name, :start_at, :end_at
belongs_to :root_account, :class_name => 'Account'
belongs_to :grading_period_group, inverse_of: :enrollment_terms
has_many :grading_periods, through: :grading_period_group
has_many :enrollment_dates_overrides
has_many :courses
has_many :enrollments, :through => :courses
@ -33,6 +35,7 @@ class EnrollmentTerm < ActiveRecord::Base
before_validation :verify_unique_sis_source_id
before_save :update_courses_later_if_necessary
before_save :destroy_orphaned_grading_period_group
include StickySisFields
are_sis_sticky :name, :start_at, :end_at
@ -164,5 +167,18 @@ class EnrollmentTerm < ActiveRecord::Base
save!
end
def destroy_orphaned_grading_period_group
is_being_destroyed = workflow_state_changed? && deleted?
had_previous_group = grading_period_group_id_changed? && grading_period_group_id_was
if is_being_destroyed || had_previous_group
grading_period_group_criteria = { grading_period_group_id: grading_period_group_id_was }
remaining_terms = EnrollmentTerm.active.where(grading_period_group_criteria).where.not(id: self)
unless remaining_terms.exists?
GradingPeriodGroup.destroy(grading_period_group_id_was)
end
end
end
scope :active, -> { where("enrollment_terms.workflow_state<>'deleted'") }
end

View File

@ -2,11 +2,12 @@ class GradingPeriodGroup < ActiveRecord::Base
include Canvas::SoftDeletable
attr_accessible # None of this model's attributes are mass-assignable
belongs_to :course
belongs_to :account
belongs_to :course
has_many :grading_periods, dependent: :destroy
has_many :enrollment_terms, inverse_of: :grading_period_group
validate :belongs_to_course_or_account_exclusive
validate :associated_with_course_or_account_or_enrollment_term?
set_policy do
given { |user| multiple_grading_periods_enabled? && (course || account).grants_right?(user, :read) }
@ -24,18 +25,37 @@ class GradingPeriodGroup < ActiveRecord::Base
end
private
def belongs_to_course_or_account_exclusive
if course.blank? && account.blank?
errors.add(:course_id, t("cannot be nil when account_id is nil"))
errors.add(:account_id, t("cannot be nil when course_id is nil"))
end
if course.present? && account.present?
def associated_with_course_or_account_or_enrollment_term?
has_enrollment_terms = enrollment_terms.loaded? ? enrollment_terms.any?(&:active?) : enrollment_terms.active.exists?
if has_enrollment_terms
validate_with_enrollment_terms
else
validate_without_enrollment_terms if active?
end
end
def validate_without_enrollment_terms
if course_id.blank? && account_id.blank?
errors.add(:enrollment_terms, t("cannot be empty when course_id is nil and account_id is nil"))
elsif course_id.present? && account_id.present?
errors.add(:course_id, t("cannot be present when account_id is present"))
errors.add(:account_id, t("cannot be present when course_id is present"))
end
end
def validate_with_enrollment_terms
if enrollment_terms.loaded?
account_ids = enrollment_terms.map(&:root_account_id)
else
account_ids = enrollment_terms.uniq.pluck(:root_account_id)
end
account_ids << self.account_id if self.account_id.present?
if account_ids.uniq.count > 1
errors.add(:enrollment_terms, t("cannot be associated with different accounts"))
end
end
def account_grading_period_allowed?
!!(account && account.feature_allowed?(:multiple_grading_periods))
end

View File

@ -0,0 +1,13 @@
class AddGradingPeriodGroupIdToEnrollmentTerm < ActiveRecord::Migration
tag :predeploy
def up
add_column :enrollment_terms, :grading_period_group_id, :integer, :limit => 8
add_index :enrollment_terms, :grading_period_group_id
add_foreign_key :enrollment_terms, :grading_period_groups
end
def down
remove_column :enrollment_terms, :grading_period_group_id
end
end

View File

@ -1,5 +1,5 @@
#
# Copyright (C) 2015 Instructure, Inc.
# Copyright (C) 2015-2016 Instructure, Inc.
#
# This file is part of Canvas.
#
@ -64,3 +64,13 @@ def create_grading_periods_for(context, opts={})
end
end
module Factories
class GradingPeriodGroupHelper
def create_for_enrollment_term(term)
grading_period_group = GradingPeriodGroup.new
grading_period_group.enrollment_terms << term
grading_period_group.save!
grading_period_group
end
end
end

View File

@ -1,5 +1,5 @@
#
# Copyright (C) 2011 Instructure, Inc.
# Copyright (C) 2011-2016 Instructure, Inc.
#
# This file is part of Canvas.
#
@ -19,6 +19,29 @@
require File.expand_path(File.dirname(__FILE__) + '/../spec_helper.rb')
describe EnrollmentTerm do
let(:group_helper) { Factories::GradingPeriodGroupHelper.new }
describe "validation" do
before(:once) do
@root_account = account_model
end
it "is valid with no grading_period_group" do
term = EnrollmentTerm.new
term.root_account = @root_account
expect(term).to be_valid
end
it "is valid with a grading period group shared with another enrollment term" do
term_1 = @root_account.enrollment_terms.create!
term_2 = @root_account.enrollment_terms.create!
group = group_helper.create_for_enrollment_term(term_1)
term_2.grading_period_group = group
expect(term_1).to be_valid
expect(term_2).to be_valid
end
end
it "should handle the translated Default Term names correctly" do
begin
account_model
@ -84,14 +107,63 @@ describe EnrollmentTerm do
end
end
describe "saving" do
before(:once) do
@account = account_model
end
context "when removing an associated grading period group" do
it "destroys the group when unshared" do
term = @account.enrollment_terms.create!
group = group_helper.create_for_enrollment_term(term)
term.grading_period_group = nil
term.save!
expect(GradingPeriodGroup.active.find_by_id(group.id)).to be_nil
end
it "does not destroy the group when associated with other enrollment terms" do
term_1 = @account.enrollment_terms.create!
group = group_helper.create_for_enrollment_term(term_1)
term_2 = @account.enrollment_terms.create!
term_2.update_attribute(:grading_period_group, group)
term_1.grading_period_group = nil
term_1.save!
expect(GradingPeriodGroup.active.find_by_id(group.id)).to eq(group)
end
end
context "when replacing an associated grading period group" do
it "destroys the group when unshared" do
term = @account.enrollment_terms.create!
group_1 = group_helper.create_for_enrollment_term(term)
group_2 = group_helper.create_for_enrollment_term(term)
expect(GradingPeriodGroup.active.find_by_id(group_1.id)).to be_nil
expect(GradingPeriodGroup.active.find_by_id(group_2.id)).to eq(group_2)
end
it "does not destroy the group when associated with other enrollment terms" do
term_1 = @account.enrollment_terms.create!
term_2 = @account.enrollment_terms.create!
group_1 = group_helper.create_for_enrollment_term(term_1)
group_1.enrollment_terms << term_2
group_1.save!
group_2 = group_helper.create_for_enrollment_term(term_1)
expect(GradingPeriodGroup.active.find_by_id(group_1.id)).to eq(group_1)
expect(GradingPeriodGroup.active.find_by_id(group_2.id)).to eq(group_2)
end
end
end
describe "deletion" do
before(:once) do
@account = account_model
end
it "should not be able to delete a default term" do
account_model
expect { @account.default_enrollment_term.destroy }.to raise_error
end
it "should not be able to delete an enrollment term with active courses" do
account_model
@term = @account.enrollment_terms.create!
course account: @account
@course.enrollment_term = @term
@ -100,9 +172,34 @@ describe EnrollmentTerm do
expect { @term.destroy }.to raise_error
@course.destroy
@term.destroy
end
it "destroys an associated grading period group" do
term = @account.enrollment_terms.create!
group = group_helper.create_for_enrollment_term(term)
term.destroy
expect(GradingPeriodGroup.active.find_by_id(group.id)).to be_nil
end
it "does not destroy grading period groups associated with other active enrollment terms" do
term_1 = @account.enrollment_terms.create!
group = group_helper.create_for_enrollment_term(term_1)
term_2 = @account.enrollment_terms.create!
term_2.update_attribute(:grading_period_group, group)
term_1.destroy
expect(GradingPeriodGroup.active.find_by_id(group.id)).to eql(group)
end
it "destroys grading period groups associated with other deleted enrollment terms" do
term_1 = @account.enrollment_terms.create!
group = group_helper.create_for_enrollment_term(term_1)
term_2 = @account.enrollment_terms.create!
term_2.update_attribute(:grading_period_group, group)
term_1.destroy
term_2.destroy
expect(GradingPeriodGroup.active.find_by_id(group.id)).to be_nil
end
end
describe "counts" do
@ -136,4 +233,54 @@ describe EnrollmentTerm do
end
end
end
describe "#grading_period_group" do
before(:once) do
@account = account_model
end
it "returns the associated grading period group" do
term = @account.enrollment_terms.create!
group = group_helper.create_for_enrollment_term(term)
expect(term.grading_period_group).to eq group
end
it "returns nil when no grading period group is associated" do
term = @account.enrollment_terms.create!
expect(term.grading_period_group).to be_nil
end
end
describe "#grading_periods" do
before(:once) do
@account = account_model
end
def create_grading_period(group, start_weeks, end_weeks)
group.grading_periods.create!({
start_date: start_weeks.weeks.ago,
end_date: end_weeks.weeks.ago,
title: "Example Grading Period"
})
end
it "returns the grading periods from the associated grading period group" do
term = @account.enrollment_terms.create!
group = group_helper.create_for_enrollment_term(term)
period_1 = create_grading_period(group, 5, 3)
period_2 = create_grading_period(group, 3, 1)
expect(term.grading_periods).to match_array [period_1, period_2]
end
it "returns an empty array when the associated group has no grading periods" do
term = @account.enrollment_terms.create!
group_helper.create_for_enrollment_term(term)
expect(term.grading_periods).to eq []
end
it "returns an empty array when no grading period group is associated" do
term = @account.enrollment_terms.create!
expect(term.grading_periods).to eq []
end
end
end

View File

@ -1,5 +1,5 @@
#
# Copyright (C) 2014 Instructure, Inc.
# Copyright (C) 2014-2016 Instructure, Inc.
#
# This file is part of Canvas.
#
@ -19,39 +19,121 @@
require File.expand_path(File.dirname(__FILE__) + '/../spec_helper.rb')
describe GradingPeriodGroup do
let(:group_helper) { Factories::GradingPeriodGroupHelper.new }
describe "validation" do
it "should not be valid without a course or account" do
grading_period_group = GradingPeriodGroup.new
expect(grading_period_group).to_not be_valid
it "is valid with only an active enrollment term" do
enrollment_term = Account.default.enrollment_terms.create!
group = GradingPeriodGroup.new
group.enrollment_terms << enrollment_term
expect(group).to be_valid
end
it "should not be valid with a course AND an account" do
it "is valid with an account" do
group = GradingPeriodGroup.new
group.account = Account.default
expect(group).to be_valid
end
it "is valid with a course" do
course = Course.create!(account: Account.default)
group = GradingPeriodGroup.new
group.course = course
expect(group).to be_valid
end
it "is valid with both an account and enrollment terms" do
term_1 = Account.default.enrollment_terms.create!
term_2 = Account.default.enrollment_terms.create!
group = GradingPeriodGroup.new
group.account = Account.default
group.enrollment_terms << term_1
group.enrollment_terms << term_2
expect(group).to be_valid
end
it "is not valid without an account, a course, or an enrollment term" do
group = GradingPeriodGroup.new
expect(group).not_to be_valid
end
it "is not valid with both an account and a course" do
course = Course.create!(account: Account.default)
group = GradingPeriodGroup.new
group.account = Account.default
group.course = course
expect(group).not_to be_valid
end
it "is valid with only deleted enrollment terms and is deleted" do
enrollment_term = Account.default.enrollment_terms.create!
enrollment_term.destroy
group = GradingPeriodGroup.new
group.enrollment_terms << enrollment_term
group.workflow_state = 'deleted'
expect(group).to be_valid
end
it "is not valid with only deleted enrollment terms and not deleted" do
enrollment_term = Account.default.enrollment_terms.create!
enrollment_term.destroy
group = GradingPeriodGroup.new
group.enrollment_terms << enrollment_term
expect(group).not_to be_valid
end
it "is not valid with only deleted enrollment terms and undeleted" do
enrollment_term = Account.default.enrollment_terms.create!
group = group_helper.create_for_enrollment_term(enrollment_term)
enrollment_term.destroy
group.reload
group.workflow_state = 'active'
expect(group).not_to be_valid
end
it "is not valid with an account and enrollment terms from different accounts" do
group = GradingPeriodGroup.new
group.account = Account.default
other_account = account_model
term = other_account.enrollment_terms.create!
group.enrollment_terms << term
expect(group).not_to be_valid
end
it "is not valid with enrollment terms associated with different accounts" do
account_1 = account_model
account_2 = account_model
term_1 = account_1.enrollment_terms.create!
term_2 = account_2.enrollment_terms.create!
group = GradingPeriodGroup.new
group.enrollment_terms << term_1
group.enrollment_terms << term_2
expect(group).not_to be_valid
end
it "is not valid with enrollment terms with different accounts and workflow states" do
account_1 = account_model
account_2 = account_model
term_1 = account_1.enrollment_terms.create!
term_2 = account_2.enrollment_terms.create!
term_2.destroy
group = GradingPeriodGroup.new
group.enrollment_terms << term_1
group.enrollment_terms << term_2
expect(group).not_to be_valid
end
it "is not able to mass-assign the account id" do
group = GradingPeriodGroup.new(account_id: Account.default.id)
expect(group.account_id).to be_nil
expect(group.account).to be_nil
end
it "is not able to mass-assign the course id" do
course = course()
grading_period_group = course.grading_period_groups.new
grading_period_group.account = Account.default
expect(grading_period_group).to_not be_valid
end
it "should be valid with an account" do
grading_period_group = Account.default.grading_period_groups.new
expect(grading_period_group).to be_valid
end
it "should not be able to mass-assign the account id" do
grading_period_group = GradingPeriodGroup.new(account_id: Account.default.id)
expect(grading_period_group).to_not be_valid
end
it "should be valid with a course" do
course = course()
grading_period_group = course.grading_period_groups.new
expect(grading_period_group).to be_valid
end
it "should not be able to mass-assign the course id" do
course = course()
grading_period_group = GradingPeriodGroup.new(course_id: course.id)
expect(grading_period_group).to_not be_valid
group = GradingPeriodGroup.new(course_id: course.id)
expect(group.course_id).to be_nil
expect(group.course).to be_nil
end
end
@ -273,4 +355,17 @@ describe GradingPeriodGroup do
end
end
end
describe "#enrollment_terms" do
it "returns the associated enrollment terms" do
account = Account.default
term_1 = account.enrollment_terms.create!
term_2 = account.enrollment_terms.create!
group = group_helper.create_for_enrollment_term(term_1)
term_2.update_attribute(:grading_period_group, group)
group.save!
group.reload
expect(group.enrollment_terms).to match_array([term_1, term_2])
end
end
end