From 9d54c80754b448b6647bbf8c9fc77f43ed4cf538 Mon Sep 17 00:00:00 2001 From: Mysti Lilla Date: Wed, 7 Oct 2020 15:06:26 -0600 Subject: [PATCH] Ensure new Lti::ToolProxies on root account can find others fixes: INTEROP-6245 flag=none Test plan - Create 2 lti tool proxies for the same plagiarism family on the root account and ensure they don't create 2 separate live event subscriptions Change-Id: I86a28011c47c3ee8c680359c5576afc0ec5e8137 Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/249556 Tested-by: Service Cloud Jenkins Reviewed-by: Weston Dransfield QA-Review: Weston Dransfield Product-Review: Mysti Lilla --- app/models/lti/tool_proxy.rb | 3 ++- spec/models/lti/tool_proxy_spec.rb | 20 ++++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/app/models/lti/tool_proxy.rb b/app/models/lti/tool_proxy.rb index 733a02a5b63..8f7d99e8f3e 100644 --- a/app/models/lti/tool_proxy.rb +++ b/app/models/lti/tool_proxy.rb @@ -171,7 +171,8 @@ module Lti # Search the accounts under the same root account to see if a subscription already exists # (we only need one subscription per root account) subscription_id = tool_proxies.joins(:account). - find_by("coalesce(accounts.root_account_id, accounts.id) = ?", context&.root_account_id)&.subscription_id + # we should replace this with the tool_proxy root_account_id if/when we fill that + find_by("coalesce(accounts.root_account_id, accounts.id) = ?", context&.resolved_root_account_id)&.subscription_id # Then search courses in case the tool was only directly installed on a course subscription_id ||= tool_proxies.joins(:course). find_by(courses: {root_account_id: self.context&.root_account_id})&.subscription_id diff --git a/spec/models/lti/tool_proxy_spec.rb b/spec/models/lti/tool_proxy_spec.rb index 0ff46359d2f..eff924e67b1 100644 --- a/spec/models/lti/tool_proxy_spec.rb +++ b/spec/models/lti/tool_proxy_spec.rb @@ -638,6 +638,26 @@ module Lti expect(tool_proxy.subscription_id).to eq 'id' end + it 'should not create a subscription if a root account tool already has one' do + ToolProxy.create!( + raw_data: {'enabled_capability' => [placement]}, + subscription_id: 'id', + context: account, + shared_secret: 'shared_secret', + guid: 'guid', + product_version: '1.0beta', + lti_version: 'LTI-2p0', + product_family: product_family, + workflow_state: 'active' + ) + + expect_any_instance_of(Lti::PlagiarismSubscriptionsHelper).not_to receive(:create_subscription) + tool_proxy.context = account + tool_proxy.raw_data['enabled_capability'] = [placement] + tool_proxy.save! + expect(tool_proxy.subscription_id).to eq 'id' + end + it 'should not create subscriptions for non-plagiarism tools' do expect_any_instance_of(Lti::PlagiarismSubscriptionsHelper).not_to receive(:create_subscription) tool_proxy.raw_data['enabled_capability'] = []