Skip to content

Commit 9c491b5

Browse files
committed
speed up accounts' courses tab
don't load all associated courses into memory just for a count Change-Id: I9d67d8297fcaf8c5289ff1f85cc4613e6c9b50c7 Reviewed-on: https://gerrit.instructure.com/5235 Tested-by: Hudson <hudson@instructure.com> Reviewed-by: JT Olds <jt@instructure.com>
1 parent 1a9edb3 commit 9c491b5

2 files changed

Lines changed: 21 additions & 15 deletions

File tree

app/controllers/accounts_controller.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,7 @@ def load_course_right_side
246246
end
247247
associated_courses = @account.associated_courses.active
248248
associated_courses = associated_courses.for_term(@term) if @term
249-
@associated_courses_count = associated_courses.uniq.count
249+
@associated_courses_count = associated_courses.count('DISTINCT course_id')
250250
@hide_enrollmentless_courses = params[:hide_enrollmentless_courses] == "1"
251251
end
252252
protected :load_course_right_side

spec/controllers/accounts_controller_spec.rb

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -19,25 +19,20 @@
1919
require File.expand_path(File.dirname(__FILE__) + '/../spec_helper')
2020

2121
describe AccountsController do
22-
def account
23-
@account = @course.account
24-
end
25-
2622
def account_with_admin_logged_in(opts = {})
27-
course_with_teacher_logged_in({:active_all => true}.merge(opts))
28-
@account = account
29-
@account.add_user(@user)
30-
@admin = @user
23+
@account = Account.default
24+
account_admin_user
25+
user_session(@admin)
3126
end
3227

3328
def cross_listed_course
34-
course_with_teacher_logged_in(:active_all => true)
35-
@account1 = account
29+
account_with_admin_logged_in
30+
@account1 = Account.create!
3631
@account1.add_user(@user)
3732
@course1 = @course
3833
@course1.account = @account1
3934
@course1.save!
40-
@account2 = account
35+
@account2 = Account.create!
4136
@course2 = course
4237
@course2.account = @account2
4338
@course2.save!
@@ -48,10 +43,8 @@ def cross_listed_course
4843

4944
describe "SIS imports" do
5045
it "should set batch mode and term if given" do
51-
course_with_teacher_logged_in(:active_all => true)
52-
@account = account
46+
account_with_admin_logged_in
5347
@account.update_attribute(:allow_sis_import, true)
54-
@account.add_user(@user)
5548
post 'sis_import_submit', :account_id => @account.id, :import_type => 'instructure_csv_zip', :batch_mode => '1'
5649
batch = SisBatch.last
5750
batch.should_not be_nil
@@ -88,4 +81,17 @@ def cross_listed_course
8881
get 'show', :id => account.id
8982
response.should redirect_to(cas_client.add_service_to_login_url(login_url))
9083
end
84+
85+
it "should count total courses correctly" do
86+
account_with_admin_logged_in
87+
course
88+
@course.course_sections.create!
89+
@course.course_sections.create!
90+
@course.update_account_associations
91+
@account.course_account_associations.length.should == 3 # one for each section, and the "nil" section
92+
93+
get 'show', :id => @account.id, :format => 'html'
94+
95+
assigns[:associated_courses_count].should == 1
96+
end
9197
end

0 commit comments

Comments
 (0)