Keep root_account_ids updated on users

Closes PLAT-5801
flag=none

This change keeps the root_account_ids column
updated on the users table. Updates to this
column are triggered whenever a record in the
user_account_associations table is created,
updated, or destroyed.

Test Plan:
This test plan assumes you have MRA and at least
two shards set up.

1. Create a user and associate them with the root
   account in Shard 1
2. Run jobs
3. Verify root_account_ids on the user is now an
   array of size 1 containing the global ID of
   the Shard 1 root account
4. Associate the same user with the root account
   of shard 2 (one way to quickly do this is by
   using User#associate_with_shard)
5. Run jobs
6. Verify root_account_ids on the user is now an
   array of size 2 containing the global IDs of
   both the Shard 1 and Shard 2 root accounts
7. Set the "sync_root_account_ids_on_user_records"
   Setting to something other than "true":
   ```
   Setting.set(
     "sync_root_account_ids_on_user_records",
     "false
   )
   ```
8. Create a new user in an account
9. Run jobs
10. Verify root_account_ids is still nil
    for the new user

Change-Id: Ia4b6552b887f58a85bc368312234db4da2237058
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/238830
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Xander Moffatt <xmoffatt@instructure.com>
Reviewed-by: Evan Battaglia <ebattaglia@instructure.com>
QA-Review: Xander Moffatt <xmoffatt@instructure.com>
Product-Review: Weston Dransfield <wdransfield@instructure.com>
This commit is contained in:
wdransfield 2020-05-26 11:19:07 -06:00 committed by Weston Dransfield
parent ad29b11846
commit 0dfd45614c
5 changed files with 115 additions and 4 deletions

View File

@ -20,6 +20,8 @@ require 'atom'
class User < ActiveRecord::Base
GRAVATAR_PATTERN = /^https?:\/\/[a-zA-Z0-9.-]+\.gravatar\.com\//
MAX_ROOT_ACCOUNT_ID_SYNC_ATTEMPTS = 5
include TurnitinID
include Pronouns
@ -431,6 +433,33 @@ class User < ActiveRecord::Base
!!@skip_updating_account_associations
end
def update_root_account_ids
# See User#associated_shards in MRA for an explanation of
# shard association levels
shards = associated_shards(:strong) + associated_shards(:weak)
refreshed_root_account_ids = Set.new
Shard.with_each_shard(shards) do
UserAccountAssociation.joins(:account).
where(user: self, accounts: { parent_account_id: nil }).
preload(:account).
find_each do |association|
refreshed_root_account_ids << association.global_account_id
end
end
self.update!(root_account_ids: refreshed_root_account_ids.to_a)
end
def update_root_account_ids_later
send_later_enqueue_args(
:update_root_account_ids,
{
max_attempts: MAX_ROOT_ACCOUNT_ID_SYNC_ATTEMPTS
}
)
end
def update_account_associations_later
self.send_later_if_production(:update_account_associations) unless self.class.skip_updating_account_associations?
end

View File

@ -22,8 +22,30 @@ class UserAccountAssociation < ActiveRecord::Base
belongs_to :user
belongs_to :account
after_commit :update_user_root_account_ids
validates_presence_of :user_id, :account_id
resolves_root_account through: :account
def for_root_account?
# TODO: Once root_account_id backfill is complete on
# user_account_associations, we can change this to
# just `account_id == root_account_id`
account_id == root_account_id || account.root_account?
end
private
def update_user_root_account_ids
# In some Canvas environments we may not want to populate
# root_account_ids due to the hight number of root account associations
# per user. This Setting allows us to control if root_account_ids syncing
# occurs.
return unless Setting.get('sync_root_account_ids_on_user_records', 'true') == 'true'
return unless for_root_account?
user.update_root_account_ids_later
end
end

View File

@ -49,9 +49,11 @@ module LiveEvents
LiveEvents.clear_context!
yield block
run_jobs
# Find the last message with a matching event_name
@event_message = stream_client.data.map do |event|
JSON.parse(event[:data])
end.find do |msg|
end.reverse.find do |msg|
msg.dig('attributes', 'event_name') == @event_name
end
end
@ -103,10 +105,11 @@ module LiveEvents
def initialize(stream_name)
@stream_name = stream_name
@data = []
end
def put_records(records:, stream_name:) # rubocop:disable Lint/UnusedMethodArgument
@data = records
@data += records
end
end

View File

@ -400,6 +400,63 @@ describe User do
expect(user.associated_root_accounts.to_a).to eql [account2]
end
describe 'update_root_account_ids' do
let_once(:root_account) { Account.default }
let_once(:sub_account) { Account.create(parent_account: root_account, name: 'sub') }
let(:user) { user_model }
let(:root_account_association) do
user.user_account_associations.create!(account: root_account)
user.user_account_associations.find_by(account: root_account)
end
let(:sub_account_association) do
user.user_account_associations.create!(account: sub_account)
user.user_account_associations.find_by(account: sub_account)
end
before do
root_account_association
sub_account_association
end
context 'when there is a single root account association' do
it 'updates root_account_ids with the root account' do
expect {
user.update_root_account_ids
}.to change {
user.root_account_ids
}.from(nil).to([root_account.global_id])
end
end
context 'when there cross-shard root account associations' do
specs_require_sharding
let(:shard_two_root_account) { account_model }
before do
@shard2.activate do
user.user_account_associations.create!(
account: shard_two_root_account
)
user.associate_with_shard(@shard2)
end
end
it 'updates root_account_ids with all root accounts' do
expect {
user.update_root_account_ids
}.to change {
user.root_account_ids&.sort
}.from(nil).to(
[root_account.global_id, shard_two_root_account.global_id].sort
)
end
end
end
describe "update_account_associations" do
it "should support incrementally adding to account associations" do
user = User.create!

View File

@ -134,8 +134,8 @@ describe "site admin jobs ui" do
it "should check current popular tags" do
filter_tags(FlavorTags::CURRENT)
expect(f("#tags-grid .slick-row:nth-child(1) .f0")).to include_text "String#reverse"
expect(f("#tags-grid .slick-row:nth-child(1) .f1")).to include_text "2"
expect(f("#tags-grid")).to include_text "String#reverse"
expect(f("#tags-grid")).to include_text "2"
end
it "should check all popular tags", priority: "2" do