subs: Fix subscriber_..._history_access to not exclude subbed guests.

Guests are supposed to have stream history access to public streams
they're subscribed to.

zerver/lib/stream_subscription.py

@@ -260,24 +260,19 @@ def subscriber_ids_with_stream_history_access(stream: Stream) -> Set[int]:
 
     1. if !history_public_to_subscribers:
           History is not available to anyone
-    2. if history_public_to_subscribers and is_web_public:
+    2. if history_public_to_subscribers:
           All subscribers can access the history including guests
-    3. if history_public_to_subscribers and !is_web_public:
-          All subscribers can access the history excluding guests
+
+    The results of this function need to be kept consistent with
+    what can_access_stream_history would dictate.
 
     """
 
     if not stream.is_history_public_to_subscribers():
         return set()
 
-    subscriptions = get_active_subscriptions_for_stream_id(
-        stream.id, include_deactivated_users=False
-    )
-    if stream.is_web_public:
-        return set(subscriptions.values_list("user_profile__id", flat=True))
-
     return set(
-        subscriptions.exclude(user_profile__role=UserProfile.ROLE_GUEST).values_list(
-            "user_profile__id", flat=True
-        )
+        get_active_subscriptions_for_stream_id(
+            stream.id, include_deactivated_users=False
+        ).values_list("user_profile__id", flat=True)
     )

zerver/tests/test_reactions.py

@@ -541,7 +541,7 @@ class ReactionEventTest(ZulipTestCase):
         # Make stream history public to subscribers
         do_change_stream_invite_only(stream, False, history_public_to_subscribers=True)
         # Since stream history is public to subscribers, reacting to
-        # message_before_id should notify all non-guest subscribers:
+        # message_before_id should notify all subscribers:
         # Iago and Hamlet.
         events = []
         with tornado_redirected_to_list(events):
@@ -553,7 +553,7 @@ class ReactionEventTest(ZulipTestCase):
         event = events[0]["event"]
         self.assertEqual(event["type"], "reaction")
         event_user_ids = set(events[0]["users"])
-        self.assertEqual(event_user_ids, {iago.id, hamlet.id})
+        self.assertEqual(event_user_ids, {iago.id, hamlet.id, polonius.id})
         remove = self.api_delete(
             iago, f"/api/v1/messages/{message_before_id}/reactions", reaction_info
         )

zerver/tests/test_subs.py

@@ -546,7 +546,9 @@ class StreamAdminTest(ZulipTestCase):
         )
         self.subscribe(hamlet, stream2.name)
         self.subscribe(polonius, stream2.name)
-        self.assertEqual({hamlet.id}, subscriber_ids_with_stream_history_access(stream2))
+        self.assertEqual(
+            {hamlet.id, polonius.id}, subscriber_ids_with_stream_history_access(stream2)
+        )
 
         stream3 = self.make_stream(
             "history_public_web_public_stream",
@@ -559,6 +561,15 @@ class StreamAdminTest(ZulipTestCase):
             {hamlet.id, polonius.id}, subscriber_ids_with_stream_history_access(stream3)
         )
 
+        stream4 = self.make_stream(
+            "regular_public_stream",
+        )
+        self.subscribe(hamlet, stream4.name)
+        self.subscribe(polonius, stream4.name)
+        self.assertEqual(
+            {hamlet.id, polonius.id}, subscriber_ids_with_stream_history_access(stream4)
+        )
+
     def test_deactivate_stream_backend(self) -> None:
         user_profile = self.example_user("hamlet")
         self.login_user(user_profile)