From e7c3d01aaae39b167a7f053ebc1199ff9a88c088 Mon Sep 17 00:00:00 2001
From: Alan Guo Xiang Tan <gxtan1990@gmail.com>
Date: Fri, 1 Apr 2022 08:58:06 +0800
Subject: [PATCH] DEV: Restore order assertion in category serializer tests.
 (#16344)

Our group fabrication creates groups with name "my_group_#{n}" where n
is the sequence number of the group being created. However, this can
cause the test to be flaky if and when a group with name `my_group_10`
is created as it will be ordered before
`my_group_9`. This commits makes the group names determinstic to
eliminate any flakiness.

This reverts commit 558bc6b746928a1fcdb2773b0447ea4a57aa7419.
---
 app/models/group.rb                          |  2 +-
 app/serializers/category_serializer.rb       |  4 ++--
 spec/serializers/category_serializer_spec.rb | 22 +++++++++++---------
 3 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/app/models/group.rb b/app/models/group.rb
index e6dc796f506..72eae6ab167 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -142,7 +142,7 @@ class Group < ActiveRecord::Base
   scope :with_smtp_configured, -> { where(smtp_enabled: true) }
 
   scope :visible_groups, Proc.new { |user, order, opts|
-    groups = self.order(order || "name ASC")
+    groups = self.order(order || "groups.name ASC")
 
     if !opts || !opts[:include_everyone]
       groups = groups.where("groups.id > 0")
diff --git a/app/serializers/category_serializer.rb b/app/serializers/category_serializer.rb
index 44382b08349..856ecf44ee6 100644
--- a/app/serializers/category_serializer.rb
+++ b/app/serializers/category_serializer.rb
@@ -37,8 +37,8 @@ class CategorySerializer < SiteCategorySerializer
         .category_groups
         .joins(:group)
         .includes(:group)
-        .merge(Group.visible_groups(scope&.user))
-        .order("groups.name").map do |cg|
+        .merge(Group.visible_groups(scope&.user, "groups.name ASC"))
+        .map do |cg|
           {
             permission_type: cg.permission_type,
             group_name: cg.group.name
diff --git a/spec/serializers/category_serializer_spec.rb b/spec/serializers/category_serializer_spec.rb
index a224861140e..aad5b4885a6 100644
--- a/spec/serializers/category_serializer_spec.rb
+++ b/spec/serializers/category_serializer_spec.rb
@@ -54,15 +54,17 @@ describe CategorySerializer do
     end
 
     context "category with group permissions configured" do
-      fab!(:private_group) { Fabricate(:group, visibility_level: Group.visibility_levels[:staff]) }
+      fab!(:private_group) { Fabricate(:group, visibility_level: Group.visibility_levels[:staff], name: 'bbb') }
 
       fab!(:user_group) do
-        Fabricate(:group, visibility_level: Group.visibility_levels[:members]).tap do |g|
+        Fabricate(:group, visibility_level: Group.visibility_levels[:members], name: 'ccc').tap do |g|
           g.add(user)
         end
       end
 
       before do
+        group.update!(name: 'aaa')
+
         category.set_permissions(
           :everyone => :readonly,
           group.name => :readonly,
@@ -76,28 +78,28 @@ describe CategorySerializer do
       it "returns the right category group permissions for an anon user" do
         json = described_class.new(category, scope: Guardian.new, root: false).as_json
 
-        expect(json[:group_permissions]).to contain_exactly(
+        expect(json[:group_permissions]).to eq([
           { permission_type: CategoryGroup.permission_types[:readonly], group_name: group.name },
-        )
+        ])
       end
 
-      it "returns the right category group permissions for a regular user" do
+      it "returns the right category group permissions for a regular user ordered by ascending group name" do
         json = described_class.new(category, scope: Guardian.new(user), root: false).as_json
 
-        expect(json[:group_permissions]).to contain_exactly(
+        expect(json[:group_permissions]).to eq([
           { permission_type: CategoryGroup.permission_types[:readonly], group_name: group.name },
           { permission_type: CategoryGroup.permission_types[:full], group_name: user_group.name },
-        )
+        ])
       end
 
-      it "returns the right category group permission for a staff user" do
+      it "returns the right category group permission for a staff user ordered by ascending group name" do
         json = described_class.new(category, scope: Guardian.new(admin), root: false).as_json
 
-        expect(json[:group_permissions]).to contain_exactly(
+        expect(json[:group_permissions]).to eq([
           { permission_type: CategoryGroup.permission_types[:readonly], group_name: group.name },
           { permission_type: CategoryGroup.permission_types[:full], group_name: private_group.name },
           { permission_type: CategoryGroup.permission_types[:full], group_name: user_group.name }
-        )
+        ])
       end
     end
   end