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

Jump to bottom

Delete notification bot messages for resolved topics, on undo in a short grace period #19181

Open
timabbott opened this issue Jul 8, 2021 · 6 comments · May be fixed by #19199
Open

Delete notification bot messages for resolved topics, on undo in a short grace period #19181

timabbott opened this issue Jul 8, 2021 · 6 comments · May be fixed by #19199
Labels
area: message-editing (topics) help wanted priority: high

Comments

@timabbott
Copy link
Sponsor Member

When you (un)resolve a topic, we send an automated notification noting that it was resolved. This can be unfortunate in the event of a misclick, since one ends up creating two spam messages. Rather than making it harder to resolve topics, we'd like to instead design a grace period. We'd modify maybe_send_resolve_topic_notifications to add the following logic:

  • If the newest message that was moved is a notification bot "marked as resolved" message (so there's no conversation that might be made confusing).
  • And that message's timestamp is within settings.RESOLVE_TOPIC_UNDO_GRACE_PERIOD_SECONDS, which defaults to 60 seconds but should be 5 seconds in a development environment environment (to make it easier to test the unresolve functionality without waiting a minute).
  • Then we'd delete the "marked as resolved" notice rather than adding a new one that says the opposite by calling do_delete_messages on the notice.

(And presumably the opposite for if you accidentally toggled in the other direction).

https://chat.zulip.org/#narrow/stream/101-design/topic/closing.2Fsolving.20a.20topic.20.28.2311154.29/near/1227897 has background.
@ligmitz
Copy link
Collaborator

ligmitz commented Jul 8, 2021

@zulipbot claim

ligmitz added a commit to ligmitz/zulip that referenced this issue Jul 9, 2021
@ligmitz
resolve topic: Add resolve topic undo grace period.
11af633 
Currently we send a notification to the topic if
it has been resolved or unresolved even if there
is an immediate event of resolving and then unresolving
or vice-versa. This adds a setting of
RESOLVE_TOPIC_UNDO_GRACE_PERIOD_SECONDS under which
if a topic has been unresolved after being resolved
immediately and the last message was the notification
of resolving, then delete the last message and don't
send a new notification and vice-versa.

Fixes zulip#19181.
ligmitz added a commit to ligmitz/zulip that referenced this issue Jul 9, 2021
@ligmitz
resolve topic: Add resolve topic undo grace period.
af9aaf4 
Currently we send a notification to the topic if
it has been resolved or unresolved even if there
is an immediate event of resolving and then unresolving
or vice-versa. This adds a setting of
RESOLVE_TOPIC_UNDO_GRACE_PERIOD_SECONDS under which
if a topic has been unresolved after being resolved
immediately and the last message was the notification
of resolving, then delete the last message and don't
send a new notification and vice-versa.

Fixes zulip#19181.
@ligmitz ligmitz linked a pull request Jul 9, 2021 that will close this issue
Open
ligmitz added a commit to ligmitz/zulip that referenced this issue Jul 9, 2021
@ligmitz
resolve topic: Add resolve topic undo grace period.
835b80e 
Currently we send a notification to the topic if
it has been resolved or unresolved even if there
is an immediate event of resolving and then unresolving
or vice-versa. This adds a setting of
RESOLVE_TOPIC_UNDO_GRACE_PERIOD_SECONDS under which
if a topic has been unresolved after being resolved
immediately and the last message was the notification
of resolving, then delete the last message and don't
send a new notification and vice-versa.

Fixes zulip#19181.
@mojavelinux
Copy link

If it's alright, I'd like to request an additional requirement for this change. Currently, when you tick the resolved icon, it bubbles the topic to the top because it has received new activity. This behavior just adds noise to the stream. The intention is to mark the topic as closed / done, not give it new life. Would it be possible for the notification to assume the timestamp of the last post on the topic and not update the unread count?

timabbott pushed a commit to ligmitz/zulip that referenced this issue Jul 23, 2021
@ligmitz @timabbott
resolve topic: Add resolve topic undo grace period.
dd3b027 
Currently we send a notification to the topic if it has been resolved
or unresolved even if there is an immediate event of resolving and
then unresolving or vice-versa. This adds a setting of
RESOLVE_TOPIC_UNDO_GRACE_PERIOD_SECONDS under which if a topic has
been unresolved after being resolved immediately and the last message
was the notification of resolving, then delete the last message and
don't send a new notification and vice-versa.

We use the new message.type field to precisely identify relevant
messages.

Fixes zulip#19181.
timabbott pushed a commit to ligmitz/zulip that referenced this issue Jul 23, 2021
@ligmitz @timabbott
resolve topic: Add resolve topic undo grace period.
0b1dba4 
Currently we send a notification to the topic if it has been resolved
or unresolved even if there is an immediate event of resolving and
then unresolving or vice-versa. This adds a setting of
RESOLVE_TOPIC_UNDO_GRACE_PERIOD_SECONDS under which if a topic has
been unresolved after being resolved immediately and the last message
was the notification of resolving, then delete the last message and
don't send a new notification and vice-versa.

We use the new message.type field to precisely identify relevant
messages.

Fixes zulip#19181.
ligmitz added a commit to ligmitz/zulip that referenced this issue Jul 28, 2021
@ligmitz
resolve topic: Add resolve topic undo grace period.
c402018 
Currently we send a notification to the topic if it has been resolved
or unresolved even if there is an immediate event of resolving and
then unresolving or vice-versa. This adds a setting of
RESOLVE_TOPIC_UNDO_GRACE_PERIOD_SECONDS under which if a topic has
been unresolved after being resolved immediately and the last message
was the notification of resolving, then delete the last message and
don't send a new notification and vice-versa.

We use the new message.type field to precisely identify relevant
messages.

Fixes zulip#19181.
ligmitz added a commit to ligmitz/zulip that referenced this issue Jul 28, 2021
@ligmitz
resolve topic: Add resolve topic undo grace period.
90840fc 
Currently we send a notification to the topic if it has been resolved
or unresolved even if there is an immediate event of resolving and
then unresolving or vice-versa. This adds a setting of
RESOLVE_TOPIC_UNDO_GRACE_PERIOD_SECONDS under which if a topic has
been unresolved after being resolved immediately and the last message
was the notification of resolving, then delete the last message and
don't send a new notification and vice-versa.

We use the new message.type field to precisely identify relevant
messages.

Fixes zulip#19181.
ligmitz added a commit to ligmitz/zulip that referenced this issue Jul 28, 2021
@ligmitz
resolve topic: Add resolve topic undo grace period.
e6d962e 
Currently we send a notification to the topic if it has been resolved
or unresolved even if there is an immediate event of resolving and
then unresolving or vice-versa. This adds a setting of
RESOLVE_TOPIC_UNDO_GRACE_PERIOD_SECONDS under which if a topic has
been unresolved after being resolved immediately and the last message
was the notification of resolving, then delete the last message and
don't send a new notification and vice-versa.

We use the new message.type field to precisely identify relevant
messages.

Fixes zulip#19181.
ligmitz added a commit to ligmitz/zulip that referenced this issue Jul 28, 2021
@ligmitz
resolve topic: Add resolve topic undo grace period.
3c74df0 
Currently we send a notification to the topic if it has been resolved
or unresolved even if there is an immediate event of resolving and
then unresolving or vice-versa. This adds a setting of
RESOLVE_TOPIC_UNDO_GRACE_PERIOD_SECONDS under which if a topic has
been unresolved after being resolved immediately and the last message
was the notification of resolving, then delete the last message and
don't send a new notification and vice-versa.

We use the new message.type field to precisely identify relevant
messages.

Fixes zulip#19181.
@gnprice gnprice changed the title Delete notification bot messages for resolved topics after a short grace period Delete notification bot messages for resolved topics, on undo in a short grace period Nov 29, 2021
@gnprice
Copy link
Member

gnprice commented Nov 29, 2021

(I've edited this title to reflect what I believe is the intention.)

@gnprice
Copy link
Member

gnprice commented Nov 29, 2021

If it's alright, I'd like to request an additional requirement for this change. Currently, when you tick the resolved icon, it bubbles the topic to the top because it has received new activity. This behavior just adds noise to the stream. The intention is to mark the topic as closed / done, not give it new life. Would it be possible for the notification to assume the timestamp of the last post on the topic and not update the unread count?

This is a good request, and this isn't the right thread for it.

This issue thread is about behavior in the case where the topic gets resolved and promptly unresolved (or vice versa). What you're asking for is behavior to reduce noise in the normal case, where the topic gets resolved and stays resolved.

There's now a separate issue #19709 which covers part of this -- not updating the unread count. (With some additional wrinkles -- if you'd rather not have those wrinkles, then either that issue, or a new issue linking to that one, would be the place to discuss that.)

I think once the notification doesn't count as unread, that will take care of this and there won't be a need for an additional hack relating to timestamps. But again either #19709, or a fresh issue linking to that one, would be the place to discuss if you think you'd want an additional behavior like that.

@timabbott timabbott added the release goal We'd like to resolve this for the current major release label Apr 14, 2022
@timabbott timabbott removed the release goal We'd like to resolve this for the current major release label Sep 23, 2022
@roanster007
Copy link
Collaborator

@zulipbot claim

@roanster007 roanster007 removed their assignment Jan 24, 2024
@roanster007
Copy link
Collaborator

I didn't realise the work on this is blocked due to database migration. Hence, un claiming it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area: message-editing (topics) help wanted priority: high
Projects
Zulip 9.0 priorities
Status: No status
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

resolve topic: Add resolve topic undo grace period. ligmitz/zulip
7 participants
@gnprice @mojavelinux @alya @timabbott @zulipbot @ligmitz @roanster007