diff --git a/app/controllers/role_overrides_controller.rb b/app/controllers/role_overrides_controller.rb index 9e8a40923db..44345b6dd09 100644 --- a/app/controllers/role_overrides_controller.rb +++ b/app/controllers/role_overrides_controller.rb @@ -224,13 +224,14 @@ class RoleOverridesController < ApplicationController # manage_calendar -- [sTADo] Add, edit and delete events on the course calendar # manage_content -- [ TADo] Manage all other course content # manage_files -- [ TADo] Manage (add / edit / delete) course files - # manage_grades -- [ TA ] Edit grades (includes assessing rubrics) + # manage_grades -- [ TA ] Edit grades # manage_groups -- [ TAD ] Manage (create / edit / delete) groups # manage_interaction_alerts -- [ Ta ] Manage alerts # manage_outcomes -- [sTaDo] Manage learning outcomes # manage_sections -- [ TaD ] Manage (create / edit / delete) course sections # manage_students -- [ TAD ] Add/remove students for the course # manage_user_notes -- [ TA ] Manage faculty journal entries + # manage_rubrics -- [ TAD ] Edit assessing rubrics # manage_wiki -- [ TADo] Manage wiki (add / edit / delete pages) # read_forum -- [STADO] View discussions # moderate_forum -- [sTADo] Moderate discussions (delete/edit others' posts, lock topics) diff --git a/app/controllers/rubric_associations_controller.rb b/app/controllers/rubric_associations_controller.rb index 5ffe1b1faf9..633fbc7321b 100644 --- a/app/controllers/rubric_associations_controller.rb +++ b/app/controllers/rubric_associations_controller.rb @@ -32,7 +32,7 @@ class RubricAssociationsController < ApplicationController rubric_id = params[:rubric_association].delete(:rubric_id) @rubric = @association ? @association.rubric : Rubric.find(rubric_id) # raise "User doesn't have access to this rubric" unless @rubric.grants_right?(@current_user, session, :read) - if !@association && !authorized_action(@context, @current_user, :manage_grades) + if !@association && !authorized_action(@context, @current_user, :manage_rubrics) return elsif !@association || authorized_action(@association, @current_user, :update) if params[:rubric] && @rubric.grants_rights?(@current_user, session, :update)[:update] diff --git a/app/controllers/rubrics_controller.rb b/app/controllers/rubrics_controller.rb index 46af1886004..2f5c62b6390 100644 --- a/app/controllers/rubrics_controller.rb +++ b/app/controllers/rubrics_controller.rb @@ -61,7 +61,7 @@ class RubricsController < ApplicationController params[:rubric_association].delete(:invitations) @association_object = RubricAssociation.get_association_object(params[:rubric_association]) params[:rubric][:user] = @current_user if params[:rubric] - if (!@association_object || authorized_action(@association_object, @current_user, :read)) && authorized_action(@context, @current_user, :manage_grades) + if (!@association_object || authorized_action(@association_object, @current_user, :read)) && authorized_action(@context, @current_user, :manage_rubrics) @association = @context.rubric_associations.find_by_id(params[:rubric_association_id]) if params[:rubric_association_id].present? @association_object ||= @association.association if @association params[:rubric_association][:association] = @association_object diff --git a/app/models/role_override.rb b/app/models/role_override.rb index 1bf8c42e16f..cff279922c0 100644 --- a/app/models/role_override.rb +++ b/app/models/role_override.rb @@ -301,7 +301,7 @@ class RoleOverride < ActiveRecord::Base ] }, :manage_grades => { - :label => lambda { t('permissions.manage_grades', "Edit grades (includes assessing rubrics)") }, + :label => lambda { t('permissions.manage_grades', "Edit grades") }, :available_to => [ 'TaEnrollment', 'TeacherEnrollment', @@ -314,6 +314,22 @@ class RoleOverride < ActiveRecord::Base 'AccountAdmin' ] }, + :manage_rubrics => { + :label => lambda { t('permissions.manage_rubrics', "Create and edit assessing rubrics") }, + :available_to => [ + 'TaEnrollment', + 'DesignerEnrollment', + 'TeacherEnrollment', + 'AccountAdmin', + 'AccountMembership' + ], + :true_for => [ + 'DesignerEnrollment', + 'TaEnrollment', + 'TeacherEnrollment', + 'AccountAdmin' + ] + }, :comment_on_others_submissions => { :label => lambda { t('permissions.comment_on_others_submissions', "View all students' submissions and make comments on them") }, :available_to => [ diff --git a/app/models/rubric.rb b/app/models/rubric.rb index 497b1c316bb..30cd47d1226 100644 --- a/app/models/rubric.rb +++ b/app/models/rubric.rb @@ -47,7 +47,7 @@ class Rubric < ActiveRecord::Base } set_policy do - given {|user, session| self.cached_context_grants_right?(user, session, :manage_grades)} + given {|user, session| self.cached_context_grants_right?(user, session, :manage_rubrics)} can :read and can :create and can :delete_associations given {|user, session| self.cached_context_grants_right?(user, session, :manage_assignments)} @@ -60,13 +60,13 @@ class Rubric < ActiveRecord::Base given {|user, session| !self.read_only && self.rubric_associations.for_grading.length < 2 && self.cached_context_grants_right?(user, session, :manage_assignments)} can :update and can :delete - given {|user, session| !self.read_only && self.rubric_associations.for_grading.length < 2 && self.cached_context_grants_right?(user, session, :manage_grades)} + given {|user, session| !self.read_only && self.rubric_associations.for_grading.length < 2 && self.cached_context_grants_right?(user, session, :manage_rubrics)} can :update and can :delete given {|user, session| self.cached_context_grants_right?(user, session, :manage_assignments)} can :delete - given {|user, session| self.cached_context_grants_right?(user, session, :manage_grades)} + given {|user, session| self.cached_context_grants_right?(user, session, :manage_rubrics)} can :delete given {|user, session| self.cached_context_grants_right?(user, session, :read) } diff --git a/app/models/rubric_assessment.rb b/app/models/rubric_assessment.rb index bc45dabf749..525a994ac9b 100644 --- a/app/models/rubric_assessment.rb +++ b/app/models/rubric_assessment.rb @@ -161,7 +161,7 @@ class RubricAssessment < ActiveRecord::Base given {|user, session| self.rubric_association && self.rubric_association.grants_rights?(user, session, :manage)[:manage] && - (self.rubric_association.association.context.grants_right?(self.assessor, nil, :manage_grades) rescue false) + (self.rubric_association.association.context.grants_right?(self.assessor, nil, :manage_rubrics) rescue false) } can :update end diff --git a/db/migrate/20130215164701_add_manage_rubrics_permission.rb b/db/migrate/20130215164701_add_manage_rubrics_permission.rb new file mode 100644 index 00000000000..751b41f5e47 --- /dev/null +++ b/db/migrate/20130215164701_add_manage_rubrics_permission.rb @@ -0,0 +1,12 @@ +class AddManageRubricsPermission < ActiveRecord::Migration + tag :predeploy + + def self.up + DataFixup::CopyRoleOverrides.send_later_if_production_enqueue_args(:run, + {:priority => Delayed::LOW_PRIORITY, :max_attempts => 1}, + :manage_grades, :manage_rubrics) + end + + def self.down + end +end diff --git a/lib/data_fixup/copy_role_overrides.rb b/lib/data_fixup/copy_role_overrides.rb new file mode 100644 index 00000000000..1a233c87d2b --- /dev/null +++ b/lib/data_fixup/copy_role_overrides.rb @@ -0,0 +1,24 @@ +module DataFixup::CopyRoleOverrides + def self.run(old_permission, new_permission) + RoleOverride.scoped(:conditions => {:permission => old_permission.to_s}).find_in_batches do |old_role_overrides| + possible_new_role_overrides = RoleOverride.find(:all, :conditions => + {:permission => new_permission.to_s, :context_id => old_role_overrides.map(&:context_id)} ) + + old_role_overrides.each do |old_role_override| + unless old_role_override.invalid? || possible_new_role_overrides.detect{|ro| + ro.context_id == old_role_override.context_id && + ro.context_type == old_role_override.context_type && + ro.enrollment_type == old_role_override.enrollment_type + } + + dup = RoleOverride.new + old_role_override.attributes.delete_if{|k,v| [:id, :permission, :created_at, :updated_at].include?(k.to_sym)}.each do |key, val| + dup.send("#{key}=", val) + end + dup.permission = new_permission.to_s + dup.save! + end + end + end + end +end diff --git a/spec/migrations/add_manage_rubrics_permission_spec.rb b/spec/migrations/add_manage_rubrics_permission_spec.rb new file mode 100644 index 00000000000..870c401af57 --- /dev/null +++ b/spec/migrations/add_manage_rubrics_permission_spec.rb @@ -0,0 +1,41 @@ +require File.expand_path(File.dirname(__FILE__) + '/../spec_helper.rb') +require 'db/migrate/20130215164701_add_manage_rubrics_permission.rb' + +describe "AddManageRubricsPermission" do + it "will copy role overrides for the new permission" do + accounts = [] + 4.times do + accounts << account_model(:parent_account => Account.default) + end + + enrollment_types = ['TeacherEnrollment', 'TaEnrollment', 'AccountAdmin', 'AccountMembership'] + bool = true + + role_overrides = [] + accounts.each do |account| + enrollment_types.each do |enrollment_type| + role_overrides << RoleOverride.create!(:context => account, :permission => 'manage_grades', + :enrollment_type => enrollment_type, :enabled => bool) + bool = !bool + end + end + + AddManageRubricsPermission.up + + new_role_overrides = RoleOverride.find(:all, :conditions => {:permission => 'manage_rubrics'}) + + role_overrides.count.should == new_role_overrides.count + role_overrides.each do |old_role_override| + new_role_override = new_role_overrides.find{|ro| + ro.context_id == old_role_override.context_id && + ro.context_type == old_role_override.context_type && + ro.enrollment_type == old_role_override.enrollment_type + } + new_role_override.should_not be_nil + + new_role_override.attributes.delete_if{|k,v| [:id, :permission, :created_at, :updated_at].include?(k.to_sym)}.should == + old_role_override.attributes.delete_if{|k,v| [:id, :permission, :created_at, :updated_at].include?(k.to_sym)} + end + + end +end diff --git a/spec/selenium/assignments_rubrics_spec.rb b/spec/selenium/assignments_rubrics_spec.rb index f66b27d2118..cd45ed42344 100644 --- a/spec/selenium/assignments_rubrics_spec.rb +++ b/spec/selenium/assignments_rubrics_spec.rb @@ -276,20 +276,18 @@ describe "assignment rubrics" do course_with_designer_logged_in end - it "should allow an designer to create a course rubric" do - pending "Bug #7136 - Rubrics cannot be created by designers" do - rubric_name = 'this is a new rubric' - get "/courses/#{@course.id}/rubrics" + it "should allow a designer to create a course rubric" do + rubric_name = 'this is a new rubric' + get "/courses/#{@course.id}/rubrics" - expect { - f('.add_rubric_link').click - replace_content(f('.rubric_title input'), rubric_name) - submit_form('#edit_rubric_form') - wait_for_ajaximations - }.to change(Rubric, :count).by(1) - refresh_page - f('#rubrics .title').text.should == rubric_name - end + expect { + f('.add_rubric_link').click + replace_content(f('.rubric_title input'), rubric_name) + submit_form('#edit_rubric_form') + wait_for_ajaximations + }.to change(Rubric, :count).by(1) + refresh_page + f('#rubrics .title').text.should == rubric_name end end end