From e5d07bb0830eea056ea85863bca3d5b093d43a88 Mon Sep 17 00:00:00 2001
From: Erik Johnston <erikj@element.io>
Date: Fri, 6 Sep 2024 11:44:37 +0100
Subject: [PATCH] Fix bump stamp for non-joined rooms (#17674)

We should only look for bump stamps in joined rooms, otherwise we should
just use the membership stream ordering.
---
 changelog.d/17674.bugfix                      |  1 +
 synapse/handlers/sliding_sync/__init__.py     | 40 +++++++++--------
 .../client/sliding_sync/test_rooms_meta.py    | 45 +++++++++++++++++++
 3 files changed, 67 insertions(+), 19 deletions(-)
 create mode 100644 changelog.d/17674.bugfix

diff --git a/changelog.d/17674.bugfix b/changelog.d/17674.bugfix
new file mode 100644
index 00000000000..bbef5005a13
--- /dev/null
+++ b/changelog.d/17674.bugfix
@@ -0,0 +1 @@
+Fix bug where we returned the wrong `bump_stamp` for invites in sliding sync response, causing incorrect ordering of invites in the room list.
diff --git a/synapse/handlers/sliding_sync/__init__.py b/synapse/handlers/sliding_sync/__init__.py
index 0f06ffaa11e..7f084cb9163 100644
--- a/synapse/handlers/sliding_sync/__init__.py
+++ b/synapse/handlers/sliding_sync/__init__.py
@@ -975,27 +975,29 @@ async def get_room_sync_data(
                     )
                 )
 
-        # Figure out the last bump event in the room
-        last_bump_event_result = (
-            await self.store.get_last_event_pos_in_room_before_stream_ordering(
-                room_id,
-                to_token.room_key,
-                event_types=SLIDING_SYNC_DEFAULT_BUMP_EVENT_TYPES,
-            )
-        )
-
         # By default, just choose the membership event position
         bump_stamp = room_membership_for_user_at_to_token.event_pos.stream
-        # But if we found a bump event, use that instead
-        if last_bump_event_result is not None:
-            _, new_bump_event_pos = last_bump_event_result
-
-            # If we've just joined a remote room, then the last bump event may
-            # have been backfilled (and so have a negative stream ordering).
-            # These negative stream orderings can't sensibly be compared, so
-            # instead we use the membership event position.
-            if new_bump_event_pos.stream > 0:
-                bump_stamp = new_bump_event_pos.stream
+
+        # Figure out the last bump event in the room if we're in the room.
+        if room_membership_for_user_at_to_token.membership == Membership.JOIN:
+            last_bump_event_result = (
+                await self.store.get_last_event_pos_in_room_before_stream_ordering(
+                    room_id,
+                    to_token.room_key,
+                    event_types=SLIDING_SYNC_DEFAULT_BUMP_EVENT_TYPES,
+                )
+            )
+
+            # But if we found a bump event, use that instead
+            if last_bump_event_result is not None:
+                _, new_bump_event_pos = last_bump_event_result
+
+                # If we've just joined a remote room, then the last bump event may
+                # have been backfilled (and so have a negative stream ordering).
+                # These negative stream orderings can't sensibly be compared, so
+                # instead we use the membership event position.
+                if new_bump_event_pos.stream > 0:
+                    bump_stamp = new_bump_event_pos.stream
 
         unstable_expanded_timeline = False
         prev_room_sync_config = previous_connection_state.room_configs.get(room_id)
diff --git a/tests/rest/client/sliding_sync/test_rooms_meta.py b/tests/rest/client/sliding_sync/test_rooms_meta.py
index 8ce5e8995e1..aac2e60586d 100644
--- a/tests/rest/client/sliding_sync/test_rooms_meta.py
+++ b/tests/rest/client/sliding_sync/test_rooms_meta.py
@@ -755,3 +755,48 @@ def test_rooms_bump_stamp_backfill(self) -> None:
         response_body, _ = self.do_sync(sync_body, tok=user1_tok)
 
         self.assertGreater(response_body["rooms"][room_id]["bump_stamp"], 0)
+
+    def test_rooms_bump_stamp_invites(self) -> None:
+        """
+        Test that `bump_stamp` is present and points to the membership event,
+        and not later events, for non-joined rooms
+        """
+
+        user1_id = self.register_user("user1", "pass")
+        user1_tok = self.login(user1_id, "pass")
+
+        user2_id = self.register_user("user2", "pass")
+        user2_tok = self.login(user2_id, "pass")
+
+        room_id = self.helper.create_room_as(
+            user2_id,
+            tok=user2_tok,
+        )
+
+        # Invite user1 to the room
+        invite_response = self.helper.invite(room_id, user2_id, user1_id, tok=user2_tok)
+
+        # More messages happen after the invite
+        self.helper.send(room_id, "message in room1", tok=user2_tok)
+
+        # We expect the bump_stamp to match the invite.
+        invite_pos = self.get_success(
+            self.store.get_position_for_event(invite_response["event_id"])
+        )
+
+        # Doing an SS request should return a `bump_stamp` of the invite event,
+        # rather than the message that was sent after.
+        sync_body = {
+            "lists": {
+                "foo-list": {
+                    "ranges": [[0, 1]],
+                    "required_state": [],
+                    "timeline_limit": 5,
+                }
+            }
+        }
+        response_body, _ = self.do_sync(sync_body, tok=user1_tok)
+
+        self.assertEqual(
+            response_body["rooms"][room_id]["bump_stamp"], invite_pos.stream
+        )