Merge branch '346435-fix-activities-followed-users-sort-order' into 'master'

Followed users events (filter ALL) should be ordered by id DESC

See merge request gitlab-org/gitlab!77436

app/finders/user_recent_events_finder.rb

@@ -49,9 +49,13 @@ class UserRecentEventsFinder
   # rubocop: disable CodeReuse/ActiveRecord
   def execute_optimized_multi(users)
     Gitlab::Pagination::Keyset::InOperatorOptimization::QueryBuilder.new(
-      scope: Event.reorder(:id),
+      scope: Event.reorder(id: :desc),
       array_scope: User.select(:id).where(id: users),
-      array_mapping_scope: -> (author_id_expression) { Event.where(Event.arel_table[:author_id].eq(author_id_expression)) },
+      # Event model has a default scope { reorder(nil) }
+      # When a relation is rordered and used as a target when merging scope,
+      # its order takes a precedence and _overwrites_ the original scope's order.
+      # Thus we have to explicitly provide `reorder` for array_mapping_scope here.
+      array_mapping_scope: -> (author_id_expression) { Event.where(Event.arel_table[:author_id].eq(author_id_expression)).reorder(id: :desc) },
       finder_query: -> (id_expression) { Event.where(Event.arel_table[:id].eq(id_expression)) }
     )
     .execute

db/post_migrate/20211217145923_add_index_to_events_on_author_id_and_action_and_id.rb

 # frozen_string_literal: true
 
 class AddIndexToEventsOnAuthorIdAndActionAndId < Gitlab::Database::Migration[1.0]
-  INDEX_NAME = 'index_events_on_author_id_and_action_and_id'
-
-  disable_ddl_transaction!
+  # no-op
+  # see: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/77436
 
   def up
-    add_concurrent_index :events, [:author_id, :action, :id], name: INDEX_NAME
+    # no-op
   end
 
   def down
-    remove_concurrent_index_by_name :events, INDEX_NAME
+    # no-op
   end
 end

db/post_migrate/20211230112517_remove_index_events_on_author_id_and_action_and_id.rb

+# frozen_string_literal: true
+
+class RemoveIndexEventsOnAuthorIdAndActionAndId < Gitlab::Database::Migration[1.0]
+  INDEX_NAME = 'index_events_on_author_id_and_action_and_id'
+
+  disable_ddl_transaction!
+
+  def up
+    remove_concurrent_index_by_name :events, name: INDEX_NAME
+  end
+
+  def down
+    # no-op
+    # The index had been added in the same milestone.
+    # Adding back the index takes a long time and should not be needed.
+  end
+end

db/post_migrate/20211230113031_add_index_to_events_on_author_id_and_id.rb

+# frozen_string_literal: true
+
+class AddIndexToEventsOnAuthorIdAndId < Gitlab::Database::Migration[1.0]
+  INDEX_NAME = 'index_events_on_author_id_and_id'
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index :events, [:author_id, :id], name: INDEX_NAME
+  end
+
+  def down
+    remove_concurrent_index_by_name :events, INDEX_NAME
+  end
+end

db/schema_migrations/20211230112517

+32c4b0ad5b52b8990b4c0ab6e832d503be6fc802a30aa2de20c7d3ced7f04c36
\ No newline at end of file

db/schema_migrations/20211230113031

+54201f4273226ed92ff0ed991fdba09c1108fccb764e6b9903e9f01e6acced1a
\ No newline at end of file

db/structure.sql

@@ -26040,12 +26040,12 @@ CREATE INDEX index_events_author_id_project_id_action_target_type_created_at ON
 
 CREATE INDEX index_events_on_action ON events USING btree (action);
 
-CREATE INDEX index_events_on_author_id_and_action_and_id ON events USING btree (author_id, action, id);
-
 CREATE INDEX index_events_on_author_id_and_created_at ON events USING btree (author_id, created_at);
 
 CREATE INDEX index_events_on_author_id_and_created_at_merge_requests ON events USING btree (author_id, created_at) WHERE ((target_type)::text = 'MergeRequest'::text);
 
+CREATE INDEX index_events_on_author_id_and_id ON events USING btree (author_id, id);
+
 CREATE INDEX index_events_on_created_at_and_id ON events USING btree (created_at, id) WHERE (created_at > '2021-08-27 00:00:00+00'::timestamp with time zone);
 
 CREATE INDEX index_events_on_group_id_partial ON events USING btree (group_id) WHERE (group_id IS NOT NULL);

spec/finders/user_recent_events_finder_spec.rb

@@ -77,23 +77,23 @@ RSpec.describe UserRecentEventsFinder do
 
         events = described_class.new(current_user, [project_owner, second_user], nil, params).execute
 
-        expect(events).to eq([private_event, internal_event, public_event])
+        expect(events).to contain_exactly(private_event, internal_event, public_event)
       end
 
       context 'with pagination params' do
         using RSpec::Parameterized::TableSyntax
 
         where(:limit, :offset, :ordered_expected_events) do
-          nil | nil   | lazy { [private_event, internal_event, public_event, private_event_second_user, internal_event_second_user, public_event_second_user] }
-          2   | nil   | lazy { [private_event, internal_event] }
-          nil | 4     | lazy { [internal_event_second_user, public_event_second_user] }
-          2   | 2     | lazy { [public_event, private_event_second_user] }
+          nil | nil   | lazy { [public_event_second_user, internal_event_second_user, private_event_second_user, public_event, internal_event, private_event] }
+          2   | nil   | lazy { [public_event_second_user, internal_event_second_user] }
+          nil | 4     | lazy { [internal_event, private_event] }
+          2   | 2     | lazy { [private_event_second_user, public_event] }
         end
 
         with_them do
           let(:params) { { limit: limit, offset: offset }.compact }
 
-          it 'returns paginated events sorted by id' do
+          it 'returns paginated events sorted by id (DESC)' do
             events = described_class.new(current_user, [project_owner, second_user], nil, params).execute
 
             expect(events).to eq(ordered_expected_events)