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

add sis_ids to groups and group categories APIs 

fixes CORE-652

test plan
 - groups response should include sis_id
 - group category response should include sis id

Change-Id: I2e2ea884e6fed333baf4a2f5df8a8d1ad0e65f1c
Reviewed-on: https://gerrit.instructure.com/137876
Reviewed-by: Cody Cutrer <cody@instructure.com>
Tested-by: Jenkins
QA-Review: Tucker McKnight <tmcknight@instructure.com>
Product-Review: Rob Orton <rob@instructure.com>
This commit is contained in:
Rob Orton 2018-01-03 14:44:22 -07:00
parent 584e05dbb6
commit 68ed1c341d
11 changed files with 108 additions and 42 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

34
app/controllers/group_categories_controller.rb
View File

@ -75,6 +75,14 @@
# "description": "If self-signup is enabled, group_limit can be set to cap the number of users in each group. If null, there is no limit.",
# "type": "integer"
# },
# "sis_group_category_id": {
# "description": "The SIS identifier for the group category. This field is only included if the user has permission to manage or view SIS information.",
# "type": "String"
# },
# "sis_import_id": {
# "description": "The unique identifier for the SIS import. This field is only included if the user has permission to manage SIS information.",
# "type": "integer"
# },
# "progress": {
# "description": "If the group category has not yet finished a randomly student assignment request, a progress object will be attached, which will contain information related to the progress of the assignment request. Refer to the Progress API for more information",
# "$ref": "Progress"
@ -106,7 +114,7 @@ class GroupCategoriesController < ApplicationController
#
# @returns [GroupCategory]
def index
@categories = @context.group_categories
@categories = @context.group_categories.preload(:root_account)
respond_to do |format|
format.json do
if authorized_action(@context, @current_user, :manage_groups)
@ -166,6 +174,9 @@ class GroupCategoriesController < ApplicationController
# Limit the maximum number of users in each group (Course Only). Requires
# self signup.
#
# @argument sis_group_category_id [String]
# The unique SIS identifier.
#
# @argument create_group_count [Integer]
# Create this number of groups (Course Only).
#
@ -189,6 +200,14 @@ class GroupCategoriesController < ApplicationController
if api_request?
includes = ["unassigned_users_count", "groups_count"]
includes.concat(params[:includes]) if params[:includes]
if (sis_id = params[:sis_group_category_id])
if @group_category.root_account.grants_right?(@current_user, :manage_sis)
@group_category.sis_source_id = sis_id
@group_category.save!
else
return render json: { message: "You must have manage_sis permission to set sis attributes" }, status: :unauthorized
end
end
render :json => group_category_json(@group_category, @current_user, session, include: includes)
else
flash[:notice] = t('notices.create_category_success', 'Category was successfully created.')
@ -221,6 +240,9 @@ class GroupCategoriesController < ApplicationController
# Limit the maximum number of users in each group (Course Only). Requires
# self signup.
#
# @argument sis_group_category_id [String]
# The unique SIS identifier.
#
# @argument create_group_count [Integer]
# Create this number of groups (Course Only).
#
@ -247,6 +269,14 @@ class GroupCategoriesController < ApplicationController
includes.concat(params[:includes]) if params[:includes]
render :json => group_category_json(@group_category, @current_user, session, :include => includes)
end
if (sis_id = params[:sis_group_category_id])
if @group_category.root_account.grants_right?(@current_user, :manage_sis)
@group_category.sis_source_id = sis_id
@group_category.save!
else
return render json: { message: "You must have manage_sis permission to set sis attributes" }, status: :unauthorized
end
end
else
return render(:json => {'status' => 'not found'}, :status => :not_found) unless @group_category
return render(:json => {'status' => 'unauthorized'}, :status => :unauthorized) if @group_category.protected?
@ -300,7 +330,7 @@ class GroupCategoriesController < ApplicationController
# @returns [Group]
def groups
if authorized_action(@context, @current_user, :manage_groups)
@groups = @group_category.groups.active.by_name
@groups = @group_category.groups.active.by_name.preload(:root_account)
@groups = Api.paginate(@groups, self, api_v1_group_category_groups_url)
render :json => @groups.map { |g| group_json(g, @current_user, session) }
end

2
app/controllers/group_memberships_controller.rb
View File

@ -96,7 +96,7 @@ class GroupMembershipsController < ApplicationController
def index
if authorized_action(@group, @current_user, :read_roster)
memberships_route = polymorphic_url([:api_v1, @group, :memberships])
scope = @group.group_memberships
scope = @group.group_memberships.preload(group: :root_account)
only_states = ALLOWED_MEMBERSHIP_FILTER
only_states = only_states & params[:filter_states] if params[:filter_states]

32
app/controllers/groups_controller.rb
View File

@ -208,7 +208,7 @@ class GroupsController < ApplicationController
format.html do
groups_scope = groups_scope.by_name
groups_scope = groups_scope.where(:context_type => params[:context_type]) if params[:context_type]
groups_scope = groups_scope.preload(:group_category, :context)
groups_scope = groups_scope.preload(:group_category, :context, :root_account)
groups = groups_scope.shard(@current_user).to_a
groups.select!{|group| group.context_type != 'Course' || group.context.grants_right?(@current_user, :read)}
@ -248,9 +248,9 @@ class GroupsController < ApplicationController
# @returns [Group]
def context_index
return unless authorized_action(@context, @current_user, :read_roster)
@groups = all_groups = @context.groups.active
.order(GroupCategory::Bookmarker.order_by, Group::Bookmarker.order_by)
.eager_load(:group_category)
@groups = all_groups = @context.groups.active.
order(GroupCategory::Bookmarker.order_by, Group::Bookmarker.order_by).
eager_load(:group_category).preload(:root_account)
unless api_request?
if @context.is_a?(Account)
@ -274,14 +274,14 @@ class GroupsController < ApplicationController
respond_to do |format|
format.html do
@categories = @context.group_categories.order("role <> 'student_organized'", GroupCategory.best_unicode_collation_key('name'))
@categories = @context.group_categories.order("role <> 'student_organized'", GroupCategory.best_unicode_collation_key('name')).preload(:root_account)
@user_groups = @current_user.group_memberships_for(@context) if @current_user
if @context.grants_right?(@current_user, session, :manage_groups)
categories_json = @categories.map{ |cat| group_category_json(cat, @current_user, session, include: ["progress_url", "unassigned_users_count", "groups_count"]) }
uncategorized = @context.groups.active.uncategorized.to_a
if uncategorized.present?
json = group_category_json(GroupCategory.uncategorized, @current_user, session)
json = group_category_json(GroupCategory.uncategorized.preload(:root_account), @current_user, session)
json["groups"] = uncategorized.map{ |group| group_json(group, @current_user, session) }
categories_json << json
end
@ -427,6 +427,9 @@ class GroupsController < ApplicationController
# The allowed file storage for the group, in megabytes. This parameter is
# ignored if the caller does not have the manage_storage_quotas permission.
#
# @argument sis_group_id [String]
# The sis ID of the group. Must have manage_sis permission to set.
#
# @example_request
# curl https://<canvas>/api/v1/groups \
# -F 'name=Math Teachers' \
@ -466,6 +469,13 @@ class GroupsController < ApplicationController
@group = @context.groups.temp_record(attrs.slice(*SETTABLE_GROUP_ATTRIBUTES))
if authorized_action(@group, @current_user, :create)
if (sis_id = params.delete :sis_group_id)
if @group.root_account.grants_right?(@current_user, :manage_sis)
@group.sis_source_id = sis_id
else
return render json: { message: "You must have manage_sis permission to set sis attributes" }, status: :unauthorized
end
end
respond_to do |format|
if @group.save
@group.add_user(@current_user, 'accepted', true) if @group.should_add_creator?(@current_user)
@ -514,6 +524,9 @@ class GroupsController < ApplicationController
# Users not in the group will be sent invitations. Existing group
# members who aren't in the list will be removed from the group.
#
# @argument sis_group_id [String]
# The sis ID of the group. Must have manage_sis permission to set.
#
# @example_request
# curl https://<canvas>/api/v1/groups/<group_id> \
# -X PUT \
@ -547,6 +560,13 @@ class GroupsController < ApplicationController
end
if authorized_action(@group, @current_user, :update)
if (sis_id = params.delete :sis_group_id)
if @group.root_account.grants_right?(@current_user, :manage_sis)
@group.sis_source_id = sis_id
else
return render json: { message: "You must have manage_sis permission to update sis attributes" }, status: :unauthorized
end
end
respond_to do |format|
@group.transaction do
@group.update_attributes(attrs.slice(*SETTABLE_GROUP_ATTRIBUTES))

4
app/models/account.rb
View File

@ -32,8 +32,8 @@ class Account < ActiveRecord::Base
has_many :all_courses, :class_name => 'Course', :foreign_key => 'root_account_id'
has_one :terms_of_service, :dependent => :destroy
has_one :terms_of_service_content, :dependent => :destroy
has_many :group_categories, -> { where(deleted_at: nil) }, as: :context, inverse_of: :context
has_many :all_group_categories, :class_name => 'GroupCategory', :as => :context, :inverse_of => :context
has_many :group_categories, as: :context, inverse_of: :context
has_many :all_group_categories, :class_name => 'GroupCategory', foreign_key: 'root_account_id', inverse_of: :root_account
has_many :groups, :as => :context, :inverse_of => :context
has_many :all_groups, :class_name => 'Group', :foreign_key => 'root_account_id'
has_many :all_group_memberships, source: 'group_memberships', through: :all_groups

1
app/models/group_category.rb
View File

@ -22,6 +22,7 @@ class GroupCategory < ActiveRecord::Base
belongs_to :context, polymorphic: [:course, :account]
belongs_to :sis_batch
belongs_to :root_account, class_name: 'Account', inverse_of: :all_group_categories
has_many :groups, :dependent => :destroy
has_many :progresses, :as => 'context', :dependent => :destroy
has_one :current_progress, -> { where(workflow_state: ['queued', 'running']).order(:created_at) }, as: :context, inverse_of: :context, class_name: 'Progress'

9
lib/api/v1/group.rb
View File

@ -71,8 +71,8 @@ module Api::V1::Group
hash['is_favorite'] = group.favorite_for_user?(user)
end
hash['html_url'] = group_url(group) if includes.include? 'html_url'
hash['sis_group_id'] = group.sis_source_id if group.context_type == 'Account' && group.account.grants_any_right?(user, session, :read_sis, :manage_sis)
hash['sis_import_id'] = group.sis_batch_id if group.context_type == 'Account' && group.account.grants_right?(user, session, :manage_sis)
hash['sis_group_id'] = group.sis_source_id if group.root_account.grants_any_right?(user, session, :read_sis, :manage_sis)
hash['sis_import_id'] = group.sis_batch_id if group.root_account.grants_right?(user, session, :manage_sis)
hash['has_submission'] = group.submission?
hash['concluded'] = group.context.concluded? || group.context.deleted?
hash['tabs'] = tabs_available_json(group, user, session, ['external']) if includes.include?('tabs')
@ -93,7 +93,10 @@ module Api::V1::Group
if includes.include?('just_created')
hash['just_created'] = membership.just_created || false
end
if membership.group.context_type == 'Account' && membership.group.account.grants_right?(user, session, :manage_sis)
if membership.group.root_account.grants_any_right?(user, session, :read_sis, :manage_sis)
hash['sis_group_id'] = membership.group.sis_source_id
end
if membership.group.root_account.grants_right?(user, session, :manage_sis)
hash['sis_import_id'] = membership.sis_batch_id
end
hash

18
lib/api/v1/group_category.rb
View File

@ -27,9 +27,10 @@ module Api::V1::GroupCategory
def group_category_json(group_category, user, session, options = {})
api_json(group_category, user, session, API_GROUP_CATEGORY_JSON_OPTS).
merge!(context_data(group_category)).
merge!(included_data(group_category, user, session, options[:include])).
merge!(group_category_data(group_category, user))
merge!(context_data(group_category)).
merge!(included_data(group_category, user, session, options[:include])).
merge!(group_category_sis(group_category, user)).
merge!(group_category_data(group_category, user))
end
private
@ -42,6 +43,17 @@ module Api::V1::GroupCategory
}
end
def group_category_sis(group_category, user)
hash = {}
if group_category.root_account.grants_any_right?(user, :read_sis, :manage_sis)
hash['sis_group_category_id'] = group_category.sis_source_id
end
if group_category.root_account.grants_right?(user, :manage_sis)
hash['sis_import_id'] = group_category.sis_batch_id
end
hash
end
def included_data(group_category, user, session, includes)
hash = {}
if includes

12
spec/apis/v1/group_categories_api_spec.rb
View File

@ -20,8 +20,8 @@ require File.expand_path(File.dirname(__FILE__) + '/../api_spec_helper')
require File.expand_path(File.dirname(__FILE__) + '/../file_uploads_spec_helper')
describe "Group Categories API", type: :request do
def category_json(category)
{
def category_json(category, user=@user)
json = {
'id' => category.id,
'name' => category.name,
'role' => category.role,
@ -36,6 +36,9 @@ describe "Group Categories API", type: :request do
'auto_leader' => category.auto_leader,
'is_member' => false
}
json['sis_group_category_id'] = category.sis_source_id if category.root_account.grants_any_right?(user, :read_sis, :manage_sis)
json['sis_import_id'] = category.sis_batch_id if category.root_account.grants_right?(user, :manage_sis)
json
end
before :once do
@ -563,24 +566,27 @@ describe "Group Categories API", type: :request do
it "should allow an admin to create an account group category" do
json = api_call(:post, "/api/v1/accounts/#{@account.id}/group_categories",
@category_path_options.merge(:action => 'create',
:sis_group_category_id => 'gc101',
:account_id => @account.to_param),
{'name' => 'name'})
category = GroupCategory.find(json["id"])
expect(json["context_type"]).to eq "Account"
expect(category.name).to eq 'name'
expect(category.sis_source_id).to eq 'gc101'
expect(json).to eq category_json(category)
end
it "should allow an admin to update a category for an account" do
api_call :put, "/api/v1/group_categories/#{@communities.id}",
@category_path_options.merge(:action => 'update',
:sis_group_category_id => 'gc101',
:group_category_id => @communities.to_param),
{:name => 'name'}
category = GroupCategory.find(@communities.id)
expect(category.name).to eq 'name'
expect(category.sis_source_id).to eq 'gc101'
end
it "should allow an admin to delete a category for an account" do
account_category = GroupCategory.create(:name => 'Groups', :context => @account)
expect(GroupCategory.find(@communities.id)).not_to eq nil

6
spec/apis/v1/groups_api_spec.rb
View File

@ -65,7 +65,7 @@ describe "Groups API", type: :request do
end
def group_category_json(group_category, user)
{
json = {
"auto_leader" => group_category.auto_leader,
"group_limit" => group_category.group_limit,
"id" => group_category.id,
@ -78,6 +78,9 @@ describe "Groups API", type: :request do
"allows_multiple_memberships" => group_category.allows_multiple_memberships?,
"is_member" => group_category.is_member?(user)
}
json['sis_group_category_id'] = group_category.sis_source_id if group_category.context.grants_any_right?(user, :read_sis, :manage_sis)
json['sis_import_id'] = group_category.sis_batch_id if group_category.context.grants_right?(user, :manage_sis)
json
end
def users_json(users, opts)
@ -106,6 +109,7 @@ describe "Groups API", type: :request do
'moderator' => membership.moderator,
}
json['sis_import_id'] = membership.sis_batch_id if membership.group.context_type == 'Account' && is_admin
json['sis_group_id'] = membership.group.sis_source_id if membership.group.context_type == 'Account' && is_admin
json
end

4
spec/lib/api/v1/group_category_spec.rb
View File

@ -27,7 +27,7 @@ end
describe "Api::V1::GroupCategory" do
describe "#group_category_json" do
let(:category){ GroupCategory.new(name: "mygroup") }
let(:category){ GroupCategory.new(name: "mygroup", root_account: Account.new) }
it "includes the auto_leader value" do
category.auto_leader = 'random'
@ -75,7 +75,7 @@ describe "Api::V1::GroupCategory" do
end
it 'checks the user against the category to set "is_member"' do
user = double
user = User.new
expect(category).to receive(:is_member?).with(user).and_return(true)
json = CategoryHarness.new.group_category_json(category, user, nil)
expect(json["is_member"]).to eq(true)

28
spec/models/account_spec.rb
View File

@ -672,27 +672,17 @@ describe Account do
end
it "group_categories should not include deleted categories" do
it "group_categories.active should not include deleted categories" do
account = Account.default
expect(account.group_categories.count).to eq 0
category1 = account.group_categories.create(:name => 'category 1')
category2 = account.group_categories.create(:name => 'category 2')
expect(account.group_categories.active.count).to eq 0
category1 = account.group_categories.create(name: 'category 1')
category2 = account.group_categories.create(name: 'category 2')
expect(account.group_categories.active.count).to eq 2
category1.destroy
account.reload
expect(account.group_categories.active.count).to eq 1
expect(account.group_categories.count).to eq 2
category1.destroy
account.reload
expect(account.group_categories.count).to eq 1
expect(account.group_categories.to_a).to eq [category2]
end
it "all_group_categories should include deleted categories" do
account = Account.default
expect(account.all_group_categories.count).to eq 0
category1 = account.group_categories.create(:name => 'category 1')
category2 = account.group_categories.create(:name => 'category 2')
expect(account.all_group_categories.count).to eq 2
category1.destroy
account.reload
expect(account.all_group_categories.count).to eq 2
expect(account.group_categories.active.to_a).to eq [category2]
end
it "should return correct values for login_handle_name_with_inference" do