optimize Account#/Course.update_account_associations a bit

* don't pull all courses in at once
 * only select necessary columns

also pick up some edge cases where we wouldn't catch courses

test plan:
 * do some sis imports
 * it shouldn't fail

Change-Id: I18d03f78e31c47de183e8a61ce7627ef1c3424ad
Reviewed-on: https://gerrit.instructure.com/21157
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Brian Palmer <brianp@instructure.com>
QA-Review: Jeremy Putnam <jeremyp@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>

app/models/account.rb

@@ -855,15 +855,43 @@ class Account < ActiveRecord::Base
   def update_account_associations
     self.shard.activate do
       account_chain_cache = {}
-      all_user_ids = []
-      all_user_ids += Course.update_account_associations(self.associated_courses, :skip_user_account_associations => true, :account_chain_cache => account_chain_cache)
+      all_user_ids = Set.new
+
+      # make sure to use the non-associated_courses associations
+      # to catch courses that didn't ever have an association created
+      scopes = if root_account?
+                [all_courses.select([:id, :account_id]),
+                 associated_courses.
+                     where("root_account_id<>?", self).
+                     except(:select).
+                     select("courses.id, courses.account_id").
+                     uniq
+                ]
+              else
+                [courses.select([:id, :account_id]),
+                 associated_courses.
+                    where("courses.account_id<>?", self).
+                    except(:select).
+                    select("courses.id, courses.account_id").
+                    uniq]
+              end
+      # match the "batch" size in Course.update_account_associations
+      scopes.each do |scope|
+        scope.find_in_batches(:batch_size => 500) do |courses|
+          Course.send(:with_exclusive_scope) do
+            all_user_ids.merge Course.update_account_associations(courses, :skip_user_account_associations => true, :account_chain_cache => account_chain_cache)
+          end
+        end
+      end
 
       # Make sure we have all users with existing account associations.
-      # (This should catch users with Pseudonyms associated with the account.)
-      all_user_ids += self.user_account_associations.pluck(:user_id)
+      all_user_ids.merge self.user_account_associations.pluck(:user_id)
+      if root_account?
+        all_user_ids.merge self.pseudonyms.active.pluck(:user_id)
+      end
 
       # Update the users' associations as well
-      User.update_account_associations(all_user_ids.uniq, :account_chain_cache => account_chain_cache)
+      User.update_account_associations(all_user_ids.to_a, :account_chain_cache => account_chain_cache)
     end
   end
   

app/models/course.rb

@@ -342,7 +342,10 @@ class Course < ActiveRecord::Base
         course_ids = courses.map(&:id)
       else
         course_ids = courses_or_course_ids
-        courses = Course.where(:id => course_ids).includes(:course_sections => [:course, :nonxlist_course]).all
+        courses = Course.where(:id => course_ids).
+            includes(:course_sections => [:course, :nonxlist_course]).
+            select([:id, :account_id]).
+            all
       end
       course_ids_to_update_user_account_associations = []
       CourseAccountAssociation.transaction do

app/models/course_account_association.rb

@@ -22,4 +22,6 @@ class CourseAccountAssociation < ActiveRecord::Base
   belongs_to :account
   has_many :account_users, :foreign_key => 'account_id', :primary_key => 'account_id'
   attr_accessible :account, :depth, :course_section
+
+  validates_presence_of :course_id, :account_id, :depth
 end

spec/models/account_spec.rb

@@ -978,4 +978,17 @@ describe Account do
       account.can_see_admin_tools_tab?(@admin).should be_true
     end
   end
+
+  describe "#update_account_associations" do
+    it "should update associations for all courses" do
+      account = Account.create!
+      c1 = account.courses.create!
+      c2 = account.courses.create!
+      account.course_account_associations.delete_all
+      account.associated_courses.should == []
+      account.update_account_associations
+      account.reload
+      account.associated_courses.sort_by(&:id).should == [c1, c2]
+    end
+  end
 end