Page MenuHomePhabricator

Double notification received for becoming a Newsletter publisher
Closed, ResolvedPublic

Description

Quim added me as a publisher (in a single step, I've confirmed with him).
I received two notifications.

Details

Related Gerrit Patches:
mediawiki/extensions/Newsletter : masterRemove duplicate newsletter-newpublisher create event

Event Timeline

Quiddity created this task.Aug 30 2017, 6:53 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 30 2017, 6:53 PM
Qgil triaged this task as Medium priority.Aug 31 2017, 9:34 AM
Qgil added a subscriber: 01tonythomas.

This bug has been confirmed by another user in another test.

@01tonythomas do you have any idea about where this might be coming from? Although technically this bug is not a big deal, it causes a poor first impression to new publishers about the stability and reliability of this new extension.

@Qgil I just tried to reproduce this - but sadly couldnt. I just get one notification. My only assumption was that the form was submitted twice. Can you check this thing now (after our yesterdays patch - which was unrelated though) ?

Johan added a subscriber: Johan.Aug 31 2017, 10:08 AM

I just got two notifications when being added as a publisher.

(And no notification when being removed as publisher, if that's relevant.)

Johan removed a subscriber: Johan.Aug 31 2017, 10:10 AM
Qgil added a comment.Aug 31 2017, 10:11 AM

@01tonythomas I just added you as publisher to https://test.wikipedia.org/wiki/Newsletter:The_Technical_Collaboration_Observer

Did you receive one or two notifications?

@01tonythomas I just added you as publisher to https://test.wikipedia.org/wiki/Newsletter:The_Technical_Collaboration_Observer
Did you receive one or two notifications?

I can see it there. Woah. Strange I cannot reproduce (making me think that someone should look at the Echo jobs logs).

Qgil added a comment.Aug 31 2017, 10:23 AM
In T174602#3569466, Johan wrote:

(And no notification when being removed as publisher, if that's relevant.)

(Thanks! This is now T174667: Newsletter extension doesn't notify users when they are removed as publishers)

Qgil added a comment.Aug 31 2017, 10:43 AM

I think I have found the problem. There is something fishy triggering notifications when the user who adds a publisher is an administrator:

  • When a publisher who is not an administrator adds a publisher, one notification is sent.
  • When a publisher who is an administrator adds a publisher, two notifications are sent.
  • When an administrator who is not a publisher adds a publisher, two notifications are sent.

I am not sure. I still think there is something with the Echo job queue here. Might not be a newsletter triggered thing as I tried condition (3) mentioned above just now - and only got *one*. Can someone from the echo team take a look, or someone with Echo jobs take a look at what is happening ?

Qgil added a subscriber: Addshore.Sep 1 2017, 12:23 PM

In any case, I don't think this is a blocker for deployment.

@Addshore I think this extension is ready to go.
The testing helped finding one major bug (fixed) and one glitch (T174602) that @01tonythomas cannot reproduce outside of test.wikipedia. That glitch only represents a very minor problem to a few and special users (users granted publisher permissions by an admin), and in fact checking whether it can be reproduced in production will be useful to find the cause.

There are 2 EchoEvent::create calls for "EchoEvent::create"

One in NewsletterDataUpdate and one in NewsletterEditPage

Addshore claimed this task.Sep 1 2017, 12:35 PM
Addshore added a project: User-Addshore.
Addshore moved this task from Unsorted 💣 to Back Burner 🏛️ on the User-Addshore board.

Change 375371 had a related patch set uploaded (by Addshore; owner: Addshore):
[mediawiki/extensions/Newsletter@master] Remove duplicate newsletter-newpublisher create event

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

There are 2 EchoEvent::create calls for "EchoEvent::create"
One in NewsletterDataUpdate and one in NewsletterEditPage

But is the NewsletterDataUpdate called on normal edits ? I cannot find that happening on my local instance. In fact, after applying the ^^ patch - I do not get a notification at all. Probably if someone is free, we can test this thing out at newsletter-test.wmflabs.org and decide for sure.

Else, there is something very related to my machine install which do not produce double notifications.

QuimGil added a subscriber: QuimGil.Sep 7 2017, 8:02 PM

I just added @Quiddity as publisher of Newsletter², a newsletter in MediaWiki.org, and he only got one notification. Should we close this task as... Resolved? If someone hits this problem again, we can reopen.

Addshore moved this task from Active 🚁 to Next on the User-Addshore board.Nov 16 2017, 3:39 PM

(Since then I have added more publishers to newsletters, and I have heard no further complaints.)

Quiddity closed this task as Resolved.Nov 16 2017, 10:14 PM

I've tested again by adding Quim to another newsletter, and it does seem to be fixed, somehow (?). I'll resolve this task, and leave a note on Addshore's unmerged patchset.

Change 375371 abandoned by Addshore:
Remove duplicate newsletter-newpublisher create event

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

Addshore moved this task from Next to Closing ✔️ on the User-Addshore board.Nov 17 2017, 6:19 PM