correct calculation of results for identical names

closes CNVS-51409
refs OUT-4149
flag=none

Test plan:
- create course with two users with the same name
- create two outcomes in course with calculation
  method decaying_average
- create two assignments with rubrics referencing those
  two outcomes
- assess the students on those assignments (ensure that
  you assess both students on one assignment, then
  both on the other), with different scores on the two
  assignments
- verify that the learning mastery gradebook shows the
  correctly averaged scores for each student on each
  outcome

Change-Id: I3bb8970b850f8dfa4e531f647fe33c89e57c75d4
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/256184
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Pat Renner <prenner@instructure.com>
Reviewed-by: Augusto Callejas <acallejas@instructure.com>
QA-Review: Pat Renner <prenner@instructure.com>
Product-Review: Jody Sailor

lib/outcomes/result_analytics.rb

@@ -59,7 +59,9 @@ module Outcomes
     #
     # Returns the resulting relation
     def order_results_for_rollup(relation)
-      relation.joins(:user).order(User.sortable_name_order_by_clause).order('learning_outcome_results.learning_outcome_id ASC, learning_outcome_results.id ASC')
+      relation.joins(:user).
+        order(User.sortable_name_order_by_clause).
+        order('users.id ASC, learning_outcome_results.learning_outcome_id ASC, learning_outcome_results.id ASC')
     end
 
     # Public: Generates a rollup of each outcome result for each user.
@@ -77,9 +79,9 @@ module Outcomes
     #            determining the current_method chosen for calculating rollups.
     #
     # Returns an Array of Rollup objects.
-    def outcome_results_rollups(results: , users: [], excludes: [], context: nil)
+    def outcome_results_rollups(results:, users: [], excludes: [], context: nil)
       ActiveRecord::Associations::Preloader.new.preload(results, :learning_outcome)
-      rollups = results.chunk(&:user_id).map do |_, user_results|
+      rollups = results.group_by(&:user_id).map do |_, user_results|
         Rollup.new(user_results.first.user, rollup_user_results(user_results, context))
       end
       if excludes.include? 'missing_user_rollups'
@@ -126,6 +128,7 @@ module Outcomes
 
     def mastery_scale_opts(context)
       return {} unless context.is_a?(Course) && context.root_account.feature_enabled?(:account_level_mastery_scales)
+
       @mastery_scale_opts ||= {}
       @mastery_scale_opts[context.asset_string] ||= begin
         method = context.resolved_outcome_calculation_method
@@ -182,6 +185,7 @@ module Outcomes
     def to_percents(count_arr)
       total = count_arr.sum
       return count_arr if total.zero?
+
       count_arr.map {|v| (100.0 * v / total).round}
     end
 

spec/lib/outcomes/result_analytics_spec.rb

@@ -259,6 +259,19 @@ describe Outcomes::ResultAnalytics do
       end
     end
 
+    it 'correctly handles users with the same name' do
+      results = [
+        outcome_from_score(5.0, {method: 'decaying_average', user: User.new(id: 20, name: 'b')}),
+        outcome_from_score(3.0, {method: 'decaying_average', user: User.new(id: 30, name: 'b')}),
+        outcome_from_score(2.0, {method: 'decaying_average', user: User.new(id: 30, name: 'b')}),
+        outcome_from_score(4.0, {method: 'decaying_average', user: User.new(id: 20, name: 'b')})
+      ]
+      users = [User.new(id: 20, name:'b'), User.new(id: 30, name: 'b')]
+      rollups = ra.outcome_results_rollups(results: results, users: users)
+      scores_by_user = [4.35, 2.35]
+      expect(rollups.flat_map(&:scores).map(&:score)).to eq scores_by_user
+    end
+
     it 'excludes missing user rollups' do
       results = [
         outcome_from_score(5.0, {user: User.new(id: 20, name: 'b')})