Skip to content

Mark-unread can fail if topic was concurrently moved #1045

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

Jump to bottom
Closed
gnprice opened this issue Nov 5, 2024 · 0 comments · Fixed by #1044
Closed

Mark-unread can fail if topic was concurrently moved #1045

gnprice opened this issue Nov 5, 2024 · 0 comments · Fixed by #1044
Labels
a-msglist The message-list screen, except what's label:a-content
Milestone
M5b: Launch goals

Comments

@gnprice
Copy link
Member

gnprice commented Nov 5, 2024

Scenario:

  • You long-press a message, because you want to mark the conversation unread starting from there.
  • While you have the action sheet open, the topic gets moved — for example, perhaps it gets resolved.
    • This causes the message list to update to the new topic, as seen for example in the app bar.
  • Then you hit the "mark as unread" option in the action sheet.

Expected behavior: The message and the subsequent conversation still get marked as unread.

Actual behavior: Nothing happens. (Concretely, the app sends a mark-as-unread request aimed at the old narrow. Typically there will be no matching messages and so nothing will happen.)

I discovered this bug in the course of making other changes to the action-sheet code. #1044 contains a fix.
@gnprice gnprice added the a-msglist The message-list screen, except what's label:a-content label Nov 5, 2024
@gnprice gnprice added this to the Launch milestone Nov 5, 2024
gnprice added a commit to gnprice/zulip-flutter that referenced this issue Nov 5, 2024
@gnprice
action_sheet: Mark as unread in current narrow, vs. from when action …
2cf1f6d 
…sheet opened

The narrow of a given MessageListPage can change over time,
in particular if viewing a topic that gets moved.

If it does, the mark-unread action should follow along.  Otherwise
we'd have a frustrating race where mark-unread just didn't work
if, for example, someone happened to resolve the topic while you
were in the middle of trying to mark as unread.

Fixes: zulip#1045
@gnprice gnprice mentioned this issue Nov 5, 2024
Merged
PIG208 pushed a commit to PIG208/zulip-flutter that referenced this issue Nov 7, 2024
@gnprice @PIG208
action_sheet: Mark as unread in current narrow, vs. from when action …
2267dd5 
…sheet opened

The narrow of a given MessageListPage can change over time,
in particular if viewing a topic that gets moved.

If it does, the mark-unread action should follow along.  Otherwise
we'd have a frustrating race where mark-unread just didn't work
if, for example, someone happened to resolve the topic while you
were in the middle of trying to mark as unread.

Fixes: zulip#1045
gnprice added a commit to gnprice/zulip-flutter that referenced this issue Nov 7, 2024
@gnprice
action_sheet: Mark as unread in current narrow, vs. from when action …
67e291b 
…sheet opened

The narrow of a given MessageListPage can change over time,
in particular if viewing a topic that gets moved.

If it does, the mark-unread action should follow along.  Otherwise
we'd have a frustrating race where mark-unread just didn't work
if, for example, someone happened to resolve the topic while you
were in the middle of trying to mark as unread.

Fixes: zulip#1045
gnprice added a commit to gnprice/zulip-flutter that referenced this issue Nov 7, 2024
@gnprice
action_sheet: Mark as unread in current narrow, vs. from when action …
3bc18be 
…sheet opened

The narrow of a given MessageListPage can change over time,
in particular if viewing a topic that gets moved.

If it does, the mark-unread action should follow along.  Otherwise
we'd have a frustrating race where mark-unread just didn't work
if, for example, someone happened to resolve the topic while you
were in the middle of trying to mark as unread.

Fixes: zulip#1045
@gnprice gnprice closed this as completed in e258773 Nov 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
a-msglist The message-list screen, except what's label:a-content
Projects
Flutter app
Status: Done
Milestone
M5b: Launch goals
Development

Successfully merging a pull request may close this issue.

Clarify action sheet's "page context"; fix small bugs gnprice/zulip-flutter
1 participant
@gnprice