Skip to content

Commit a5ccb6b

Browse files
committed
fake arel!
closes CNVS-4705 * use the fake_arel gem to get a good portion of the way there * override fake_arel's AR override even more to get proper behavior of select and group merging * add even more Rails 3 query methods to Scope (except, reorder, pluck, uniq) * fix some spots in our code that break with the new semantics test plan: * test all the things! Change-Id: I4290d00db407f3250570df4e89c8c78283fe5f5f Reviewed-on: https://gerrit.instructure.com/18427 Reviewed-by: Brian Palmer <brianp@instructure.com> Tested-by: Jenkins <jenkins@instructure.com> QA-Review: Clare Hetherington <clare@instructure.com> Product-Review: Cody Cutrer <cody@instructure.com>
1 parent 4c355ae commit a5ccb6b

19 files changed

Lines changed: 311 additions & 187 deletions

Gemfile

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ gem 'daemons', '1.1.0'
1313
gem 'diff-lcs', '1.1.2', :require => 'diff/lcs'
1414
gem 'encrypted_cookie_store-instructure', '1.0.2', :require => 'encrypted_cookie_store'
1515
gem 'erubis', '2.7.0'
16+
gem 'fake_arel', '1.0.0'
1617
gem 'ffi', '1.1.5'
1718
gem 'hairtrigger', '0.1.14'
1819
gem 'sass', '3.2.3'

app/controllers/context_controller.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -324,7 +324,8 @@ def prior_users
324324
if authorized_action(@context, @current_user, [:manage_students, :manage_admin_users, :read_prior_roster])
325325
@prior_users = @context.prior_users.
326326
where(Enrollment.not_fake.proxy_options[:conditions]).
327-
by_top_enrollment(:select => "users.*, NULL AS prior_enrollment").
327+
select("users.*, NULL AS prior_enrollment").
328+
by_top_enrollment.
328329
paginate(:page => params[:page], :per_page => 20)
329330

330331
users = @prior_users.index_by(&:id)

app/controllers/gradebooks_controller.rb

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -223,17 +223,9 @@ def gradebook_init_json
223223
params[:user_ids] ||= params[:user_id]
224224
user_ids = params[:user_ids].split(",").map(&:to_i) if params[:user_ids]
225225
assignment_ids = params[:assignment_ids].split(",").map(&:to_i) if params[:assignment_ids]
226-
# you need to specify specifically which submission fields you want returned to the gradebook here
227-
scope_options = {
228-
:select => ["assignment_id", "attachment_id", "grade", "grade_matches_current_submission", "group_id", "has_rubric_assessment", "id", "score", "submission_comments_count", "submission_type", "submitted_at", "url", "user_id"].join(" ,")
229-
}
230-
if user_ids && assignment_ids
231-
@submissions = @context.submissions.scoped(scope_options).find(:all, :conditions => {:user_id => user_ids, :assignment_id => assignment_ids})
232-
elsif user_ids
233-
@submissions = @context.submissions.scoped(scope_options).find(:all, :conditions => {:user_id => user_ids})
234-
else
235-
@submissions = @context.submissions.scoped(scope_options)
236-
end
226+
@submissions = @context.submissions
227+
@submissions = @submissions.where(:user_id => user_ids) if user_ids
228+
@submissions = @submissions.where(:assignment_id => assignment_ids) if assignment_ids
237229
render :json => @submissions.to_json(:include => [:attachments, :quiz_submission, :submission_comments])
238230
end
239231
end

app/controllers/question_banks_controller.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ class QuestionBanksController < ApplicationController
2424

2525
def index
2626
if @context == @current_user || authorized_action(@context, @current_user, :manage_assignments)
27-
@question_banks = @context.assessment_question_banks.active
27+
@question_banks = @context.assessment_question_banks.active.all
2828
if params[:include_bookmarked] == '1'
2929
@question_banks += @current_user.assessment_question_banks.active
3030
end

app/controllers/user_notes_controller.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ def user_notes
4141
@is_course = true
4242
end
4343
count = @users.count
44-
@users = @users.order_by_sortable_name(:select=> "name, users.id, last_user_note", :order=>"last_user_note ASC")
44+
@users = @users.select("name, users.id, last_user_note").order("last_user_note").order_by_sortable_name
4545
@users = @users.paginate(:page => params[:page], :per_page => 20, :total_entries=>count)
4646
# rails gets confused by :include => :courses, because has_current_student_enrollments above references courses in a subquery
4747
User.send(:preload_associations, @users, :courses)

app/controllers/users_controller.rb

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -195,11 +195,10 @@ def index
195195
if @context && @context.is_a?(Account) && @query
196196
@users = @context.users_name_like(@query)
197197
elsif params[:enrollment_term_id].present? && @root_account == @context
198-
@users = @context.fast_all_users.scoped({
199-
:joins => :courses,
200-
:conditions => ["courses.enrollment_term_id = ?", params[:enrollment_term_id]],
201-
:group => @context.connection.group_by('users.id', 'users.name', 'users.sortable_name')
202-
})
198+
# fast_all_users already specifies a select for the distinct to work on
199+
@users = @context.fast_all_users.joins(:courses).
200+
where(:courses => { :enrollment_term_id => params[:enrollment_term_id] })
201+
@users = @users.uniq
203202
elsif !api_request?
204203
@users = @context.fast_all_users
205204
end

app/models/account.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -354,7 +354,7 @@ def all_users(limit=250)
354354

355355
def fast_all_users(limit=nil)
356356
@cached_fast_all_users ||= {}
357-
@cached_fast_all_users[limit] ||= self.all_users(limit).active.order_by_sortable_name.scoped(:select => "users.id, users.name, users.sortable_name")
357+
@cached_fast_all_users[limit] ||= self.all_users(limit).active.select("users.id, users.name, users.sortable_name").order_by_sortable_name
358358
end
359359

360360
def users_not_in_groups_sql(groups, opts={})

app/models/appointment_group.rb

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -313,8 +313,9 @@ def possible_participants(registration_status=nil)
313313

314314
def participant_ids
315315
appointments_participants.
316-
scoped(:select => 'context_id', :conditions => ["calendar_events.context_type = ?", participant_type]).
317-
map(&:context_id)
316+
except(:order).
317+
where(:context_type => participant_type).
318+
pluck(:context_id)
318319
end
319320

320321
def participant_table

app/models/course.rb

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -425,7 +425,7 @@ def update_account_associations
425425
end
426426

427427
def associated_accounts
428-
self.non_unique_associated_accounts.uniq
428+
self.non_unique_associated_accounts.all.uniq
429429
end
430430

431431
named_scope :recently_started, lambda {
@@ -1314,7 +1314,7 @@ def send_final_grades_to_endpoint(publishing_user)
13141314
# 'publish_final_grades'
13151315

13161316
self.recompute_student_scores_without_send_later
1317-
enrollments = self.student_enrollments.not_fake.order_by_sortable_name(:include => [:user, :course_section])
1317+
enrollments = self.student_enrollments.not_fake.includes(:user, :course_section).order_by_sortable_name
13181318

13191319
errors = []
13201320
posts_to_make = []
@@ -1392,7 +1392,7 @@ def gradebook_to_csv(options = {})
13921392
includes = [:user, :course_section]
13931393
includes = {:user => :pseudonyms, :course_section => []} if options[:include_sis_id]
13941394
scope = options[:user] ? self.enrollments_visible_to(options[:user]) : self.student_enrollments
1395-
student_enrollments = scope.order_by_sortable_name(:include => includes) # remove duplicate enrollments for students enrolled in multiple sections
1395+
student_enrollments = scope.includes(includes).order_by_sortable_name # remove duplicate enrollments for students enrolled in multiple sections
13961396
student_enrollments = student_enrollments.all.uniq_by(&:user_id)
13971397

13981398
calc = GradeCalculator.new(student_enrollments.map(&:user_id), self,

app/models/enrollment.rb

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -898,9 +898,13 @@ def self.cached_temporary_invitations(email)
898898
end
899899
end
900900

901-
def self.order_by_sortable_name(options = {})
902-
add_sort_key!(options, User.sortable_name_order_by_clause('users'))
903-
uber_scope(options)
901+
def self.order_by_sortable_name
902+
clause = User.sortable_name_order_by_clause('users')
903+
scope = self.order(clause)
904+
if scope.scope(:find, :select)
905+
scope = scope.select(clause)
906+
end
907+
scope
904908
end
905909

906910
def self.top_enrollment_by(key, rank_order = :default)

0 commit comments

Comments
 (0)