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

Create new "Manage course admin roles" granulars and FF 

Refs FOO-171
flag=granular_permissions_manage_admin_users

Creates the granular permissions for adding and removing teachers,
course designers, and TAs in courses. The granulars and group are
behind a root-account feature flag for now.

Test plan:
* Make sure the migration runs error-free
* UI looks the same as it used to with the feature flag off
* New granular permissions show up in the UI with the FF on
  (note that they will not actually work yet)
* I'd welcome suggestions for better wording of the text
  for the granulars on the permissions page

Change-Id: Ia11859f0cb69084606fb0c4cef5174cbb223cce7
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/248227
Reviewed-by: August Thornton <august@instructure.com>
QA-Review: August Thornton <august@instructure.com>
Product-Review: August Thornton <august@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
This commit is contained in:
Charley Kline 2020-09-22 11:31:44 -05:00
parent 8a57a05f89
commit 9831b444e0
8 changed files with 252 additions and 36 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
app/coffeescripts/views/courses/roster/RosterUserView.js
View File

@ -103,12 +103,15 @@ export default class RosterUserView extends Backbone.View {
json.canLinkStudents = json.isObserver && !ENV.course.concluded
json.canViewLoginIdColumn = ENV.permissions.view_user_logins
json.canViewSisIdColumn = ENV.permissions.read_sis
const candoAdminActions =
ENV.permissions.allow_course_admin_actions || ENV.permissions.manage_admin_users
json.canManage = _.some(['TeacherEnrollment', 'DesignerEnrollment', 'TaEnrollment'], et =>
this.model.hasEnrollmentType(et)
)
? ENV.permissions.manage_admin_users
? candoAdminActions
: this.model.hasEnrollmentType('ObserverEnrollment')
? ENV.permissions.manage_admin_users || ENV.permissions.manage_students
? candoAdminActions || ENV.permissions.manage_students
: ENV.permissions.manage_students
json.customLinks = this.model.get('custom_links')

3
app/jsx/permissions/templates.js
View File

@ -43,7 +43,8 @@ export const PERMISSION_DETAIL_SECTIONS = [
]
export const GROUP_PERMISSION_DESCRIPTIONS = {
manage_wiki: () => I18n.t('Create / Delete / Update Pages')
manage_wiki: () => I18n.t('Create / Delete / Update Pages'),
manage_course_admin_users: () => I18n.t('TAs / Observers / Designers')
}
export const generateActionTemplates = (

2
app/models/group.rb
View File

@ -537,6 +537,7 @@ class Group < ActiveRecord::Base
can :delete and
can :manage and
can :manage_admin_users and
can :allow_course_admin_actions and
can :manage_students and
can :moderate_forum and
can :update
@ -559,6 +560,7 @@ class Group < ActiveRecord::Base
can :delete and
can :manage and
can :manage_admin_users and
can :allow_course_admin_actions and
can :manage_calendar and
can :manage_content and
can :manage_files and

157
app/models/role_override.rb
View File

@ -463,21 +463,150 @@ class RoleOverride < ActiveRecord::Base
:true_for => %w(TeacherEnrollment TaEnrollment DesignerEnrollment AccountAdmin),
:available_to => %w(TeacherEnrollment TaEnrollment DesignerEnrollment AccountAdmin AccountMembership)
},
:manage_admin_users => {
:label => lambda { t('permissions.manage_admin_users', "Add/remove other teachers, course designers or TAs to the course") },
:label_v2 => lambda { t("Users - add / remove teachers, course designers, or TAs in courses") },
:available_to => [
'TaEnrollment',
'DesignerEnrollment',
'TeacherEnrollment',
'AccountAdmin',
'AccountMembership'
],
:true_for => [
manage_admin_users: {
label: lambda { t("permissions.manage_admin_users", "Add/remove other teachers, course designers or TAs to the course") },
label_v2: lambda { t("Users - add / remove teachers, course designers, or TAs in courses") },
available_to: [
'TaEnrollment',
'DesignerEnrollment',
'TeacherEnrollment',
'AccountAdmin'
]
},
'AccountAdmin',
'AccountMembership'
],
true_for: [
"TeacherEnrollment",
"AccountAdmin"
],
account_allows: lambda { |a| !a.root_account.feature_enabled?(:granular_permissions_manage_admin_users) }
},
allow_course_admin_actions: {
label: lambda { t("Allow administrative actions in courses") },
label_v2: lambda { t("Users - allow administrative actions in courses") },
available_to: [
'TaEnrollment',
'DesignerEnrollment',
'TeacherEnrollment',
'AccountAdmin',
'AccountMembership'
],
true_for: [
"TeacherEnrollment",
"AccountAdmin"
],
account_allows: lambda { |a| a.root_account.feature_enabled?(:granular_permissions_manage_admin_users) }
},
add_ta_to_course: {
label: lambda { t("Add TAs to courses") },
label_v2: lambda { t("TAs - Add") },
available_to: [
"TaEnrollment",
"DesignerEnrollment",
"TeacherEnrollment",
"AccountAdmin",
"AccountMembership"
],
true_for: [
"TeacherEnrollment",
"AccountAdmin"
],
group: "manage_course_admin_users",
group_label: lambda { t("Users - Admin users in courses") },
account_allows: lambda { |a| a.root_account.feature_enabled?(:granular_permissions_manage_admin_users) }
},
remove_ta_from_course: {
label: lambda { t("Remove TAs from courses") },
label_v2: lambda { t("TAs - Remove") },
available_to: [
"TaEnrollment",
"DesignerEnrollment",
"TeacherEnrollment",
"AccountAdmin",
"AccountMembership"
],
true_for: [
"TeacherEnrollment",
"AccountAdmin"
],
group: "manage_course_admin_users",
group_label: lambda { t("Users - Admin users in courses") },
account_allows: lambda { |a| a.root_account.feature_enabled?(:granular_permissions_manage_admin_users) }
},
add_observer_to_course: {
label: lambda { t("Add Observers to courses") },
label_v2: lambda { t("Observers - Add") },
available_to: [
"TaEnrollment",
"DesignerEnrollment",
"TeacherEnrollment",
"AccountAdmin",
"AccountMembership"
],
true_for: [
"TeacherEnrollment",
"AccountAdmin"
],
group: "manage_course_admin_users",
group_label: lambda { t("Users - Admin users in courses") },
account_allows: lambda { |a| a.root_account.feature_enabled?(:granular_permissions_manage_admin_users) }
},
remove_observer_from_course: {
label: lambda { t("Remove Observers from courses") },
label_v2: lambda { t("Observers - Remove") },
available_to: [
"TaEnrollment",
"DesignerEnrollment",
"TeacherEnrollment",
"AccountAdmin",
"AccountMembership"
],
true_for: [
"TeacherEnrollment",
"AccountAdmin"
],
group: "manage_course_admin_users",
group_label: lambda { t("Users - Admin users in courses") },
account_allows: lambda { |a| a.root_account.feature_enabled?(:granular_permissions_manage_admin_users) }
},
add_designer_to_course: {
label: lambda { t("Add Designers to courses") },
label_v2: lambda { t("Designers - Add") },
available_to: [
"TaEnrollment",
"DesignerEnrollment",
"TeacherEnrollment",
"AccountAdmin",
"AccountMembership"
],
true_for: [
"TeacherEnrollment",
"AccountAdmin"
],
group: "manage_course_admin_users",
group_label: lambda { t("Users - Admin users in courses") },
account_allows: lambda { |a| a.root_account.feature_enabled?(:granular_permissions_manage_admin_users) }
},
remove_designer_from_course: {
label: lambda { t("Remove Designers from courses") },
label_v2: lambda { t("Designers - Remove") },
available_to: [
"TaEnrollment",
"DesignerEnrollment",
"TeacherEnrollment",
"AccountAdmin",
"AccountMembership"
],
true_for: [
"TeacherEnrollment",
"AccountAdmin"
],
group: "manage_course_admin_users",
group_label: lambda { t("Users - Admin users in courses") },
account_allows: lambda { |a| a.root_account.feature_enabled?(:granular_permissions_manage_admin_users) }
},
:manage_assignments => {
:label => lambda { t('permissions.manage_assignments', "Manage (add / edit / delete) assignments and quizzes") },
:label_v2 => lambda { t("Assignments and Quizzes - add / edit / delete") },

11
config/feature_flags/app_fundamentals_release_flags.yml Normal file
View File

@ -0,0 +1,11 @@
---
default_recaptcha_registration_enable:
state: hidden
display_name: Enable Recaptcha by default for self-registration
description: Requires Recaptcha for self-registration by default
applies_to: SiteAdmin
granular_permissions_manage_admin_users:
state: hidden
applies_to: RootAccount
display_name: Granular permissions for Add/Remove admin roles from Course
description: Adds granular permissions for adding and removing each role, and a separate permission for other course admin actions

6
config/feature_flags/support_users_release_flags.yml
View File

@ -1,6 +0,0 @@
---
default_recaptcha_registration_enable:
state: hidden
display_name: Enable Recaptcha by default for self-registration
description: Requires Recaptcha for self-registration by default
applies_to: SiteAdmin

36
db/migrate/20200922200047_granular_manage_admin_users_permissions.rb Normal file
View File

@ -0,0 +1,36 @@
# frozen_string_literal: true
#
# Copyright (C) 2020 - present 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 <http://www.gnu.org/licenses/>.
#
class GranularManageAdminUsersPermissions < ActiveRecord::Migration[5.2]
tag :postdeploy
def up
DataFixup::AddRoleOverridesForNewPermission.run(:manage_admin_users, :allow_course_admin_actions)
DataFixup::AddRoleOverridesForNewPermission.run(:manage_admin_users, :add_ta_to_course)
DataFixup::AddRoleOverridesForNewPermission.run(:manage_admin_users, :remove_ta_from_course)
DataFixup::AddRoleOverridesForNewPermission.run(:manage_admin_users, :add_observer_to_course)
DataFixup::AddRoleOverridesForNewPermission.run(:manage_admin_users, :remove_observer_from_course)
DataFixup::AddRoleOverridesForNewPermission.run(:manage_admin_users, :add_designer_to_course)
DataFixup::AddRoleOverridesForNewPermission.run(:manage_admin_users, :remove_designer_from_course)
end
def down
raise ActiveRecord::IrreversibleMigration
end
end

66
spec/apis/v1/roles_api_spec.rb
View File

@ -440,18 +440,9 @@ describe "Roles API", type: :request do
end
describe "json response" do
it "should return the expected json format" do
json = api_call_with_settings
expect(json.keys.sort).to eq ["account", "base_role_type", "created_at", "id", "label",
"last_updated_at", "permissions", "role", "workflow_state"]
expect(json["account"]["id"]).to eq @account.id
expect(json["id"]).to eq @role.id
expect(json["role"]).to eq @role_name
expect(json["base_role_type"]).to eq Role::DEFAULT_ACCOUNT_TYPE
# make sure all the expected keys are there, but don't assert on a
# *only* the expected keys, since plugins may have added more.
expect([
before :each do
@account.root_account.disable_feature!(:granular_permissions_manage_admin_users)
@expected_permissions = [
"become_user", "change_course_state", "create_collaborations",
"create_conferences", "manage_account_memberships",
"manage_account_settings", "manage_admin_users", "manage_alerts",
@ -466,7 +457,56 @@ describe "Roles API", type: :request do
"read_question_banks", "read_reports", "read_roster",
"read_sis", "send_messages", "view_all_grades", "view_group_pages",
"view_statistics"
] - json["permissions"].keys).to be_empty
]
end
it "should return the expected json format with granular admin user permission off" do
json = api_call_with_settings
expect(json.keys.sort).to eq ["account", "base_role_type", "created_at", "id", "label",
"last_updated_at", "permissions", "role", "workflow_state"]
expect(json["account"]["id"]).to eq @account.id
expect(json["id"]).to eq @role.id
expect(json["role"]).to eq @role_name
expect(json["base_role_type"]).to eq Role::DEFAULT_ACCOUNT_TYPE
# make sure all the expected keys are there, but don't assert on a
# *only* the expected keys, since plugins may have added more.
expect(@expected_permissions - json["permissions"].keys).to be_empty
expect(json["permissions"][@permission]).to eq({
"explicit" => false,
"readonly" => false,
"enabled" => false,
"locked" => false
})
end
it "should return the expected json format with granular admin user permission on" do
@account.root_account.enable_feature!(:granular_permissions_manage_admin_users)
# no longer have manage_admin_users, instead we have the new ones
expected_perms = @expected_permissions - ["manage_admin_users"]
expected_perms += [
"allow_course_admin_actions",
"add_ta_to_course",
"add_designer_to_course",
"add_observer_to_course",
"remove_ta_from_course",
"remove_designer_from_course",
"remove_observer_from_course"
]
json = api_call_with_settings
expect(json.keys.sort).to eq ["account", "base_role_type", "created_at", "id", "label",
"last_updated_at", "permissions", "role", "workflow_state"]
expect(json["account"]["id"]).to eq @account.id
expect(json["id"]).to eq @role.id
expect(json["role"]).to eq @role_name
expect(json["base_role_type"]).to eq Role::DEFAULT_ACCOUNT_TYPE
# make sure all the expected keys are there, but don't assert on a
# *only* the expected keys, since plugins may have added more.
expect(expected_perms - json["permissions"].keys).to be_empty
expect(json["permissions"][@permission]).to eq({
"explicit" => false,