Skip to content
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.

Sign up for GitHub

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

Group message data #1537

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
+127 −266
Open

Group message data #1537

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
15a7c86
views: Use `self` to re-use instance variables in MsgInfoView class.
Niloth-p Jul 22, 2024
c734d2a
core/views: Stack inner popups, add support for exiting all popups.
Niloth-p Jul 22, 2024
e316327
views/core: Delete all message info popup parameters from inner popups.
Niloth-p Jul 22, 2024
9e5036e
views: Delete the calling arguments from MsgInfoView's show_* calls.
Niloth-p Jul 22, 2024
9051d4f
refactor: views/helper/core/messages: Group message info in a TypedDict.
Niloth-p Jul 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
173 changes: 60 additions & 113 deletions tests/ui_tools/test_popups.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,11 @@
from zulipterminal.api_types import Message
from zulipterminal.config.keys import is_command_key, keys_for_command
from zulipterminal.config.ui_mappings import EDIT_MODE_CAPTIONS
from zulipterminal.helper import CustomProfileData, TidiedUserInfo
from zulipterminal.helper import (
CustomProfileData,
MessageInfoPopupContent,
TidiedUserInfo,
)
from zulipterminal.ui_tools.messages import MessageBox
from zulipterminal.ui_tools.views import (
AboutView,
Expand Down Expand Up @@ -503,31 +507,25 @@ def mock_external_classes(self, mocker: MockerFixture, msg_box: MessageBox) -> N
self.full_rendered_message = FullRenderedMsgView(
controller=self.controller,
message=self.message,
topic_links=OrderedDict(),
message_links=OrderedDict(),
time_mentions=list(),
Comment on lines -506 to -508
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Removing dead/unnecessary code is a refactor - it shouldn't change the behavior :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, thank you!

title="Full Rendered Message",
)

def test_init(self, msg_box: MessageBox) -> None:
assert self.full_rendered_message.title == "Full Rendered Message"
assert self.full_rendered_message.controller == self.controller
assert self.full_rendered_message.message == self.message
assert self.full_rendered_message.topic_links == OrderedDict()
assert self.full_rendered_message.message_links == OrderedDict()
assert self.full_rendered_message.time_mentions == list()
assert self.full_rendered_message.header.widget_list == msg_box.header
assert self.full_rendered_message.footer.widget_list == msg_box.footer

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re the commit text for the 'stack' change, my only nit would be re the end of the 'why stack popups' that, we don't need to pass the entire data around... unless we need it :) None of these now do, but as per discussion there are upcoming PRs that will do so.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, is that so? I can't seem to recall that discussion.
The change to the commit text sounds better anyways.
Glad to know that the rest is fine.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've not rechecked, but was likely referring to Sashank's comment here, where he is expecting this will be necessary for spoilers.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, that's covered by this PR. Won't need to pass around any data for that.

@pytest.mark.parametrize("key", keys_for_command("MSG_INFO"))
def test_keypress_exit_popup(
def test_keypress_exit_all_popups(
self, key: str, widget_size: Callable[[Widget], urwid_Size]
) -> None:
size = widget_size(self.full_rendered_message)

self.full_rendered_message.keypress(size, key)

assert self.controller.exit_popup.called
assert self.controller.exit_all_popups.called

def test_keypress_exit_popup_invalid_key(
self, widget_size: Callable[[Widget], urwid_Size]
Expand All @@ -546,19 +544,14 @@ def test_keypress_exit_popup_invalid_key(
*keys_for_command("EXIT_POPUP"),
},
)
def test_keypress_show_msg_info(
def test_keypress_exit_to_msg_info(
self, key: str, widget_size: Callable[[Widget], urwid_Size]
) -> None:
size = widget_size(self.full_rendered_message)

self.full_rendered_message.keypress(size, key)

self.controller.show_msg_info.assert_called_once_with(
msg=self.message,
topic_links=OrderedDict(),
message_links=OrderedDict(),
time_mentions=list(),
)
assert self.controller.exit_popup.called


class TestFullRawMsgView:
Expand All @@ -579,31 +572,25 @@ def mock_external_classes(self, mocker: MockerFixture, msg_box: MessageBox) -> N
self.full_raw_message = FullRawMsgView(
controller=self.controller,
message=self.message,
topic_links=OrderedDict(),
message_links=OrderedDict(),
time_mentions=list(),
title="Full Raw Message",
)

def test_init(self, msg_box: MessageBox) -> None:
assert self.full_raw_message.title == "Full Raw Message"
assert self.full_raw_message.controller == self.controller
assert self.full_raw_message.message == self.message
assert self.full_raw_message.topic_links == OrderedDict()
assert self.full_raw_message.message_links == OrderedDict()
assert self.full_raw_message.time_mentions == list()
assert self.full_raw_message.header.widget_list == msg_box.header
assert self.full_raw_message.footer.widget_list == msg_box.footer

@pytest.mark.parametrize("key", keys_for_command("MSG_INFO"))
def test_keypress_exit_popup(
def test_keypress_exit_all_popups(
self, key: str, widget_size: Callable[[Widget], urwid_Size]
) -> None:
size = widget_size(self.full_raw_message)

self.full_raw_message.keypress(size, key)

assert self.controller.exit_popup.called
assert self.controller.exit_all_popups.called

def test_keypress_exit_popup_invalid_key(
self, widget_size: Callable[[Widget], urwid_Size]
Expand All @@ -622,19 +609,14 @@ def test_keypress_exit_popup_invalid_key(
*keys_for_command("EXIT_POPUP"),
},
)
def test_keypress_show_msg_info(
def test_keypress_exit_to_msg_info(
self, key: str, widget_size: Callable[[Widget], urwid_Size]
) -> None:
size = widget_size(self.full_raw_message)

self.full_raw_message.keypress(size, key)

self.controller.show_msg_info.assert_called_once_with(
msg=self.message,
topic_links=OrderedDict(),
message_links=OrderedDict(),
time_mentions=list(),
)
assert self.controller.exit_popup.called


class TestEditHistoryView:
Expand All @@ -654,31 +636,25 @@ def mock_external_classes(self, mocker: MockerFixture) -> None:
self.edit_history_view = EditHistoryView(
controller=self.controller,
message=self.message,
topic_links=OrderedDict(),
message_links=OrderedDict(),
time_mentions=list(),
title="Edit History",
)

def test_init(self) -> None:
assert self.edit_history_view.controller == self.controller
assert self.edit_history_view.message == self.message
assert self.edit_history_view.topic_links == OrderedDict()
assert self.edit_history_view.message_links == OrderedDict()
assert self.edit_history_view.time_mentions == list()
self.controller.model.fetch_message_history.assert_called_once_with(
message_id=self.message["id"],
)

@pytest.mark.parametrize("key", keys_for_command("MSG_INFO"))
def test_keypress_exit_popup(
def test_keypress_exit_all_popups(
self, key: str, widget_size: Callable[[Widget], urwid_Size]
) -> None:
size = widget_size(self.edit_history_view)

self.edit_history_view.keypress(size, key)

assert self.controller.exit_popup.called
assert self.controller.exit_all_popups.called

def test_keypress_exit_popup_invalid_key(
self, widget_size: Callable[[Widget], urwid_Size]
Expand All @@ -700,12 +676,7 @@ def test_keypress_show_msg_info(

self.edit_history_view.keypress(size, key)

self.controller.show_msg_info.assert_called_once_with(
msg=self.message,
topic_links=OrderedDict(),
message_links=OrderedDict(),
time_mentions=list(),
)
assert self.controller.exit_popup.called

@pytest.mark.parametrize(
"snapshot",
Expand Down Expand Up @@ -955,10 +926,22 @@ def test_keypress_exit_popup(
assert self.controller.exit_popup.called


@pytest.fixture
def message_info_content() -> MessageInfoPopupContent:
return MessageInfoPopupContent(
topic_links=OrderedDict(),
message_links=OrderedDict(),
time_mentions=list(),
)


class TestMsgInfoView:
@pytest.fixture(autouse=True)
def mock_external_classes(
self, mocker: MockerFixture, message_fixture: Message
self,
mocker: MockerFixture,
message_fixture: Message,
message_info_content: MessageInfoPopupContent,
) -> None:
self.controller = mocker.Mock()
mocker.patch.object(
Expand All @@ -975,34 +958,40 @@ def mock_external_classes(
"Tue Mar 13 10:55:22",
"Tue Mar 13 10:55:37",
]
self.message = message_fixture
self.message_info_content = message_info_content
self.msg_info_view = MsgInfoView(
self.controller,
message_fixture,
self.message,
"Message Information",
OrderedDict(),
OrderedDict(),
list(),
self.message_info_content,
)

def test_init(self, message_fixture: Message) -> None:
assert self.msg_info_view.msg == message_fixture
def test_init(self) -> None:
assert self.msg_info_view.msg == self.message
assert self.msg_info_view.topic_links == OrderedDict()
assert self.msg_info_view.message_links == OrderedDict()
assert self.msg_info_view.time_mentions == list()

def test_pop_up_info_order(self, message_fixture: Message) -> None:
def test_pop_up_info_order(self) -> None:
topic_links = OrderedDict([("https://bar.com", ("topic", 1, True))])
message_links = OrderedDict([("image.jpg", ("image", 1, True))])
msg_info_view = MsgInfoView(
self.controller,
message_fixture,
title="Message Information",
message_info_content = MessageInfoPopupContent(
topic_links=topic_links,
message_links=message_links,
time_mentions=list(),
)
msg_info_view = MsgInfoView(
self.controller,
self.message,
title="Message Information",
message_info_content=message_info_content,
)
msg_links = msg_info_view.button_widgets
assert msg_links == [message_links, topic_links]
assert msg_links == [
message_info_content["message_links"],
message_info_content["topic_links"],
]

def test_keypress_any_key(
self, widget_size: Callable[[Widget], urwid_Size]
Expand All @@ -1029,7 +1018,6 @@ def test_keypress_any_key(
)
def test_keypress_edit_history(
self,
message_fixture: Message,
key: str,
widget_size: Callable[[Widget], urwid_Size],
realm_allow_edit_history: bool,
Expand All @@ -1041,78 +1029,39 @@ def test_keypress_edit_history(
self.controller.model.initial_data = {
"realm_allow_edit_history": realm_allow_edit_history,
}
msg_info_view = MsgInfoView(
self.controller,
message_fixture,
title="Message Information",
topic_links=OrderedDict(),
message_links=OrderedDict(),
time_mentions=list(),
)
size = widget_size(msg_info_view)
size = widget_size(self.msg_info_view)

msg_info_view.keypress(size, key)
self.msg_info_view.keypress(size, key)

if msg_info_view.show_edit_history_label:
if self.msg_info_view.show_edit_history_label:
self.controller.show_edit_history.assert_called_once_with(
message=message_fixture,
topic_links=OrderedDict(),
message_links=OrderedDict(),
time_mentions=list(),
message=self.message
)
else:
self.controller.show_edit_history.assert_not_called()

@pytest.mark.parametrize("key", keys_for_command("FULL_RENDERED_MESSAGE"))
def test_keypress_full_rendered_message(
self,
message_fixture: Message,
key: str,
widget_size: Callable[[Widget], urwid_Size],
self, key: str, widget_size: Callable[[Widget], urwid_Size]
) -> None:
msg_info_view = MsgInfoView(
self.controller,
message_fixture,
title="Message Information",
topic_links=OrderedDict(),
message_links=OrderedDict(),
time_mentions=list(),
)
size = widget_size(msg_info_view)
size = widget_size(self.msg_info_view)

msg_info_view.keypress(size, key)
self.msg_info_view.keypress(size, key)

self.controller.show_full_rendered_message.assert_called_once_with(
message=message_fixture,
topic_links=OrderedDict(),
message_links=OrderedDict(),
time_mentions=list(),
message=self.message
)

@pytest.mark.parametrize("key", keys_for_command("FULL_RAW_MESSAGE"))
def test_keypress_full_raw_message(
self,
message_fixture: Message,
key: str,
widget_size: Callable[[Widget], urwid_Size],
self, key: str, widget_size: Callable[[Widget], urwid_Size]
) -> None:
msg_info_view = MsgInfoView(
self.controller,
message_fixture,
title="Message Information",
topic_links=OrderedDict(),
message_links=OrderedDict(),
time_mentions=list(),
)
size = widget_size(msg_info_view)
size = widget_size(self.msg_info_view)

msg_info_view.keypress(size, key)
self.msg_info_view.keypress(size, key)

self.controller.show_full_raw_message.assert_called_once_with(
message=message_fixture,
topic_links=OrderedDict(),
message_links=OrderedDict(),
time_mentions=list(),
message=self.message
)

@pytest.mark.parametrize(
Expand Down Expand Up @@ -1211,9 +1160,7 @@ def test_height_reactions(
self.controller,
varied_message,
"Message Information",
OrderedDict(),
OrderedDict(),
list(),
self.message_info_content,
)
# 12 = 7 labels + 2 blank lines + 1 'Reactions' (category)
# + 4 reactions (excluding 'Message Links').
Expand Down
Loading
Loading