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.

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

Update mark as read stream and topic to use /messages/flags/narrow endpoint. #27372

Closed
laurynmm opened this issue Oct 25, 2023 · 11 comments · Fixed by #28560
Closed

Update mark as read stream and topic to use /messages/flags/narrow endpoint. #27372

laurynmm opened this issue Oct 25, 2023 · 11 comments · Fixed by #28560

Comments

@laurynmm
Copy link
Collaborator

As part of deprecating legacy unread endpoints (#23598), the web-app frontend code for marking a stream and topic as read should be updated to use the new /messages/flags/narrow endpoint.

Current use of legacy unread endpoints in unread_ops.js, CODE LINK

export function mark_stream_as_read(stream_id, cont) {
    channel.post({
        url: "/json/mark_stream_as_read",
        data: {stream_id},
        success: cont,
    });
}

export function mark_topic_as_read(stream_id, topic, cont) {
    channel.post({
        url: "/json/mark_topic_as_read",
        data: {stream_id, topic_name: topic},
        success: cont,
    });
}

It likely makes sense to reuse/refactor some of the work done for mark_as_unread_from_here to use the new endpoint (see this commit) for these functions.

CZO thread

@zulipbot
Copy link
Member

Hello @zulip/server-refactoring members, this issue was labeled with the "area: refactoring" label, so you may want to check it out!

@lata-11
Copy link
Collaborator

lata-11 commented Oct 27, 2023

@zulipbot claim

@zulipbot
Copy link
Member

Welcome to Zulip, @lata-11! We just sent you an invite to collaborate on this repository at https://github.com/zulip/zulip/invitations. Please accept this invite in order to claim this issue and begin a fun, rewarding experience contributing to Zulip!

Here's some tips to get you off to a good start:

As you work on this issue, you'll also want to refer to the Zulip code contribution guide, as well as the rest of the developer documentation on that site.

See you on the other side (that is, the pull request side)!

@zulipbot
Copy link
Member

@marco-zan This issue cannot be claimed, as someone else is already working on it. Please see our contributor guide for advice on finding an issue to work on. Thanks!

@marco-zan
Copy link
Collaborator

Hi @lata-11 sorry to bother you. Speaking on the zulip-chat related thread with the founders i found out how to resolve this issue and actually started working. In fact i am ready to do the first commit to fix the mark stream as read function. If you didn't start working on it yet, can i please claim it?

@lata-11
Copy link
Collaborator

lata-11 commented Oct 30, 2023

Hi @marco-zan, I am still trying to figure out but it seems you had already figured it out. So you can claim it.

@lata-11
Copy link
Collaborator

lata-11 commented Oct 30, 2023

@zulipbot abandon

@marco-zan
Copy link
Collaborator

Thank you

@marco-zan
Copy link
Collaborator

@zulipbot claim

marco-zan added a commit to marco-zan/zulip that referenced this issue Oct 30, 2023
The code is heavly inspired by the mark_all_as_read function on the same
file and so gains the batch and ui reporting features

Partially fixes zulip#27372
marco-zan added a commit to marco-zan/zulip that referenced this issue Nov 14, 2023
Change discussed on the PR, avoids code duplication to mark many messages
as read using a narrow and an anchor. The cont attribute is eliminated
because it was never used

fixes zulip#27372
marco-zan added a commit to marco-zan/zulip that referenced this issue Nov 14, 2023
The code is simple by using mark_bulk_as_read, similarly to mark_stream_as_read
but by changing the narrow to also use the topic name. The cont parameter
is also removed because it was never used

Fixes zulip#27372
@zulipbot
Copy link
Member

zulipbot commented Nov 24, 2023

@marco-zan You have been unassigned from this issue because you have not made any updates for over 14 days. Please feel free to reclaim the issue if you decide to pick up again. Thanks!

@shubham-padia
Copy link
Member

@zulipbot claim

shubham-padia pushed a commit to shubham-padia/zulip that referenced this issue Jan 13, 2024
Fixes zulip#27372.
This commit refactors mark_all_as_read in unread_ops to
mark_stream_topic_or_all_as_read to include marking stream/topic as read.
The unread_ops API reamins the same to mark stream/topic as read but the
underlying implementation now uses a different endpoint.

I did not create a new function for this task since a lot of the code
would have been shared with mark_all_as_read
shubham-padia pushed a commit to shubham-padia/zulip that referenced this issue Jan 13, 2024
Fixes zulip#27372.
This commit refactors mark_all_as_read in unread_ops to
mark_stream_topic_or_all_as_read to include marking stream/topic as read.
The unread_ops API reamins the same to mark stream/topic as read but the
underlying implementation now uses a different endpoint.

I did not create a new function for this task since a lot of the code
would have been shared with mark_all_as_read

mark_stream_topic_or_all_as_read will mark all messages as read if
no type is provided.
shubham-padia pushed a commit to shubham-padia/zulip that referenced this issue Jan 13, 2024
Fixes zulip#27372.
This commit refactors mark_all_as_read in unread_ops to
mark_stream_topic_or_all_as_read to include marking stream/topic as read.
The unread_ops API reamins the same to mark stream/topic as read but the
underlying implementation now uses a different endpoint.

I did not create a new function for this task since a lot of the code
would have been shared with mark_all_as_read

mark_stream_topic_or_all_as_read will mark all messages as read if
no type is provided.
shubham-padia pushed a commit to shubham-padia/zulip that referenced this issue Jan 13, 2024
Fixes zulip#27372.
This commit refactors the original mark_all_as_read in unread_ops to
mark_stream_topic_or_all_as_read to include marking stream/topic as read.
The unread_ops API reamins the same to mark stream/topic as read but the
underlying implementation now uses a different endpoint. A new function
mark_all_as_read has been added so that the unread_ops API remains the
same.

I did not create a new function for this task since a lot of the code
would have been shared with mark_all_as_read
shubham-padia pushed a commit to shubham-padia/zulip that referenced this issue Jan 13, 2024
Fixes zulip#27372.
This commit refactors the original mark_all_as_read in unread_ops to
mark_stream_topic_or_all_as_read to include marking stream/topic as read.
The unread_ops API reamins the same to mark stream/topic as read but the
underlying implementation now uses a different endpoint. A new function
mark_all_as_read has been added so that the unread_ops API remains the
same.

I did not create a new function for this task since a lot of the code
would have been shared with mark_all_as_read
shubham-padia pushed a commit to shubham-padia/zulip that referenced this issue Jan 13, 2024
Fixes zulip#27372.
This commit refactors the original mark_all_as_read in unread_ops to
mark_stream_topic_or_all_as_read to include marking stream/topic as read.
The unread_ops API reamins the same to mark stream/topic as read but the
underlying implementation now uses a different endpoint. A new function
mark_all_as_read has been added so that the unread_ops API remains the
same.

I did not create a new function for this task since a lot of the code
would have been shared with mark_all_as_read
shubham-padia pushed a commit to shubham-padia/zulip that referenced this issue Jan 13, 2024
Fixes zulip#27372.
This commit refactors the original mark_all_as_read in unread_ops to
mark_bulk_as_read to include marking stream/topic as read.
The unread_ops API reamins the same to mark stream/topic as read but the
underlying implementation now uses a different endpoint. A new function
mark_all_as_read has been added so that the unread_ops API remains the
same.

I did not create a new function for this task since a lot of the code
would have been shared with mark_all_as_read
shubham-padia pushed a commit to shubham-padia/zulip that referenced this issue Jan 13, 2024
Fixes zulip#27372.
This commit refactors the original mark_all_as_read in unread_ops to
bulk_mark_mesages_as_read to include marking stream/topic as read.
The unread_ops API reamins the same to mark stream/topic as read but the
underlying implementation now uses a different endpoint. A new function
mark_all_as_read has been added so that the unread_ops API remains the
same.

I did not create a new function for this task since a lot of the code
would have been shared with mark_all_as_read
timabbott pushed a commit to shubham-padia/zulip that referenced this issue Jan 14, 2024
This commit refactors the original mark_all_as_read in unread_ops to
bulk_mark_mesages_as_read to include marking stream/topic as read.
The unread_ops API reamins the same to mark stream/topic as read but the
underlying implementation now uses a different endpoint. A new function
mark_all_as_read has been added so that the unread_ops API remains the
same.

Fixes zulip#27372.
timabbott pushed a commit that referenced this issue Jan 14, 2024
This commit refactors the original mark_all_as_read in unread_ops to
bulk_mark_mesages_as_read to include marking stream/topic as read.
The unread_ops API reamins the same to mark stream/topic as read but the
underlying implementation now uses a different endpoint. A new function
mark_all_as_read has been added so that the unread_ops API remains the
same.

Fixes #27372.
mananbordia pushed a commit to mananbordia/zulip that referenced this issue Feb 27, 2024
This commit refactors the original mark_all_as_read in unread_ops to
bulk_mark_mesages_as_read to include marking stream/topic as read.
The unread_ops API reamins the same to mark stream/topic as read but the
underlying implementation now uses a different endpoint. A new function
mark_all_as_read has been added so that the unread_ops API remains the
same.

Fixes zulip#27372.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment