diff --git a/app/models/lti/tool_proxy.rb b/app/models/lti/tool_proxy.rb index bd8d4957ccc..1a87dff8bc8 100644 --- a/app/models/lti/tool_proxy.rb +++ b/app/models/lti/tool_proxy.rb @@ -34,5 +34,17 @@ module Lti validates_uniqueness_of :guid validates_inclusion_of :workflow_state, in: ['active', 'deleted', 'disabled'] + def self.find_active_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'). + 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 + ToolProxy.joins("JOIN (#{subquery}) bindings on lti_tool_proxies.id = bindings.tool_proxy_id").where('bindings.enabled = true') + end + end end \ No newline at end of file diff --git a/app/models/lti/tool_proxy_binding.rb b/app/models/lti/tool_proxy_binding.rb index d7c4a3aaee2..d71edf29135 100644 --- a/app/models/lti/tool_proxy_binding.rb +++ b/app/models/lti/tool_proxy_binding.rb @@ -25,7 +25,7 @@ module Lti belongs_to :context, :polymorphic => true has_many :links, :class_name => 'Lti::LtiLink', foreign_key: 'tool_proxy_binding_id' - validates_presence_of :tool_proxy, :context, :enabled + validates_presence_of :tool_proxy, :context validates_inclusion_of :context_type, :allow_nil => true, :in => ['Course', 'Account'] end diff --git a/spec/models/account_spec.rb b/spec/models/account_spec.rb index 395c8b41a0f..609c7790907 100644 --- a/spec/models/account_spec.rb +++ b/spec/models/account_spec.rb @@ -975,6 +975,16 @@ describe Account do @account3.account_chain.should == [@account3, @account2, @account1] end end + + it "returns parent accounts in order up the tree" do + account1 = Account.create! + account2 = account1.sub_accounts.create! + account3 = account2.sub_accounts.create! + account4 = account3.sub_accounts.create! + + account4.account_chain.should == [account4, account3, account2, account1] + end + end describe "#can_see_admin_tools_tab?" do diff --git a/spec/models/lti/tool_proxy_spec.rb b/spec/models/lti/tool_proxy_spec.rb index a9c4e61dfce..6ae555994d3 100644 --- a/spec/models/lti/tool_proxy_spec.rb +++ b/spec/models/lti/tool_proxy_spec.rb @@ -20,9 +20,9 @@ require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb') module Lti describe ToolProxy do - let (:account) {Account.create} - let (:product_family) {ProductFamily.create(vendor_code: '123', product_code:'abc', vendor_name:'acme', root_account:account)} - let (:resource_handler) {ResourceHandler.new} + let (:account) { Account.create } + let (:product_family) { ProductFamily.create(vendor_code: '123', product_code: 'abc', vendor_name: 'acme', root_account: account) } + let (:resource_handler) { ResourceHandler.new } describe 'validations' do @@ -40,14 +40,14 @@ module Lti it 'requires a shared_secret' do subject.shared_secret = nil subject.save - error = subject.errors.find {|e| e == [:shared_secret, "can't be blank"]} + error = subject.errors.find { |e| e == [:shared_secret, "can't be blank"] } error.should_not == nil end it 'requires a guid' do subject.guid = nil subject.save - error = subject.errors.find {|e| e == [:guid, "can't be blank"]} + error = subject.errors.find { |e| e == [:guid, "can't be blank"] } error.should_not == nil end @@ -102,6 +102,103 @@ module Lti subject.errors[:raw_data].should include("can't be blank") end + describe "#find_active_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) } + let(:sub_account_2_1) { Account.create(parent_account: sub_account_1_1) } + + + 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.count.should == 1 + proxies.first.should == tool_proxy + end + + 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.count.should == 1 + proxies.first.should == tool_proxy + end + + it 'finds a tool_proxy for a course binding' do + 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.count.should == 1 + proxies.first.should == 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) + proxies.count.should == 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) + proxies.count.should == 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.count.should == 0 + end + + it 'handles multiple tool_proxies' do + tool_proxy1 = create_tool_proxy(context: sub_account_2_1) + 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.count.should == 2 + proxies.should include(tool_proxy1) + proxies.should include(tool_proxy2) + end + + it 'handles multiple bindings' do + 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.count.should == 1 + proxies.first.should == tool_proxy + 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) + tool_proxy.bindings.create!(context: sub_account_2_1, enabled: false) + proxies = described_class.find_active_proxies_for_context(sub_account_2_1) + proxies.count.should == 0 + end + + end + + end + + def create_tool_proxy(opts = {}) + default_opts = { + shared_secret: 'shared_secret', + guid: SecureRandom.uuid, + product_version: '1.0beta', + lti_version: 'LTI-2p0', + product_family: product_family, + workflow_state: 'active', + raw_data: 'some raw data' + } + ToolProxy.create(default_opts.merge(opts)) end end