Optimize assignment_external_tools LTI placement
Closes: CORE-3167 Currently: we have a feature where people can register an LTI tool to show up on any page load to: a) Assignment create and edit. b) Discussion topic create and edit. c) Quiz create and edit. So that means that on every page load to any of those pages (some of the most common pages in canvas) we do an additional XHR request to see what tools are registered to that Course/Acccount/User to see if we should show anything in that LTI placement on the page. That takes a non-trivial amount of time and I wanted to see if I could help in 2 ways: 1. the xhr request to see what tools are registered represents a good chunk of the total requests we handle on our app servers, can we help that? 2. fetching the XHR counts against our New Relic load times, can we do it sooner so it is ready sooner? To help #1 I just put an expires_in 10.minutes on the http response for that controller action because it is something that does not change that often. For anyone trying to test adding new ones and see it show up immediately, they just need to do a “hard refresh” in the browser (command+shift+R) To help #2 I used prefetch_xhr to send the fetch request for the data sooner, meaning the complete page should load faster for users and this should never be the thing we are waiting on for newRelic to consider the page loaded. Test Plan: 1. Configure a new LTI at the account / course level with a assignment_edit launch. 2. Verify that the launch happens in three different contexts: a) Assignment create and edit. b) Discussion topic create and edit. c) Quiz create and edit. 3. For each context above (#2) verify the launch happens as it did before. 4. Look in your browser network panel. You should see that the request To: …/lti_apps/launch_definitions?placements%5B%5D=assignment_view Is of type: “Fetch” instead of “XHR” and it happens sooner in the Waterfall. 5. If you hit the page again, you should see that the “time” is < 1ms because it was loaded from the browser’s cache 6. Review api docs at: doc/api/file.assignment_external_tools.html experiment with options and verify apps work as expected. Change-Id: I2986fa281d1f85d4e544b763647b8e3137d42e00 Reviewed-on: https://gerrit.instructure.com/199384 Reviewed-by: Mark Valentine <mvalentine@instructure.com> Reviewed-by: Rob Orton <rob@instructure.com> Tested-by: Jenkins QA-Review: Trevor Byington <tbyington@instructure.com> Product-Review: Ryan Shaw <ryan@instructure.com>
This commit is contained in:
parent
6cbc442a04
commit
9fb6a85c48
|
@ -52,7 +52,11 @@ module Lti
|
||||||
named_context_url(@context, :api_v1_context_launch_definitions_url, include_host: true),
|
named_context_url(@context, :api_v1_context_launch_definitions_url, include_host: true),
|
||||||
pagination_args
|
pagination_args
|
||||||
)
|
)
|
||||||
format.json { render :json => AppLaunchCollator.launch_definitions(launch_defs, placements) }
|
format.json do
|
||||||
|
cancel_cache_buster
|
||||||
|
expires_in 10.minutes
|
||||||
|
render :json => AppLaunchCollator.launch_definitions(launch_defs, placements)
|
||||||
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -1195,6 +1195,15 @@ module ApplicationHelper
|
||||||
super(attachment, uuid || attachment.uuid, url_options)
|
super(attachment, uuid || attachment.uuid, url_options)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def prefetch_assignment_external_tools
|
||||||
|
content_tag(:div, id: 'assignment_external_tools') do
|
||||||
|
prefetch_xhr(api_v1_course_launch_definitions_path(
|
||||||
|
@context,
|
||||||
|
'placements[]' => 'assignment_view'
|
||||||
|
))
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
if CANVAS_RAILS5_1
|
if CANVAS_RAILS5_1
|
||||||
# this is for rails 5.1. it can be deleted when we upgrade to RAILS_5.2. rails 5.2 includes a preload_link_tag method
|
# this is for rails 5.1. it can be deleted when we upgrade to RAILS_5.2. rails 5.2 includes a preload_link_tag method
|
||||||
def preload_link_tag(source, options = {})
|
def preload_link_tag(source, options = {})
|
||||||
|
|
|
@ -23,6 +23,7 @@ import ReactDOM from 'react-dom'
|
||||||
import I18n from 'i18n!moderated_grading'
|
import I18n from 'i18n!moderated_grading'
|
||||||
import 'compiled/jquery.rails_flash_notifications'
|
import 'compiled/jquery.rails_flash_notifications'
|
||||||
import iframeAllowances from '../external_apps/lib/iframeAllowances'
|
import iframeAllowances from '../external_apps/lib/iframeAllowances'
|
||||||
|
import {asJson, getPrefetchedXHR} from '@instructure/js-utils'
|
||||||
|
|
||||||
class AssignmentExternalTools extends React.Component {
|
class AssignmentExternalTools extends React.Component {
|
||||||
constructor(props) {
|
constructor(props) {
|
||||||
|
@ -61,30 +62,17 @@ class AssignmentExternalTools extends React.Component {
|
||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
|
|
||||||
getTools() {
|
async getTools() {
|
||||||
const self = this;
|
const url = encodeURI(`${this.getDefinitionsUrl()}?placements[]=${this.props.placement}`)
|
||||||
const toolsUrl = this.getDefinitionsUrl();
|
|
||||||
const data = {
|
|
||||||
'placements[]': this.props.placement
|
|
||||||
};
|
|
||||||
|
|
||||||
$.ajax({
|
try {
|
||||||
type: 'GET',
|
const request = getPrefetchedXHR(url) || fetch(url, {headers: {Accept: 'application/json'}})
|
||||||
url: toolsUrl,
|
const tools = await asJson(request)
|
||||||
data,
|
tools.forEach(t => (t.launch = this.getLaunch(t)))
|
||||||
success: $.proxy(function(data) {
|
this.setState({tools})
|
||||||
const tool_data = data
|
} catch (e) {
|
||||||
for (let i = 0; i < data.length; i++) {
|
$.flashError(I18n.t('Error retrieving assignment external tools'))
|
||||||
tool_data[i].launch = this.getLaunch(data[i]);
|
}
|
||||||
}
|
|
||||||
this.setState({
|
|
||||||
tools: tool_data
|
|
||||||
});
|
|
||||||
}, self),
|
|
||||||
error(_xhr) {
|
|
||||||
$.flashError(I18n.t('Error retrieving assignment external tools'));
|
|
||||||
}
|
|
||||||
});
|
|
||||||
}
|
}
|
||||||
|
|
||||||
getDefinitionsUrl() {
|
getDefinitionsUrl() {
|
||||||
|
|
|
@ -355,7 +355,7 @@
|
||||||
|
|
||||||
</div>
|
</div>
|
||||||
|
|
||||||
<div id="assignment_external_tools"></div>
|
<%= prefetch_assignment_external_tools %>
|
||||||
|
|
||||||
<% if @headers != false %>
|
<% if @headers != false %>
|
||||||
<div id="module_sequence_footer"></div>
|
<div id="module_sequence_footer"></div>
|
||||||
|
|
|
@ -301,5 +301,5 @@ content_is_locked = !!js_env.dig(:MASTER_COURSE_DATA, "is_master_course_child_co
|
||||||
</div>
|
</div>
|
||||||
</div>
|
</div>
|
||||||
<% end %>
|
<% end %>
|
||||||
<div id="assignment_external_tools"></div>
|
<%= prefetch_assignment_external_tools %>
|
||||||
</div>
|
</div>
|
||||||
|
|
|
@ -383,7 +383,7 @@
|
||||||
<%= render 'cant_go_back_warning' %>
|
<%= render 'cant_go_back_warning' %>
|
||||||
<% end %>
|
<% end %>
|
||||||
|
|
||||||
<div id="assignment_external_tools"></div>
|
<%= prefetch_assignment_external_tools %>
|
||||||
|
|
||||||
<div id="module_sequence_footer"></div>
|
<div id="module_sequence_footer"></div>
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue