-
-
Notifications
You must be signed in to change notification settings - Fork 259
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Handling Empty Narrows #1543
base: main
Are you sure you want to change the base?
Handling Empty Narrows #1543
Changes from 1 commit
4992dbe
936cf2f
6cac57d
f8503f8
dcb213a
0fb5bf0
df2feb4
299a1c1
3444552
2a66297
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -48,6 +48,7 @@ def test_init(self, mocker, message_type, set_fields): | |
display_recipient=[ | ||
{"id": 7, "email": "[email protected]", "full_name": "Boo is awesome"} | ||
], | ||
id=457823, | ||
stream_id=5, | ||
subject="hi", | ||
sender_email="[email protected]", | ||
|
@@ -71,6 +72,7 @@ def test_private_message_to_self(self, mocker): | |
display_recipient=[ | ||
{"full_name": "Foo Foo", "email": "[email protected]", "id": None} | ||
], | ||
id=457823, | ||
sender_id=9, | ||
content="<p> self message. </p>", | ||
sender_full_name="Foo Foo", | ||
|
@@ -82,6 +84,7 @@ def test_private_message_to_self(self, mocker): | |
MODULE + ".MessageBox._is_private_message_to_self", return_value=True | ||
) | ||
mocker.patch.object(MessageBox, "main_view") | ||
|
||
msg_box = MessageBox(message, self.model, None) | ||
|
||
assert msg_box.recipient_emails == ["[email protected]"] | ||
|
@@ -753,6 +756,7 @@ def test_main_view(self, mocker, message, last_message): | |
"color": "#bd6", | ||
}, | ||
} | ||
self.model.controller.is_in_empty_narrow = False | ||
MessageBox(message, self.model, last_message) | ||
|
||
@pytest.mark.parametrize( | ||
|
@@ -1149,8 +1153,11 @@ def test_main_view_compact_output( | |
): | ||
message_fixture.update({"id": 4}) | ||
varied_message = dict(message_fixture, **to_vary_in_each_message) | ||
self.model.controller.is_in_empty_narrow = False | ||
msg_box = MessageBox(varied_message, self.model, varied_message) | ||
|
||
view_components = msg_box.main_view() | ||
|
||
assert len(view_components) == 1 | ||
assert isinstance(view_components[0], Padding) | ||
|
||
|
@@ -1160,7 +1167,9 @@ def test_main_view_generates_EDITED_label( | |
messages = messages_successful_response["messages"] | ||
for message in messages: | ||
self.model.index["edited_messages"].add(message["id"]) | ||
self.model.controller.is_in_empty_narrow = False | ||
msg_box = MessageBox(message, self.model, message) | ||
|
||
view_components = msg_box.main_view() | ||
|
||
label = view_components[0].original_widget.contents[0] | ||
|
@@ -1186,6 +1195,7 @@ def test_update_message_author_status( | |
): | ||
message = message_fixture | ||
last_msg = dict(message, **to_vary_in_last_message) | ||
self.model.controller.is_in_empty_narrow = False | ||
|
||
msg_box = MessageBox(message, self.model, last_msg) | ||
|
||
|
@@ -1389,6 +1399,7 @@ def test_keypress_EDIT_MESSAGE( | |
to_vary_in_each_message["subject"] = "" | ||
varied_message = dict(message_fixture, **to_vary_in_each_message) | ||
message_type = varied_message["type"] | ||
self.model.controller.is_in_empty_narrow = False | ||
msg_box = MessageBox(varied_message, self.model, message_fixture) | ||
size = widget_size(msg_box) | ||
msg_box.model.user_id = 1 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,6 +29,46 @@ def create_msg_box_list( | |
focus_msg = None | ||
last_msg = last_message | ||
muted_msgs = 0 # No of messages that are muted. | ||
|
||
# Add a dummy message if no new or old messages are found | ||
if not message_list and not last_msg: | ||
message_type = "stream" if model.stream_id is not None else "private" | ||
dummy_message = { | ||
Comment on lines
+33
to
+36
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. While I know that the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To clarify
|
||
"id": None, | ||
"content": ( | ||
"No search hits" if model.is_search_narrow() else "No messages here" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I couldn't trigger 'No search hits' from here, and trying to do so without the last commit triggered a crash instead. The 'No messages here' works fine as a placeholder for now, though I'm not sure of the text. |
||
), | ||
"is_me_message": False, | ||
"flags": ["read"], | ||
"reactions": [], | ||
"type": message_type, | ||
"stream_id": model.stream_id if message_type == "stream" else None, | ||
"stream_name": model.stream_dict[model.stream_id]["name"] | ||
if message_type == "stream" | ||
else None, | ||
"subject": next( | ||
(subnarrow[1] for subnarrow in model.narrow if subnarrow[0] == "topic"), | ||
"No topics in channel", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is possible, but difficult to achieve with the current model with Zulip - I think stream events would have to be absent. Did you manually test this? |
||
) | ||
if message_type == "stream" | ||
else None, | ||
"display_recipient": ( | ||
model.stream_dict[model.stream_id]["name"] | ||
if message_type == "stream" | ||
else [ | ||
{ | ||
"email": model.user_id_email_dict[recipient_id], | ||
"full_name": model.user_dict[ | ||
model.user_id_email_dict[recipient_id] | ||
]["full_name"], | ||
"id": recipient_id, | ||
} | ||
for recipient_id in model.recipients | ||
] | ||
), | ||
} | ||
message_list.append(dummy_message) | ||
|
||
for msg in message_list: | ||
if is_unsubscribed_message(msg, model): | ||
continue | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do use this message data later, but apparently not these attributes - even though we test for them! In any case, based on usage, it seems like they should be set independently of the message type if we are to use them? So that may be a useful part of the earlier refactor.