Page MenuHomePhabricator

MentionPresentationModel fatals for notifications with no title
Closed, ResolvedPublic

Description

  1. Mention user U on page P
  2. User U receives a notification
  3. Delete page P
  4. Restore page P (optional)
  5. User U receives internal_api_error_BadMethodCallException when trying to view their notification flyout

For example, see https://test.wikipedia.org/wiki/User_talk:Roan_Kattouw_%28WMF%29 , which I deleted and restored. This has broken the alert flyout on testwiki for Catrope and Legoktm.

Event Timeline

Catrope assigned this task to Legoktm.
Catrope raised the priority of this task from to Unbreak Now!.
Catrope updated the task description. (Show Details)
Catrope subscribed.

The error is PHP Fatal error: Call to a member function getText() on a non-object in /home/catrope/git/mediawiki/extensions/Echo/includes/formatters/MentionPresentationModel.php on line 51. This is because MentionPresentationModel doesn't take into account that $event->getTitle() can be null. EchoBasicFormatter::setTitleLink() deals with this by inserting a hideous [No page placeholder (T52829: Echo notifications should show the page name instead of [No page] if the page was deleted)

Change 249354 had a related patch set uploaded (by Catrope):
Comment out presentation model for mentions since it fatals on deleted pages

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

Change 249354 merged by jenkins-bot:
Comment out presentation model for mentions since it fatals on deleted pages

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

Change 249411 had a related patch set uploaded (by Legoktm):
Comment out presentation model for mentions since it fatals on deleted pages

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

Change 249411 merged by jenkins-bot:
Comment out presentation model for mentions since it fatals on deleted pages

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

Change 249444 had a related patch set uploaded (by Legoktm):
Allow presentation models to indicate a notification can't be formatted

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

Legoktm lowered the priority of this task from Unbreak Now! to High.Oct 28 2015, 5:19 PM

Lowering priority since the fatal has been worked around.

Change 249444 merged by jenkins-bot:
Allow presentation models to indicate a notification can't be formatted

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

Checked in betalabs - no more BadMethodCallException.
The issue with [No page] as in T52829: Echo notifications should show the page name instead of [No page] if the page was deleted is still present. Clicking on notifications which refer to [No page], the deleted & restored page is perfectly displayed with all content.