Skip to content

Commit 4748aba

Browse files
Nick Clowardsimonista
authored andcommitted
extract canvas sort gem
fixes: CNVS-11890 Test Plan: Canvas Should not be broken, test requests that use the SortLast and SortFirst class. One is account settings. Make sure its sorted correctly. Another is submissions_controller show method and make sure the visible rubric assesments are sorted correctly. Change-Id: I75a61475a928e069d6566fcc51305a6c442ec5a0 Reviewed-on: https://gerrit.instructure.com/32014 Tested-by: Jenkins <jenkins@instructure.com> Reviewed-by: Simon Williams <simon@instructure.com> Product-Review: Simon Williams <simon@instructure.com> QA-Review: Simon Williams <simon@instructure.com>
1 parent f0a7554 commit 4748aba

42 files changed

Lines changed: 297 additions & 163 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

Gemfile.d/other_stuff.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,7 @@
121121
gem 'canvas_http', :path => 'gems/canvas_http'
122122
gem 'canvas_mimetype_fu', :path => 'gems/canvas_mimetype_fu'
123123
gem 'canvas_sanitize', :path => 'gems/canvas_sanitize'
124+
gem 'canvas_sort', :path => 'gems/canvas_sort'
124125
gem 'canvas_statsd', :path => 'gems/canvas_statsd'
125126
gem 'canvas_stringex', :path => 'gems/canvas_stringex'
126127
gem 'canvas_uuid', :path => 'gems/canvas_uuid'

app/controllers/accounts_controller.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -484,7 +484,7 @@ def settings
484484
@account.available_account_roles.each_with_index do |type, idx|
485485
order_hash[type] = idx
486486
end
487-
@account_users = @account_users.select(&:user).sort_by{|au| [order_hash[au.membership_type] || SortLast, Canvas::ICU.collation_key(au.user.sortable_name)] }
487+
@account_users = @account_users.select(&:user).sort_by{|au| [order_hash[au.membership_type] || CanvasSort::Last, Canvas::ICU.collation_key(au.user.sortable_name)] }
488488
@alerts = @account.alerts
489489
@role_types = RoleOverride.account_membership_types(@account)
490490
@enrollment_types = RoleOverride.enrollment_types

app/controllers/calendar_events_api_controller.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -556,7 +556,7 @@ def public_feed
556556
end
557557
end
558558

559-
@events = @events.sort_by { |e| [e.start_at || SortLast, Canvas::ICU.collation_key(e.title)] }
559+
@events = @events.sort_by { |e| [e.start_at || CanvasSort::Last, Canvas::ICU.collation_key(e.title)] }
560560

561561
@contexts.each do |context|
562562
log_asset_access("calendar_feed:#{context.asset_string}", "calendar", 'other')

app/controllers/communication_channels_controller.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -257,7 +257,7 @@ def confirm
257257
@merge_opportunities.last.last.sort! { |a, b| Canvas::ICU.compare(a.account.name, b.account.name) }
258258
end
259259
end
260-
@merge_opportunities.sort_by! { |a| [a.first == @current_user ? SortFirst : SortLast, Canvas::ICU.collation_key(a.first.name)] }
260+
@merge_opportunities.sort_by! { |a| [a.first == @current_user ? CanvasSort::First : CanvasSort::Last, Canvas::ICU.collation_key(a.first.name)] }
261261

262262
js_env :PASSWORD_POLICY => @domain_root_account.password_policy
263263

app/controllers/content_migrations_controller.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ def index
150150
if api_request?
151151
render :json => content_migration_json_hash
152152
else
153-
@plugins = ContentMigration.migration_plugins(true).sort_by {|p| [p.metadata(:sort_order) || SortLast, p.metadata(:select_text)]}
153+
@plugins = ContentMigration.migration_plugins(true).sort_by {|p| [p.metadata(:sort_order) || CanvasSort::Last, p.metadata(:select_text)]}
154154

155155
options = @plugins.map{|p| {:label => p.metadata(:select_text), :id => p.id}}
156156

app/controllers/context_controller.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -331,7 +331,7 @@ def roster_user_services
331331
@users.each_with_index{|u, i| @users_hash[u.id] = u; @users_order_hash[u.id] = i }
332332
@current_user_services = {}
333333
@current_user.user_services.each{|s| @current_user_services[s.service] = s }
334-
@services = UserService.for_user(@users.except(:select, :order)).sort_by{|s| @users_order_hash[s.user_id] || SortLast}
334+
@services = UserService.for_user(@users.except(:select, :order)).sort_by{|s| @users_order_hash[s.user_id] || CanvasSort::Last}
335335
@services = @services.select{|service|
336336
!UserService.configured_service?(service.service) || feature_and_service_enabled?(service.service.to_sym)
337337
}

app/controllers/question_banks_controller.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ def index
3232
@question_banks += @context.inherited_assessment_question_banks.active
3333
end
3434
@question_banks = @question_banks.select{|b| b.grants_right?(@current_user, nil, :manage) } if params[:managed] == '1'
35-
@question_banks = Canvas::ICU.collate_by(@question_banks.uniq) { |b| b.title || SortLast }
35+
@question_banks = Canvas::ICU.collate_by(@question_banks.uniq) { |b| b.title || CanvasSort::Last }
3636
respond_to do |format|
3737
format.html
3838
format.json { render :json => @question_banks.map{ |b| b.as_json(methods: [:cached_context_short_name, :assessment_question_count]) }}

app/controllers/quizzes/quizzes_controller.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ def index
3939
redirect_to fabulous_quizzes_course_quizzes_path
4040
end
4141
return unless tab_enabled?(@context.class::TAB_QUIZZES)
42-
@quizzes = @context.quizzes.active.include_assignment.sort_by{|q| [(q.assignment ? q.assignment.due_at : q.lock_at) || SortLast, Canvas::ICU.collation_key(q.title || SortFirst)]}
42+
@quizzes = @context.quizzes.active.include_assignment.sort_by{|q| [(q.assignment ? q.assignment.due_at : q.lock_at) || CanvasSort::Last, Canvas::ICU.collation_key(q.title || CanvasSort::First)]}
4343

4444
# draft state - only filter by available? for students
4545
if @context.feature_enabled?(:draft_state)

app/controllers/submissions_controller.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ def show
235235
if @assignment.muted? && !@submission.grants_right?(@current_user, :read_grade)
236236
@visible_rubric_assessments = []
237237
else
238-
@visible_rubric_assessments = @submission.rubric_assessments.select{|a| a.grants_rights?(@current_user, session, :read)[:read]}.sort_by{|a| [a.assessment_type == 'grading' ? SortFirst : SortLast, Canvas::ICU.collation_key(a.assessor_name)] }
238+
@visible_rubric_assessments = @submission.rubric_assessments.select{|a| a.grants_rights?(@current_user, session, :read)[:read]}.sort_by{|a| [a.assessment_type == 'grading' ? CanvasSort::First : CanvasSort::Last, Canvas::ICU.collation_key(a.assessor_name)] }
239239
end
240240

241241
@assessment_request = @submission.assessment_requests.find_by_assessor_id(@current_user.id) rescue nil

app/models/assignment.rb

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1298,7 +1298,7 @@ def representatives(user)
12981298

12991299
def visible_rubric_assessments_for(user)
13001300
if self.rubric_association
1301-
self.rubric_association.rubric_assessments.select{|a| a.grants_rights?(user, :read)[:read]}.sort_by{|a| [a.assessment_type == 'grading' ? SortFirst : SortLast, Canvas::ICU.collation_key(a.assessor_name)] }
1301+
self.rubric_association.rubric_assessments.select{|a| a.grants_rights?(user, :read)[:read]}.sort_by{|a| [a.assessment_type == 'grading' ? CanvasSort::First : CanvasSort::Last, Canvas::ICU.collation_key(a.assessor_name)] }
13021302
end
13031303
end
13041304

@@ -1396,7 +1396,7 @@ def assign_peer_reviews
13961396
}.sort_by { |c|
13971397
[
13981398
# prefer those who still need more reviews done.
1399-
assessment_request_counts[c.id] < self.peer_review_count ? SortFirst : SortLast,
1399+
assessment_request_counts[c.id] < self.peer_review_count ? CanvasSort::First : CanvasSort::Last,
14001400
# then prefer those who are assigned fewer reviews at this point --
14011401
# this helps avoid loops where everybody is reviewing those who are
14021402
# reviewing them, leaving the final assignee out in the cold.
@@ -1796,7 +1796,7 @@ def <=>(comparable)
17961796

17971797
def sort_key
17981798
# undated assignments go last
1799-
[due_at || SortLast, Canvas::ICU.collation_key(title)]
1799+
[due_at || CanvasSort::Last, Canvas::ICU.collation_key(title)]
18001800
end
18011801

18021802
def special_class; nil; end

0 commit comments

Comments
 (0)