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

conversations: improve recipient search 

fixes CNVS-4806, CNVS-4482

test plan:
  * open conversations
  * compose a new message
  * select a course
  * verify that you can search for a section in the course by name
  * verify that you can search for a group in the course by name
  * verify that you can send a message to an entire group if you have
      permissions to send it to the entire course

  * create a course with two sections
  * add students to the sections and add a TA who can only see
      students in one section
  * masquerade as the TA
  * verify that the other section does not appear in the recipient
      search section list
  * verify that users in the other section do not appear in the
      recipient search student list

  * create a course in a concluded term
  * open new conversations
  * verify that the course is listed in the concluded section of the
      filter list
  * masquerade as a teacher for the course
  * open old conversations
  * verify that the concluded course is present in the filter search
  * verify that the course is not present in the recipient search

Change-Id: I5a4e62de4e182cf0859dc47a3f8ad694f616a3bc
Reviewed-on: https://gerrit.instructure.com/27433
Tested-by: Jenkins <jenkins@instructure.com>
QA-Review: Matt Fairbourn <mfairbourn@instructure.com>
Reviewed-by: Zach Pendleton <zachp@instructure.com>
Product-Review: Braden Anderson <banderson@instructure.com>
This commit is contained in:
Braden Anderson 2013-12-06 15:11:44 -07:00
parent 504ecc7009
commit 1d25cfb67e
11 changed files with 107 additions and 57 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
app/coffeescripts/collections/CourseCollection.coffee
View File

@ -9,3 +9,4 @@ define [
initialize: () ->
super()
@setParam('state', ['unpublished', 'available', 'completed'])
@setParam('include', ['term'])

1
app/coffeescripts/conversations/Inbox.coffee
View File

@ -568,6 +568,7 @@ define [
selectAllText
baseData:
permissions: ["send_messages_all"]
messageable_only: true
return if $scope

39
app/coffeescripts/views/conversations/AutocompleteView.coffee
View File

@ -42,15 +42,17 @@ define [
# Internal: Currently selected results.
tokens: []
# Internal: A cache of per-course permissions.
# Internal: A cache of per-context permissions.
permissions: {}
# Internal: A cache of URL results.
cache: {}
# Internal: Construct the search URL for the given term.
url: (term) ->
baseURL = '/api/v1/search/recipients?'
params = { search: term, per_page: 20, 'permissions[]': 'send_messages_all' }
params = { search: term, per_page: 20, 'permissions[]': 'send_messages_all', synthetic_contexts: true }
params.context = @currentContext.id if @currentContext
params.synthetic_contexts = true unless term
baseURL + _.reduce(params, (queryString, v, k) ->
queryString.push("#{k}=#{v}")
@ -249,6 +251,7 @@ define [
#
# Returns nothing.
_onSearchResultLoad: =>
@cache[@currentUrl] = @resultCollection.toJSON()
_.extend(@permissions, @_getPermissions())
@_addEveryoneResult(@resultCollection) unless @excludeAll or !@_canSendToAll()
shouldDrawResults = @resultCollection.length
@ -264,7 +267,8 @@ define [
# Returns a boolean.
_canSendToAll: ->
return false unless @currentContext
@permissions[@_currentCourseOrGroup().id]
key = @currentContext.id.replace(/_(students|teachers)$/, '')
@permissions[key]
# Internal: Return permissions hashes from the current results.
#
@ -277,14 +281,6 @@ define [
map
, {}
# Internal: Return the current course context.
#
# Returns a context object.
_currentCourseOrGroup: ->
return @currentContext if @currentContext.id.match(/^(course|group)_\d+$/)
for context in @parentContexts
return context if context.id.match(/^(course|group)_\d+$/)
# Internal: Add, if appropriate, an "All in %{context}" result to the
# search results.
#
@ -335,10 +331,17 @@ define [
__fetchResults: (fetchIfEmpty = false) ->
return unless @$input.val() or fetchIfEmpty
@currentRequest?.abort()
@currentRequest = @resultCollection.fetch
url: @url(@$input.val())
success: @_onSearchResultLoad
@toggleResultList(true)
url = @url(@$input.val())
@currentUrl = url
if @cache[url]
@resultCollection.reset(@cache[url])
@toggleResultList(true)
@_onSearchResultLoad()
else
@currentRequest = @resultCollection.fetch
url: @url(@$input.val())
success: @_onSearchResultLoad
@toggleResultList(true)
# Internal: Delete the last token.
#
@ -370,7 +373,7 @@ define [
@currentContext = @parentContexts.pop()
@__fetchResults(true)
else if @selectedModel.get('isContext')
@parentContexts.push(@currentContext)
@parentContexts.push(@currentContext) if @currentContext
@$input.val('')
@currentContext =
id: @selectedModel.id
@ -430,7 +433,7 @@ define [
@currentContext = @parentContexts.pop()
@__fetchResults(true)
else if $target.hasClass('context')
@parentContexts.push(@currentContext)
@parentContexts.push(@currentContext) if @currentContext
@$input.val('')
@currentContext =
id: $target.data('id')

7
app/coffeescripts/views/conversations/CourseSelectionView.coffee
View File

@ -4,6 +4,7 @@ define [
'Backbone'
'compiled/views/conversations/SearchableSubmenuView'
'jst/conversations/courseOptions'
'jquery.instructure_date_and_time'
'use!vendor/bootstrap/bootstrap-dropdown'
'use!vendor/bootstrap-select/bootstrap-select'
], (I18n, _, {View, Collection}, SearchableSubmenuView, template) ->
@ -27,9 +28,13 @@ define [
super()
more = []
concluded = []
now = $.fudgeDateForProfileTimezone(new Date)
@options.courses.all.each((course) =>
if @options.courses.favorites.get(course.id) then return
collection = if course.get('workflow_state') == 'completed' then concluded else more
is_complete = course.get('workflow_state') == 'completed' ||
(course.get('end_at') && new Date(course.get('end_at')) < now) ||
(course.get('term').end_at && new Date(course.get('term').end_at) < now)
collection = if is_complete then concluded else more
collection.push(course.toJSON())
)
data =

6
app/coffeescripts/widget/TokenSelector.coffee
View File

@ -329,7 +329,7 @@ define [
list.appendTo(@$menu)
@list = list
@autoSelectFirst()
, 100
, 200
preparePost: (data) ->
postData = $.extend({}, @options.baseData ? {}, data ? {}, {search: @input.val().replace(/^\s+|\s+$/g, "")})
@ -341,12 +341,14 @@ define [
postData.exclude = postData.exclude.concat @input.tokenValues()
postData
lastFetch: null
collectionForQuery: (query) ->
@lastFetch?.abort()
cacheKey = JSON.stringify(query)
unless @cache[cacheKey]?
collection = new RecipientCollection
collection.url = @url
collection.fetch data: query
@lastFetch = collection.fetch data: query
@cache[cacheKey] = collection
@cache[cacheKey]

15
app/controllers/search_controller.rb
View File

@ -110,8 +110,10 @@ class SearchController < ApplicationController
params[:user_id] = api_find(User, params[:user_id]).id
end
permissions = params[:permissions] || []
permissions << :send_messages if params[:messageable_only]
load_all_contexts :context => get_admin_search_context(params[:context]),
:permissions => params[:permissions]
:permissions => permissions
types = (params[:types] || [] + [params[:type]]).compact
types |= [:course, :section, :group] if types.delete('context')
@ -139,6 +141,7 @@ class SearchController < ApplicationController
:context => params[:context],
:synthetic_contexts => params[:synthetic_contexts],
:include_inactive => params[:include_inactive],
:messageable_only => params[:messageable_only],
:exclude_ids => MessageableUser.context_recipients(exclude),
:search_all_contexts => params[:search_all_contexts],
:types => types[:context]
@ -223,7 +226,8 @@ class SearchController < ApplicationController
result = sections + groups
else # otherwise we show synthetic contexts
result = synthetic_contexts_for(course, context_name)
result << {:id => "#{context_name}_sections", :name => t(:course_sections, "Course Sections"), :item_count => sections.size, :type => :context} if sections.size > 1
found_custom_sections = sections.any? { |s| s[:id] != course[:default_section_id] }
result << {:id => "#{context_name}_sections", :name => t(:course_sections, "Course Sections"), :item_count => sections.size, :type => :context} if found_custom_sections
result << {:id => "#{context_name}_groups", :name => t(:student_groups, "Student Groups"), :item_count => groups.size, :type => :context} if groups.size > 0
return result
end
@ -278,10 +282,13 @@ class SearchController < ApplicationController
# more permission (possibly inherited from the course, like
# :send_messages_all)
ret[:permissions] = course_for_section(context)[:permissions]
elsif context[:type] == :group && context[:parent]
ret[:permissions] = course_for_group(context)[:permissions]
end
ret[:context_name] = context[:context_name] if context[:context_name] && context_name.nil?
ret
}
result = result.select{ |context| context[:permissions].include? :send_messages } if options[:messageable_only]
result.reject!{ |context| terms.any?{ |part| !context[:name].downcase.include?(part) } } if terms.present?
result.reject!{ |context| exclude.include?(context[:id]) }
@ -292,6 +299,10 @@ class SearchController < ApplicationController
@contexts[:courses][section[:parent][:course]]
end
def course_for_group(group)
course_for_section(group)
end
def synthetic_contexts_for(course, context)
@skip_users = true
# TODO: move the aggregation entirely into the DB. we only select a little

7
app/helpers/search_helper.rb
View File

@ -43,7 +43,8 @@ module SearchHelper
:term => term_for_course.call(course),
:state => type == :current ? :active : (course.recently_ended? ? :recently_active : :inactive),
:available => type == :current && course.available?,
:permissions => course.grants_rights?(@current_user)
:permissions => course.grants_rights?(@current_user),
:default_section_id => course.default_section.id
}
end
end
@ -81,7 +82,9 @@ module SearchHelper
if context.is_a?(Course)
add_courses.call [context], :current
add_sections.call context.course_sections
visibility = context.enrollment_visibility_level_for(@current_user, context.section_visibilities_for(@current_user), true)
sections = visibility == :sections ? context.sections_visible_to(@current_user) : context.course_sections
add_sections.call sections
add_groups.call context.groups
else
add_courses.call @current_user.concluded_courses.with_each_shard, :concluded

12
lib/messageable_user/calculator.rb
View File

@ -416,14 +416,10 @@ class MessageableUser
:include_concluded_students => false,
:course_workflow_state => course.workflow_state))
scope =
if options[:admin_context]
scope.where(full_visibility_clause([course]))
else
case course_visibility(course)
when :full then scope.where(full_visibility_clause([course]))
when :sections then scope.where(section_visibility_clause([course]))
when :restricted then scope.where(restricted_visibility_clause([course]))
end
case course_visibility(course)
when :full then scope.where(full_visibility_clause([course]))
when :sections then scope.where(section_visibility_clause([course]))
when :restricted then scope.where(restricted_visibility_clause([course]))
end
scope = scope.where(observer_restriction_clause) if student_courses.present?
scope = scope.where('enrollments.type' => enrollment_types) if enrollment_types

49
spec/controllers/search_controller_spec.rb
View File

@ -53,6 +53,19 @@ describe SearchController do
response.body.should include('billy')
end
it "should allow filtering out non-messageable courses" do
course_with_student_logged_in(:active_all => true)
@course.update_attribute(:name, "course1")
@course2 = course(:active_all => 1)
@course2.enroll_student(@user).accept
@course2.update_attribute(:name, "course2")
term = @course2.root_account.enrollment_terms.create! :name => "Fall", :end_at => 1.day.ago
@course2.update_attributes! :enrollment_term => term
get 'recipients', {search: 'course', :messageable_only => true}
response.body.should include('course1')
response.body.should_not include('course2')
end
context "with admin_context" do
it "should return nothing if the user doesn't have rights" do
user_session(user)
@ -93,6 +106,42 @@ describe SearchController do
response.body.should include(@student.name)
end
end
context "with section privilege limitations" do
before do
course_with_teacher_logged_in(:active_all => true)
@section = @course.course_sections.create!(:name => 'Section1')
@section2 = @course.course_sections.create!(:name => 'Section2')
@enrollment.update_attribute(:course_section, @section)
@enrollment.update_attribute(:limit_privileges_to_course_section, true)
@student1 = user_with_pseudonym(:active_all => true, :name => 'Student1', :username => 'student1@instructure.com')
@section.enroll_user(@student1, 'StudentEnrollment', 'active')
@student2 = user_with_pseudonym(:active_all => true, :name => 'Student2', :username => 'student2@instructure.com')
@section2.enroll_user(@student2, 'StudentEnrollment', 'active')
end
it "should exclude non-messageable contexts" do
get 'recipients', {
:context => "course_#{@course.id}",
:synthetic_contexts => true
}
response.body.should include('"name":"Course Sections"')
get 'recipients', {
:context => "course_#{@course.id}_sections",
:synthetic_contexts => true
}
response.body.should include('Section1')
response.body.should_not include('Section2')
end
it "should exclude non-messageable users" do
get 'recipients', {
:context => "course_#{@course.id}_students"
}
response.body.should include('Student1')
response.body.should_not include('Student2')
end
end
end
end

9
spec/lib/messageable_user/calculator_spec.rb
View File

@ -987,15 +987,6 @@ describe "MessageableUser::Calculator" do
should_not include(@student.id)
end
end
context "with admin_context" do
it "should treat the course as if fully visible" do
student_in_course(:active_all => true, :section => @course.course_sections.create!)
Enrollment.limit_privileges_to_course_section!(@course, @viewing_user, true)
@calculator.messageable_users_in_course(@course, :admin_context => @course).map(&:id).
should include(@student.id)
end
end
end
describe "messageable_users_in_section" do

18
spec/selenium/conversations_recipient_finder_spec.rb
View File

@ -57,7 +57,6 @@ describe "conversations recipient finder" do
end
it "should respect permissions" do
# only affects courses/sections, not groups
RoleOverride.create!(:context => Account.default, :permission => 'send_messages_all', :enrollment_type => 'TeacherEnrollment', :enabled => false)
browse_menu
@ -84,21 +83,10 @@ describe "conversations recipient finder" do
end
browse "Student Groups" do
menu.should == ["the group"]
browse("the group") { menu.should == ["Select All", "nobody@example.com", "student 1"] }
browse("the group") { menu.should == ["nobody@example.com", "student 1"] }
end
end
browse("the group") { menu.should == ["Select All", "nobody@example.com", "student 1"] }
end
it "should return recently concluded courses" do
@course.complete!
browse_menu
menu.should == ["the course", "the group"]
search("course") do
menu.should == ["the course"]
end
browse("the group") { menu.should == ["nobody@example.com", "student 1"] }
end
it "should not show concluded enrollments as students in the course" do
@ -119,7 +107,7 @@ describe "conversations recipient finder" do
@course.update_attribute :conclude_at, 1.year.ago
browse_menu
menu.should == ["the group"]
menu.should == ["No results found"]
search("course") do
menu.should == ["No results found"]