sort users on course people page by sortable_name.

fixes #10347, #10440

this commit also replaces the display of login_id with the
user's email address.

test plan:
  * create a course with users;
  * navigate to courses/:id/users and verify that students and
    teachers/tas are being sorted by sortable_name;
  * make some test calls to /api/v1/courses/:id/users and verify
    that it continues to work as expected.
  * confirm that user's email address is displayed instead of their
    login_id.

Change-Id: Id67557d98e8e2c0c42402ee64528e1f27c91b78c
Reviewed-on: https://gerrit.instructure.com/13592
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Ryan Florence <ryanf@instructure.com>
This commit is contained in:
Zach Pendleton 2012-09-10 15:07:48 -06:00
parent ce1de9c2df
commit 64dd2248e8
6 changed files with 98 additions and 159 deletions

View File

@ -19,54 +19,48 @@
require [
'jquery'
'underscore'
'compiled/collections/EnrollmentCollection'
'compiled/collections/UserCollection'
'compiled/collections/SectionCollection'
'compiled/views/courses/RosterView'
], ($, _, EnrollmentCollection, SectionCollection, RosterView) ->
], ($, _, UserCollection, SectionCollection, RosterView) ->
rosterPage =
init: ->
@loadEnvironment()
@cacheElements()
@createCollections()
# Load environment
course = ENV.context_asset_string.split('_')[1]
url = "/api/v1/courses/#{course}/users"
fetchOptions =
include: ['avatar_url', 'enrollments', 'email']
per_page: 50
# Get the course ID and create the enrollments API url.
#
# @api public
# @return nothing
loadEnvironment: ->
@course = ENV.context_asset_string.split('_')[1]
@url = "/api/v1/courses/#{@course}/enrollments"
# Cache elements
$studentList = $('.student_roster .user_list')
$teacherList = $('.teacher_roster .user_list')
# Store DOM elements used.
#
# @api public
# @return nothing
cacheElements: ->
@$studentList = $('.student_roster .user_list')
@$teacherList = $('.teacher_roster .user_list')
# Create views
sections = new SectionCollection(ENV.SECTIONS)
students = new UserCollection
teachers = new UserCollection
# Create the view and collection objects needed for the page.
#
# @api public
# @return nothing
createCollections: ->
@sections = new SectionCollection(ENV.SECTIONS)
students = new EnrollmentCollection
teachers = new EnrollmentCollection
_.each [students, teachers], (c) ->
c.url = url
c.sections = sections
_.each [students, teachers], (c) =>
c.url = @url
c.sections = @sections
studentOptions = add: false, data: _.extend({}, fetchOptions, enrollment_type: 'student')
teacherOptions = add: false, data: _.extend({}, fetchOptions, enrollment_type: ['teacher', 'ta'])
@studentView = new RosterView
el: @$studentList
collection: students
requestOptions: type: ['StudentEnrollment']
@teacherView = new RosterView
el: @$teacherList
collection: teachers
requestOptions: type: ['TeacherEnrollment', 'TaEnrollment']
studentView = new RosterView
collection: students
el: $studentList
fetchOptions: studentOptions
teacherView = new RosterView
collection: teachers
el: $teacherList
fetchOptions: teacherOptions
# Add events
students.on('reset', studentView.render, studentView)
teachers.on('reset', teacherView.render, teacherView)
# Fetch roster
studentView.$el.disableWhileLoading(students.fetch(studentOptions))
teacherView.$el.disableWhileLoading(teachers.fetch(teacherOptions))
# Start loading the page.
rosterPage.init()

View File

@ -27,44 +27,3 @@ define [
class EnrollmentCollection extends PaginatedCollection
model: Enrollment
# Format returned responses by flattening the enrollment/user objects
# returned and adding a section name if given sections.
#
# @param response {Object} - A parsed JSON object from the server.
#
# @api private
# @return a formatted JSON response
parse: (response) ->
_.map(response, @flattenEnrollment)
super
# Add the returned user elements to the parent enrollment object to
# make templating easier (e.g. remove all {{#with}} calls in Handlebars.
#
# @param enrollment {Object} - An enrollment object w/ a user sub-object.
#
# @api private
# @return a formatted enrollment JSON object
flattenEnrollment: (enrollment) =>
id = enrollment.user.id
delete enrollment.user.id
enrollment[key] = value for key, value of enrollment.user
enrollment.user.id = id
@storeSection(enrollment) if @sections?
enrollment
# If the collection has been assigned a SectionCollection as @sections,
# use the course_section_id to find the section name and add it as
# course_section_name to the enrollment.
#
# NOTE: This function side-effects the passed enrollment to add the
# given column. It doesn't return anything.
#
# @param enrollment {Object} - An enrollment object.
#
# @api private
# @return nothing
storeSection: (enrollment) ->
section = @sections.find((section) -> section.get('id') == enrollment.course_section_id)
enrollment.course_section_name = section.get('name')

View File

@ -23,90 +23,68 @@ define [
'jst/courses/RosterUser'
], ($, _, PaginatedView, rosterUser) ->
# This view displays a paginated collection of users inside of a course.
# RosterView: Display a paginated collection of users inside of a course.
#
# @examples
# Examples
#
# view = RosterView.new
# el: $('...')
# collection: EnrollmentCollection.new(...')
#
# view.collection.on('reset', view.render)
# view = new RosterView el: $('..'), collection: new UserCollection(...)
# view.collection.on('reset', view.render, view)
# view.collection.fetch(...)
class RosterView extends PaginatedView
# Default options to be passed to the server on each request for new
# collection records.
fetchOptions:
include: ['avatar_url']
per_page: 50
# Create and configure a new RosterView.
# Public: Create a new instance.
#
# @param el {jQuery} - The parent element (should have overflow: hidden and
# a height for infinite scroll).
# @param collection {EnrollmentCollection} - The collection to retrieve
# results from.
# @param options {Object} - Configuration options.
# - requestOptions: options to be passed w/ every server call.
#
# @examples
#
# view = new RosterView
# el: $(...)
# collection: new EnrollmentCollection
# url: ...
# sections: ENV.SECTIONS
# requestOptions:
# type: ['StudentEnrollment']
# include: ['avatar_url']
# per_page: 25
#
# @api public
# @return a RosterView.
initialize: (options) ->
@fetchOptions =
data: _.extend({}, @fetchOptions, options.requestOptions)
add: false
@collection.on('reset', @render, this)
# fetchOptions - Options to be passed to @collection.fetch(). Needs to be
# passed for subsequent page gets (see PaginatedView).
initialize: ({fetchOptions}) ->
@paginationScrollContainer = @$el
@$el.disableWhileLoading(@collection.fetch(@fetchOptions))
super(fetchOptions: @fetchOptions)
super(fetchOptions: fetchOptions)
# Append newly fetched records to the roster list.
# Public: Append new records to the roster list.
#
# @api private
# @return nothing.
# Returns nothing.
render: ->
users = @combinedSectionEnrollments(@collection)
enrollments = _.map(users, @renderUser)
@$el.append(enrollments.join(''))
@combineSectionNames(@collection)
@appendCourseId(@collection)
html = _.map(@collection.models, @renderUser)
@$el.append(html.join(''))
super
# Create the HTML for a given user record.
# Public: Return HTML for a given record.
#
# @param enrollment - An enrollment model.
# user - The user object to render as HTML.
#
# @api private
# @return nothing.
renderUser: (enrollment) ->
rosterUser(enrollment.toJSON())
# Returns an HTML string.
renderUser: (user) ->
rosterUser(user.toJSON())
# Take users in multiple sections and combine their section names
# into an array to be displayed in a list.
# Internal: Mutate a user collection, adding a sectionNames property to
# each child model.
#
# @param collection {EnrollmentCollection} - Enrollments to format.
# collection - The collection to alter.
#
# @api private
# @return an array of user models.
combinedSectionEnrollments: (collection) ->
users = collection.groupBy (enrollment) -> enrollment.get('user_id')
enrollments = _.reduce users, (list, enrollments, key) ->
enrollment = enrollments[0]
names = _.map(enrollments, (e) -> e.get('course_section_name'))
# do it this way instead of calling .set(...) so that we don't fire an
# extra page load from PaginatedView.
enrollment.attributes.course_section_name = _.uniq(names)
list.push(enrollment)
list
, []
enrollments
# Returns nothing.
combineSectionNames: (collection) ->
collection.each (user) =>
user.set('sectionNames', @getSections(user), silent: true)
# Internal: Mutate a user collection, adding a course_id attribute to
# each child model.
#
# collection - The collection to alter.
#
# Returns nothing
appendCourseId: (collection) ->
collection.each (user) ->
user.set('course_id', user.get('enrollments')[0].course_id, silent: true)
# Internal: Get the names of a user's sections.
#
# user - The user to return section names for.
#
# Return an array of section names.
getSections: (user) ->
sections = user.get('enrollments').map (enrollment) =>
@collection.sections.find (section) -> enrollment.course_section_id == section.id
_.uniq(_.map(sections, (section) -> section.get('name')))

View File

@ -298,11 +298,11 @@ class CoursesController < ApplicationController
def users
get_context
if authorized_action(@context, @current_user, :read_roster)
enrollment_type = "#{params[:enrollment_type].capitalize}Enrollment" if params[:enrollment_type]
enrollment_type = Array(params[:enrollment_type]).map { |e| "#{e.capitalize}Enrollment" } if params[:enrollment_type]
users = @context.users_visible_to(@current_user)
# TODO: convert this to the good user sorting stuff
users = users.scoped(:order => "users.sortable_name")
users = users.scoped(:conditions => ["enrollments.type = ? ", enrollment_type]) if enrollment_type
users = users.scoped(:conditions => ["enrollments.type IN (?) ", enrollment_type]) if enrollment_type
# If a user_id is passed in, modify the page parameter so that the page
# that contains that user is returned.

View File

@ -4,12 +4,12 @@
</a>
<div class="user-details">
<a class="user_name" href="/courses/{{course_id}}/users/{{user_id}}">{{name}}</a>
<a class="user_name" href="/courses/{{course_id}}/users/{{id}}">{{name}}</a>
<div class="more_info">
<div class="short_name">{{short_name}}</div>
<div class="email">{{login_id}}</div>
<div class="email">{{email}}</div>
<ul class="sections">
{{#each course_section_name}}
{{#each sectionNames}}
<li class="section">{{this}}</li>
{{/each}}
</ul>

View File

@ -637,6 +637,14 @@ describe CoursesController, :type => :integration do
:only => USER_API_FIELDS).map {|x| x["id"]}.sort
end
it "should accept an array of enrollment_types" do
json = api_call(:get, "/api/v1/courses/#{@course1.id}/users",
{:controller => 'courses', :action => 'users', :course_id => @course1.to_param, :format => 'json' },
:enrollment_type => ['student', 'teacher'], :include => ['enrollments'])
json.map { |u| u['enrollments'].map { |e| e['type'] } }.flatten.uniq.sort.should == %w{StudentEnrollment TeacherEnrollment}
end
it "should not include sis user id or login id for non-admins" do
RoleOverride.create!(:context => Account.default, :permission => 'read_sis', :enrollment_type => 'TeacherEnrollment', :enabled => false)
student_in_course(:course => @course2, :active_all => true, :name => 'Zombo')