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

create scope for installed apps 

fixes PLAT-819

test-plan:
the api at http://{host}/api/v1/{context}/{context_id}/lti_apps
should include disabled lti2 apps

regression test the external tabs selection of assignments and modules

Change-Id: I77e504ac0b6a9edd5eb77abea6c980778bcf0ed4
Reviewed-on: https://gerrit.instructure.com/46862
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Eric Berry <ericb@instructure.com>
QA-Review: August Thornton <august@instructure.com>
Product-Review: Nathan Mills <nathanm@instructure.com>
This commit is contained in:
Nathan Mills 2015-01-09 13:29:02 -07:00
parent 1ba618892f
commit 92b5489e98
3 changed files with 42 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
app/models/lti/tool_proxy.rb
View File

@ -36,11 +36,19 @@ module Lti
validates_inclusion_of :workflow_state, in: ['active', 'deleted', 'disabled']
def self.find_active_proxies_for_context(context)
find_all_proxies_for_context(context).where('lti_tool_proxies.workflow_state = ?', 'active')
end
def self.find_installed_proxies_for_context(context)
find_all_proxies_for_context(context).where('lti_tool_proxies.workflow_state <> ?', 'deleted')
end
def self.find_all_proxies_for_context(context)
account_ids = context.account_chain.map { |a| a.id }
account_sql_string = account_ids.each_with_index.map { |x, i| "('Account',#{x},#{i})" }.unshift("('#{context.class.name}',#{context.id},#{0})").join(',')
subquery = ToolProxyBinding.select('DISTINCT ON (lti_tool_proxies.id) lti_tool_proxy_bindings.*').joins(:tool_proxy).where('lti_tool_proxies.workflow_state = ?', 'active').
subquery = ToolProxyBinding.select('DISTINCT ON (lti_tool_proxies.id) lti_tool_proxy_bindings.*').joins(:tool_proxy).
joins("INNER JOIN ( VALUES #{account_sql_string}) as x(context_type, context_id, ordering) ON lti_tool_proxy_bindings.context_type = x.context_type AND lti_tool_proxy_bindings.context_id = x.context_id").
where('(lti_tool_proxy_bindings.context_type = ? AND lti_tool_proxy_bindings.context_id = ?) OR (lti_tool_proxy_bindings.context_type = ? AND lti_tool_proxy_bindings.context_id IN (?))', context.class.name, context.id, 'Account', account_ids).
order('lti_tool_proxies.id, x.ordering').to_sql

2
lib/lti/app_collator.rb
View File

@ -24,7 +24,7 @@ module Lti
def bookmarked_collection
external_tools_scope = ContextExternalTool.all_tools_for(@context)
external_tools_collection = BookmarkedCollection.wrap(ExternalToolNameBookmarker, external_tools_scope)
tool_proxy_scope = ToolProxy.find_active_proxies_for_context(@context)
tool_proxy_scope = ToolProxy.find_installed_proxies_for_context(@context)
tool_proxy_collection = BookmarkedCollection.wrap(ToolProxyNameBookmarker, tool_proxy_scope)
BookmarkedCollection.merge(
['external_tools', external_tools_collection],

53
spec/models/lti/tool_proxy_spec.rb
View File

@ -102,7 +102,7 @@ module Lti
expect(subject.errors[:raw_data]).to include("can't be blank")
end
describe "#find_active_proxies_for_context" do
describe "#find_proxies_for_context" do
let(:root_account) { Account.create }
let(:sub_account_1_1) { Account.create(parent_account: root_account) }
let(:sub_account_1_2) { Account.create(parent_account: root_account) }
@ -112,7 +112,7 @@ module Lti
it 'finds a tool_proxy' do
tool_proxy = create_tool_proxy(context: sub_account_2_1)
tool_proxy.bindings.create!(context: sub_account_2_1)
proxies = described_class.find_active_proxies_for_context(sub_account_2_1)
proxies = described_class.find_all_proxies_for_context(sub_account_2_1)
expect(proxies.count).to eq 1
expect(proxies.first).to eq tool_proxy
end
@ -120,7 +120,7 @@ module Lti
it 'finds a tool_proxy for a parent account' do
tool_proxy = create_tool_proxy(context: sub_account_1_1)
tool_proxy.bindings.create!(context: sub_account_1_1)
proxies = described_class.find_active_proxies_for_context(sub_account_2_1)
proxies = described_class.find_all_proxies_for_context(sub_account_2_1)
expect(proxies.count).to eq 1
expect(proxies.first).to eq tool_proxy
end
@ -129,30 +129,16 @@ module Lti
course = Course.create!(account: sub_account_2_1)
tool_proxy = create_tool_proxy(context: course)
tool_proxy.bindings.create!(context: course)
proxies = described_class.find_active_proxies_for_context(course)
proxies = described_class.find_all_proxies_for_context(course)
expect(proxies.count).to eq 1
expect(proxies.first).to eq tool_proxy
end
it "doesn't return tool_proxies that are disabled" do
tool_proxy = create_tool_proxy(context: sub_account_2_1, workflow_state: 'disabled')
tool_proxy.bindings.create!(context: sub_account_2_1)
proxies = described_class.find_active_proxies_for_context(sub_account_2_1)
expect(proxies.count).to eq 0
end
it "doesn't return tool_proxies that are deleted" do
tool_proxy = create_tool_proxy(context: sub_account_2_1, workflow_state: 'deleted')
tool_proxy.bindings.create!(context: sub_account_2_1)
proxies = described_class.find_active_proxies_for_context(sub_account_2_1)
expect(proxies.count).to eq 0
end
it "doesn't return tool_proxies when closest ancestor is disabled" do
tool_proxy = create_tool_proxy(context: sub_account_2_1)
tool_proxy.bindings.create!(context: sub_account_2_1, enabled: false)
tool_proxy.bindings.create!(context: sub_account_1_1)
proxies = described_class.find_active_proxies_for_context(sub_account_2_1)
proxies = described_class.find_all_proxies_for_context(sub_account_2_1)
expect(proxies.count).to eq 0
end
@ -161,7 +147,7 @@ module Lti
tool_proxy1.bindings.create!(context: sub_account_2_1)
tool_proxy2 = create_tool_proxy(context: sub_account_1_1)
tool_proxy2.bindings.create!(context: sub_account_1_1)
proxies = described_class.find_active_proxies_for_context(sub_account_2_1)
proxies = described_class.find_all_proxies_for_context(sub_account_2_1)
expect(proxies.count).to eq 2
expect(proxies).to include(tool_proxy1)
expect(proxies).to include(tool_proxy2)
@ -171,11 +157,36 @@ module Lti
tool_proxy = create_tool_proxy(context: sub_account_1_1)
tool_proxy.bindings.create!(context: sub_account_1_1)
tool_proxy.bindings.create!(context: sub_account_2_1)
proxies = described_class.find_active_proxies_for_context(sub_account_2_1)
proxies = described_class.find_all_proxies_for_context(sub_account_2_1)
expect(proxies.count).to eq 1
expect(proxies.first).to eq tool_proxy
end
describe "#find_active_proxies_for_context" do
it "doesn't return tool_proxies that are disabled" do
tool_proxy = create_tool_proxy(context: sub_account_2_1, workflow_state: 'disabled')
tool_proxy.bindings.create!(context: sub_account_2_1)
proxies = described_class.find_active_proxies_for_context(sub_account_2_1)
expect(proxies.count).to eq 0
end
it "doesn't return tool_proxies that are deleted" do
tool_proxy = create_tool_proxy(context: sub_account_2_1, workflow_state: 'deleted')
tool_proxy.bindings.create!(context: sub_account_2_1)
proxies = described_class.find_active_proxies_for_context(sub_account_2_1)
expect(proxies.count).to eq 0
end
end
describe "#find_installed_proxies_for_context" do
it "doesn't return tool_proxies that are deleted" do
tool_proxy = create_tool_proxy(context: sub_account_2_1, workflow_state: 'deleted')
tool_proxy.bindings.create!(context: sub_account_2_1)
proxies = described_class.find_installed_proxies_for_context(sub_account_2_1)
expect(proxies.count).to eq 0
end
end
it "doesn't return tool proxies that are enabled at a higher binding and disabled at a lower binding" do
tool_proxy = create_tool_proxy(context: sub_account_1_1)
tool_proxy.bindings.create!(context: sub_account_1_1)