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

Add create 1.3 tool config to external tools controlller. 

Deleted create_tool_from_tool_config,
moved functionality into create.
Edited external tools json to include version.

fixes PLAT-4437

Test Plan:
-Create LTI key
-Create context external tool using client of LTI key
	-Should return success
-Create context external tool with bad client id
	-Should return 404
-Create a 1.1 tool using same endpoint
	-Original functionality should still work
-Create a context external tool with disabled client id
	-Should return 422

Change-Id: I5fce06db376f5eca13e6d41b9bb38f9f25c3c8d5
Reviewed-on: https://gerrit.instructure.com/192664
Reviewed-by: Marc Phillips <mphillips@instructure.com>
QA-Review: Marc Phillips <mphillips@instructure.com>
Product-Review: Marc Phillips <mphillips@instructure.com>
Tested-by: Jenkins
This commit is contained in:
Drake Harper 2019-05-08 13:16:27 -06:00 committed by Marc Phillips
parent d906d279ed
commit 67a86c19e3
6 changed files with 66 additions and 113 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

51
app/controllers/external_tools_controller.rb
View File

@ -862,14 +862,19 @@ class ExternalToolsController < ApplicationController
# -F 'config_type=by_url' \
# -F 'config_url=https://example.com/ims/lti/tool_config.xml'
def create
external_tool_params = (params[:external_tool] || params).to_unsafe_h
@tool = @context.context_external_tools.new
if request.content_type == 'application/x-www-form-urlencoded'
custom_fields = Lti::AppUtil.custom_params(request.raw_post)
external_tool_params[:custom_fields] = custom_fields if custom_fields.present?
if params.key?(:client_id)
raise ActiveRecord::RecordInvalid unless developer_key.active?
@tool = developer_key.tool_configuration.new_external_tool(@context)
else
external_tool_params = (params[:external_tool] || params).to_unsafe_h
@tool = @context.context_external_tools.new
if request.content_type == 'application/x-www-form-urlencoded'
custom_fields = Lti::AppUtil.custom_params(request.raw_post)
external_tool_params[:custom_fields] = custom_fields if custom_fields.present?
end
set_tool_attributes(@tool, external_tool_params)
check_for_duplication(@tool)
end
set_tool_attributes(@tool, external_tool_params)
check_for_duplication(@tool)
if @tool.errors.blank? && @tool.save
invalidate_nav_tabs_cache(@tool)
if api_request?
@ -929,32 +934,6 @@ class ExternalToolsController < ApplicationController
end
end
# @API Create Tool from ToolConfiguration
# @internal
# Creates context_external_tool from attached tool_configuration of
# the provided developer_key if not already present in context.
# DeveloperKey must have a ToolConfiguration to create tool or 404 will be raised.
# Will return an existing ContextExternalTool if one already exists.
#
# @argument account_id [String]
# if account
#
# @argument course_id [String]
# if course
#
# @argument developer_key_id [String]
#
# @returns ContextExternalTool
def create_tool_from_tool_config
cet = fetch_existing_tool_in_context_chain
if cet.nil?
cet = developer_key.tool_configuration.new_external_tool(@context)
cet.save!
invalidate_nav_tabs_cache(cet)
end
render json: external_tool_json(cet, @context, @current_user, session)
end
# @API Edit an external tool
# Update the specified external tool. Uses same parameters as create
#
@ -1199,12 +1178,8 @@ class ExternalToolsController < ApplicationController
head :not_found
end
def fetch_existing_tool_in_context_chain
ContextExternalTool.all_tools_for(@context).where(developer_key: developer_key).take
end
def developer_key
@_developer_key = DeveloperKey.nondeleted.find(params[:developer_key_id])
@_developer_key = DeveloperKey.nondeleted.find(params[:client_id])
end
def delete_tool(tool)

2
config/routes.rb
View File

@ -1258,8 +1258,6 @@ CanvasRails::Application.routes.draw do
post "#{context}s/:#{context}_id/create_tool_with_verification", action: :create_tool_with_verification, as: "#{context}_create_tool_with_verification"
put "#{context}s/:#{context}_id/external_tools/:external_tool_id", action: :update, as: "#{context}_external_tools_update"
delete "#{context}s/:#{context}_id/external_tools/:external_tool_id", action: :destroy, as: "#{context}_external_tools_delete"
post "#{context}s/:#{context}_id/developer_keys/:developer_key_id/create_tool", action: :create_tool_from_tool_config
post "#{context}s/:#{context}_id/developer_keys/:developer_key_id/create_tool", action: :create_tool_from_tool_config
end
get "groups/:group_id/external_tools", action: :index, as: "group_external_tools"

1
lib/api/v1/external_tools.rb
View File

@ -39,6 +39,7 @@ module Api::V1::ExternalTools
json['selection_height'] = tool.settings[:selection_height] if tool.settings.key? :selection_height
json['icon_url'] = tool.settings[:icon_url] if tool.settings.key? :icon_url
json['not_selectable'] = tool.not_selectable
json['version'] = tool.use_1_3? ? '1.3' : '1.1'
extension_types.each do |type|
if json[type]
json[type]['label'] = tool.label_for(type, I18n.locale)

1
spec/apis/v1/external_tools_api_spec.rb
View File

@ -614,6 +614,7 @@ describe ExternalToolsController, type: :request do
"not_selectable"=> et ? et.not_selectable : nil,
"workflow_state"=>"public",
"vendor_help_link"=>nil,
"version"=>"1.1",
"resource_selection"=>
{"text"=>"",
"url"=>"http://www.example.com/ims/lti/resource",

116
spec/controllers/external_tools_controller_spec.rb
View File

@ -954,6 +954,49 @@ describe ExternalToolsController do
end
let(:xml_response) { OpenStruct.new({body: xml}) }
context 'with client id' do
include_context 'lti_1_3_spec_helper'
subject { ContextExternalTool.find_by(id: JSON.parse(response.body)['id']) }
let(:tool_configuration) { Lti::ToolConfiguration.create! settings: settings, developer_key: developer_key }
let(:developer_key) { DeveloperKey.create!(account: account) }
let_once(:user) { account_admin_user(account: account) }
let_once(:account) { account_model }
let(:params) do
{
client_id: developer_key.id,
account_id: account
}
end
before do
user_session(user)
tool_configuration
post 'create', params: params, format: 'json'
end
it { is_expected.to_not be_nil }
context 'with invalid client id' do
let(:params) { super().merge(client_id: "bad client id") }
it 'return 404' do
expect(response).to have_http_status :not_found
end
end
context 'with inactive developer key' do
let(:developer_key) do
dev_key = super()
dev_key.deactivate!
dev_key
end
it 'return 422' do
expect(response).to have_http_status :unprocessable_entity
end
end
end
context 'tool duplication' do
shared_examples_for 'detects duplication in context' do
let(:params) { raise "Override in specs" }
@ -1654,79 +1697,6 @@ describe ExternalToolsController do
it { is_expected.to be_not_found }
end
end
describe '#create_tool_from_tool_config' do
subject { post :create_tool_from_tool_config, params: params }
shared_examples_for 'tool configuration does not exist' do
let(:tool_configuration) { nil }
it { is_expected.to be_not_found }
end
shared_examples_for 'reuses an exisiting ContextExternalTool' do
let(:tool_context) { raise 'Override in spec' }
let(:cet) do
cet = tool_configuration.new_external_tool(tool_context)
cet.save!
cet
end
before do
cet
subject
end
it 'returns the existing tool' do
expect(json_parse['id']).to eq cet.id
end
end
shared_examples_for 'a context that can create a tool' do
let(:create_tool_context) { raise 'Override in spec' }
it 'creates a ContextExternalTool' do
expect { subject }.to change { ContextExternalTool.count }.by(1)
expect(ContextExternalTool.first.context_id).to eq create_tool_context.id
end
it_behaves_like 'reuses an exisiting ContextExternalTool' do
let(:tool_context) { create_tool_context }
end
end
context 'when an account' do
let(:params) { { account_id: sub_account.id, developer_key_id: dev_key_id } }
it_behaves_like 'basic devkey behavior'
it_behaves_like 'tool configuration does not exist'
it_behaves_like 'a context that can create a tool' do
let(:create_tool_context) { sub_account }
end
end
context 'when a course' do
let(:params) { { course_id: course.id, developer_key_id: dev_key_id } }
it_behaves_like 'basic devkey behavior'
it_behaves_like 'tool configuration does not exist'
it_behaves_like 'a context that can create a tool' do
let(:create_tool_context) { course }
end
it_behaves_like 'reuses an exisiting ContextExternalTool' do
let(:tool_context) { sub_account }
end
it_behaves_like 'reuses an exisiting ContextExternalTool' do
let(:tool_context) { account }
end
end
end
end
def opaque_id(asset)

8
spec/lib/api/v1/external_tools_spec.rb
View File

@ -50,6 +50,14 @@ describe Api::V1::ExternalTools do
expect(json['updated_at']).to eq tool.updated_at
expect(json['privacy_level']).to eq tool.privacy_level
expect(json['custom_fields']).to eq tool.custom_fields
expect(json['version']).to eq "1.1"
end
it "generates json with 1.3 version" do
tool.use_1_3 = true
tool.save!
json = controller.external_tool_json(tool, @course, @student, nil)
expect(json['version']).to eq "1.3"
end
it "gets default extension settings" do