diff --git a/app/coffeescripts/ember/screenreader_gradebook/tests/integration/muter_component.spec.coffee b/app/coffeescripts/ember/screenreader_gradebook/tests/integration/muter_component.spec.coffee index 56ac07cf381..39669210b37 100644 --- a/app/coffeescripts/ember/screenreader_gradebook/tests/integration/muter_component.spec.coffee +++ b/app/coffeescripts/ember/screenreader_gradebook/tests/integration/muter_component.spec.coffee @@ -65,7 +65,7 @@ define [ sendSuccess(server, "#{ENV.GRADEBOOK_OPTIONS.context_url}/assignments/#{@con.get('selectedAssignment.id')}/mute", false) andThen => dialog = find('.ui-dialog:visible', 'body') - equal(dialog.length, 0, 'the dialog is closed WOOOOOO') + equal(dialog.length, 0, 'the dialog is closed') checkChecked(false) checkLabel(ariaUnmuted) equal @con.get('selectedAssignment.muted'), false @@ -103,7 +103,7 @@ define [ sendSuccess(server, "#{ENV.GRADEBOOK_OPTIONS.context_url}/assignments/#{@con.get('selectedAssignment.id')}/mute", true) andThen => dialog = find('.ui-dialog:visible', 'body') - equal(dialog.length, 0, 'the dialog is closed WOOOOOO') + equal(dialog.length, 0, 'the dialog is closed') checkChecked(true) checkLabel(ariaMuted) equal @con.get('selectedAssignment.muted'), true diff --git a/app/controllers/assignments_api_controller.rb b/app/controllers/assignments_api_controller.rb index 59f05bfc98d..ff380acd6d7 100644 --- a/app/controllers/assignments_api_controller.rb +++ b/app/controllers/assignments_api_controller.rb @@ -952,6 +952,8 @@ class AssignmentsApiController < ApplicationController end end + private + def save_and_render_response @assignment.content_being_saved_by(@current_user) if update_api_assignment(@assignment, params[:assignment], @current_user) diff --git a/app/controllers/courses_controller.rb b/app/controllers/courses_controller.rb index f9bb623d7b4..8c90e542db4 100644 --- a/app/controllers/courses_controller.rb +++ b/app/controllers/courses_controller.rb @@ -1932,7 +1932,7 @@ class CoursesController < ApplicationController standard_id = params[:course].delete :grading_standard_id if @course.grants_right?(@current_user, session, :manage_grades) if standard_id.present? - grading_standard = GradingStandard.standards_for(@course).where(id: standard_id).first + grading_standard = GradingStandard.for(@course).where(id: standard_id).first @course.grading_standard = grading_standard if grading_standard else @course.grading_standard = nil diff --git a/app/controllers/grading_periods_controller.rb b/app/controllers/grading_periods_controller.rb index 8223ef19a81..60dc322fd84 100644 --- a/app/controllers/grading_periods_controller.rb +++ b/app/controllers/grading_periods_controller.rb @@ -68,10 +68,11 @@ class GradingPeriodsController < ApplicationController # def index if authorized_action(@context, @current_user, :read) - # inheritance check instead of #get_context? - @grading_periods = @context.grading_periods.active.order('start_date') - json, meta = paginate_for(@grading_periods) - render json: serialize_jsonapi(json, meta) + # TODO: inheritance check instead of #get_context? + grading_periods = GradingPeriod.for(@context).sort_by(&:start_date) + paginated_grading_periods, meta = paginate_for(grading_periods) + + render json: serialize_json_api(paginated_grading_periods, meta) end end @@ -87,8 +88,10 @@ class GradingPeriodsController < ApplicationController # def show @grading_period = @context.grading_periods.active.find(params[:id]) + # TODO: if there is no grading period found then this action will return + # an empty body which is probably not what we want if @grading_period && authorized_action(@grading_period, @current_user, :read) - render json: serialize_jsonapi(@grading_period) + render json: serialize_json_api(@grading_period) end end @@ -119,7 +122,7 @@ class GradingPeriodsController < ApplicationController @grading_period = grading_period_group.grading_periods.new(grading_period_params) if @grading_period && authorized_action(@grading_period, @current_user, :manage) if @grading_period.save - render json: serialize_jsonapi(@grading_period) + render json: serialize_json_api(@grading_period) else render json: @grading_period.errors, status: :bad_request end @@ -150,7 +153,7 @@ class GradingPeriodsController < ApplicationController if @grading_period && authorized_action(@grading_period, @current_user, :manage) if @grading_period.update_attributes(grading_period_params) - render json: serialize_jsonapi(@grading_period) + render json: serialize_json_api(@grading_period) else render json: @grading_period.errors, status: :bad_request end @@ -170,15 +173,15 @@ class GradingPeriodsController < ApplicationController end end - protected + private + def paginate_for(grading_periods) - meta = {} - grading_periods, meta = Api.jsonapi_paginate(grading_periods, self, named_context_url(@context, :api_v1_context_grading_periods_url)) + paginated_grading_periods, meta = Api.jsonapi_paginate(grading_periods, self, named_context_url(@context, :api_v1_context_grading_periods_url)) meta[:primaryCollection] = 'grading_periods' - return grading_periods, meta + [paginated_grading_periods, meta] end - def serialize_jsonapi(grading_periods, meta = {}) + def serialize_json_api(grading_periods, meta = {}) grading_periods = Array.wrap(grading_periods) Canvas::APIArraySerializer.new(grading_periods, { diff --git a/app/controllers/grading_standards_controller.rb b/app/controllers/grading_standards_controller.rb index 483d86e24ec..1c0fd8edc86 100644 --- a/app/controllers/grading_standards_controller.rb +++ b/app/controllers/grading_standards_controller.rb @@ -29,7 +29,7 @@ class GradingStandardsController < ApplicationController :MULTIPLE_GRADING_PERIODS => multiple_grading_periods?, :DEFAULT_GRADING_STANDARD_DATA => GradingStandard.default_grading_standard }) - @standards = GradingStandard.standards_for(@context).sorted.limit(100) + @standards = GradingStandard.for(@context).sorted.limit(100) respond_to do |format| format.html { } format.json { @@ -58,7 +58,7 @@ class GradingStandardsController < ApplicationController end def update - @standard = GradingStandard.standards_for(@context).find(params[:id]) + @standard = GradingStandard.for(@context).find(params[:id]) if authorized_action(@standard, @current_user, :manage) @standard.user = @current_user respond_to do |format| @@ -72,7 +72,7 @@ class GradingStandardsController < ApplicationController end def destroy - @standard = GradingStandard.standards_for(@context).find(params[:id]) + @standard = GradingStandard.for(@context).find(params[:id]) if authorized_action(@standard, @current_user, :manage) respond_to do |format| if @standard.destroy @@ -83,4 +83,11 @@ class GradingStandardsController < ApplicationController end end end + + private + + def default_data + GradingStandard.default_grading_standard + end + end diff --git a/app/models/grading_period.rb b/app/models/grading_period.rb index 3d4d74d49fd..1a67846e98a 100644 --- a/app/models/grading_period.rb +++ b/app/models/grading_period.rb @@ -1,7 +1,8 @@ class GradingPeriod < ActiveRecord::Base - attr_accessible :weight, :start_date, :end_date, :title include Workflow + attr_accessible :weight, :start_date, :end_date, :title + belongs_to :grading_period_group, :inverse_of => :grading_periods has_many :grading_period_grades @@ -9,9 +10,9 @@ class GradingPeriod < ActiveRecord::Base validate :validate_dates set_policy do - [:read, :manage].each do |permission| - given { |user| grading_period_group.grants_right?(user, permission) } - can permission + [:read, :manage].each do |action| + given { |user| grading_period_group.grants_right?(user, action) } + can action end end @@ -22,34 +23,44 @@ class GradingPeriod < ActiveRecord::Base scope :active, -> { where workflow_state: "active" } scope :current, -> { where("start_date <= ? AND end_date >= ?", Time.now, Time.now) } + scope :grading_periods_by, ->(context_with_ids) { + joins(:grading_period_group).where(grading_period_groups: context_with_ids) + } + def self.for(context) + "GradingPeriod::#{context.class}GradingPeriodFinder".constantize.new(context).grading_periods + end + + # save the previous definition of `destroy` and alias it to `destroy!` + # Note: `destroy!` now does NOT throw errors while the newly defined + # `destroy` DOES throw errors due to `save!` alias_method :destroy!, :destroy def destroy - update_attribute :workflow_state, "deleted" + self.workflow_state = 'deleted' + save! end def assignments(assignment_scope) # TODO: avoid wasteful queries - assignments = assignment_scope.where( - "due_at BETWEEN ? AND ?", - start_date, end_date - ).all + assignments = assignment_scope.where( "due_at BETWEEN ? AND ?", start_date, end_date) - if self.last? + if last? assignments + assignment_scope.where(due_at: nil) else assignments end end + private + def last? grading_period_group.grading_periods.last == self end def validate_dates - if self.start_date && self.end_date - errors.add(:end_date, t('errors.invalid_grading_period_end_date', "Grading period end date precedes start date")) if self.end_date < self.start_date + if start_date && end_date && end_date < start_date + errors.add(:end_date, t('errors.invalid_grading_period_end_date', + 'Grading period end date precedes start date')) end end - private :validate_dates end diff --git a/app/models/grading_period/account_grading_period_finder.rb b/app/models/grading_period/account_grading_period_finder.rb new file mode 100644 index 00000000000..04cadf711d3 --- /dev/null +++ b/app/models/grading_period/account_grading_period_finder.rb @@ -0,0 +1,34 @@ +# +# Copyright (C) 2015 Instructure, Inc. +# +# This file is part of Canvas. +# +# Canvas is free software: you can redistribute it and/or modify it under +# the terms of the GNU Affero General Public License as published by the Free +# Software Foundation, version 3 of the License. +# +# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY +# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR +# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more +# details. +# +# You should have received a copy of the GNU Affero General Public License along +# with this program. If not, see . +# + +class GradingPeriod + class AccountGradingPeriodFinder + + def initialize(account) + @account = account + end + + def grading_periods + account_ids = Account.all_accounts_for(account).map(&:id).uniq + GradingPeriod.active.grading_periods_by(account_id: account_ids) + end + + private + attr_reader :account + end +end diff --git a/app/models/grading_period/course_grading_period_finder.rb b/app/models/grading_period/course_grading_period_finder.rb new file mode 100644 index 00000000000..a8bf5bfd062 --- /dev/null +++ b/app/models/grading_period/course_grading_period_finder.rb @@ -0,0 +1,36 @@ +# +# Copyright (C) 2015 Instructure, Inc. +# +# This file is part of Canvas. +# +# Canvas is free software: you can redistribute it and/or modify it under +# the terms of the GNU Affero General Public License as published by the Free +# Software Foundation, version 3 of the License. +# +# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY +# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR +# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more +# details. +# +# You should have received a copy of the GNU Affero General Public License along +# with this program. If not, see . +# + +class GradingPeriod + class CourseGradingPeriodFinder + + def initialize(course) + @course = course + end + + def grading_periods + account_ids = Account.all_accounts_for(course.account).map(&:id).uniq + + GradingPeriod.active.grading_periods_by(course_id: course.id) + + GradingPeriod.active.grading_periods_by(account_id: account_ids) + end + + private + attr_reader :course + end +end diff --git a/app/models/grading_standard.rb b/app/models/grading_standard.rb index 7512a9d5fee..5254195d032 100644 --- a/app/models/grading_standard.rb +++ b/app/models/grading_standard.rb @@ -69,6 +69,12 @@ class GradingStandard < ActiveRecord::Base can :manage end + def self.for(context) + context_codes = [context.asset_string] + context_codes.concat Account.all_accounts_for(context).map(&:asset_string) + GradingStandard.active.where(:context_code => context_codes.uniq) + end + def version read_attribute(:version).presence || 1 end @@ -189,12 +195,6 @@ class GradingStandard < ActiveRecord::Base res end - def self.standards_for(context) - context_codes = [context.asset_string] - context_codes.concat Account.all_accounts_for(context).map(&:asset_string) - GradingStandard.active.where(:context_code => context_codes.uniq) - end - def standard_data=(params={}) params ||= {} res = {} diff --git a/app/models/importers/assignment_importer.rb b/app/models/importers/assignment_importer.rb index ccf20851e80..de5a64b52c0 100644 --- a/app/models/importers/assignment_importer.rb +++ b/app/models/importers/assignment_importer.rb @@ -127,7 +127,7 @@ module Importers gs = context.grading_standards.where(migration_id: hash[:grading_standard_migration_id]).first item.grading_standard = gs if gs elsif hash[:grading_standard_id] && migration - gs = GradingStandard.standards_for(context).where(id: hash[:grading_standard_id]).first unless migration.cross_institution? + gs = GradingStandard.for(context).where(id: hash[:grading_standard_id]).first unless migration.cross_institution? if gs item.grading_standard = gs if gs else diff --git a/app/models/importers/course_content_importer.rb b/app/models/importers/course_content_importer.rb index 96a6d5fbe4b..01696c9480b 100644 --- a/app/models/importers/course_content_importer.rb +++ b/app/models/importers/course_content_importer.rb @@ -275,7 +275,7 @@ module Importers migration.add_warning(t(:copied_grading_standard_warning, "Couldn't find copied grading standard for the course.")) end elsif settings[:grading_standard_id].present? - if gs = GradingStandard.standards_for(course).where(id: settings[:grading_standard_id]).first + if gs = GradingStandard.for(course).where(id: settings[:grading_standard_id]).first course.grading_standard = gs else migration.add_warning(t(:account_grading_standard_warning,"Couldn't find account grading standard for the course." )) diff --git a/config/routes.rb b/config/routes.rb index ec7efdcda18..e6dbbb27583 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1373,6 +1373,7 @@ CanvasRails::Application.routes.draw do put 'courses/:course_id/quizzes/:quiz_id/submissions/:id', action: :update, as: 'course_quiz_submission_update' post 'courses/:course_id/quizzes/:quiz_id/submissions/:id/complete', action: :complete, as: 'course_quiz_submission_complete' end + scope(:controller => 'quizzes/outstanding_quiz_submissions') do get 'courses/:course_id/quizzes/:quiz_id/outstanding_quiz_submissions', :action => :index, :path_name => 'outstanding_quiz_submission_index' post 'courses/:course_id/quizzes/:quiz_id/outstanding_quiz_submissions', :action => :grade, :path_name => 'outstanding_quiz_submission_grade' diff --git a/lib/api/v1/assignment.rb b/lib/api/v1/assignment.rb index 96fc9d29aec..f0ca7907735 100644 --- a/lib/api/v1/assignment.rb +++ b/lib/api/v1/assignment.rb @@ -386,7 +386,7 @@ module Api::V1::Assignment if update_params.has_key?("grading_standard_id") standard_id = update_params.delete("grading_standard_id") if standard_id.present? - grading_standard = GradingStandard.standards_for(context).where(id: standard_id).first + grading_standard = GradingStandard.for(context).where(id: standard_id).first assignment.grading_standard = grading_standard if grading_standard else assignment.grading_standard = nil diff --git a/app/models/open_object.rb b/lib/open_object.rb similarity index 100% rename from app/models/open_object.rb rename to lib/open_object.rb diff --git a/spec/apis/v1/grading_periods_api_spec.rb b/spec/apis/v1/grading_periods_api_spec.rb index 28d058ad23e..7c629dd184c 100644 --- a/spec/apis/v1/grading_periods_api_spec.rb +++ b/spec/apis/v1/grading_periods_api_spec.rb @@ -19,6 +19,7 @@ require File.expand_path(File.dirname(__FILE__) + '/../api_spec_helper') describe GradingPeriodsController, type: :request do + context "A grading period is associated with an account." do before :once do @account = Account.default account_admin_user(account: @account) @@ -27,42 +28,7 @@ describe GradingPeriodsController, type: :request do @account.grading_periods.last.destroy end - context "A grading period is associated with an account." do context "multiple grading periods feature flag turned on" do - describe 'GET index' do - def get_index(raw = false, data = {}, headers = {}) - helper = method(raw ? :raw_api_call : :api_call) - helper.call(:get, - "/api/v1/accounts/#{@account.id}/grading_periods", - { controller: 'grading_periods', action: 'index', format: 'json', account_id: @account.id }, - data, - headers) - end - - it "returns all existing grading periods" do - json = get_index - periods_json = json['grading_periods'] - expect(periods_json.size).to eq(2) - - periods_json.each do |period| - expect(period).to have_key('id') - expect(period).to have_key('weight') - expect(period).to have_key('start_date') - expect(period).to have_key('end_date') - expect(period).to have_key('title') - expect(period).to have_key('permissions') - end - end - - it "paginates to the jsonapi standard" do - json = get_index(false, {}, 'Accept' => 'application/vnd.api+json') - - expect(json).to have_key('meta') - expect(json['meta']).to have_key('pagination') - expect(json['meta']['primaryCollection']).to eq 'grading_periods' - end - end - describe 'GET show' do before :once do @grading_period = @account.grading_periods.first @@ -186,40 +152,6 @@ describe GradingPeriodsController, type: :request do end context "multiple grading periods feature flag turned on" do - describe 'GET index' do - def get_index(raw = false, data = {}, headers = {}) - helper = method(raw ? :raw_api_call : :api_call) - helper.call(:get, - "/api/v1/courses/#{@course.id}/grading_periods", - { controller: 'grading_periods', action: 'index', format: 'json', course_id: @course.id }, - data, - headers) - end - - it "returns all existing grading periods" do - json = get_index - periods_json = json['grading_periods'] - expect(periods_json.size).to eq(2) - - periods_json.each do |period| - expect(period).to have_key('id') - expect(period).to have_key('weight') - expect(period).to have_key('start_date') - expect(period).to have_key('end_date') - expect(period).to have_key('title') - expect(period).to have_key('permissions') - end - end - - it "paginates to the jsonapi standard" do - json = get_index(false, {}, 'Accept' => 'application/vnd.api+json') - - expect(json).to have_key('meta') - expect(json['meta']).to have_key('pagination') - expect(json['meta']['primaryCollection']).to eq 'grading_periods' - end - end - describe 'GET show' do before :once do @grading_period = @course.grading_periods.first diff --git a/spec/controllers/grading_periods_controller_spec.rb b/spec/controllers/grading_periods_controller_spec.rb new file mode 100644 index 00000000000..7b2e20ebb17 --- /dev/null +++ b/spec/controllers/grading_periods_controller_spec.rb @@ -0,0 +1,86 @@ +# +# Copyright (C) 2015 Instructure, Inc. +# +# This file is part of Canvas. +# +# Canvas is free software: you can redistribute it and/or modify it under +# the terms of the GNU Affero General Public License as published by the Free +# Software Foundation, version 3 of the License. +# +# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY +# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR +# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more +# details. +# +# You should have received a copy of the GNU Affero General Public License along +# with this program. If not, see . +# +require_relative '../spec_helper' + +describe GradingPeriodsController do + describe "GET index" do + let(:root_account) { Account.default } + let(:sub_account) { root_account.sub_accounts.create! } + + let(:create_grading_periods) { -> (context, start) { + context.grading_period_groups.create! + .grading_periods.create!(weight: 1, + start_date: start, + end_date: (Time.zone.now + 10.days)) + } + } + + let(:remove_while) { -> (string) { + string.sub(%r{^while\(1\);}, '') + } + } + + before do + account_admin_user(account: sub_account) + user_session(@admin) + + root_account.allow_feature!(:multiple_grading_periods) + root_account.enable_feature!(:multiple_grading_periods) + + create_grading_periods.call(root_account, 3.days.ago) + create_grading_periods.call(sub_account, Time.zone.now) + end + + context "when context is a sub account" do + before do + get :index, {account_id: sub_account.id} + @json = JSON.parse(remove_while.call(response.body)) + end + + it "contains two grading periods" do + expect(@json['grading_periods'].count).to eql 2 + end + + it "paginates" do + expect(@json).to have_key('meta') + expect(@json['meta']).to have_key('pagination') + expect(@json['meta']['primaryCollection']).to eq 'grading_periods' + end + + it "is ordered by start_date" do + expect(@json['grading_periods']).to be_sorted_by('start_date') + end + end + + context "when context is a course" do + before do + course = Course.create!(account: sub_account) + + create_grading_periods.call(course, 5.days.ago) + + get :index, {course_id: course.id} + @json = JSON.parse(remove_while.call(response.body)) + end + + it "contains three grading periods" do + expect(@json['grading_periods'].count).to eql 3 + end + end + end +end + diff --git a/spec/models/grading_period/account_grading_period_finder_spec.rb b/spec/models/grading_period/account_grading_period_finder_spec.rb new file mode 100644 index 00000000000..a2ebd6b52ea --- /dev/null +++ b/spec/models/grading_period/account_grading_period_finder_spec.rb @@ -0,0 +1,39 @@ +# +# Copyright (C) 2015 Instructure, Inc. +# +# This file is part of Canvas. +# +# Canvas is free software: you can redistribute it and/or modify it under +# the terms of the GNU Affero General Public License as published by the Free +# Software Foundation, version 3 of the License. +# +# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY +# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR +# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more +# details. +# +# You should have received a copy of the GNU Affero General Public License along +# with this program. If not, see . +# + +require 'spec_helper' + +describe GradingPeriod::AccountGradingPeriodFinder do + describe "#grading_periods" do + let(:grading_period_params) { { weight: 0, start_date: Time.now, end_date: 2.days.from_now } } + let(:root_account) { Account.create! } + let(:sub_account) { root_account.sub_accounts.create! } + let!(:root_account_grading_period) { root_account.grading_period_groups.create! + .grading_periods.create!(grading_period_params) } + let!(:sub_account_grading_period) { sub_account.grading_period_groups.create! + .grading_periods.create!(grading_period_params) } + + context "when context is an account" do + subject { GradingPeriod::AccountGradingPeriodFinder.new(sub_account).grading_periods } + it "finds grading periods for the account, and all associated accounts" do + + is_expected.to eq [root_account_grading_period, sub_account_grading_period] + end + end + end +end diff --git a/spec/models/grading_period/course_grading_period_finder_spec.rb b/spec/models/grading_period/course_grading_period_finder_spec.rb new file mode 100644 index 00000000000..4d7cc72a58b --- /dev/null +++ b/spec/models/grading_period/course_grading_period_finder_spec.rb @@ -0,0 +1,41 @@ +# +# Copyright (C) 2015 Instructure, Inc. +# +# This file is part of Canvas. +# +# Canvas is free software: you can redistribute it and/or modify it under +# the terms of the GNU Affero General Public License as published by the Free +# Software Foundation, version 3 of the License. +# +# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY +# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR +# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more +# details. +# +# You should have received a copy of the GNU Affero General Public License along +# with this program. If not, see . +# + +require 'spec_helper' + +describe GradingPeriod::CourseGradingPeriodFinder do + describe "#grading_periods" do + let(:grading_period_params) { { weight: 0, start_date: Time.now, end_date: 2.days.from_now } } + let(:root_account) { Account.create! } + let(:sub_account) { root_account.sub_accounts.create! } + let(:course) { Course.create!(account: sub_account) } + + let!(:course_grading_period) { course.grading_period_groups.create! + .grading_periods.create!(grading_period_params) } + let!(:root_account_grading_period) { root_account.grading_period_groups.create! + .grading_periods.create!(grading_period_params) } + let!(:sub_account_grading_period) { sub_account.grading_period_groups.create! + .grading_periods.create!(grading_period_params) } + + it "finds grading periods for the course, and all associated accounts" do + grading_periods = GradingPeriod::CourseGradingPeriodFinder.new(course).grading_periods + expect(grading_periods).to eq [course_grading_period, root_account_grading_period, sub_account_grading_period] + end + end +end + diff --git a/spec/models/grading_period_permissions_spec.rb b/spec/models/grading_period_permissions_spec.rb new file mode 100644 index 00000000000..a46a6ae869c --- /dev/null +++ b/spec/models/grading_period_permissions_spec.rb @@ -0,0 +1,199 @@ +# +# Copyright (C) 2015 Instructure, Inc. +# +# This file is part of Canvas. +# +# Canvas is free software: you can redistribute it and/or modify it under +# the terms of the GNU Affero General Public License as published by the Free +# Software Foundation, version 3 of the License. +# +# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY +# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR +# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more +# details. +# +# You should have received a copy of the GNU Affero General Public License along +# with this program. If not, see . +# + +require_relative '../spec_helper' + +describe GradingPeriod, "permissions:" do + context "course belonging to root account" do + before(:once) do + @root_account = Account.default + @sub_account = @root_account.sub_accounts.create! + course_with_teacher(account: @root_account, active_all: true) + course_with_student(course: @course, active_all: true) + @root_account_period = grading_periods(context: @root_account, count: 1).first + @sub_account_period = grading_periods(context: @sub_account, count: 1).first + @course_period = grading_periods(context: @course, count: 1).first + end + + context "root-account admin" do + before(:once) do + account_admin_user(account: @root_account) + end + + it "should be able to read and manage root-account level grading periods" do + expect(@root_account_period.rights_status(@admin, :read, :manage)).to eq({ read: true, manage: true }) + end + + it "should be able to read and manage sub-account level grading periods" do + expect(@sub_account_period.rights_status(@admin, :read, :manage)).to eq({ read: true, manage: true }) + end + + it "should be able to read and manage course level grading periods" do + expect(@course_period.rights_status(@admin, :read, :manage)).to eq({ read: true, manage: true }) + end + end + + context "sub-account admin" do + before(:once) do + account_admin_user(account: @sub_account) + end + + it "should NOT be able to manage root-account level grading periods, but should be able to read them" do + expect(@root_account_period.rights_status(@admin, :read, :manage)).to eq({ read: true, manage: false }) + end + + it "should be able to read and manage sub-account level grading periods" do + expect(@sub_account_period.rights_status(@admin, :read, :manage)).to eq({ read: true, manage: true }) + end + + + it "should NOT be able to read or manage course level grading periods, when the course is under the root-account" do + expect(@course_period.rights_status(@admin, :read, :manage)).to eq({ read: false, manage: false }) + end + end + + context "teacher" do + it "should NOT be able to manage root-account level grading periods, but should be able to read them" do + expect(@root_account_period.rights_status(@teacher, :read, :manage)).to eq({ read: true, manage: false }) + end + + it "should NOT be able to read or manage sub-account level grading period" do + expect(@sub_account_period.rights_status(@teacher, :read, :manage)).to eq({ read: false, manage: false }) + end + + it "should be able to read and manage course level grading periods" do + expect(@course_period.rights_status(@teacher, :read, :manage)).to eq({ read: true, manage: true}) + end + end + + context "student" do + it "should NOT be able to manage root-account level grading periods, but should be able to read them" do + expect(@root_account_period.rights_status(@student, :read, :manage)).to eq({ read: true, manage: false }) + end + + it "should NOT be able to read or manage sub-account level grading period" do + expect(@sub_account_period.rights_status(@student, :read, :manage)).to eq({ read: false, manage: false }) + end + + it "should NOT be able to manage course level grading periods, but should be able to read them" do + expect(@course_period.rights_status(@student, :read, :manage)).to eq({ read: true, manage: false}) + end + end + + context "multiple grading periods feature flag turned off" do + before(:once) do + account_admin_user(account: @root_account) + @root_account.disable_feature! :multiple_grading_periods + end + + it "should return false for all permissions" do + expect(@course_period.rights_status(@admin, :read, :manage)).to eq({ read: false, manage: false }) + end + end + end + + context "course belonging to sub-account" do + before(:once) do + @root_account = Account.default + @sub_account = @root_account.sub_accounts.create! + course_with_teacher(account: @sub_account, active_all: true) + course_with_student(course: @course, active_all: true) + @root_account_period = grading_periods(context: @root_account, count: 1).first + @sub_account_period = grading_periods(context: @sub_account, count: 1).first + @course_period = grading_periods(context: @course, count: 1).first + end + + context "root-account admin" do + before(:once) do + account_admin_user(account: @root_account) + end + + it "should be able to read and manage root-account level grading periods" do + expect(@root_account_period.rights_status(@admin, :read, :manage)).to eq({ read: true, manage: true }) + end + + it "should be able to read and manage sub-account level grading periods" do + expect(@sub_account_period.rights_status(@admin, :read, :manage)).to eq({ read: true, manage: true }) + end + + + it "should be able to read and manage course level grading periods" do + expect(@course_period.rights_status(@admin, :read, :manage)).to eq({ read: true, manage: true }) + end + end + + context "sub-account admin" do + before(:once) do + account_admin_user(account: @sub_account) + end + + it "should NOT be able to manage root-account level grading periods, but should be able to read them" do + expect(@root_account_period.rights_status(@admin, :read, :manage)).to eq({ read: true, manage: false }) + end + + it "should be able to read and manage sub-account level grading periods" do + expect(@sub_account_period.rights_status(@admin, :read, :manage)).to eq({read: true, manage: true }) + end + + + it "should be able to read and manage course level grading periods, when the course is under the sub-account" do + expect(@course_period.rights_status(@admin, :read, :manage)).to eq({read: true, manage: true }) + end + end + + context "teacher" do + it "should NOT be able to manage root-account level grading periods, but should be able to read them" do + expect(@root_account_period.rights_status(@teacher, :read, :manage)).to eq({ read: true, manage: false }) + end + + it "should NOT be able to manage sub-account level grading periods, but should be able to read them" do + expect(@sub_account_period.rights_status(@teacher, :read, :manage)).to eq({ read: true, manage: false }) + end + + it "should be able to read and manage course level grading periods" do + expect(@course_period.rights_status(@teacher, :read, :manage)).to eq({ read: true, manage: true }) + end + end + + context "student" do + it "should NOT be able to manage root-account level grading periods, but should be able to read them" do + expect(@root_account_period.rights_status(@student, :read, :manage)).to eq({ read: true, manage: false }) + end + + it "should NOT be able to manage sub-account level grading periods, but should be able to read them" do + expect(@sub_account_period.rights_status(@student, :read, :manage)).to eq({ read: true, manage: false }) + end + + it "should NOT be able to manage course level grading periods, but should be able to read them" do + expect(@course_period.rights_status(@student, :read, :manage)).to eq({ read: true, manage: false}) + end + end + + context "multiple grading periods feature flag turned off" do + before(:once) do + account_admin_user(account: @sub_account) + @root_account.disable_feature! :multiple_grading_periods + end + + it "should return false for all permissions" do + expect(@course_period.rights_status(@admin, :read, :manage)).to eq({ read: false, manage: false }) + end + end + end +end + diff --git a/spec/models/grading_period_spec.rb b/spec/models/grading_period_spec.rb index f8b816ea623..3cf44c5aa31 100644 --- a/spec/models/grading_period_spec.rb +++ b/spec/models/grading_period_spec.rb @@ -16,247 +16,88 @@ # with this program. If not, see . # -require File.expand_path(File.dirname(__FILE__) + '/../spec_helper.rb') +require_relative '../spec_helper' describe GradingPeriod do - before :once do - @grading_period = GradingPeriod.create(course_id: 1, start_date: Time.zone.now, end_date: 1.day.from_now) + subject { GradingPeriod.new(params) } + + let(:params) do + { course_id: 1, start_date: Time.zone.now, end_date: 1.day.from_now } end - describe "validation" do - it "should be valid with appropriate input" do - expect(@grading_period).to be_valid + it { is_expected.to be_valid } + + it "requires a start_date" do + grading_period = GradingPeriod.new(params.except(:start_date)) + expect(grading_period).to_not be_valid + end + + it "requires an end_date" do + grading_period = GradingPeriod.new(params.except(:end_date)) + expect(grading_period).to_not be_valid + end + + it "requires start_date to be before end_date" do + subject.update_attributes(start_date: Time.zone.now, end_date: 1.day.ago) + is_expected.to_not be_valid + end + + describe "#destroy" do + before { subject.destroy } + + it "marks workflow as deleted" do + expect(subject.workflow_state).to eq "deleted" end - it "should require a start_date" do - expect(GradingPeriod.create(course_id: 1, end_date: 1.day.from_now)).to_not be_valid + it "does not destroy" do + expect(subject).to_not be_destroyed end - it "should require an end_date" do - expect(GradingPeriod.create(course_id: 1, start_date: Time.zone.now)).to_not be_valid - end + end - context "when end_date is before the start_date" do - it "should not be able to create a grading period with end_date before the start_date" do - expect(GradingPeriod.create(course_id: 1, start_date: 2.days.from_now, end_date: Time.zone.now)).to_not be_valid + describe "#destroy!" do + before { subject.destroy! } + + it { is_expected.to be_destroyed } + end + + describe "#for" do + + context "when context is an account" do + let(:account) { Account.new } + let(:finder) { mock } + + it "delegates calls" do + GradingPeriod::AccountGradingPeriodFinder.expects(:new).with(account).once.returns(finder) + finder.expects(:grading_periods).once + GradingPeriod.for(account) end + end - it "should not be able to update the end_date to be before the start_date" do - grading_period = GradingPeriod.create(course_id: 1, start_date: Time.zone.now, end_date: 1.day.from_now) - expect(grading_period).to be_valid - grading_period.update_attributes(end_date: 1.day.ago) - expect(grading_period).not_to be_valid - end + context "when context is a course" do + let(:course) { Course.new } + let(:finder) { mock } - it "should not be able to update the start_date to be after the end_date" do - grading_period = GradingPeriod.create(course_id: 1, start_date: Time.zone.now, end_date: 1.day.from_now) - expect(grading_period).to be_valid - grading_period.update_attributes(start_date: 2.days.from_now) - expect(grading_period).not_to be_valid + it "delegates calls" do + GradingPeriod::CourseGradingPeriodFinder.expects(:new).with(course).once.returns(finder) + finder.expects(:grading_periods).once + GradingPeriod.for(course) end end end - describe '#destroy' do - it 'soft deletes' do - @grading_period.destroy - expect(@grading_period).to be_deleted - expect(@grading_period).to_not be_destroyed - end - end - - describe '#assignments' do - before :once do + describe "#assignments" do + it "filters assignments for grading period" do course_with_teacher active_all: true - @gp1, @gp2 = grading_periods count: 2 - @a1, @a2 = [@gp1, @gp2].map { |gp| + gp1, gp2 = grading_periods count: 2 + a1, a2 = [gp1, gp2].map { |gp| @course.assignments.create! due_at: gp.start_date + 1 } # no due date goes in final grading period - @a3 = @course.assignments.create! - end - - it 'filters assignments for grading period' do - expect(@gp1.assignments(@course.assignments)).to eq [@a1] - expect(@gp2.assignments(@course.assignments)).to eq [@a2, @a3] - end - end - - describe "permissions:" do - context "course belonging to root account" do - before(:once) do - @root_account = Account.default - @sub_account = @root_account.sub_accounts.create! - course_with_teacher(account: @root_account, active_all: true) - course_with_student(course: @course, active_all: true) - @root_account_period = grading_periods(context: @root_account, count: 1).first - @sub_account_period = grading_periods(context: @sub_account, count: 1).first - @course_period = grading_periods(context: @course, count: 1).first - end - - context "root-account admin" do - before(:once) do - account_admin_user(account: @root_account) - end - - it "should be able to read and manage root-account level grading periods" do - expect(@root_account_period.rights_status(@admin, :read, :manage)).to eq({ read: true, manage: true }) - end - - it "should be able to read and manage sub-account level grading periods" do - expect(@sub_account_period.rights_status(@admin, :read, :manage)).to eq({ read: true, manage: true }) - end - - it "should be able to read and manage course level grading periods" do - expect(@course_period.rights_status(@admin, :read, :manage)).to eq({ read: true, manage: true }) - end - end - - context "sub-account admin" do - before(:once) do - account_admin_user(account: @sub_account) - end - - it "should NOT be able to manage root-account level grading periods, but should be able to read them" do - expect(@root_account_period.rights_status(@admin, :read, :manage)).to eq({ read: true, manage: false }) - end - - it "should be able to read and manage sub-account level grading periods" do - expect(@sub_account_period.rights_status(@admin, :read, :manage)).to eq({ read: true, manage: true }) - end - - - it "should NOT be able to read or manage course level grading periods, when the course is under the root-account" do - expect(@course_period.rights_status(@admin, :read, :manage)).to eq({ read: false, manage: false }) - end - end - - context "teacher" do - it "should NOT be able to manage root-account level grading periods, but should be able to read them" do - expect(@root_account_period.rights_status(@teacher, :read, :manage)).to eq({ read: true, manage: false }) - end - - it "should NOT be able to read or manage sub-account level grading period" do - expect(@sub_account_period.rights_status(@teacher, :read, :manage)).to eq({ read: false, manage: false }) - end - - it "should be able to read and manage course level grading periods" do - expect(@course_period.rights_status(@teacher, :read, :manage)).to eq({ read: true, manage: true}) - end - end - - context "student" do - it "should NOT be able to manage root-account level grading periods, but should be able to read them" do - expect(@root_account_period.rights_status(@student, :read, :manage)).to eq({ read: true, manage: false }) - end - - it "should NOT be able to read or manage sub-account level grading period" do - expect(@sub_account_period.rights_status(@student, :read, :manage)).to eq({ read: false, manage: false }) - end - - it "should NOT be able to manage course level grading periods, but should be able to read them" do - expect(@course_period.rights_status(@student, :read, :manage)).to eq({ read: true, manage: false}) - end - end - - context "multiple grading periods feature flag turned off" do - before(:once) do - account_admin_user(account: @root_account) - @root_account.disable_feature! :multiple_grading_periods - end - - it "should return false for all permissions" do - expect(@course_period.rights_status(@admin, :read, :manage)).to eq({ read: false, manage: false }) - end - end - end - context "course belonging to sub-account" do - before(:once) do - @root_account = Account.default - @sub_account = @root_account.sub_accounts.create! - course_with_teacher(account: @sub_account, active_all: true) - course_with_student(course: @course, active_all: true) - @root_account_period = grading_periods(context: @root_account, count: 1).first - @sub_account_period = grading_periods(context: @sub_account, count: 1).first - @course_period = grading_periods(context: @course, count: 1).first - end - - context "root-account admin" do - before(:once) do - account_admin_user(account: @root_account) - end - - it "should be able to read and manage root-account level grading periods" do - expect(@root_account_period.rights_status(@admin, :read, :manage)).to eq({ read: true, manage: true }) - end - - it "should be able to read and manage sub-account level grading periods" do - expect(@sub_account_period.rights_status(@admin, :read, :manage)).to eq({ read: true, manage: true }) - end - - - it "should be able to read and manage course level grading periods" do - expect(@course_period.rights_status(@admin, :read, :manage)).to eq({ read: true, manage: true }) - end - end - - context "sub-account admin" do - before(:once) do - account_admin_user(account: @sub_account) - end - - it "should NOT be able to manage root-account level grading periods, but should be able to read them" do - expect(@root_account_period.rights_status(@admin, :read, :manage)).to eq({ read: true, manage: false }) - end - - it "should be able to read and manage sub-account level grading periods" do - expect(@sub_account_period.rights_status(@admin, :read, :manage)).to eq({read: true, manage: true }) - end - - - it "should be able to read and manage course level grading periods, when the course is under the sub-account" do - expect(@course_period.rights_status(@admin, :read, :manage)).to eq({read: true, manage: true }) - end - end - - context "teacher" do - it "should NOT be able to manage root-account level grading periods, but should be able to read them" do - expect(@root_account_period.rights_status(@teacher, :read, :manage)).to eq({ read: true, manage: false }) - end - - it "should NOT be able to manage sub-account level grading periods, but should be able to read them" do - expect(@sub_account_period.rights_status(@teacher, :read, :manage)).to eq({ read: true, manage: false }) - end - - it "should be able to read and manage course level grading periods" do - expect(@course_period.rights_status(@teacher, :read, :manage)).to eq({ read: true, manage: true }) - end - end - - context "student" do - it "should NOT be able to manage root-account level grading periods, but should be able to read them" do - expect(@root_account_period.rights_status(@student, :read, :manage)).to eq({ read: true, manage: false }) - end - - it "should NOT be able to manage sub-account level grading periods, but should be able to read them" do - expect(@sub_account_period.rights_status(@student, :read, :manage)).to eq({ read: true, manage: false }) - end - - it "should NOT be able to manage course level grading periods, but should be able to read them" do - expect(@course_period.rights_status(@student, :read, :manage)).to eq({ read: true, manage: false}) - end - end - - context "multiple grading periods feature flag turned off" do - before(:once) do - account_admin_user(account: @sub_account) - @root_account.disable_feature! :multiple_grading_periods - end - - it "should return false for all permissions" do - expect(@course_period.rights_status(@admin, :read, :manage)).to eq({ read: false, manage: false }) - end - end + a3 = @course.assignments.create! + expect(gp1.assignments(@course.assignments)).to eq [a1] + expect(gp2.assignments(@course.assignments)).to eq [a2, a3] end end end + diff --git a/spec/models/grading_standard_spec.rb b/spec/models/grading_standard_spec.rb index cec3bdbb5f4..1c978634598 100644 --- a/spec/models/grading_standard_spec.rb +++ b/spec/models/grading_standard_spec.rb @@ -76,11 +76,11 @@ describe GradingStandard do compare_schemes(standard.data, GradingStandard.default_grading_standard) end - context "standards_for" do + context "#for" do it "should return standards that match the context" do grading_standard_for @course - standards = GradingStandard.standards_for(@course) + standards = GradingStandard.for(@course) expect(standards.length).to eq 1 expect(standards[0].id).to eq @standard.id end @@ -88,7 +88,7 @@ describe GradingStandard do it "should include standards made in the parent account" do grading_standard_for @course.root_account - standards = GradingStandard.standards_for(@course) + standards = GradingStandard.for(@course) expect(standards.length).to eq 1 expect(standards[0].id).to eq @standard.id end @@ -104,7 +104,7 @@ describe GradingStandard do @course.assignments.create!(:title => "hi", :grading_standard_id => gs.id) end - standards = GradingStandard.standards_for(@course).sorted + standards = GradingStandard.for(@course).sorted expect(standards.length).to eq 2 expect(standards.map(&:id)).to eq [gs.id, gs2.id] end @@ -115,7 +115,7 @@ describe GradingStandard do gs2.title = nil gs2.save! - standards = GradingStandard.standards_for(@course).sorted + standards = GradingStandard.for(@course).sorted expect(standards.length).to eq 2 expect(standards.map(&:id)).to eq [gs.id, gs2.id] end diff --git a/spec/selenium/grading_periods_spec.rb b/spec/selenium/grading_periods_spec.rb index 349ec07b064..b5002c6432b 100644 --- a/spec/selenium/grading_periods_spec.rb +++ b/spec/selenium/grading_periods_spec.rb @@ -25,10 +25,6 @@ describe "grading periods" do end it "should show grading periods created by an associated account" do - pending("this test marked as pending until the grading periods API - is changed to return grading periods created at the account - level for a given course (in addition to returning grading periods - created at the course level") account_grading_period = create_grading_periods_for(@course.root_account).first get "/courses/#{@course.id}/grading_standards" expect(f("#period_title_#{account_grading_period.id}").attribute("value")).to eq(account_grading_period.title) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 3e1d9c9a642..7f228331c2e 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -56,6 +56,8 @@ ENV["RAILS_ENV"] = 'test' require File.expand_path('../../config/environment', __FILE__) unless defined?(Rails) require 'rspec/rails' +Dir[Rails.root.join("spec/support/**/*.rb")].each { |f| require f } + ActionView::TestCase::TestController.view_paths = ApplicationController.view_paths module RSpec::Rails diff --git a/spec/support/sorted_by_matcher.rb b/spec/support/sorted_by_matcher.rb new file mode 100644 index 00000000000..0f5a831a05c --- /dev/null +++ b/spec/support/sorted_by_matcher.rb @@ -0,0 +1,6 @@ +RSpec::Matchers.define :be_sorted_by do |attr| + match do |records| + a = records.map{ |record| record.fetch(attr) } + a.sort == a + end +end