topic history: Fix fetching topic history of public streams.

Apparently, we did essentially all the work to support showing full
topic history to newly subscribed users from a data flow perspective,
but didn't actually enable this feature by having the topic history
endpoint grant access to historical topics.  This fixes that gap.

I'm not altogether happy with how the code and tests read for this
feature; the code itself has more duplication than I'd like, and the
tests do too, but it works.
This commit is contained in:
Tim Abbott
2018-03-11 20:59:20 -07:00
parent 9e52a9f879
commit ef92fcbe2b
3 changed files with 65 additions and 5 deletions

View File

@@ -196,7 +196,25 @@ def realm_user_count(realm: Realm) -> int:
return UserProfile.objects.filter(realm=realm, is_active=True, is_bot=False).count() return UserProfile.objects.filter(realm=realm, is_active=True, is_bot=False).count()
def get_topic_history_for_stream(user_profile: UserProfile, def get_topic_history_for_stream(user_profile: UserProfile,
recipient: Recipient) -> List[Dict[str, Any]]: recipient: Recipient,
public_history: bool) -> List[Dict[str, Any]]:
cursor = connection.cursor()
if public_history:
query = '''
SELECT
"zerver_message"."subject" as topic,
max("zerver_message".id) as max_message_id
FROM "zerver_message"
WHERE (
"zerver_message"."recipient_id" = %s
)
GROUP BY (
"zerver_message"."subject"
)
ORDER BY max("zerver_message".id) DESC
'''
cursor.execute(query, [recipient.id])
else:
query = ''' query = '''
SELECT SELECT
"zerver_message"."subject" as topic, "zerver_message"."subject" as topic,
@@ -214,7 +232,6 @@ def get_topic_history_for_stream(user_profile: UserProfile,
) )
ORDER BY max("zerver_message".id) DESC ORDER BY max("zerver_message".id) DESC
''' '''
cursor = connection.cursor()
cursor.execute(query, [user_profile.id, recipient.id]) cursor.execute(query, [user_profile.id, recipient.id])
rows = cursor.fetchall() rows = cursor.fetchall()
cursor.close() cursor.close()

View File

@@ -24,6 +24,7 @@ from zerver.lib.actions import (
do_create_user, do_create_user,
get_client, get_client,
do_add_alert_words, do_add_alert_words,
do_change_stream_invite_only,
) )
from zerver.lib.message import ( from zerver.lib.message import (
@@ -87,6 +88,7 @@ class TopicHistoryTest(ZulipTestCase):
recipient = get_stream_recipient(stream.id) recipient = get_stream_recipient(stream.id)
def create_test_message(topic: str) -> int: def create_test_message(topic: str) -> int:
# TODO: Clean this up to send messages the normal way.
hamlet = self.example_user('hamlet') hamlet = self.example_user('hamlet')
message = Message.objects.create( message = Message.objects.create(
@@ -144,6 +146,46 @@ class TopicHistoryTest(ZulipTestCase):
topic2_msg_id, topic2_msg_id,
]) ])
# Now try as cordelia, who we imagine as a totally new user in
# that she doesn't have UserMessage rows. We should see the
# same results for a public stream.
self.login(self.example_email("cordelia"))
result = self.client_get(endpoint, dict())
self.assert_json_success(result)
history = result.json()['topics']
# We only look at the most recent three topics, because
# the prior fixture data may be unreliable.
history = history[:3]
self.assertEqual([topic['name'] for topic in history], [
'topic0',
'topic1',
'topic2',
])
self.assertIn('topic0', [topic['name'] for topic in history])
self.assertEqual([topic['max_id'] for topic in history], [
topic0_msg_id,
topic1_msg_id,
topic2_msg_id,
])
# Now make stream private, but subscribe cordelia
do_change_stream_invite_only(stream, True)
self.subscribe(self.example_user("cordelia"), stream.name)
result = self.client_get(endpoint, dict())
self.assert_json_success(result)
history = result.json()['topics']
history = history[:3]
# Cordelia doesn't have these recent history items when we
# wasn't subscribed in her results.
self.assertNotIn('topic0', [topic['name'] for topic in history])
self.assertNotIn('topic1', [topic['name'] for topic in history])
self.assertNotIn('topic2', [topic['name'] for topic in history])
def test_bad_stream_id(self) -> None: def test_bad_stream_id(self) -> None:
email = self.example_email("iago") email = self.example_email("iago")
self.login(email) self.login(email)

View File

@@ -440,6 +440,7 @@ def get_topics_backend(request: HttpRequest, user_profile: UserProfile,
result = get_topic_history_for_stream( result = get_topic_history_for_stream(
user_profile=user_profile, user_profile=user_profile,
recipient=recipient, recipient=recipient,
public_history=not stream.invite_only,
) )
return json_success(dict(topics=result)) return json_success(dict(topics=result))