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

fix grading periods api to allow proper delete functionality 

fixes CNVS-30548
fixes CNVS-30739

test plan:
- create a course with a teacher and enable multiple grading periods
- create a grading period set and three grading periods in the
  browser, called 'Account Period 1', 'Account Period 2', and
  'Account Period 3'

  course index:
  - hit GET /api/v1/courses/<Course ID>/grading_periods
  - ensure that the request succeeds
  - ensure that all three account periods are returned

  course show:
  - hit GET /api/v1/courses/<Course ID>/grading_periods/
    <Grading Period ID>, where the grading period is any of
    the three account periods
  - ensure that the request succeeds

- create a grading period group off the course by using
  course.grading_period_groups.create!()
- create a grading period by using group.grading_periods.create!(
  title: 'Course Period 1', start_date: Time.now, end_date: Time.now,
  close_date: Time.now)
- create two more grading periods called 'Course Period 2' and
  'Course Period 3'

  course index:
  - hit GET /api/v1/courses/<Course ID>/grading_periods
  - ensure that the request succeeds
  - ensure that only all three of the course grading periods
    are returned

  course show:
  - hit GET /api/v1/courses/<Course ID>/grading_periods/
    <Grading Period ID>, where the grading period is a
    course grading period
  - ensure the request succeeds
  - hit it again with an account grading period
  - ensure the request fails

  course update:
  - hit PUT /api/v1/courses/<Course ID>/grading_periods/
    <Grading Period ID>, where the grading period is a
    course grading period
  - include the argument
    { grading_periods: [{ title: 'New Title' }] }
  - ensure the request succeeds
  - ensure the title of the grading period is changed in the
    console
  - hit it again with an account grading period with the
    same arguments
  - ensure the request fails

  course delete:
  - hit DELETE /api/v1/courses/<Course ID>/grading_periods/
    <Grading Period ID>, where the grading period is a
    course grading period
  - ensure the request succeeds
  - ensure the period has its workflow state set to deleted
    in the console
  - hit it again with an account grading period
  - ensure the request fails

  account index:
  - hit GET /api/v1/accounts/<Account ID>/grading_periods
  - ensure the request succeeds
  - ensure that only all three of the account grading periods
    are returned

  account delete:
  - hit DELETE /api/v1/accounts/<Account ID>/grading_periods/
    <Grading Period ID>, where the grading period is an
    account grading period
  - ensure the request succeeds
  - ensure the period has its workflow state set to deleted
    in the console
  - hit it again with a course grading period
  - ensure the request fails

Change-Id: I34e43e10aadb30a349e1932c132554c44f7ec24a
Reviewed-on: https://gerrit.instructure.com/86961
Reviewed-by: Neil Gupta <ngupta@instructure.com>
Reviewed-by: Spencer Olson <solson@instructure.com>
Tested-by: Jenkins
QA-Review: KC Naegle <knaegle@instructure.com>
Product-Review: Christi Wruck
This commit is contained in:
Nick Pitrak 2016-08-04 17:07:54 -05:00 committed by Gentry Beckmann
parent fb07f513ed
commit ebdd8419db
12 changed files with 569 additions and 457 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
app/controllers/assignment_groups_api_controller.rb
View File

@ -46,7 +46,7 @@ class AssignmentGroupsApiController < ApplicationController
override_dates = value_to_boolean(params[:override_assignment_dates] || true)
assignments = @assignment_group.visible_assignments(@current_user)
if params[:grading_period_id].present? && multiple_grading_periods?
assignments = GradingPeriod.context_find(@context, params[:grading_period_id]).assignments(assignments)
assignments = GradingPeriod.for(@context).find_by(id: params[:grading_period_id]).assignments(assignments)
end
if assignments.any? && includes.include?('submission')
submissions = submissions_hash(['submission'], assignments)

2
app/controllers/assignment_groups_controller.rb
View File

@ -370,7 +370,7 @@ class AssignmentGroupsController < ApplicationController
end
def filter_assignments_by_grading_period(assignments, course)
grading_period = GradingPeriod.context_find(course, params.fetch(:grading_period_id))
grading_period = GradingPeriod.for(course).find_by(id: params.fetch(:grading_period_id))
return assignments unless grading_period
if params[:scope_assignments_to_student] &&

4
app/controllers/enrollments_api_controller.rb
View File

@ -313,7 +313,7 @@ class EnrollmentsApiController < ApplicationController
end
grading_period = @context.courses.lazy.map do |course|
GradingPeriod.context_find(course, params[:grading_period_id])
GradingPeriod.for(course).find_by(id: params[:grading_period_id])
end.detect(&:present?)
else
unless multiple_grading_periods?
@ -321,7 +321,7 @@ class EnrollmentsApiController < ApplicationController
return false
end
grading_period = GradingPeriod.context_find(@context, params[:grading_period_id])
grading_period = GradingPeriod.for(@context).find_by(id: params[:grading_period_id])
end
unless grading_period

2
app/controllers/gradebooks_controller.rb
View File

@ -127,7 +127,7 @@ class GradebooksController < ApplicationController
.reject(&:muted?)
if multiple_grading_periods? && @current_grading_period_id && !view_all_grading_periods?
current_period = GradingPeriod.context_find(@context, @current_grading_period_id)
current_period = GradingPeriod.for(@context).find_by(id: @current_grading_period_id)
visible_assignments = current_period.assignments_for_student(visible_assignments, opts[:student])
end

53
app/controllers/grading_periods_controller.rb
View File

@ -79,17 +79,13 @@ class GradingPeriodsController < ApplicationController
#
def index
if authorized_action(@context, @current_user, :read)
# FIXME: This action is no longer used with Accounts.
#
# Team question: What should we do if external clients are still
# potentially going to expect grading periods on accounts by calling
# to this endpoint?
if @context.is_a? Account
grading_periods = []
grading_periods = @context.grading_periods.active.order(:start_date)
read_only = false
else
grading_periods = GradingPeriod.for(@context).sort_by(&:start_date)
grading_periods = GradingPeriod.for(@context).order(:start_date)
read_only = grading_periods.present? && grading_periods.first.grading_period_group.account_id.present?
end
read_only = grading_periods.present? && grading_periods.first.grading_period_group.account_id.present?
paginated_grading_periods, meta = paginate_for(grading_periods)
respond_to do |format|
format.json do
@ -140,13 +136,13 @@ class GradingPeriodsController < ApplicationController
def update
grading_period_params = params[:grading_periods].first
if authorized_action(grading_period, @current_user, :update)
if authorized_action(grading_period(inherit: false), @current_user, :update)
respond_to do |format|
if grading_period.update_attributes(grading_period_params)
format.json { render json: serialize_json_api(grading_period) }
if grading_period(inherit: false).update_attributes(grading_period_params)
format.json { render json: serialize_json_api(grading_period(inherit: false)) }
else
format.json do
render json: grading_period.errors, status: :unprocessable_entity
render json: grading_period(inherit: false).errors, status: :unprocessable_entity
end
end
end
@ -159,12 +155,8 @@ class GradingPeriodsController < ApplicationController
# <b>204 No Content</b> response code is returned if the deletion was
# successful.
def destroy
unless can_destroy_in_context?(grading_period)
return render_unauthorized_action
end
if authorized_action(grading_period, @current_user, :delete)
grading_period.destroy
if authorized_action(grading_period(inherit: false), @current_user, :delete)
grading_period(inherit: false).destroy
respond_to do |format|
format.json { head :no_content }
end
@ -179,16 +171,16 @@ class GradingPeriodsController < ApplicationController
private
def grading_period
def grading_period(inherit: true)
@grading_period ||= begin
grading_period = GradingPeriod.context_find(@context, params[:id])
grading_period = GradingPeriod.for(@context, inherit: inherit).find_by(id: params[:id])
fail ActionController::RoutingError.new('Not Found') if grading_period.blank?
grading_period
end
end
def account_batch_update
periods = find_or_build_periods_for_account(params[:grading_periods])
periods = find_or_build_periods(params[:grading_periods])
unless batch_update_rights?(periods)
return render_unauthorized_action
end
@ -213,7 +205,7 @@ class GradingPeriodsController < ApplicationController
end
def course_batch_update
periods = find_or_build_periods_for_course(params[:grading_periods])
periods = find_or_build_periods(params[:grading_periods])
unless batch_update_rights?(periods)
return render_unauthorized_action
end
@ -267,18 +259,9 @@ class GradingPeriodsController < ApplicationController
sorted_periods.select { |period| period.errors.present? }.map(&:errors)
end
def find_or_build_periods_for_account(periods_params)
def find_or_build_periods(periods_params)
periods_params.map do |period_params|
period = grading_period_group.grading_periods.find(period_params[:id]) if period_params[:id].present?
period ||= grading_period_group.grading_periods.build
period.assign_attributes(period_params.except(:id))
period
end
end
def find_or_build_periods_for_course(periods_params)
periods_params.map do |period_params|
period = GradingPeriod.context_find(@context, period_params[:id])
period = GradingPeriod.for(@context, inherit: false).find_by(id: period_params[:id]) if period_params[:id].present?
period ||= context_grading_period_group.grading_periods.build
period.assign_attributes(period_params.except(:id))
period
@ -303,10 +286,6 @@ class GradingPeriodsController < ApplicationController
periods.empty? || periods.first.grading_period_group.account_id.blank?
end
def can_destroy_in_context?(period)
@context.is_a?(Account) || period.grading_period_group.account_id.blank?
end
def context_grading_period_group
@context.grading_period_groups.active.first_or_create
end

2
app/controllers/users_controller.rb
View File

@ -199,7 +199,7 @@ class UsersController < ApplicationController
course = enrollment.course
grading_period_id = params[:grading_period_id].to_i
grading_period = GradingPeriod.context_find(course, grading_period_id)
grading_period = GradingPeriod.for(course).find_by(id: grading_period_id)
grading_periods = {
course.id => {
:periods => [grading_period],

1
app/models/account.rb
View File

@ -43,6 +43,7 @@ class Account < ActiveRecord::Base
has_many :enrollment_terms, :foreign_key => 'root_account_id'
has_many :active_enrollment_terms, -> { where("enrollment_terms.workflow_state<>'deleted'") }, class_name: 'EnrollmentTerm', foreign_key: 'root_account_id'
has_many :grading_period_groups, inverse_of: :root_account, dependent: :destroy
has_many :grading_periods, through: :grading_period_groups
has_many :enrollments, -> { where("enrollments.type<>'StudentViewEnrollment'") }, foreign_key: 'root_account_id'
has_many :all_enrollments, :class_name => 'Enrollment', :foreign_key => 'root_account_id'
has_many :sub_accounts, -> { where("workflow_state<>'deleted'") }, class_name: 'Account', foreign_key: 'parent_account_id'

20
app/models/grading_period.rb
View File

@ -50,13 +50,12 @@ class GradingPeriod < ActiveRecord::Base
end
end
def self.for(course)
periods = active.grading_periods_by(course_id: course.id)
if periods.exists?
periods
def self.for(context, inherit: true)
grading_periods = context.grading_periods.active
if context.is_a?(Course) && inherit && grading_periods.empty?
context.enrollment_term.grading_periods.active
else
grading_period_group_ids = EnrollmentTerm.select(:grading_period_group_id).where(id: course.enrollment_term)
active.where(grading_period_group_id: grading_period_group_ids)
grading_periods
end
end
@ -64,15 +63,6 @@ class GradingPeriod < ActiveRecord::Base
self.for(context).find(&:current?)
end
# Takes a context and a grading_period_id and returns a grading period
# if it is in the for collection. Uses Enumberable#find to query
# collection.
def self.context_find(context, grading_period_id)
self.for(context).find do |grading_period|
grading_period.id == grading_period_id.to_i
end
end
def account_group?
grading_period_group.course_id.nil?
end

2
app/presenters/grade_summary_presenter.rb
View File

@ -144,7 +144,7 @@ class GradeSummaryPresenter
end
def grading_period_assignments(grading_period_id, assignments)
grading_period = GradingPeriod.context_find(@context, grading_period_id)
grading_period = GradingPeriod.for(@context).find_by(id: grading_period_id)
if grading_period
grading_period.assignments_for_student(assignments, student)
else

2
lib/gradebook_exporter.rb
View File

@ -42,7 +42,7 @@ class GradebookExporter
# grading_period_id == 0 means no grading period selected
unless @options[:grading_period_id].try(:to_i) == 0
grading_period = GradingPeriod.context_find @course, @options[:grading_period_id]
grading_period = GradingPeriod.for(@course).find_by(id: @options[:grading_period_id])
end
calc = GradeCalculator.new(student_enrollments.map(&:user_id), @course,

870
spec/controllers/grading_periods_controller_spec.rb
View File

@ -28,6 +28,36 @@ describe GradingPeriodsController do
let(:course) { sub_account.courses.create! }
def create_course_grading_period(course, opts = {})
group = group_helper.legacy_create_for_course(course)
period = period_helper.create_for_group(group, opts)
course.enrollment_term.update_attribute(:grading_period_group_id, group)
period
end
def create_account_grading_period(account, opts = {})
group = group_helper.create_for_account(account)
period = period_helper.create_for_group(group, opts)
course.enrollment_term.update_attribute(:grading_period_group_id, group)
period
end
def login_admin
user = User.create!
root_account.account_users.create!(user: user)
user_session(user)
end
def login_sub_account
user_session(account_admin_user(account: sub_account))
end
def expect_grading_period_id_match(json, period)
expect(json['grading_periods'].count).to eql(1)
returned_period = json['grading_periods'].first
expect(returned_period['id']).to eql(period.id.to_s)
end
before do
account_admin_user(account: root_account)
user_session(@admin)
@ -36,480 +66,554 @@ describe GradingPeriodsController do
end
describe 'GET index' do
let(:get_index!) { get :index, { course_id: course.id } }
let(:json) { json_parse }
context 'with course grading periods' do
before do
period_helper.create_with_group_for_course(course)
end
it 'contains grading periods owned by the course' do
get_index!
expect(json['grading_periods'].count).to eql(1)
end
it 'paginates' do
get_index!
expect(json).to have_key('meta')
expect(json['meta']).to have_key('pagination')
expect(json['meta']['primaryCollection']).to eql('grading_periods')
end
it 'is ordered by start_date' do
get_index!
expect(json['grading_periods']).to be_sorted_by('start_date')
end
it "sets 'grading_periods_read_only' to false" do
get_index!
expect(json['grading_periods_read_only']).to eql(false)
end
it "paginates" do
create_course_grading_period(course)
get :index, { course_id: course.id }
expect(json_parse).to have_key('meta')
expect(json_parse['meta']).to have_key('pagination')
expect(json_parse['meta']['primaryCollection']).to eql('grading_periods')
end
context 'with no grading periods' do
it "sets 'grading_periods_read_only' to false" do
get_index!
expect(json['grading_periods_read_only']).to eql(false)
end
end
context 'with account grading periods' do
it "sets 'grading_periods_read_only' to true" do
group = group_helper.create_for_account(root_account)
course.enrollment_term.update_attribute(:grading_period_group_id, group)
period_helper.create_for_group(group)
get_index!
expect(json['grading_periods_read_only']).to eql(true)
end
end
context 'with root account admins' do
before do
period_helper.create_with_group_for_course(course)
end
describe 'with root account admins' do
it 'disallows creating grading periods' do
root_account.enable_feature!(:multiple_grading_periods)
get_index!
expect(json['can_create_grading_periods']).to eql(false)
get :index, { course_id: course.id }
expect(json_parse['can_create_grading_periods']).to eql(false)
end
it 'allows toggling grading periods when multiple grading periods are enabled' do
root_account.enable_feature!(:multiple_grading_periods)
get_index!
expect(json['can_toggle_grading_periods']).to eql(true)
get :index, { course_id: course.id }
expect(json_parse['can_toggle_grading_periods']).to eql(true)
end
it "returns 'not found' when multiple grading periods are allowed" do
root_account.allow_feature!(:multiple_grading_periods)
get_index!
get :index, { course_id: course.id }
expect(response).to be_not_found
end
it "returns 'not found' when multiple grading periods are disabled" do
root_account.disable_feature!(:multiple_grading_periods)
get_index!
get :index, { course_id: course.id }
expect(response).to be_not_found
end
end
context 'with sub account admins' do
let(:sub_account_admin) { account_admin_user(account: sub_account) }
before do
period_helper.create_with_group_for_course(course)
end
describe 'with sub account admins' do
it 'disallows creating grading periods' do
root_account.enable_feature!(:multiple_grading_periods)
user_session(sub_account_admin)
get_index!
expect(json['can_create_grading_periods']).to eql(false)
login_sub_account
get :index, { course_id: course.id }
expect(json_parse['can_create_grading_periods']).to eql(false)
end
it 'disallows toggling grading periods when multiple grading periods are root account enabled' do
root_account.enable_feature!(:multiple_grading_periods)
user_session(sub_account_admin)
get_index!
expect(json['can_toggle_grading_periods']).to eql(false)
login_sub_account
get :index, { course_id: course.id }
expect(json_parse['can_toggle_grading_periods']).to eql(false)
end
it "returns 'not found' when multiple grading periods are root account disabled" do
root_account.disable_feature!(:multiple_grading_periods)
user_session(sub_account_admin)
get_index!
login_sub_account
get :index, { course_id: course.id }
expect(response).to be_not_found
end
it "returns 'not found' when multiple grading periods are allowed" do
root_account.allow_feature!(:multiple_grading_periods)
user_session(sub_account_admin)
get_index!
login_sub_account
get :index, { course_id: course.id }
expect(response).to be_not_found
end
it "returns 'not found' when multiple grading periods are sub account disabled" do
root_account.allow_feature!(:multiple_grading_periods)
sub_account.disable_feature!(:multiple_grading_periods)
user_session(sub_account_admin)
get_index!
login_sub_account
get :index, { course_id: course.id }
expect(response).to be_not_found
end
end
describe 'with course context' do
it "can get any course associated grading periods with read_only set to false" do
period = create_course_grading_period(course)
get :index, { course_id: course.id }
expect_grading_period_id_match(json_parse, period)
expect(json_parse['grading_periods_read_only']).to eql(false)
end
it 'is ordered by start_date' do
create_course_grading_period(course, start_date: 2.days.from_now)
create_course_grading_period(course, start_date: 5.days.from_now)
create_course_grading_period(course, start_date: 3.days.from_now)
get :index, { course_id: course.id }
expect(json_parse['grading_periods']).to be_sorted_by('start_date')
end
it "can get any account associated grading periods with read_only set to true" do
period = create_account_grading_period(root_account)
get :index, { course_id: course.id }
expect_grading_period_id_match(json_parse, period)
expect(json_parse['grading_periods_read_only']).to eql(true)
end
it "gets course associated grading periods if both are available" do
course_period = create_course_grading_period(course)
account_period = create_account_grading_period(root_account)
get :index, { course_id: course.id }
expect_grading_period_id_match(json_parse, course_period)
end
it "sets read_only to false if no grading periods are given" do
get :index, { course_id: course.id }
expect(json_parse['grading_periods_read_only']).to eql(false)
end
end
describe 'with account context' do
it "can get any account associated grading periods with read_only set to false" do
period = create_account_grading_period(root_account)
get :index, { account_id: root_account.id }
expect_grading_period_id_match(json_parse, period)
expect(json_parse['grading_periods_read_only']).to eql(false)
end
it 'is ordered by start_date' do
create_account_grading_period(root_account, start_date: 2.days.from_now)
create_account_grading_period(root_account, start_date: 5.days.from_now)
create_account_grading_period(root_account, start_date: 3.days.from_now)
get :index, { account_id: root_account.id }
expect(json_parse['grading_periods']).to be_sorted_by('start_date')
end
it "cannot get any course associated grading periods" do
period = create_course_grading_period(course)
get :index, { account_id: root_account.id }
expect(json_parse['grading_periods'].count).to eql(0)
end
it "sets read_only to false if no grading periods are given" do
get :index, { account_id: root_account.id }
expect(json_parse['grading_periods_read_only']).to eql(false)
end
end
end
describe 'GET show' do
it 'contains grading periods owned by the course' do
grading_period = period_helper.create_with_group_for_course(course)
get :show, { course_id: course.id, id: grading_period.id }
json = json_parse
expect(json['grading_periods'].count).to eql(1)
period = json['grading_periods'].first
expect(period['id']).to eql(grading_period.id.to_s)
it 'can show course associated grading periods' do
period = create_course_grading_period(course)
get :show, { course_id: course.id, id: period.to_param }
expect_grading_period_id_match(json_parse, period)
end
it 'can show account associated grading periods' do
period = create_account_grading_period(root_account)
get :show, { course_id: course.id, id: period.to_param }
expect_grading_period_id_match(json_parse, period)
end
it 'returns the expected attributes' do
grading_period = period_helper.create_with_group_for_course(course)
get :show, { course_id: course.id, id: grading_period.id }
period = create_course_grading_period(course)
get :show, { course_id: course.id, id: period.to_param }
expected_attributes = [
"id", "grading_period_group_id", "start_date", "end_date",
"close_date", "weight", "title", "permissions"
"id",
"grading_period_group_id",
"start_date",
"end_date",
"close_date",
"weight",
"title",
"permissions"
]
period_attributes = json_parse['grading_periods'].first.keys
expect(period_attributes).to match_array(expected_attributes)
end
end
describe "PATCH batch_update for a grading period set" do
let(:period_1_params) do
{
title: 'First Grading Period',
start_date: 2.days.ago(now).to_s,
end_date: 2.days.from_now(now).to_s
}
describe "PUT update" do
before(:each) do
login_admin
end
let(:period_2_params) do
{
title: 'Second Grading Period',
start_date: 2.days.from_now(now).to_s,
end_date: 4.days.from_now(now).to_s
}
end
let(:group) { group_helper.create_for_account(root_account) }
let(:period_1) { group.grading_periods.create!(period_1_params) }
let(:period_2) { group.grading_periods.create!(period_2_params) }
context 'given two consecutive persisted periods' do
it "compares the in memory periods' dates for overlapping" do
patch :batch_update, {
set_id: group.id,
grading_periods: [
period_1_params.merge(id: period_1.id, end_date: 3.days.from_now(now), close_date: 3.days.from_now(now)),
period_2_params.merge(id: period_2.id, start_date: 3.days.from_now(now))
]
}
expect(period_1.reload.end_date).to eql(3.days.from_now(now))
expect(period_2.reload.start_date).to eql(3.days.from_now(now))
it "can update any course associated grading periods" do
period = create_course_grading_period(course, { title: 'Grading Period' })
put :update, {
course_id: course.id,
id: period.to_param,
grading_periods: [{
title: 'Grading Period New'
}]
}
expect(period.reload.title).to eql('Grading Period New')
end
it "cannot update any account associated grading periods" do
period = create_account_grading_period(root_account, { title: 'Grading Period' })
put :update, {
course_id: course.id,
id: period.to_param,
grading_periods: [{
title: 'Grading Period New'
}]
}
expect(period.reload.title).to eql('Grading Period')
expect(response).to be_not_found
end
end
describe "DELETE destroy" do
before(:each) do
login_admin
end
describe "with course context" do
it "can destroy any course associated grading periods" do
period = create_course_grading_period(course)
delete :destroy, { course_id: course.id, id: period.to_param }
expect(period.reload).to be_deleted
end
it "does not paginate" do
period_params = (1..11).map do|i|
it "cannot destroy any account associated grading periods" do
period = create_account_grading_period(root_account)
delete :destroy, { course_id: course.id, id: period.to_param }
expect(period.reload).not_to be_deleted
expect(response).to be_not_found
end
end
describe "with account context" do
it "can destroy any account associated grading periods" do
period = create_account_grading_period(root_account)
delete :destroy, { account_id: root_account.id, id: period.to_param }
expect(period.reload).to be_deleted
end
it "cannot destroy any course associated grading periods" do
period = create_course_grading_period(course)
delete :destroy, { account_id: root_account.id, id: period.to_param }
expect(period.reload).not_to be_deleted
expect(response).to be_not_found
end
end
end
describe "PATCH batch_update" do
describe "with account context" do
describe "with account associated grading periods" do
let(:period_1_params) do
{
title: "Period #{i}",
start_date: i.days.from_now(now),
end_date: (i+1).days.from_now(now)
title: 'First Grading Period',
start_date: 2.days.ago(now).to_s,
end_date: 2.days.from_now(now).to_s
}
end
patch :batch_update, {
set_id: group.id,
grading_periods: period_params
}
json = JSON.parse(response.body)
expect(json).not_to have_key('meta')
expect(json.fetch('grading_periods').count).to eql 11
end
end
let(:period_2_params) do
{
title: 'Second Grading Period',
start_date: 2.days.from_now(now).to_s,
end_date: 4.days.from_now(now).to_s
}
end
let(:group) { group_helper.create_for_account(root_account) }
let(:period_1) { group.grading_periods.create!(period_1_params) }
let(:period_2) { group.grading_periods.create!(period_2_params) }
context "as a user associated with the root account" do
before do
user = User.create!
root_account.account_users.create!(:user => user)
user_session(user)
end
it "can create a single grading period" do
expect do
patch :batch_update, { set_id: group.id, grading_periods: [period_1_params] }
end.to change { group.grading_periods.count }.by 1
end
it "can create multiple grading periods" do
expect do
it "compares the in memory periods' dates for overlapping" do
patch :batch_update, {
set_id: group.id,
grading_periods: [period_1_params, period_2_params]
grading_periods: [
period_1_params.merge(id: period_1.id, end_date: 3.days.from_now(now), close_date: 3.days.from_now(now)),
period_2_params.merge(id: period_2.id, start_date: 3.days.from_now(now))
]
}
end.to change { group.grading_periods.count }.by 2
expect(period_1.reload.end_date).to eql(3.days.from_now(now))
expect(period_2.reload.start_date).to eql(3.days.from_now(now))
end
it "does not paginate" do
period_params = (1..11).map do|i|
{
title: "Period #{i}",
start_date: i.days.from_now(now),
end_date: (i+1).days.from_now(now)
}
end
patch :batch_update, {
set_id: group.id,
grading_periods: period_params
}
json = JSON.parse(response.body)
expect(json).not_to have_key('meta')
expect(json.fetch('grading_periods').count).to eql 11
end
describe "with root account admins" do
before do
login_admin
end
it "can create a single grading period" do
expect do
patch :batch_update, { set_id: group.id, grading_periods: [period_1_params] }
end.to change { group.grading_periods.count }.by 1
end
it "can create multiple grading periods" do
expect do
patch :batch_update, {
set_id: group.id,
grading_periods: [period_1_params, period_2_params]
}
end.to change { group.grading_periods.count }.by 2
end
it "can update a single grading period" do
patch :batch_update, { set_id: group.id, grading_periods: [
period_1_params.merge(id: period_1.id, title: 'Updated Title')
] }
expect(group.reload.grading_periods.find(period_1.id).title).to eq 'Updated Title'
end
it "can update multiple grading periods" do
patch :batch_update, { set_id: group.id, grading_periods: [
period_1_params.merge(id: period_1.id, title: 'Original Title'),
period_2_params.merge(id: period_2.id, title: 'Updated Title')
] }
expect(group.reload.grading_periods.find(period_1.id).title).to eql('Original Title')
expect(group.reload.grading_periods.find(period_2.id).title).to eql('Updated Title')
end
it "can create and update multiple grading periods" do
period_1 = group.grading_periods.create!(period_1_params)
expect do
patch :batch_update, { set_id: group.id, grading_periods: [
period_1_params.merge(id: period_1.id, title: 'A Different Title'),
period_2_params
] }
end.to change { group.grading_periods.count }.by 1
expect(group.reload.grading_periods.find(period_1.id).title).to eql('A Different Title')
end
end
end
it "can update a single grading period" do
patch :batch_update, { set_id: group.id, grading_periods: [
period_1_params.merge(id: period_1.id, title: 'Updated Title')
] }
expect(group.reload.grading_periods.find(period_1.id).title).to eq 'Updated Title'
end
describe "with course associated grading periods" do
let(:period_1_params) do
{
title: 'Original Title',
start_date: 2.days.ago(now).to_s,
end_date: 2.days.from_now(now).to_s
}
end
let(:group) { group_helper.legacy_create_for_course(course) }
let(:period_1) { group.grading_periods.create!(period_1_params) }
it "can update multiple grading periods" do
patch :batch_update, { set_id: group.id, grading_periods: [
period_1_params.merge(id: period_1.id, title: 'Original Title'),
period_2_params.merge(id: period_2.id, title: 'Updated Title')
] }
expect(group.reload.grading_periods.find(period_1.id).title).to eql('Original Title')
expect(group.reload.grading_periods.find(period_2.id).title).to eql('Updated Title')
end
before(:each) do
login_admin
end
it "can create and update multiple grading periods" do
period_1 = group.grading_periods.create!(period_1_params)
expect do
it "cannot update any grading periods" do
patch :batch_update, { set_id: group.id, grading_periods: [
period_1_params.merge(id: period_1.id, title: 'A Different Title'),
period_2_params
period_1_params.merge(id: period_1.id, title: 'Updated Title')
] }
end.to change { group.grading_periods.count }.by 1
expect(group.reload.grading_periods.find(period_1.id).title).to eql('A Different Title')
end
end
end
expect(period_1.reload.title).to eql('Original Title')
expect(GradingPeriod.for(course).find(period_1.id).title).to eql('Original Title')
end
describe "PATCH batch_update for course grading periods" do
let(:period_1_params) do
{
title: 'First Grading Period',
start_date: 2.days.ago(now).to_s,
end_date: 2.days.from_now(now).to_s
}
end
let(:period_2_params) do
{
title: 'Second Grading Period',
start_date: 2.days.from_now(now).to_s,
end_date: 4.days.from_now(now).to_s
}
end
let(:group) { group_helper.legacy_create_for_course(course) }
let(:period_1) { group.grading_periods.create!(period_1_params) }
let(:period_2) { group.grading_periods.create!(period_2_params) }
context 'given two consecutive persisted periods' do
it "compares the in memory periods' dates for overlapping" do
patch :batch_update, {
course_id: course.id,
grading_periods: [
period_1_params.merge(id: period_1.id, end_date: 3.days.from_now(now), close_date: 3.days.from_now(now)),
period_2_params.merge(id: period_2.id, start_date: 3.days.from_now(now))
]
}
expect(period_1.reload.end_date).to eql(3.days.from_now(now))
expect(period_2.reload.start_date).to eql(3.days.from_now(now))
it "responds with 404 not found upon failure" do
patch :batch_update, { set_id: group.id, grading_periods: [
period_1_params.merge(id: period_1.id, title: 'Updated Title')
] }
expect(response).to be_not_found
end
end
end
context "as a user associated with the root account" do
before do
user = User.create!
root_account.account_users.create!(:user => user)
user_session(user)
end
describe "with course context" do
describe "with course associated grading periods" do
let(:period_1_params) do
{
title: 'First Grading Period',
start_date: 2.days.ago(now).to_s,
end_date: 2.days.from_now(now).to_s
}
end
let(:period_2_params) do
{
title: 'Second Grading Period',
start_date: 2.days.from_now(now).to_s,
end_date: 4.days.from_now(now).to_s
}
end
let(:group) { group_helper.legacy_create_for_course(course) }
let(:period_1) { group.grading_periods.create!(period_1_params) }
let(:period_2) { group.grading_periods.create!(period_2_params) }
it "cannot create a single grading period" do
expect do
patch :batch_update, { course_id: course.id, grading_periods: [period_1_params] }
end.not_to change { course.grading_periods.count }
expect(response.status).to eql(Rack::Utils.status_code(:unauthorized))
end
it "cannot create multiple grading periods" do
expect do
it "compares the in memory periods' dates for overlapping" do
patch :batch_update, {
course_id: course.id,
grading_periods: [period_1_params, period_2_params]
grading_periods: [
period_1_params.merge(id: period_1.id, end_date: 3.days.from_now(now), close_date: 3.days.from_now(now)),
period_2_params.merge(id: period_2.id, start_date: 3.days.from_now(now))
]
}
end.not_to change { course.grading_periods.count }
expect(period_1.reload.end_date).to eql(3.days.from_now(now))
expect(period_2.reload.start_date).to eql(3.days.from_now(now))
end
it "responds with json upon success" do
patch :batch_update, { course_id: course.id, grading_periods: [] }
expect(response).to be_ok
json = JSON.parse(response.body)
expect(json['grading_periods']).to be_empty
expect(json).not_to includes('errors')
end
it "responds with json upon failure" do
period = period_helper.create_with_group_for_course(course)
patch :batch_update, { course_id: course.id, grading_periods: [{id: period.id, title: ''}] }
expect(response).not_to be_ok
json = JSON.parse(response.body)
expect(json['errors']).to be_present
expect(json).not_to have_key('grading_periods')
end
describe "with root account admins" do
before do
login_admin
end
it "cannot create a single grading period" do
expect do
patch :batch_update, { course_id: course.id, grading_periods: [period_1_params] }
end.not_to change { course.grading_periods.count }
expect(response.status).to eql(Rack::Utils.status_code(:unauthorized))
end
it "cannot create multiple grading periods" do
expect do
patch :batch_update, {
course_id: course.id,
grading_periods: [period_1_params, period_2_params]
}
end.not_to change { course.grading_periods.count }
end
it "can update a single grading period" do
patch :batch_update, { course_id: course.id, grading_periods: [
period_1_params.merge(id: period_1.id, title: 'Original Title')
] }
expect(course.grading_periods.find(period_1.id).title).to eql('Original Title')
end
it "can update multiple grading periods" do
patch :batch_update, { course_id: course.id, grading_periods: [
period_1_params.merge(id: period_1.id, title: 'Original Title'),
period_2_params.merge(id: period_2.id, title: 'Updated Title')
] }
expect(course.grading_periods.find(period_1.id).title).to eql('Original Title')
expect(course.grading_periods.find(period_2.id).title).to eql('Updated Title')
end
it "cannot create and update multiple grading periods" do
period_1 = group.grading_periods.create!(period_1_params)
expect do
patch :batch_update, { course_id: course.id, grading_periods: [
period_1_params.merge(id: period_1.id, title: 'Original Title'),
period_2_params
] }
end.not_to change { course.grading_periods.count }
expect(course.grading_periods.find(period_1.id).title).not_to eql('Original Title')
end
end
describe "with sub account admins" do
before do
login_sub_account
end
it "cannot create a single grading period" do
expect do
patch :batch_update, { course_id: course.id, grading_periods: [period_1_params] }
end.not_to change { course.grading_periods.count }
expect(response.status).to eql(Rack::Utils.status_code(:unauthorized))
end
it "cannot create multiple grading periods" do
expect do
patch :batch_update, {
course_id: course.id,
grading_periods: [period_1_params, period_2_params]
}
end.not_to change { course.grading_periods.count }
end
it "can update a single grading period" do
patch :batch_update, { course_id: course.id, grading_periods: [
period_1_params.merge(id: period_1.id, title: 'Original Title')
] }
expect(course.grading_periods.find(period_1.id).title).to eql('Original Title')
end
it "can update multiple grading periods" do
patch :batch_update, { course_id: course.id, grading_periods: [
period_1_params.merge(id: period_1.id, title: 'Original Title'),
period_2_params.merge(id: period_2.id, title: 'Updated Title')
] }
expect(course.grading_periods.find(period_1.id).title).to eql('Original Title')
expect(course.grading_periods.find(period_2.id).title).to eql('Updated Title')
end
it "cannot create and update multiple grading periods" do
period_1 = group.grading_periods.create!(period_1_params)
expect do
patch :batch_update, { course_id: course.id, grading_periods: [
period_1_params.merge(id: period_1.id, title: 'Original Title'),
period_2_params
] }
end.not_to change { course.grading_periods.count }
expect(course.grading_periods.find(period_1.id).title).not_to eql('Original Title')
end
end
end
it "can update a single grading period" do
patch :batch_update, { course_id: course.id, grading_periods: [
period_1_params.merge(id: period_1.id, title: 'Original Title')
] }
expect(course.grading_periods.find(period_1.id).title).to eql('Original Title')
end
it "can update multiple grading periods" do
patch :batch_update, { course_id: course.id, grading_periods: [
period_1_params.merge(id: period_1.id, title: 'Original Title'),
period_2_params.merge(id: period_2.id, title: 'Updated Title')
] }
expect(course.grading_periods.find(period_1.id).title).to eql('Original Title')
expect(course.grading_periods.find(period_2.id).title).to eql('Updated Title')
end
it "cannot create and update multiple grading periods" do
period_1 = group.grading_periods.create!(period_1_params)
expect do
patch :batch_update, { course_id: course.id, grading_periods: [
period_1_params.merge(id: period_1.id, title: 'Original Title'),
period_2_params
] }
end.not_to change { course.grading_periods.count }
expect(course.grading_periods.find(period_1.id).title).not_to eql('Original Title')
end
end
context "as a user associated with a sub account" do
before do
user = User.create!
sub_account.account_users.create!(:user => user)
user_session(user)
end
it "cannot create a single grading period" do
expect do
patch :batch_update, { course_id: course.id, grading_periods: [period_1_params] }
end.not_to change { course.grading_periods.count }
expect(response.status).to eql(Rack::Utils.status_code(:unauthorized))
end
it "cannot create multiple grading periods" do
expect do
patch :batch_update, {
course_id: course.id,
grading_periods: [period_1_params, period_2_params]
describe "with account associated grading periods" do
let(:period_1_params) do
{
title: 'Original Title',
start_date: 2.days.ago(now).to_s,
end_date: 2.days.from_now(now).to_s
}
end.not_to change { course.grading_periods.count }
end
end
let(:group) { group_helper.create_for_account(root_account) }
let(:period_1) { group.grading_periods.create!(period_1_params) }
it "can update a single grading period" do
patch :batch_update, { course_id: course.id, grading_periods: [
period_1_params.merge(id: period_1.id, title: 'Original Title')
] }
expect(course.grading_periods.find(period_1.id).title).to eql('Original Title')
end
before(:each) do
login_admin
course.enrollment_term.update_attribute(:grading_period_group_id, group)
end
it "can update multiple grading periods" do
patch :batch_update, { course_id: course.id, grading_periods: [
period_1_params.merge(id: period_1.id, title: 'Original Title'),
period_2_params.merge(id: period_2.id, title: 'Updated Title')
] }
expect(course.grading_periods.find(period_1.id).title).to eql('Original Title')
expect(course.grading_periods.find(period_2.id).title).to eql('Updated Title')
end
it "cannot create and update multiple grading periods" do
period_1 = group.grading_periods.create!(period_1_params)
expect do
it "cannot update any grading periods" do
patch :batch_update, { course_id: course.id, grading_periods: [
period_1_params.merge(id: period_1.id, title: 'Original Title'),
period_2_params
period_1_params.merge(id: period_1.id, title: 'Updated Title')
] }
end.not_to change { course.grading_periods.count }
expect(course.grading_periods.find(period_1.id).title).not_to eql('Original Title')
expect(period_1.reload.title).to eql('Original Title')
expect(GradingPeriod.for(course).find(period_1.id).title).to eql('Original Title')
end
it "responds with json upon failure" do
patch :batch_update, { course_id: course.id, grading_periods: [
period_1_params.merge(id: period_1.id, title: 'Updated Title')
] }
expect(response.status).to eql(Rack::Utils.status_code(:unauthorized))
json = JSON.parse(response.body)
expect(json['errors']).to be_present
expect(json).not_to have_key('grading_periods')
end
end
end
it "responds with json upon success" do
patch :batch_update, { course_id: course.id, grading_periods: [] }
expect(response).to be_ok
json = JSON.parse(response.body)
expect(json['grading_periods']).to be_empty
expect(json).not_to includes('errors')
end
it "responds with json upon failure" do
period = period_helper.create_with_group_for_course(course)
patch :batch_update, { course_id: course.id, grading_periods: [{id: period.id, title: ''}] }
expect(response).not_to be_ok
json = JSON.parse(response.body)
expect(json['errors']).to be_present
expect(json).not_to have_key('grading_periods')
end
end
describe "PATCH batch_update with a course for account-level grading periods" do
let(:period_1_params) do
{
title: 'Original Title',
start_date: 2.days.ago(now).to_s,
end_date: 2.days.from_now(now).to_s
}
end
let(:group) { group_helper.create_for_account(root_account) }
let(:period_1) { group.grading_periods.create!(period_1_params) }
before(:each) do
user = User.create!
root_account.account_users.create!(:user => user)
user_session(user)
course.enrollment_term.update_attribute(:grading_period_group_id, group)
end
it "cannot update any grading periods" do
patch :batch_update, { course_id: course.id, grading_periods: [
period_1_params.merge(id: period_1.id, title: 'Updated Title')
] }
expect(period_1.reload.title).to eql('Original Title')
expect(GradingPeriod.for(course).find(period_1.id).title).to eql('Original Title')
end
it "responds with json upon failure" do
patch :batch_update, { course_id: course.id, grading_periods: [
period_1_params.merge(id: period_1.id, title: 'Updated Title')
] }
expect(response.status).to eql(Rack::Utils.status_code(:unauthorized))
json = JSON.parse(response.body)
expect(json['errors']).to be_present
expect(json).not_to have_key('grading_periods')
end
end
describe "DELETE destroy with a course for course-level grading periods" do
it "can destroy any grading periods" do
user = User.create!
root_account.account_users.create!(:user => user)
user_session(user)
group = group_helper.legacy_create_for_course(course)
period = period_helper.create_for_group(group)
course.enrollment_term.update_attribute(:grading_period_group_id, group)
delete :destroy, { course_id: course.id, id: period.to_param }
expect(period.reload).to be_deleted
end
end
describe "DELETE destroy with a course for account-level grading periods" do
let(:group) { group_helper.create_for_account(root_account) }
let(:period) { period_helper.create_for_group(group) }
before(:each) do
user = User.create!
root_account.account_users.create!(:user => user)
user_session(user)
course.enrollment_term.update_attribute(:grading_period_group_id, group)
end
it "cannot destroy any grading periods" do
delete :destroy, { course_id: course.id, id: period.to_param }
expect(period.reload).not_to be_deleted
expect(GradingPeriod.for(course).find(period.id).title).to eql('Example Grading Period')
end
it "responds with json upon failure" do
delete :destroy, { course_id: course.id, id: period.to_param }
expect(response.status).to eql(Rack::Utils.status_code(:unauthorized))
json = JSON.parse(response.body)
expect(json['errors']).to be_present
expect(json).not_to have_key('grading_periods')
end
end
end

66
spec/models/grading_period_spec.rb
View File

@ -196,6 +196,58 @@ describe GradingPeriod do
period_2.save
expect(GradingPeriod.for(@course)).to match_array([period_1])
end
it "does not include grading periods from the course enrollment term group if inherit is false" do
group = group_helper.create_for_account(@root_account)
term.update_attribute(:grading_period_group_id, group)
period_1 = period_helper.create_with_weeks_for_group(group, 5, 3)
period_2 = period_helper.create_with_weeks_for_group(group, 3, 1)
period_2.workflow_state = :deleted
period_2.save
expect(GradingPeriod.for(@course, inherit: false)).to match_array([])
end
end
context "when context is an account" do
before(:once) do
@root_account = account_model
@sub_account = @root_account.sub_accounts.create!
@course = Course.create!(account: @sub_account)
end
it "finds all grading periods on an account" do
group_1 = group_helper.create_for_account(@root_account)
group_2 = group_helper.create_for_account(@root_account)
period_1 = period_helper.create_with_weeks_for_group(group_1, 5, 3)
period_2 = period_helper.create_with_weeks_for_group(group_2, 3, 1)
expect(GradingPeriod.for(@root_account)).to match_array([period_1, period_2])
end
it "returns an empty array when the account has no grading period groups" do
expect(GradingPeriod.for(@root_account)).to match_array([])
end
it "returns an empty array when the account has no grading periods" do
group_helper.create_for_account(@root_account)
expect(GradingPeriod.for(@root_account)).to match_array([])
end
it "does not return grading periods on the course directly" do
group = group_helper.legacy_create_for_course(@course)
period_1 = period_helper.create_with_weeks_for_group(group, 5, 3)
period_2 = period_helper.create_with_weeks_for_group(group, 3, 1)
expect(GradingPeriod.for(@root_account)).to match_array([])
end
it "includes only 'active' grading periods from the account grading period group" do
group_1 = group_helper.create_for_account(@root_account)
group_2 = group_helper.create_for_account(@root_account)
period_1 = period_helper.create_with_weeks_for_group(group_1, 5, 3)
period_2 = period_helper.create_with_weeks_for_group(group_2, 3, 1)
period_2.workflow_state = :deleted
period_2.save
expect(GradingPeriod.for(@root_account)).to match_array([period_1])
end
end
end
@ -223,20 +275,6 @@ describe GradingPeriod do
end
end
describe ".context_find" do
let(:account) { mock }
let(:finder) { mock }
let(:grading_period) { mock }
let(:id) { 1 }
it "delegates" do
grading_period.expects(:id).returns(1)
GradingPeriod.expects(:for).with(account).returns([grading_period])
expect(GradingPeriod.context_find(account, id)).to eq grading_period
end
end
describe "#assignments" do
let!(:first_assignment) { course.assignments.create!(due_at: first_grading_period.start_date + 1.second) }
let!(:second_assignment) { course.assignments.create!(due_at: second_grading_period.start_date + 1.seconds) }