Improve delivery of notifications for comments posted in close succession
Closed, ResolvedPublic
Actions

Assigned To

Authored By

	ppelberg
	Jun 25 2021, 1:03 AM

Description

This task involves the work with improving how notifications about new comments, posted in close succession, by the same person, are delivered.

Behavior

Case 1: new comments, posted in different sections, by the same person, in close succession

Person A subscribes to Topic 1 and Topic 2
Person B posts Comment 1 in Topic 1 and then quickly thereafter, posts Comment 2 in Topic 2

Current experienced

Person A receives a new comment notification about Comment 1

Desired experience

✅ Person A receives a new comment notification about Comment 1
✅ Person A receives a new comment notification about Comment 2

Case 2: new comments, posted in the same section, by the same person, in close succession

Person Z subscribes to Topic 3
Person Y posts Comment 3 in Topic 3 and then quickly thereafter, posts Comment 4 in Topic 3

Current experienced

Person A receives a new comment notification about Comment 3

Desired experience

✅ Person Z sees a new notification bundle for the two new comments that were posted in Topic 3
✅ Person Z clicks the notification bundle and sees discrete notifications for Comment 3 and Comment 4

Done

The Desired experiences for Cases 1 & 2 are implemented

Details

	Project	Branch	Lines +/-	Subject
	mediawiki/extensions/DiscussionTools	master	+155 -16	Improve notifications for comments posted in close succession
	mediawiki/extensions/DiscussionTools	master	+171 -0	Test cases for comments posted in close succession

Customize query in gerrit

Related Objects
Search...

		Status	Subtype	Assigned	Task
		Resolved		matmarex	T285528 Improve delivery of notifications for comments posted in close succession
		Resolved		matmarex	T286608 Add unit tests for DiscussionTools topic subscription notification

Event Timeline

ppelberg created this task.Jun 25 2021, 1:03 AM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJun 25 2021, 1:03 AM

ppelberg edited projects, added Editing-team; removed Editing-team (Tracking).Jun 25 2021, 1:04 AM

ppelberg moved this task from Backlog to Triaged on the DiscussionTools board.

ppelberg moved this task from Untriaged to Upcoming on the Editing-team board.

ppelberg mentioned this in T274279: Pre-deployment regression testing (topic subscriptions).

matmarex added a subscriber: matmarex.Jun 25 2021, 11:09 AM

After thinking about this a bit more, I've realized that there are a few more interesting cases to consider. But I'm sure that we can still make improvements.

We should definitely add unit tests for all of the below.

Let's say we're starting in the following situation, where you just added the comment E:

== A ==
B. --[[User:X]]

== C ==
D. --[[User:Y]]
:E. --~~~~

(this is not real, correct talk page markup, but you know what I mean)

Your case 1 is:

== A ==
B. --[[User:X]]
:F. --~~~~

== C ==
D. --[[User:Y]]
:E. --~~~~

Your case 2 is:

== A ==
B. --[[User:X]]

== C ==
D. --[[User:Y]]
:E. --~~~~
:F. --~~~~

Alternative scenario inspired by case 1 (case 3) – the comment E is moved, instead of a new comment being added (perhaps you realized belatedly that you clicked [edit source] on the wrong section). We currently do not notify for moved comments, but in this case, we should probably notify users subscribed to section A?

== A ==
B. --[[User:X]]
:E. --~~~~

== C ==
D. --[[User:Y]]

Alternative scenario inspired by case 2 (case 4) – the comment is added to the existing comment E. We have no way to notify in this case, we can't distinguish this from a typo correction to comment E (of which we don't want to notify).

== A ==
B. --[[User:X]]

== C ==
D. --[[User:Y]]
:E. --~~~~ F. --~~~~

Alternative scenario inspired by case 2 (case 5) – comment F is added above the existing comment. We can't actually tell which comment is old and which is new; if this happens, we'll probably send a notification with a snippet of comment E again. This is probably not a big deal, since this isn't really done…?

== A ==
B. --[[User:X]]

== C ==
D. --[[User:Y]]
:F. --~~~~
:E. --~~~~

Now imagine the existing discussion had two more comments, and again you just added the comment E:

== A ==
B. --[[User:X]]

== C ==
D. --[[User:Y]]
:D1. --[[User:X]]
:D2. --[[User:Y]]
::E. --~~~~

Alternative scenario inspired by case 3 (case 6) – the comment E is moved, but we should not notify, because the comment is still in the same section, so everyone alreayd got notified.

== A ==
B. --[[User:X]]

== C ==
D. --[[User:Y]]
:D1. --[[User:X]]
::E. --~~~~
:D2. --[[User:Y]]

Alternative scenario inspired by case 5 (case 7) – comment F is added above the existing comment, but we can distinguish the new comment F from comment E because they have different parent comments:

== A ==
B. --[[User:X]]

== C ==
D. --[[User:Y]]
:D1. --[[User:X]]
::F. --~~~~
:D2. --[[User:Y]]
::E. --~~~~

ppelberg edited projects, added Editing-team (Kanban Board); removed Editing-team.Jun 28 2021, 3:27 PM

matmarex added a subtask: T286608: Add unit tests for DiscussionTools topic subscription notification.Jul 13 2021, 8:15 PM

matmarex claimed this task.Jul 15 2021, 7:15 PM

matmarex moved this task from Incoming to Doing on the Editing-team (Kanban Board) board.

Change 705414 had a related patch set uploaded (by Bartosz Dziewoński; author: Bartosz Dziewoński):

[mediawiki/extensions/DiscussionTools@master] Test cases for comments posted in close succession

https://gerrit.wikimedia.org/r/705414

Change 705415 had a related patch set uploaded (by Bartosz Dziewoński; author: Bartosz Dziewoński):

[mediawiki/extensions/DiscussionTools@master] Improve delivery of notifications for comments posted in close succession

https://gerrit.wikimedia.org/r/705415

matmarex moved this task from Doing to Code Review on the Editing-team (Kanban Board) board.Jul 19 2021, 4:58 PM

ppelberg closed subtask T286608: Add unit tests for DiscussionTools topic subscription notification as Resolved.Jul 29 2021, 12:08 AM

Change 705414 merged by jenkins-bot:

[mediawiki/extensions/DiscussionTools@master] Test cases for comments posted in close succession

https://gerrit.wikimedia.org/r/705414

ReleaseTaggerBot added a project: MW-1.37-notes (1.37.0-wmf.18; 2021-08-09).Aug 3 2021, 9:00 PM

Change 705415 merged by jenkins-bot:

[mediawiki/extensions/DiscussionTools@master] Improve notifications for comments posted in close succession

https://gerrit.wikimedia.org/r/705415

Maintenance_bot removed a project: Patch-For-Review.Aug 4 2021, 12:10 AM

The new functionality has automated integration tests, and I don't think it's a good use of time to be furiously clicking trying to leave multiple comments within a minute, so skipping QA.

In T285528#7259485, @matmarex wrote:

The new functionality has automated integration tests, and I don't think it's a good use of time to be furiously clicking trying to leave multiple comments within a minute, so skipping QA.

Understood.

For posterity, noting down what @DLynch helped me to understand:

We now have unit tests written for the cases Bartosz documented in T285528#7177220.
And because we've now explicitly "written down" how we think said cases should behave, we will know when we make changes that break this behavior
If and when "2." happens we can do a minimum of one of following two things: 1) Revise the tests to adapt to how we think the experience should be, 2) Fix the "bug" that is causing the test to fail

ppelberg closed this task as Resolved.Aug 11 2021, 4:15 PM

Improve delivery of notifications for comments posted in close successionClosed, ResolvedPublicActions