improve blueprint associations performance for many teachers

we try to return teacher information but having thousands
of teachers slows things down so just return a count
(like we do on the account course search)

test plan:
* have a course with more than 5 teachers
* should be able to search for and add them to a
blueprint course and have the UI show a count
instead of every teacher's name

closes #LA-863

Change-Id: Ie4d2294d04492e52d9f6854886458871c7f050d9
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/231920
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
QA-Review: Jeremy Stanley <jeremy@instructure.com>
Product-Review: James Williams <jamesw@instructure.com>
Reviewed-by: Jeremy Stanley <jeremy@instructure.com>
This commit is contained in:
James Williams 2020-03-27 13:24:02 -06:00
parent 446706abc7
commit de6df6b789
7 changed files with 28 additions and 7 deletions

View File

@ -279,6 +279,7 @@ class MasterCourses::MasterTemplatesController < ApplicationController
courses = Api.paginate(scope, self, api_v1_course_blueprint_associated_courses_url)
can_read_sis = @course.account.grants_any_right?(@current_user, :read_sis, :manage_sis)
preload_teachers(courses)
json = courses.map do |course|
course_summary_json(course, can_read_sis: can_read_sis, include_teachers: true)
end

View File

@ -54,6 +54,7 @@ const ApiClient = {
{blueprint_associated: 'false'},
{'include[]': 'term'},
{'include[]': 'teachers'},
{teacher_limit: '5'},
{search_term: search},
{enrollment_term_id: term}
])
@ -62,7 +63,7 @@ const ApiClient = {
},
getAssociations({masterCourse}) {
const params = this._queryString([{per_page: '100'}])
const params = this._queryString([{per_page: '100'}, {teacher_limit: '5'}])
return this._depaginate(
`/api/v1/courses/${masterCourse.id}/blueprint_templates/default/associated_courses?${params}`

View File

@ -176,7 +176,11 @@ export default class AssociationsTable extends React.Component {
<td>{this.renderCellText(course.term.name)}</td>
<td>{this.renderCellText(course.sis_course_id)}</td>
<td>
{this.renderCellText(course.teachers.map(teacher => teacher.display_name).join(', '))}
{this.renderCellText(
course.teachers
? course.teachers.map(teacher => teacher.display_name).join(', ')
: I18n.t('%{teacher_count} teachers', {teacher_count: course.teacher_count})
)}
</td>
<td className="bca-associations__x-col">
<form
@ -214,7 +218,11 @@ export default class AssociationsTable extends React.Component {
<td>{this.renderCellText(course.term.name)}</td>
<td>{this.renderCellText(course.sis_course_id)}</td>
<td>
{this.renderCellText(course.teachers.map(teacher => teacher.display_name).join(', '))}
{this.renderCellText(
course.teachers
? course.teachers.map(teacher => teacher.display_name).join(', ')
: I18n.t('%{teacher_count} teachers', {teacher_count: course.teacher_count})
)}
</td>
<td className="bca-associations__x-col">
<form

View File

@ -193,7 +193,11 @@ export default class CoursePickerTable extends React.Component {
<td>{this.renderCellText(course.term.name)}</td>
<td>{this.renderCellText(course.sis_course_id)}</td>
<td>
{this.renderCellText(course.teachers.map(teacher => teacher.display_name).join(', '))}
{this.renderCellText(
course.teachers
? course.teachers.map(teacher => teacher.display_name).join(', ')
: I18n.t('%{teacher_count} teachers', {teacher_count: course.teacher_count})
)}
</td>
</tr>
))

View File

@ -45,7 +45,8 @@ propTypes.course = shape({
shape({
display_name: string.isRequired
})
).isRequired,
),
teacher_count: string,
sis_course_id: string
})
propTypes.courseList = arrayOf(propTypes.course)

View File

@ -183,7 +183,7 @@ module Api::V1::Course
def preload_teachers(courses)
threshold = params[:teacher_limit].presence&.to_i
if threshold
scope = TeacherEnrollment.active_or_pending.where(:course_id => courses).distinct.select(:user_id, :course_id)
scope = TeacherEnrollment.where.not(:workflow_state => %w{deleted rejected}).where(:course_id => courses).distinct.select(:user_id, :course_id)
teacher_counts = Enrollment.from("(#{scope.to_sql}) AS t").group("t.course_id").count
to_preload = []
courses.each do |course|

View File

@ -99,7 +99,13 @@ module Api::V1::MasterCourses
hash = api_json(course, @current_user, session, :only => %w{id name course_code})
hash['sis_course_id'] = course.sis_source_id if can_read_sis
hash['term_name'] = course.enrollment_term.name
hash['teachers'] = course.teachers.map { |teacher| user_display_json(teacher) } if opts[:include_teachers]
if opts[:include_teachers]
if course.teacher_count
hash['teacher_count'] = course.teacher_count
else
hash['teachers'] = course.teachers.map { |teacher| user_display_json(teacher) }
end
end
hash
end