adds user search functionality to the group api

see the @argument search_term in the following doc:

  # @API List group's users
  #
  # Returns a list of users in the group.
  #
  # @argument search_term (optional)
  #   The partial name or full ID of the users to match and return in the results list.
  #   Must be at least 3 characters.
  #
  # @example_request
  #     curl https://<canvas>/api/v1/groups/1/users \
  #          -H 'Authorization: Bearer <token>'
  #
  # @returns [User]

test plan)

1) verify the specs work
2) make a request (see above)
   (e.g. https://<canvas>/api/v1/groups/1/users?search_term=bob)
3) verify it returns the users in the specified group with the related name or full ID

fixes #CNVS-6152

Change-Id: I034b8b6ec2779895408276a894a3cf978baee12b
Reviewed-on: https://gerrit.instructure.com/21252
Tested-by: Jenkins <jenkins@instructure.com>
Product-Review: Marc LeGendre <marc@instructure.com>
QA-Review: Marc LeGendre <marc@instructure.com>
Reviewed-by: Jon Jensen <jon@instructure.com>
Tested-by: Landon Wilkins <lwilkins@instructure.com>
This commit is contained in:
Landon Wilkins 2013-06-05 19:21:29 -06:00 committed by Landon Wilkins
parent b2e9773832
commit 4c7ab5a576
6 changed files with 74 additions and 23 deletions

View File

@ -394,7 +394,7 @@ class CoursesController < ApplicationController
end
if search_term
users = UserSearch.for_user_in_course(search_term, @context, @current_user, search_params)
users = UserSearch.for_user_in_context(search_term, @context, @current_user, search_params)
else
users = UserSearch.scope_for(@context, @current_user, search_params)
end

View File

@ -567,16 +567,35 @@ class GroupsController < ApplicationController
#
# Returns a list of users in the group.
#
# @argument search_term (optional)
# The partial name or full ID of the users to match and return in the results list.
# Must be at least 3 characters.
#
# @example_request
# curl https://<canvas>/api/v1/groups/1/users \
# curl https://<canvas>/api/v1/groups/1/users \
# -H 'Authorization: Bearer <token>'
#
# @returns [User]
def users
return unless authorized_action(@context, @current_user, :read)
@users = Api.paginate(@context.users, self, api_v1_group_users_url)
render :json => @users.map { |u| user_json(u, @current_user, session) }
search_term = params[:search_term]
if search_term && search_term.size < 3
return render \
:json => {
"status" => "argument_error",
"message" => "search_term of 3 or more characters is required" },
:status => :bad_request
end
if search_term
users = UserSearch.for_user_in_context(search_term, @context, @current_user)
else
users = UserSearch.scope_for(@context, @current_user)
end
users = Api.paginate(users, self, api_v1_group_users_url)
render :json => users.map { |u| user_json(u, @current_user, session) }
end
def edit

View File

@ -383,6 +383,10 @@ class Group < ActiveRecord::Base
can :leave
end
def users_visible_to(user)
grants_rights?(user, :read) ? users : users.where("?", false)
end
# Helper needed by several permissions, use grants_right?(user, :participate)
def can_participate?(user)
return false unless user.present? && self.context.present?

View File

@ -1,7 +1,7 @@
module UserSearch
def self.for_user_in_course(search_term, course, searcher, options = {})
base_scope = scope_for(course, searcher, options.slice(:enrollment_type, :enrollment_role))
def self.for_user_in_context(search_term, context, searcher, options = {})
base_scope = scope_for(context, searcher, options.slice(:enrollment_type, :enrollment_role))
if search_term.to_s =~ Api::ID_REGEX
user = base_scope.find_by_id(search_term)
return [user] if user
@ -29,11 +29,11 @@ module UserSearch
wildcard_pattern(search_term, :type => pattern_type)
end
def self.scope_for(course, searcher, options={})
def self.scope_for(context, searcher, options={})
enrollment_role = Array(options[:enrollment_role]) if options[:enrollment_role]
enrollment_type = Array(options[:enrollment_type]) if options[:enrollment_type]
users = course.users_visible_to(searcher).uniq.order_by_sortable_name
users = context.users_visible_to(searcher).uniq.order_by_sortable_name
if enrollment_role
users = users.where("COALESCE(enrollments.role_name, enrollments.type) IN (?) ", enrollment_role)

View File

@ -524,6 +524,16 @@ describe "Groups API", :type => :integration do
end
context "users" do
let(:api_url) { "/api/v1/groups/#{@community.id}/users.json" }
let(:api_route) do
{
:controller => 'groups',
:action => 'users',
:group_id => @community.to_param,
:format => 'json'
}
end
it "should return users in a group" do
expected_keys = %w{id name sortable_name short_name}
json = api_call(:get, "/api/v1/groups/#{@community.id}/users",
@ -541,6 +551,24 @@ describe "Groups API", :type => :integration do
{ :controller => 'groups', :action => 'users', :group_id => @community.to_param, :format => 'json' })
response.code.should == '401'
end
it "returns an error when search_term is fewer than 3 characters" do
json = api_call(:get, api_url, api_route, {:search_term => '12'}, {}, :expected_status => 400)
json["status"].should == "argument_error"
json["message"].should == "search_term of 3 or more characters is required"
end
it "returns a list of users" do
expected_keys = %w{id name sortable_name short_name}
json = api_call(:get, api_url, api_route, {:search_term => 'value'})
json.count.should == 1
json.each do |user|
(user.keys & expected_keys).sort.should == expected_keys.sort
@community.users.map(&:id).should include(user['id'])
end
end
end
context "group files" do

View File

@ -2,10 +2,10 @@ require File.expand_path( '../spec_helper' , File.dirname(__FILE__))
describe UserSearch do
describe '.for_user_in_course' do
describe '.for_user_in_context' do
let(:search_names) { ['Rose Tyler', 'Martha Jones', 'Rosemary Giver', 'Martha Stewart', 'Tyler Pickett', 'Jon Stewart', 'Stewart Little'] }
let(:course) { Course.create! }
let(:users) { UserSearch.for_user_in_course('Stewart', course, user).to_a }
let(:users) { UserSearch.for_user_in_context('Stewart', course, user).to_a }
let(:names) { users.map(&:name) }
let(:user) { User.last }
let(:student) { User.find_by_name(search_names.last) }
@ -38,7 +38,7 @@ describe UserSearch do
it 'does not contain users I am not allowed to see' do
unenrolled_user = User.create!(:name => 'Unenrolled User')
search_results = UserSearch.for_user_in_course('Stewart', course, unenrolled_user).map(&:name)
search_results = UserSearch.for_user_in_context('Stewart', course, unenrolled_user).map(&:name)
search_results.should == []
end
@ -50,14 +50,14 @@ describe UserSearch do
end
it 'will find teachers' do
results = UserSearch.for_user_in_course('Tyler', course, user)
results = UserSearch.for_user_in_context('Tyler', course, user)
results.map(&:name).should include('Tyler Teacher')
end
describe 'filtering by role' do
subject { names }
describe 'to a single role' do
let(:users) { UserSearch.for_user_in_course('Tyler', course, user, :enrollment_type => 'student' ).to_a }
let(:users) { UserSearch.for_user_in_context('Tyler', course, user, :enrollment_type => 'student' ).to_a }
it { should include('Rose Tyler') }
it { should include('Tyler Pickett') }
@ -65,7 +65,7 @@ describe UserSearch do
end
describe 'to multiple roles' do
let(:users) { UserSearch.for_user_in_course('Tyler', course, student, :enrollment_type => ['ta', 'teacher'] ).to_a }
let(:users) { UserSearch.for_user_in_context('Tyler', course, student, :enrollment_type => ['ta', 'teacher'] ).to_a }
before do
ta = User.create!(:name => 'Tyler TA')
TaEnrollment.create!(:user => ta, :course => course, :workflow_state => 'active')
@ -78,7 +78,7 @@ describe UserSearch do
describe 'with the broader role parameter' do
let(:users) { UserSearch.for_user_in_course('Tyler', course, student, :enrollment_role => 'ObserverEnrollment' ).to_a }
let(:users) { UserSearch.for_user_in_context('Tyler', course, student, :enrollment_role => 'ObserverEnrollment' ).to_a }
before do
ta = User.create!(:name => 'Tyler Observer')
@ -105,14 +105,14 @@ describe UserSearch do
end
it 'will match against an sis id' do
UserSearch.for_user_in_course("SOME_SIS", course, user).should == [user]
UserSearch.for_user_in_context("SOME_SIS", course, user).should == [user]
end
it 'can match an SIS id and a user name in the same query' do
pseudonym.sis_user_id = "MARTHA_SIS_ID"
pseudonym.save!
other_user = User.find_by_name('Martha Stewart')
results = UserSearch.for_user_in_course("martha", course, user)
results = UserSearch.for_user_in_context("martha", course, user)
results.should include(user)
results.should include(other_user)
end
@ -123,24 +123,24 @@ describe UserSearch do
before { user.communication_channels.create!(:path => 'the.giver@example.com', :path_type => CommunicationChannel::TYPE_EMAIL) }
it 'matches against an email' do
UserSearch.for_user_in_course("the.giver", course, user).should == [user]
UserSearch.for_user_in_context("the.giver", course, user).should == [user]
end
it 'can match an email and a name in the same query' do
results = UserSearch.for_user_in_course("giver", course, user)
results = UserSearch.for_user_in_context("giver", course, user)
results.should include(user)
results.should include(User.find_by_name('Rosemary Giver'))
end
it 'will not match channels where the type is not email' do
user.communication_channels.last.update_attributes!(:path_type => CommunicationChannel::TYPE_TWITTER)
UserSearch.for_user_in_course("the.giver", course, user).should == []
UserSearch.for_user_in_context("the.giver", course, user).should == []
end
end
describe 'searching by a DB ID' do
it 'matches against the database id' do
UserSearch.for_user_in_course(user.id, course, user).should == [user]
UserSearch.for_user_in_context(user.id, course, user).should == [user]
end
end
end
@ -169,12 +169,12 @@ describe UserSearch do
pseudonym.sis_user_id = "SOME_SIS_ID"
pseudonym.unique_id = "SOME_UNIQUE_ID@example.com"
pseudonym.save!
UserSearch.for_user_in_course("SOME_SIS", course, user).should == []
UserSearch.for_user_in_context("SOME_SIS", course, user).should == []
end
it 'does not match against emails' do
user.communication_channels.create!(:path => 'the.giver@example.com', :path_type => CommunicationChannel::TYPE_EMAIL)
UserSearch.for_user_in_course("the.giver", course, user).should == []
UserSearch.for_user_in_context("the.giver", course, user).should == []
end
end
end