actions.py: Refactor generate_topic_history_from_db_rows for clarity.

This refactors the generate_topic_history_from_db_rows function to not
depend upon the assumption of rows passed as parameter to be sorted in
reverse order of max_message_id field.

Additionally, we add sorting and some tests that verify correct
handling of these cases.
This commit is contained in:
Tim Abbott
2018-07-14 10:08:35 +05:30
parent 63ec8b08b8
commit bf1ad714da
2 changed files with 42 additions and 17 deletions

View File

@@ -214,26 +214,25 @@ def activity_change_requires_seat_update(user: UserProfile) -> bool:
return user.realm.has_seat_based_plan and not user.is_bot
def generate_topic_history_from_db_rows(rows: List[Tuple[str, int]]) -> List[Dict[str, Any]]:
canonical_topic_names = set() # type: Set[str]
history = []
# This algorithm relies on the fact that `rows` is reverse-sorted
# by `max_message_id`, so that we will correctly use the highest
# max_message_id where multiple topics have the same canonical
# name.
#
# Arguably, we should just reimplment this using a dict rather
# than a set to remove that assumption for better readability.
canonical_topic_names = {} # type: Dict[str, Tuple[int, str]]
for (topic_name, max_message_id) in rows:
canonical_name = topic_name.lower()
if canonical_name in canonical_topic_names:
if canonical_name not in canonical_topic_names:
canonical_topic_names[canonical_name] = (max_message_id, topic_name)
continue
canonical_topic_names.add(canonical_name)
if max_message_id > canonical_topic_names[canonical_name][0]:
canonical_topic_names[canonical_name] = (max_message_id, topic_name)
history = []
for canonical_topic, (max_message_id, topic_name) in canonical_topic_names.items():
history.append(dict(
name=topic_name,
max_id=max_message_id))
return history
max_id=max_message_id)
)
return sorted(history, key=lambda x: -x['max_id'])
def get_topic_history_for_stream(user_profile: UserProfile,
recipient: Recipient,