From a408a8c7fec1d403509aaead1b34dbb4cc6c8830 Mon Sep 17 00:00:00 2001
From: jorg-vr <jorg.vr@live.be>
Date: Tue, 3 Oct 2023 10:14:21 +0200
Subject: [PATCH] Fix sorting for most recent submission per user

---
 app/controllers/submissions_controller.rb     |  8 ++-
 .../submissions_controller_test.rb            | 52 +++++++++++++++++++
 2 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/app/controllers/submissions_controller.rb b/app/controllers/submissions_controller.rb
index 99afaefa09..dd382a6d45 100644
--- a/app/controllers/submissions_controller.rb
+++ b/app/controllers/submissions_controller.rb
@@ -188,8 +188,14 @@ def set_submissions
 
     @course_membership = CourseMembership.find_by(user: @user, course: @course) if @user.present? && @course.present?
 
+    return unless params[:most_recent_per_user]
+
     # this cannot use has_scope, because we need the scopes in this method
     # to be applied before this one
-    @submissions = @submissions.most_recent_per_user if params[:most_recent_per_user]
+    @submissions = @submissions.most_recent_per_user
+    # reapplies the order_by scope if present in the params
+    # this is needed because the previous line creates a group by query, which breaks the order_by scope
+    @submissions = Submission.where(id: @submissions.pluck(:id)) # otherwise the group_by breaks order_by scopes that use joins
+    @submissions = apply_scopes(@submissions, { order_by: params[:order_by] }) if params[:order_by].present?
   end
 end
diff --git a/test/controllers/submissions_controller_test.rb b/test/controllers/submissions_controller_test.rb
index f6b9a3de63..8bd38f4d4f 100644
--- a/test/controllers/submissions_controller_test.rb
+++ b/test/controllers/submissions_controller_test.rb
@@ -384,4 +384,56 @@ def expected_score_string(*args)
       assert_match expected_score_string(feedback.score, feedback.maximum_score), response.body
     end
   end
+
+  test 'should be able to order most recent submissions by user' do
+    u = create :user, first_name: 'abcd'
+    u2 = create :user, first_name: 'efgh'
+    course = create :course, series_count: 1, activities_per_series: 1
+    e = course.series.first.exercises.first
+    create :submission, exercise: e, user: u, course: course, created_at: 2.minutes.ago, status: :correct
+    least_recent = create :submission, exercise: e, user: u2, course: course, created_at: 1.minute.ago, status: :wrong
+    most_recent = create :submission, exercise: e, user: u, course: course, status: :running
+
+    get course_series_activity_submissions_path(course, course.series.first, e), params: { most_recent_per_user: true, format: :json }
+
+    assert_equal 2, response.parsed_body.count
+    assert_equal most_recent.id, response.parsed_body.first['id']
+    assert_equal least_recent.id, response.parsed_body.second['id']
+
+    get course_series_activity_submissions_path(course, course.series.first, e), params: { most_recent_per_user: true, order_by: { column: 'created_at', direction: 'ASC' }, format: :json }
+
+    assert_equal 2, response.parsed_body.count
+    assert_equal least_recent.id, response.parsed_body.first['id']
+    assert_equal most_recent.id, response.parsed_body.second['id']
+
+    get course_series_activity_submissions_path(course, course.series.first, e), params: { most_recent_per_user: true, order_by: { column: 'created_at', direction: 'DESC' }, format: :json }
+
+    assert_equal 2, response.parsed_body.count
+    assert_equal most_recent.id, response.parsed_body.first['id']
+    assert_equal least_recent.id, response.parsed_body.second['id']
+
+    get course_series_activity_submissions_path(course, course.series.first, e), params: { most_recent_per_user: true, order_by: { column: 'user', direction: 'DESC' }, format: :json }
+
+    assert_equal 2, response.parsed_body.count
+    assert_equal least_recent.id, response.parsed_body.first['id']
+    assert_equal most_recent.id, response.parsed_body.second['id']
+
+    get course_series_activity_submissions_path(course, course.series.first, e), params: { most_recent_per_user: true, order_by: { column: 'user', direction: 'ASC' }, format: :json }
+
+    assert_equal 2, response.parsed_body.count
+    assert_equal most_recent.id, response.parsed_body.first['id']
+    assert_equal least_recent.id, response.parsed_body.second['id']
+
+    get course_series_activity_submissions_path(course, course.series.first, e), params: { most_recent_per_user: true, order_by: { column: 'status', direction: 'DESC' }, format: :json }
+
+    assert_equal 2, response.parsed_body.count
+    assert_equal most_recent.id, response.parsed_body.first['id']
+    assert_equal least_recent.id, response.parsed_body.second['id']
+
+    get course_series_activity_submissions_path(course, course.series.first, e), params: { most_recent_per_user: true, order_by: { column: 'status', direction: 'ASC' }, format: :json }
+
+    assert_equal 2, response.parsed_body.count
+    assert_equal least_recent.id, response.parsed_body.first['id']
+    assert_equal most_recent.id, response.parsed_body.second['id']
+  end
 end