Skip to content

Commit 57df32e

Browse files
mounilKshahmounilKshah
committed
refactor: Migrate subject -> topic.
This commit contains migration for message["subject"] to message ["topic"] as per the modern API with the oldest support being v2.1. It also contains the updated tests for the migration.
1 parent 898bb2e commit 57df32e

File tree

12 files changed

+74
-65
lines changed

12 files changed

+74
-65
lines changed

‎tests/conftest.py

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -416,7 +416,7 @@ def msg_template_factory(
416416
msg_type: str,
417417
timestamp: int,
418418
*,
419-
subject: str = "",
419+
topic: str = "",
420420
stream_id: Optional[int] = None,
421421
recipients: Union[str, List[Dict[str, Any]]] = "PTEST",
422422
) -> Message:
@@ -435,7 +435,8 @@ def msg_template_factory(
435435
flags=["read"],
436436
sender_id=5140,
437437
content_type="text/x-markdown",
438-
subject=subject,
438+
topic=topic,
439+
subject=topic,
439440
reactions=[],
440441
subject_links=[],
441442
avatar_url="dummy_avatar_url",
@@ -459,15 +460,15 @@ def msg_template_factory(
459460
@pytest.fixture
460461
def stream_msg_template() -> Message:
461462
msg_template = msg_template_factory(
462-
537286, "stream", 1520918722, subject="Test", stream_id=205
463+
537286, "stream", 1520918722, topic="Test", stream_id=205
463464
)
464465
return msg_template
465466

466467

467468
@pytest.fixture
468469
def extra_stream_msg_template() -> Message:
469470
msg_template = msg_template_factory(
470-
537289, "stream", 1520918740, subject="Test", stream_id=205
471+
537289, "stream", 1520918740, topic="Test", stream_id=205
471472
)
472473
return msg_template
473474

‎tests/model/test_model.py

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -951,7 +951,7 @@ def test_update_stream_message(
951951
old_stream_name="stream",
952952
):
953953
self.client.update_message = mocker.Mock(return_value=response)
954-
model.index["messages"][req["message_id"]]["subject"] = old_topic
954+
model.index["messages"][req["message_id"]]["topic"] = old_topic
955955
model.index["messages"][req["message_id"]][
956956
"display_recipient"
957957
] = old_stream_name
@@ -1487,7 +1487,7 @@ def test__handle_message_event_with_flags(self, mocker, model, message_fixture):
14871487
"response, narrow, recipients, log",
14881488
[
14891489
case(
1490-
{"type": "stream", "stream_id": 1, "subject": "FOO", "id": 1},
1490+
{"type": "stream", "stream_id": 1, "topic": "FOO", "id": 1},
14911491
[],
14921492
frozenset(),
14931493
["msg_w"],
@@ -1505,7 +1505,7 @@ def test__handle_message_event_with_flags(self, mocker, model, message_fixture):
15051505
"type": "stream",
15061506
"id": 1,
15071507
"stream_id": 1,
1508-
"subject": "FOO",
1508+
"topic": "FOO",
15091509
"display_recipient": "a",
15101510
},
15111511
[["stream", "a"]],
@@ -1518,7 +1518,7 @@ def test__handle_message_event_with_flags(self, mocker, model, message_fixture):
15181518
"type": "stream",
15191519
"id": 1,
15201520
"stream_id": 1,
1521-
"subject": "b",
1521+
"topic": "b",
15221522
"display_recipient": "a",
15231523
},
15241524
[["stream", "a"], ["topic", "b"]],
@@ -1531,7 +1531,7 @@ def test__handle_message_event_with_flags(self, mocker, model, message_fixture):
15311531
"type": "stream",
15321532
"id": 1,
15331533
"stream_id": 1,
1534-
"subject": "b",
1534+
"topic": "b",
15351535
"display_recipient": "a",
15361536
},
15371537
[["stream", "c"], ["topic", "b"]],
@@ -1573,7 +1573,7 @@ def test__handle_message_event_with_flags(self, mocker, model, message_fixture):
15731573
"type": "stream",
15741574
"id": 1,
15751575
"stream_id": 1,
1576-
"subject": "c",
1576+
"topic": "c",
15771577
"display_recipient": "a",
15781578
"flags": ["mentioned"],
15791579
},
@@ -2122,7 +2122,7 @@ def _set_topics_to_old_and_new(event):
21222122
model.controller.update_screen.assert_called_once_with()
21232123

21242124
@pytest.mark.parametrize(
2125-
"subject, narrow, new_log_len",
2125+
"topic, narrow, new_log_len",
21262126
[
21272127
("foo", [["stream", "boo"], ["topic", "foo"]], 2),
21282128
("foo", [["stream", "boo"], ["topic", "not foo"]], 1),
@@ -2135,11 +2135,11 @@ def _set_topics_to_old_and_new(event):
21352135
],
21362136
)
21372137
def test__update_rendered_view(
2138-
self, mocker, model, subject, narrow, new_log_len, msg_id=1
2138+
self, mocker, model, topic, narrow, new_log_len, msg_id=1
21392139
):
21402140
msg_w = mocker.Mock()
21412141
other_msg_w = mocker.Mock()
2142-
msg_w.original_widget.message = {"id": msg_id, "subject": subject}
2142+
msg_w.original_widget.message = {"id": msg_id, "topic": topic}
21432143
model.narrow = narrow
21442144
other_msg_w.original_widget.message = {"id": 2}
21452145
self.controller.view.message_view = mocker.Mock(log=[msg_w, other_msg_w])
@@ -2159,7 +2159,7 @@ def test__update_rendered_view(
21592159
assert model.controller.update_screen.called
21602160

21612161
@pytest.mark.parametrize(
2162-
"subject, narrow, narrow_changed",
2162+
"topic, narrow, narrow_changed",
21632163
[
21642164
("foo", [["stream", "boo"], ["topic", "foo"]], False),
21652165
("foo", [["stream", "boo"], ["topic", "not foo"]], True),
@@ -2172,11 +2172,11 @@ def test__update_rendered_view(
21722172
],
21732173
)
21742174
def test__update_rendered_view_change_narrow(
2175-
self, mocker, model, subject, narrow, narrow_changed, msg_id=1
2175+
self, mocker, model, topic, narrow, narrow_changed, msg_id=1
21762176
):
21772177
msg_w = mocker.Mock()
21782178
other_msg_w = mocker.Mock()
2179-
msg_w.original_widget.message = {"id": msg_id, "subject": subject}
2179+
msg_w.original_widget.message = {"id": msg_id, "topic": topic}
21802180
model.narrow = narrow
21812181
self.controller.view.message_view = mocker.Mock(log=[msg_w])
21822182
# New msg widget generated after updating index.

‎tests/server_url/test_server_url.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ def test_encode_stream(
3434
"type": "stream",
3535
"stream_id": 23,
3636
"display_recipient": "zulip terminal",
37-
"subject": "#test-here #T1 #T2 #T3",
37+
"topic": "#test-here #T1 #T2 #T3",
3838
},
3939
(
4040
"https://chat.zulip.org/#narrow/stream/23-zulip-terminal"
@@ -48,7 +48,7 @@ def test_encode_stream(
4848
"type": "stream",
4949
"stream_id": 425,
5050
"display_recipient": "/ in a stream name ?",
51-
"subject": "abc + de = abcde",
51+
"topic": "abc + de = abcde",
5252
},
5353
(
5454
"https://foo-bar.co.in/#narrow/stream/425-.2F-in-a-stream-name-.3F"

‎tests/ui/test_ui_tools.py

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1290,7 +1290,7 @@ def test_init(self, mocker, message_type, set_fields):
12901290
{"id": 7, "email": "boo@zulip.com", "full_name": "Boo is awesome"}
12911291
],
12921292
stream_id=5,
1293-
subject="hi",
1293+
topic="hi",
12941294
sender_email="foo@zulip.com",
12951295
sender_id=4209,
12961296
type=message_type,
@@ -1933,7 +1933,7 @@ def test_soup2markup(self, content, expected_markup, mocker):
19331933
"sender_full_name": "aaron",
19341934
"submessages": [],
19351935
"stream_id": 5,
1936-
"subject": "Verona2",
1936+
"topic": "Verona2",
19371937
"id": 37,
19381938
"subject_links": [],
19391939
"content": (
@@ -1971,7 +1971,7 @@ def test_soup2markup(self, content, expected_markup, mocker):
19711971
],
19721972
"sender_full_name": "Iago",
19731973
"submessages": [],
1974-
"subject": "",
1974+
"topic": "",
19751975
"id": 107,
19761976
"subject_links": [],
19771977
"content": "<p>what are you planning to do this week</p>",
@@ -2005,7 +2005,7 @@ def test_main_view(self, mocker, message, last_message):
20052005
"type": "stream",
20062006
"display_recipient": "Verona",
20072007
"stream_id": 5,
2008-
"subject": "Test topic",
2008+
"topic": "Test topic",
20092009
"is_me_message": True, # will be overridden by test function.
20102010
"flags": [],
20112011
"content": "", # will be overridden by test function.
@@ -2043,7 +2043,7 @@ def test_main_view_renders_slash_me(self, mocker, message, content, is_me_messag
20432043
"type": "stream",
20442044
"display_recipient": "Verona",
20452045
"stream_id": 5,
2046-
"subject": "Test topic",
2046+
"topic": "Test topic",
20472047
"flags": [],
20482048
"is_me_message": False,
20492049
"content": "<p>what are you planning to do this week</p>",
@@ -2057,7 +2057,7 @@ def test_main_view_renders_slash_me(self, mocker, message, content, is_me_messag
20572057
"to_vary_in_last_message",
20582058
[
20592059
{"display_recipient": "Verona offtopic"},
2060-
{"subject": "Test topic (previous)"},
2060+
{"topic": "Test topic (previous)"},
20612061
{"type": "private"},
20622062
],
20632063
ids=[
@@ -2218,7 +2218,7 @@ def test_msg_generates_search_and_header_bar(
22182218
"type": "stream",
22192219
"display_recipient": "Verona",
22202220
"stream_id": 5,
2221-
"subject": "Test topic",
2221+
"topic": "Test topic",
22222222
"flags": [],
22232223
"is_me_message": False,
22242224
"content": "<p>what are you planning to do this week</p>",
@@ -2412,7 +2412,7 @@ def test_keypress_STREAM_MESSAGE(
24122412
],
24132413
[
24142414
case(
2415-
{"sender_id": 2, "timestamp": 45, "subject": "test"},
2415+
{"sender_id": 2, "timestamp": 45, "topic": "test"},
24162416
True,
24172417
60,
24182418
{"stream": False, "private": False},
@@ -2424,7 +2424,7 @@ def test_keypress_STREAM_MESSAGE(
24242424
id="msg_sent_by_other_user_with_topic",
24252425
),
24262426
case(
2427-
{"sender_id": 1, "timestamp": 1, "subject": "test"},
2427+
{"sender_id": 1, "timestamp": 1, "topic": "test"},
24282428
True,
24292429
60,
24302430
{"stream": False, "private": False},
@@ -2437,7 +2437,7 @@ def test_keypress_STREAM_MESSAGE(
24372437
id="topic_edit_only_after_time_limit",
24382438
),
24392439
case(
2440-
{"sender_id": 1, "timestamp": 45, "subject": "test"},
2440+
{"sender_id": 1, "timestamp": 45, "topic": "test"},
24412441
False,
24422442
60,
24432443
{"stream": False, "private": False},
@@ -2449,7 +2449,7 @@ def test_keypress_STREAM_MESSAGE(
24492449
id="realm_editing_not_allowed",
24502450
),
24512451
case(
2452-
{"sender_id": 1, "timestamp": 45, "subject": "test"},
2452+
{"sender_id": 1, "timestamp": 45, "topic": "test"},
24532453
True,
24542454
60,
24552455
{"stream": True, "private": True},
@@ -2458,7 +2458,7 @@ def test_keypress_STREAM_MESSAGE(
24582458
id="realm_editing_allowed_and_within_time_limit",
24592459
),
24602460
case(
2461-
{"sender_id": 1, "timestamp": 1, "subject": "test"},
2461+
{"sender_id": 1, "timestamp": 1, "topic": "test"},
24622462
True,
24632463
0,
24642464
{"stream": True, "private": True},
@@ -2467,7 +2467,7 @@ def test_keypress_STREAM_MESSAGE(
24672467
id="no_msg_body_edit_limit",
24682468
),
24692469
case(
2470-
{"sender_id": 1, "timestamp": 1, "subject": "(no topic)"},
2470+
{"sender_id": 1, "timestamp": 1, "topic": "(no topic)"},
24712471
True,
24722472
60,
24732473
{"stream": False, "private": False},
@@ -2480,7 +2480,7 @@ def test_keypress_STREAM_MESSAGE(
24802480
id="msg_sent_by_me_with_no_topic",
24812481
),
24822482
case(
2483-
{"sender_id": 2, "timestamp": 1, "subject": "(no topic)"},
2483+
{"sender_id": 2, "timestamp": 1, "topic": "(no topic)"},
24842484
True,
24852485
60,
24862486
{"stream": False, "private": False},
@@ -2493,7 +2493,7 @@ def test_keypress_STREAM_MESSAGE(
24932493
id="msg_sent_by_other_with_no_topic",
24942494
),
24952495
case(
2496-
{"sender_id": 1, "timestamp": 1, "subject": "(no topic)"},
2496+
{"sender_id": 1, "timestamp": 1, "topic": "(no topic)"},
24972497
False,
24982498
60,
24992499
{"stream": False, "private": False},
@@ -2505,7 +2505,7 @@ def test_keypress_STREAM_MESSAGE(
25052505
id="realm_editing_not_allowed_for_no_topic",
25062506
),
25072507
case(
2508-
{"sender_id": 1, "timestamp": 45, "subject": "(no topic)"},
2508+
{"sender_id": 1, "timestamp": 45, "topic": "(no topic)"},
25092509
True,
25102510
0,
25112511
{"stream": True, "private": True},
@@ -2529,7 +2529,7 @@ def test_keypress_EDIT_MESSAGE(
25292529
key,
25302530
):
25312531
if message_fixture["type"] == "private":
2532-
to_vary_in_each_message["subject"] = ""
2532+
to_vary_in_each_message["topic"] = ""
25332533
varied_message = dict(message_fixture, **to_vary_in_each_message)
25342534
message_type = varied_message["type"]
25352535
msg_box = MessageBox(varied_message, self.model, message_fixture)
@@ -2551,7 +2551,7 @@ def test_keypress_EDIT_MESSAGE(
25512551

25522552
if expect_editing_to_succeed[message_type]:
25532553
assert write_box.msg_edit_state.message_id == varied_message["id"]
2554-
assert write_box.msg_edit_state.old_topic == varied_message["subject"]
2554+
assert write_box.msg_edit_state.old_topic == varied_message["topic"]
25552555
write_box.msg_write_box.set_edit_text.assert_called_once_with(
25562556
"Edit this message"
25572557
)

‎tests/ui/test_utils.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@
5959
{
6060
"type": "stream",
6161
"stream_id": 3,
62-
"subject": "foo koo",
62+
"topic": "foo koo",
6363
# ...
6464
},
6565
[],

‎tests/ui_tools/test_boxes.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1339,6 +1339,7 @@ def test_keypress_SEND_MESSAGE_no_topic(
13391339
)
13401340
else:
13411341
write_box.model.send_stream_message.assert_called_once_with(
1342+
# FIXME: This parameter is causing an error while testing
13421343
stream=write_box.stream_write_box.edit_text,
13431344
topic=topic_sent_to_server,
13441345
content=write_box.msg_write_box.edit_text,

‎zulipterminal/api_types.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ class Message(TypedDict, total=False):
3535
timestamp: int
3636
client: str
3737
subject: str # Only for stream msgs.
38+
# Changing subject -> topic as per the migration in Zulip 2.0
39+
topic: str # Only for stream msgs.
3840
# NOTE: new in Zulip 3.0 / ZFL 1, replacing `subject_links`
3941
# NOTE: API response format of `topic_links` changed in Zulip 4.0 / ZFL 46
4042
topic_links: List[Any]

‎zulipterminal/helper.py

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ def update_unreads(unreads: Dict[KeyT, int], key: KeyT) -> None:
165165
if message["type"] == "stream":
166166
stream_id = message["stream_id"]
167167
update_unreads(
168-
unread_counts["unread_topics"], (stream_id, message["subject"])
168+
unread_counts["unread_topics"], (stream_id, message["topic"])
169169
)
170170
update_unreads(unread_counts["streams"], stream_id)
171171
# self-pm has only one display_recipient
@@ -217,7 +217,7 @@ def _set_count_in_view(
217217

218218
if msg_type == "stream":
219219
stream_id = message["stream_id"]
220-
msg_topic = message["subject"]
220+
msg_topic = message["topic"]
221221
if controller.model.is_muted_stream(stream_id):
222222
add_to_counts = False # if muted, don't add to eg. all_msg
223223
else:
@@ -350,7 +350,7 @@ def index_messages(messages: List[Message], model: Any, index: Index) -> Index:
350350
'sender_full_name': 'Iago',
351351
'flags': [],
352352
'sender_email': 'iago@zulip.com',
353-
'subject': '',
353+
'topic': '',
354354
'subject_links': [],
355355
'sender_id': 73,
356356
'type': 'private',
@@ -386,7 +386,7 @@ def index_messages(messages: List[Message], model: Any, index: Index) -> Index:
386386
'display_recipient': 'Verona',
387387
'flags': [],
388388
'reactions': [],
389-
'subject': 'Verona2',
389+
'topic': 'Verona2',
390390
'stream_id': 32,
391391
},
392392
},
@@ -436,12 +436,12 @@ def index_messages(messages: List[Message], model: Any, index: Index) -> Index:
436436
if (
437437
msg["type"] == "stream"
438438
and len(narrow) == 2
439-
and narrow[1][1] == msg["subject"]
439+
and narrow[1][1] == msg["topic"]
440440
):
441441
topics_in_stream = index["topic_msg_ids"][msg["stream_id"]]
442-
if not topics_in_stream.get(msg["subject"]):
443-
topics_in_stream[msg["subject"]] = set()
444-
topics_in_stream[msg["subject"]].add(msg["id"])
442+
if not topics_in_stream.get(msg["topic"]):
443+
topics_in_stream[msg["topic"]] = set()
444+
topics_in_stream[msg["topic"]].add(msg["id"])
445445

446446
return index
447447

0 commit comments

Comments
 (0)