Skip to content

Commit

Permalink
refactor: Build query in find_first_unread_anchor().
Browse files Browse the repository at this point in the history
This is a bit more than a pure refactor, because we duplicate a
chunk of code to calculate a query inside of
find_first_unread_anchor(), so we're doing a bit more work
than before.

We need this refactoring to start decoupling find_first_unread_anchor
from get_messages_backend for the case where include_history is
True.  This will happen in a subsequent commit.

The only test that changes here is a direct test on
find_first_unread_anchor().  All other tests pass without
modification, and we have decent coverage on get_messages_backend.
  • Loading branch information
Steve Howell committed Apr 5, 2018
1 parent 345d44b commit b64117d
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 16 deletions.
11 changes: 0 additions & 11 deletions zerver/tests/test_narrow.py
Original file line number Diff line number Diff line change
Expand Up @@ -1760,21 +1760,10 @@ def test_find_first_unread_anchor(self) -> None:

user_profile = hamlet

# TODO: Make it so that find_first_unread_anchor() does not require
# the incoming query to join to zerver_usermessage.
query = select([column("message_id"), column("flags")],
column("user_profile_id") == literal(user_profile.id),
join(table("zerver_usermessage"), table("zerver_message"),
literal_column("zerver_usermessage.message_id") ==
literal_column("zerver_message.id")))
inner_msg_id_col = column("message_id")

anchor = find_first_unread_anchor(
sa_conn=sa_conn,
inner_msg_id_col=inner_msg_id_col,
user_profile=user_profile,
narrow=[],
query=query,
)
self.assertEqual(anchor, first_message_id)

Expand Down
30 changes: 25 additions & 5 deletions zerver/views/messages.py
Original file line number Diff line number Diff line change
Expand Up @@ -622,10 +622,32 @@ def add_narrow_conditions(user_profile: UserProfile,
return (query, is_search)

def find_first_unread_anchor(sa_conn: Any,
inner_msg_id_col: ColumnElement,
user_profile: UserProfile,
narrow: List[Dict[str, Any]],
query: Query) -> int:
narrow: List[Dict[str, Any]]) -> int:
# We always need UserMessage in our query, because it has the unread
# flag for the user.
need_user_message = True

# TODO:
# We err on the side of putting Message in our query, but there are
# probably situations that we don't need it. We may eventually try
# to make add_narrow_conditions() and similar functions help us make
# possible optimizations.
need_message = True

query, inner_msg_id_col = get_base_query_for_search(
user_profile=user_profile,
need_message=need_message,
need_user_message=need_user_message,
)

query, is_search = add_narrow_conditions(
user_profile=user_profile,
inner_msg_id_col=inner_msg_id_col,
query=query,
narrow=narrow,
)

condition = column("flags").op("&")(UserMessage.flags.read.mask) == 0

# We exclude messages on muted topics when finding the first unread
Expand Down Expand Up @@ -716,10 +738,8 @@ def get_messages_backend(request: HttpRequest, user_profile: UserProfile,
if use_first_unread_anchor:
anchor = find_first_unread_anchor(
sa_conn,
inner_msg_id_col,
user_profile,
narrow,
query
)

anchored_to_left = (anchor == 0)
Expand Down

0 comments on commit b64117d

Please sign in to comment.