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

moar root_account_id = 0 prep 

refs FOO-1693

Change-Id: I25c9b8f53c13cc5c50555306dafb632462bf30e8
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/266125
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jacob Burroughs <jburroughs@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
This commit is contained in:
Cody Cutrer 2021-06-01 15:16:05 -06:00
parent 118a7fa7b3
commit c9eae18f48
14 changed files with 64 additions and 38 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
app/models/account.rb
View File

@ -546,6 +546,10 @@ class Account < ActiveRecord::Base
end
def check_downstream_caches
# dummy account has no downstream
return if dummy?
return if ActiveRecord::Base.in_migration
keys_to_clear = []
keys_to_clear << :account_chain if self.saved_change_to_parent_account_id? || self.saved_change_to_root_account_id?
if self.saved_change_to_brand_config_md5? || (@old_settings && @old_settings[:sub_account_includes] != settings[:sub_account_includes])
@ -1443,7 +1447,7 @@ class Account < ActiveRecord::Base
self.course_template_id = nil if course_template_id == 0 && root_account?
return if [nil, 0].include?(course_template_id)
unless course_template.root_account_id == [root_account_id, id].compact.first
unless course_template.root_account_id == resolved_root_account_id
errors.add(:course_template_id, t('Course template must be in the same root account'))
end
unless course_template.template?
@ -1577,6 +1581,10 @@ class Account < ActiveRecord::Base
self == Account.site_admin
end
def dummy?
local_id.zero?
end
def display_name
self.name
end
@ -1956,6 +1964,7 @@ class Account < ActiveRecord::Base
end
scope :root_accounts, -> { where(root_account_id: [0, nil]).where.not(id: 0) }
scope :non_root_accounts, -> { where.not(root_account_id: [0, nil]) }
scope :processing_sis_batch, -> { where("accounts.current_sis_batch_id IS NOT NULL").order(:updated_at) }
scope :name_like, lambda { |name| where(wildcard('accounts.name', name)) }
scope :active, -> { where("accounts.workflow_state<>'deleted'") }
@ -2147,7 +2156,18 @@ class Account < ActiveRecord::Base
relation_delegate_class(ActiveRecord::AssociationRelation).prepend(DomainRootAccountCache)
def self.ensure_dummy_root_account
Account.find_or_create_by!(id: 0) if Rails.env.test?
return unless Rails.env.test?
dummy = Account.find_by(id: 0)
return if dummy
# this needs to be thread safe because parallel specs might all try to create at once
transaction(requires_new: true) do
Account.create!(id: 0, workflow_state: 'deleted', name: "Dummy Root Account", root_account_id: 0)
rescue ActiveRecord::UniqueConstraintViolation
# somebody else created it. we don't even need to return it, just clean up the transaction
raise ActiveRecord::Rollback
end
end
def roles_with_enabled_permission(permission)

2
app/models/auditors/active_record/grade_change_record.rb
View File

@ -42,7 +42,7 @@ module Auditors::ActiveRecord
def ar_attributes_from_event_stream(record)
attrs_hash = record.attributes.except('id', 'version_number')
root_account_id = Account.where(id: record.account_id).pluck(:root_account_id).first
root_account_id = Account.where(id: record.account_id).select(:id, :root_account_id).take.resolved_root_account_id
attrs_hash['request_id'] ||= "MISSING"
attrs_hash['uuid'] = record.id
attrs_hash['account_id'] = Shard.relative_id_for(record.account_id, Shard.current, Shard.current)

5
app/models/user.rb
View File

@ -654,10 +654,7 @@ class User < ActiveRecord::Base
current_associations[key] = [aa.id, aa.depth]
end
account_id_to_root_account_id = Account.where(id: precalculated_associations&.keys).pluck(:id, :root_account_id).reduce({}) do |cache, fields|
cache[fields[0]] = fields[1] || fields[0]
cache
end
account_id_to_root_account_id = Account.where(id: precalculated_associations&.keys).pluck(:id, Arel.sql(Account.resolved_root_account_id_sql)).to_h
users_or_user_ids.uniq.sort_by{|u| u.try(:id) || u}.each do |user_id|
if user_id.is_a? User

2
lib/api/v1/account.rb
View File

@ -42,12 +42,14 @@ module Api::V1::Account
attributes = %w(id name parent_account_id root_account_id workflow_state uuid)
if read_only
return api_json(account, user, session, :only => attributes).tap do |hash|
hash['root_account_id'] = nil if account.root_account?
hash['default_time_zone'] = account.default_time_zone.tzinfo.name
end
end
methods = %w(default_storage_quota_mb default_user_storage_quota_mb default_group_storage_quota_mb)
api_json(account, user, session, :only => attributes, :methods => methods).tap do |hash|
hash['root_account_id'] = nil if account.root_account?
hash['default_time_zone'] = account.default_time_zone.tzinfo.name
hash['sis_account_id'] = account.sis_source_id if !account.root_account? && account.root_account.grants_any_right?(user, :read_sis, :manage_sis)
hash['sis_import_id'] = account.sis_batch_id if !account.root_account? && account.root_account.grants_right?(user, session, :manage_sis)

2
lib/data_fixup/move_sub_account_grading_periods_to_courses.rb
View File

@ -61,7 +61,7 @@ module DataFixup::MoveSubAccountGradingPeriodsToCourses
end
def self.destroy_sub_account_grading_period_groups_and_grading_periods
account_subquery = Account.where.not(root_account_id: nil)
account_subquery = Account.non_root_accounts
groups = GradingPeriodGroup.active.where(account_id: account_subquery)
groups.find_ids_in_batches do |group_ids_chunk|
GradingPeriodGroup.where(id: group_ids_chunk).update_all(workflow_state: "deleted")

13
lib/data_fixup/populate_root_account_id_on_models.rb
View File

@ -61,7 +61,7 @@ module DataFixup::PopulateRootAccountIdOnModels
AccountUser => :account,
AssessmentQuestion => :assessment_question_bank,
AssessmentQuestionBank => :context,
AssetUserAccess => [:context_course, :context_group, {context_account: [:root_account_id, :id]}],
AssetUserAccess => [:context_course, :context_group, {context_account: Account.resolved_root_account_id_sql}],
AssignmentGroup => :context,
AssignmentOverride => [:assignment, :quiz],
AssignmentOverrideStudent => [:assignment, :quiz],
@ -91,7 +91,7 @@ module DataFixup::PopulateRootAccountIdOnModels
Favorite => :context,
Folder => [:account, :course, :group],
GradingPeriod => :grading_period_group,
GradingPeriodGroup => [{root_account: [:root_account_id, :id]}, :course],
GradingPeriodGroup => [{root_account: Account.resolved_root_account_id_sql}, :course],
GradingStandard => :context,
GroupCategory => :context,
GroupMembership => :group,
@ -316,7 +316,7 @@ module DataFixup::PopulateRootAccountIdOnModels
case association
when Hash
association.each_with_object({}) do |(assoc, column), memo|
memo[assoc.to_sym] = column.is_a?(Array) ? column.map(&:to_sym) : column.to_sym
memo[assoc.to_sym] = column
end
when Array
association.reduce({}){|memo, assoc| memo.merge(hash_association(assoc))}
@ -347,7 +347,7 @@ module DataFixup::PopulateRootAccountIdOnModels
# :context_quiz=>:root_account_id
# }
# Accounts are a special case, since subaccounts will have a root_account_id but root accounts
# have a nil root_account_id and will just use their id instead
# have a 0 root_account_id and will just use their id instead
# Eg: ContextExternalTool with association of {context: :root_account_id} becomes
# {
# :account=>[:root_account_id, :id],
@ -363,11 +363,11 @@ module DataFixup::PopulateRootAccountIdOnModels
if assoc_options[:polymorphic].present?
assoc_options[:polymorphic].each do |poly_a|
poly_a = poly_a.keys.first if poly_a.is_a? Hash
account_columns = [:root_account_id, :id] if poly_a == :account
account_columns = Account.resolved_root_account_id_sql if poly_a == :account
memo[:"#{prefix}#{poly_a}"] = account_columns || columns
end
else
columns = [:root_account_id, :id] if assoc == :account
columns = Account.resolved_root_account_id_sql if assoc == :account
memo[assoc] = columns
end
end
@ -541,6 +541,7 @@ module DataFixup::PopulateRootAccountIdOnModels
end
def self.create_column_names(assoc, columns)
return columns if columns.is_a?(String)
names = Array(columns).map{|column| "#{assoc.klass.table_name}.#{column}"}
names.count == 1 ? names.first : "COALESCE(#{names.join(', ')})"
end

2
lib/data_fixup/reassociate_grading_period_groups.rb
View File

@ -21,7 +21,7 @@ module DataFixup::ReassociateGradingPeriodGroups
def self.run
# associates root account grading period groups with enrollment terms
GradingPeriodGroup.active.where.not(account_id: nil).find_in_batches do |groups|
account_subquery = Account.where(id: groups.map(&:account_id), root_account_id: nil)
account_subquery = Account.root_accounts.where(id: groups.map(&:account_id))
term_ids = EnrollmentTerm.active.where(root_account_id: account_subquery).pluck(:id)
groups.each do |group|
EnrollmentTerm.

4
spec/controllers/lti/account_lookup_controller_spec.rb
View File

@ -55,7 +55,7 @@ describe Lti::AccountLookupController do
'name' => acct.name,
'workflow_state' => acct.workflow_state,
'parent_account_id' => acct.parent_account_id,
'root_account_id' => acct.root_account_id
'root_account_id' => nil
)
expect(body['id']).to be_a(Integer)
expect(body['uuid']).to be_a(String)
@ -100,7 +100,7 @@ describe Lti::AccountLookupController do
'name' => acct.name,
'workflow_state' => acct.workflow_state,
'parent_account_id' => acct.parent_account_id,
'root_account_id' => acct.root_account_id
'root_account_id' => nil
)
expect(body['id']).to be_a(Integer)
end

22
spec/lib/data_fixup/populate_root_account_id_on_models_spec.rb
View File

@ -25,7 +25,7 @@ describe DataFixup::PopulateRootAccountIdOnModels do
@cm = @course.context_modules.create!
@cm.update_columns(root_account_id: nil)
user_model
Account.find_or_create_by!(id: 0).update(name: 'Dummy Root Account', workflow_state: 'deleted', root_account_id: nil)
Account.ensure_dummy_root_account
end
# add additional models here as they are calculated and added to migration_tables.
@ -891,12 +891,6 @@ describe DataFixup::PopulateRootAccountIdOnModels do
{submission: :id, assignment: :root_account_id}
)
end
it 'should turn string associations/columns into symbols' do
expect(DataFixup::PopulateRootAccountIdOnModels.hash_association(
[{'submission' => ['root_account_id', 'id']}, 'assignment']
)).to eq({submission: [:root_account_id, :id], assignment: :root_account_id})
end
end
describe '#replace_polymorphic_associations' do
@ -918,7 +912,7 @@ describe DataFixup::PopulateRootAccountIdOnModels do
course: [:root_account_id, :id],
learning_outcome_group: [:root_account_id, :id],
assignment: [:root_account_id, :id],
account: [:root_account_id, :id],
account: Account.resolved_root_account_id_sql,
quiz: [:root_account_id, :id],
context_module: :root_account_id
}
@ -933,7 +927,7 @@ describe DataFixup::PopulateRootAccountIdOnModels do
course: [:root_account_id, :id],
learning_outcome_group: :root_account_id,
assignment: :root_account_id,
account: [:root_account_id, :id],
account: Account.resolved_root_account_id_sql,
quiz: :root_account_id
}
)
@ -958,8 +952,8 @@ describe DataFixup::PopulateRootAccountIdOnModels do
ContextExternalTool, {course: :root_account_id, account: :root_account_id}
)).to eq(
{
:account=>[:root_account_id, :id],
:course=>:root_account_id
:account => Account.resolved_root_account_id_sql,
:course => :root_account_id
}
)
end
@ -1065,19 +1059,19 @@ describe DataFixup::PopulateRootAccountIdOnModels do
describe '#create_column_names' do
it 'should create a single column name' do
expect(DataFixup::PopulateRootAccountIdOnModels.create_column_names(Assignment.reflections["course"], 'root_account_id')).to eq(
expect(DataFixup::PopulateRootAccountIdOnModels.create_column_names(Assignment.reflections["course"], :root_account_id)).to eq(
'courses.root_account_id'
)
end
it 'should coalesce multiple column names on a table' do
expect(DataFixup::PopulateRootAccountIdOnModels.create_column_names(Course.reflections["account"], ['root_account_id', :id])).to eq(
expect(DataFixup::PopulateRootAccountIdOnModels.create_column_names(Course.reflections["account"], [:root_account_id, :id])).to eq(
"COALESCE(accounts.root_account_id, accounts.id)"
)
end
it 'should use actual table names for strangely named columns' do
expect(DataFixup::PopulateRootAccountIdOnModels.create_column_names(AssetUserAccess.reflections["context_course"], 'root_account_id')).to eq(
expect(DataFixup::PopulateRootAccountIdOnModels.create_column_names(AssetUserAccess.reflections["context_course"], :root_account_id)).to eq(
'courses.root_account_id'
)
end

14
spec/lib/data_fixup/populate_root_account_ids_on_learning_outcomes_spec.rb
View File

@ -52,12 +52,16 @@ describe DataFixup::PopulateRootAccountIdsOnLearningOutcomes do
end
context 'no context id' do
specs_require_sharding
it 'sets root_account_ids when there is one root account on shard' do
account_model
lo = LearningOutcome.create!(context_id: nil, short_description: 'test')
lo.update_column(:root_account_ids, nil)
populate(lo)
expect(lo.reload.root_account_ids).to eq [Account.root_accounts.first.id]
@shard1.activate do
account_model
lo = LearningOutcome.create!(context_id: nil, short_description: 'test')
lo.update_column(:root_account_ids, nil)
populate(lo)
expect(lo.reload.root_account_ids).to eq [Account.root_accounts.first.id]
end
end
it 'sets root_account_ids when there are multiple root accounts on shard' do

1
spec/models/account_spec.rb
View File

@ -1746,6 +1746,7 @@ describe Account do
# should clear caches
to_be_subaccount.parent_account = account
to_be_subaccount.root_account = account
to_be_subaccount.save!
expect(to_be_subaccount.default_storage_quota).to eq 10.megabytes
end

5
spec/models/pseudonym_spec.rb
View File

@ -353,6 +353,11 @@ describe Pseudonym do
context "sharding" do
specs_require_sharding
let_once(:account2) { @shard1.activate { Account.create! } }
before(:once) do
# need these instantiated before we set up our mocks
Account.default
account2
end
it "should only query the pertinent shard" do
expect(Pseudonym).to receive(:associated_shards).with('abc').and_return([@shard1])

2
spec/models/user_spec.rb
View File

@ -127,6 +127,7 @@ describe User do
account2 = account_model
account1.parent_account = account2
account1.root_account = account2
account1.save!
@course.reload
@user.reload
@ -159,6 +160,7 @@ describe User do
expect(user.associated_accounts.first).to eql(account1)
account1.parent_account = account2
account1.root_account = account2
account1.save!
user.reload

4
spec/support/test_database_utils.rb
View File

@ -101,10 +101,10 @@ module TestDatabaseUtils
each_connection do |connection|
table_names = get_table_names(connection)
next if table_names.empty?
connection.execute("TRUNCATE TABLE #{table_names.map { |t| connection.quote_table_name(t) }.join(',')}")
end
Account.find_or_create_by!(id: 0).
update(name: 'Dummy Root Account', workflow_state: 'deleted', root_account_id: nil)
Account.ensure_dummy_root_account
end
def get_sequences(connection)