FIX: Sort group owners and members together (#15708)

Sorting group members worked always kept the group owners at the top of
the list. This commit keeps the group owners at the top of the list only
when no order exists.

app/controllers/groups_controller.rb

@@ -251,7 +251,7 @@ class GroupsController < ApplicationController
       Discourse.deprecate(":desc is deprecated please use :asc instead", output_in_test: true, drop_from: '2.9.0')
       dir = (params[:desc] && params[:desc].present?) ? 'DESC' : 'ASC'
     end
-    order = ""
+    order = "NOT group_users.owner"
 
     if params[:requesters]
       guardian.ensure_can_edit!(group)
@@ -305,21 +305,15 @@ class GroupsController < ApplicationController
       end
     end
 
-    users = users.joins(:user_option).select('users.*, user_options.timezone, group_users.created_at as added_at')
-
-    members = users
-      .order('NOT group_users.owner')
+    users = users
+      .includes(:primary_group)
+      .joins(:user_option)
+      .select('users.*, user_options.timezone, group_users.created_at as added_at')
       .order(order)
       .order(username_lower: dir)
-      .limit(limit)
-      .offset(offset)
-      .includes(:primary_group)
 
-    owners = users
-      .order(order)
-      .order(username_lower: dir)
-      .where('group_users.owner')
-      .includes(:primary_group)
+    members = users.limit(limit).offset(offset)
+    owners = users.where('group_users.owner')
 
     render json: {
       members: serialize_data(members, GroupUserSerializer),

spec/requests/groups_controller_spec.rb

@@ -547,6 +547,23 @@ describe GroupsController do
 
       expect(members.last['added_at']).to eq(first_user.created_at.as_json)
     end
+
+    it "can sort items" do
+      sign_in(user)
+      group.update!(visibility_level: Group.visibility_levels[:logged_on_users])
+      other_user = Fabricate(:user)
+      group.add_owner(other_user)
+
+      get "/groups/#{group.name}/members.json"
+
+      expect(response.parsed_body["members"].map { |u| u["id"] }).to eq([other_user.id, user.id])
+      expect(response.parsed_body["owners"].map { |u| u["id"] }).to eq([other_user.id])
+
+      get "/groups/#{group.name}/members.json?order=added_at&asc=1"
+
+      expect(response.parsed_body["members"].map { |u| u["id"] }).to eq([user.id, other_user.id])
+      expect(response.parsed_body["owners"].map { |u| u["id"] }).to eq([other_user.id])
+    end
   end
 
   describe '#posts_feed' do