Skip to content

Commit 7303de4

Browse files
author
Brad Horrocks
committed
Optimized endpints for pulling back lots O' results
You can now exclude pseudonym info on api requests. users_json will now preload section information when you include[]=section Fixes CNVS-14836 Fixes CNVS-13093 Fixes CNVS-12932 Fixes CNVS-16216 Fixes CNVS-16218 Test Plan: - Create a course with 800 users - Create a group set; add 150 groups to set. - Load groups page it should load way faster than before - Make sure search isn't slow Change-Id: Icff28b00141fb3573b9ed9493e8dc38521a368e4 Reviewed-on: https://gerrit.instructure.com/43356 Reviewed-by: Jacob Fugal <jacob@instructure.com> Reviewed-by: Andrew Butterfield <abutterfield@instructure.com> Tested-by: Jenkins <jenkins@instructure.com> Reviewed-by: Joel Hough <joel@instructure.com> QA-Review: Steven Shepherd <sshepherd@instructure.com> Product-Review: Brad Horrocks <bhorrocks@instructure.com>
1 parent 8287cc1 commit 7303de4

11 files changed

Lines changed: 80 additions & 23 deletions

File tree

app/coffeescripts/collections/GroupUserCollection.coffee

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ define [
1414
@optionProperty 'category'
1515

1616
url: ->
17-
@url = "/api/v1/groups/#{@group.id}/users?per_page=50"
17+
@url = "/api/v1/groups/#{@group.id}/users?per_page=50&include[]=sections&exclude[]=pseudonym"
1818

1919
initialize: (models) ->
2020
super

app/coffeescripts/collections/UnassignedGroupUserCollection.coffee

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ define [
66
class UnassignedGroupUserCollection extends GroupUserCollection
77

88
url: ->
9-
_url = "/api/v1/group_categories/#{@category.id}/users?per_page=50"
9+
_url = "/api/v1/group_categories/#{@category.id}/users?per_page=50&include[]=sections&exclude[]=pseudonym"
1010
_url += "&unassigned=true" unless @category.get('allows_multiple_memberships')
1111
@url = _url
1212

@@ -16,4 +16,19 @@ define [
1616
@category.get('allows_multiple_memberships')
1717

1818
increment: (amount) ->
19-
@category.increment 'unassigned_users_count', amount
19+
@category.increment 'unassigned_users_count', amount
20+
21+
search: (filter, options) ->
22+
options = options || {}
23+
options.reset = true
24+
25+
if filter && filter.length >= 3
26+
options.url = @url + "&search_term=" + filter
27+
@filtered = true
28+
return @fetch(options)
29+
else if @filtered
30+
@filtered = false
31+
options.url = @url
32+
return @fetch(options)
33+
34+
# do nothing

app/coffeescripts/views/groups/manage/GroupCategoriesView.coffee

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@ define [
4040
@refreshTabs()
4141
@loadTabFromUrl()
4242

43-
4443
refreshTabs: ->
4544
if @collection.length > 0
4645
@$tabs.find('ul.ui-tabs-nav li.static').remove()
@@ -132,6 +131,7 @@ define [
132131
@loadPanelView($panel)
133132

134133
loadPanelView: ($panel) ->
134+
# there is a bug here where we load the first tab, then immediately load the tab from the hash
135135
if !$panel.data('loaded')
136136
model = $panel.data('model')
137137
categoryView = new GroupCategoryView model: model

app/coffeescripts/views/groups/manage/GroupCategoryView.coffee

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,15 @@
11
define [
22
'i18n!groups'
33
'Backbone'
4+
'underscore'
45
'compiled/views/groups/manage/GroupCategoryDetailView'
56
'compiled/views/groups/manage/GroupsView'
67
'compiled/views/groups/manage/UnassignedUsersView'
78
'compiled/views/groups/manage/AddUnassignedMenu'
89
'jst/groups/manage/groupCategory'
910
'compiled/jquery.rails_flash_notifications'
1011
'jquery.disableWhileLoading'
11-
], (I18n, {View}, GroupCategoryDetailView, GroupsView, UnassignedUsersView, AddUnassignedMenu, template) ->
12+
], (I18n, {View}, _, GroupCategoryDetailView, GroupsView, UnassignedUsersView, AddUnassignedMenu, template) ->
1213

1314
class GroupCategoryView extends View
1415

@@ -55,6 +56,12 @@ define [
5556
groupsCollection: @groups
5657
}
5758

59+
filterChange: (event) ->
60+
search_term = event.target.value
61+
@options.unassignedUsersView.setFilter(search_term)
62+
63+
@_setUnassignedHeading(@originalCount) unless search_term.length >= 3
64+
5865
attach: ->
5966
@model.on 'destroy', @remove, this
6067
@model.on 'change', => @groupsView.updateDetails()
@@ -73,6 +80,11 @@ define [
7380

7481
cacheEls: ->
7582
super
83+
84+
if !@attachedFilter
85+
@$filterUnassignedUsers.on "keyup", _.debounce(@filterChange.bind(this), 300)
86+
@attachedFilter = true
87+
7688
# need to be set before their afterRender's run (i.e. before this
7789
# view's afterRender)
7890
@groupsView.$externalFilter = @$filter
@@ -84,6 +96,10 @@ define [
8496

8597
setUnassignedHeading: ->
8698
count = @model.unassignedUsersCount() ? 0
99+
@originalCount = @originalCount || count
100+
@_setUnassignedHeading(count)
101+
102+
_setUnassignedHeading: (count) ->
87103
@unassignedUsersView.render() if @unassignedUsersView
88104
@$unassignedUsersHeading.text(
89105
if @model.get('allows_multiple_memberships')

app/coffeescripts/views/groups/manage/GroupUserView.coffee

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
define [
2+
'i18n!groups'
23
'underscore'
34
'Backbone'
45
'jst/groups/manage/groupUser'
5-
], (_, {View}, template) ->
6+
], (I18n, _, {View}, template) ->
67

78
class GroupUserView extends View
89

@@ -14,6 +15,8 @@ define [
1415
className: 'group-user'
1516

1617
template: template
18+
attributes:
19+
"data-tooltip": "top"
1720

1821
els:
1922
'.al-trigger': '$userActions'
@@ -25,6 +28,7 @@ define [
2528
@model.on 'change', @render, this
2629

2730
afterRender: ->
31+
@$el.attr("title", I18n.t('Sections: %{sections}', sections: @model.get('sections')))
2832
@$el.data('model', @model)
2933

3034
highlight: ->

app/coffeescripts/views/groups/manage/UnassignedUsersView.coffee

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
define [
2+
'i18n!groups'
23
'jquery'
34
'underscore'
45
'compiled/views/groups/manage/GroupUsersView'
56
'compiled/views/groups/manage/AssignToGroupMenu'
67
'compiled/views/groups/manage/Scrollable'
7-
'compiled/views/Filterable'
88
'jst/groups/manage/groupUsers'
9-
], ($, _, GroupUsersView, AssignToGroupMenu, Scrollable, Filterable, template) ->
9+
], (I18n, $, _, GroupUsersView, AssignToGroupMenu, Scrollable, template) ->
1010

1111
class UnassignedUsersView extends GroupUsersView
1212

@@ -24,8 +24,9 @@ define [
2424
GroupUsersView::els,
2525
'.no-results-wrapper': '$noResultsWrapper'
2626
'.no-results': '$noResults'
27+
'.invalid-filter': '$invalidFilter'
2728

28-
@mixin Filterable, Scrollable
29+
@mixin Scrollable
2930

3031
dropOptions:
3132
accept: '.group-user'
@@ -37,13 +38,11 @@ define [
3738
@collection.on 'reset', @render
3839
@collection.on 'remove', @render
3940
@collection.on 'moved', @highlightUser
40-
@collection.on 'filterOut', _(=> @checkScroll()).debounce(50)
4141

4242
@collection.once 'fetch', => @$noResultsWrapper.hide()
4343
@collection.on 'fetched:last', => @$noResultsWrapper.show()
4444

4545
afterRender: ->
46-
@$filter = @$externalFilter
4746
super
4847
@collection.load('first')
4948
@$el.parent().droppable(_.extend({}, @dropOptions))
@@ -77,6 +76,15 @@ define [
7776
hideAssignToGroup: ->
7877
@assignToGroupMenu?.hide()
7978

79+
setFilter: (search_term, options) ->
80+
searchDefer = @collection.search(search_term, options)
81+
searchDefer.always(=>
82+
if search_term.length < 3
83+
shouldShow = search_term.length > 0
84+
@$invalidFilter.toggleClass("hidden", !shouldShow)
85+
@$noResultsWrapper.toggle(shouldShow)
86+
) if searchDefer
87+
8088
canAssignToGroup: ->
8189
@options.canAssignToGroup and @groupsCollection.length
8290

app/controllers/group_categories_controller.rb

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -332,7 +332,6 @@ def users
332332
end
333333

334334
search_term = params[:search_term].presence
335-
336335
search_params = params.slice(:search_term)
337336
search_params[:enrollment_type] = "student" if @context.is_a? Course
338337

@@ -347,7 +346,7 @@ def users
347346
end
348347

349348
users = Api.paginate(users, self, api_v1_group_category_users_url)
350-
render :json => users.map { |u| user_json(u, @current_user, session, [], @context) }
349+
render :json => users_json(users, @current_user, session, Array(params[:include]), @context, nil, Array(params[:exclude]))
351350
end
352351

353352
# @API Assign unassigned members

app/controllers/groups_controller.rb

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -626,15 +626,14 @@ def users
626626
return unless authorized_action(@context, @current_user, :read)
627627

628628
search_term = params[:search_term].presence
629-
630629
if search_term
631630
users = UserSearch.for_user_in_context(search_term, @context, @current_user, session)
632631
else
633632
users = UserSearch.scope_for(@context, @current_user)
634633
end
635634

636635
users = Api.paginate(users, self, api_v1_group_users_url)
637-
render :json => users_json(users, @current_user, session, Array(params[:include]))
636+
render :json => users_json(users, @current_user, session, Array(params[:include]), @context, nil, Array(params[:exclude]))
638637
end
639638

640639
def edit

app/views/jst/groups/manage/groupUsers.handlebars

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
{{#ifAny count loading}}
2-
<ul class="collectionViewItems unstyled"></ul>
32
<div class="no-results-wrapper">
43
<div class="no-results hidden">
54
{{#ifEqual ENV.group_user_type "student"}}
@@ -8,7 +7,11 @@
87
{{#t "no_matching_users"}}No matching users.{{/t}}
98
{{/ifEqual}}
109
</div>
10+
<div class="invalid-filter hidden">
11+
{{#t "greater_than_three"}}Please enter a search term with three or more characters{{/t}}
12+
</div>
1113
</div>
14+
<ul class="collectionViewItems unstyled"></ul>
1215
<div class="paginatedLoadingIndicator"></div>
1316
{{else}}
1417
<div class="no-results">

lib/api/v1/user.rb

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,11 @@ def user_json_preloads(users, preload_email=false)
3636
end
3737
end
3838

39-
def user_json(user, current_user, session, includes = [], context = @context, enrollments = nil)
39+
def user_json(user, current_user, session, includes = [], context = @context, enrollments = nil, excludes = [])
4040
includes ||= []
41+
excludes ||= []
4142
api_json(user, current_user, session, API_USER_JSON_OPTS).tap do |json|
42-
if user_json_is_admin?(context, current_user)
43+
if !excludes.include?('pseudonym') && user_json_is_admin?(context, current_user)
4344
include_root_account = @domain_root_account.trust_exists?
4445
if sis_pseudonym = user.sis_pseudonym_for(@domain_root_account, include_root_account)
4546
# the sis fields on pseudonym are poorly named -- sis_user_id is
@@ -67,6 +68,13 @@ def user_json(user, current_user, session, includes = [], context = @context, en
6768
if includes.include?('email') && context.grants_right?(current_user, session, :read_as_admin)
6869
json[:email] = user.email
6970
end
71+
72+
if includes.include?('sections')
73+
json[:sections] = user.enrollments.
74+
map(&:course_section).compact.uniq.
75+
map(&:name).join(", ")
76+
end
77+
7078
json[:locale] = user.locale if includes.include?('locale')
7179
json[:confirmation_url] = user.communication_channels.email.first.try(:confirmation_url) if includes.include?('confirmation_url')
7280

@@ -80,8 +88,13 @@ def user_json(user, current_user, session, includes = [], context = @context, en
8088
end
8189
end
8290

83-
def users_json(users, current_user, session, includes = [], context = @context, enrollments = nil)
84-
users.map{ |user| user_json(user, current_user, session, includes, context, enrollments) }
91+
def users_json(users, current_user, session, includes = [], context = @context, enrollments = nil, excludes = [])
92+
93+
if includes.include?('sections')
94+
ActiveRecord::Associations::Preloader.new(users, enrollments: :course_section).run
95+
end
96+
97+
users.map{ |user| user_json(user, current_user, session, includes, context, enrollments, excludes) }
8598
end
8699

87100
# this mini-object is used for secondary user responses, when we just want to

0 commit comments

Comments
 (0)