Page MenuHomePhabricator

flow-topic-resolved should use board as event title, not topic
Closed, ResolvedPublic

Description

Currently, flow-topic-resolved notifications set the title (i.e. $event->getTitle() and event_page_id) to the topic (e.g. Topic:fdsjkfjasd8u433), while every other flow-* notification type sets the title to the board (e.g. Talk:Foo). We should make flow-topic-resolved use board titles too, for consistency and to work better with the upcoming special page sidebar feature.

Event Timeline

Catrope created this task.Jun 9 2016, 9:58 PM
Restricted Application added subscribers: Zppix, Aklapper. · View Herald TranscriptJun 9 2016, 9:58 PM

(While doing this, take care that the primary/secondary link generation code and anything else using $event->getTitle() keeps working.)

I thought I fixed it with https://gerrit.wikimedia.org/r/#/c/283869 on April 17th.

I thought I fixed it with https://gerrit.wikimedia.org/r/#/c/283869 on April 17th.

That's possible, I didn't look very closely (I just picked a random event and looked at its title). I'll look again and look at recent events this time.

Change 294715 had a related patch set uploaded (by Catrope):
Follow-up 3253600cef: Actually notify topic watchers of topic-resolved/topic-reopened

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

Change 294715 had a related patch set uploaded (by Catrope):
Follow-up 3253600cef: Actually notify topic watchers of topic-resolved/topic-reopened
https://gerrit.wikimedia.org/r/294715

Sorry, mistagged.

You're right, this is now fixed. We may want to write a script that fixes old entries though, because pre-April notifications will still have the topic as their title.

Do we want to backfill this or let new notifications be classified correctly and old ones fall off?

The script would need to replace the event_page_ids pointing to topics by their parent board ids.

Change 295435 had a related patch set uploaded (by Catrope):
Add maintenance script to update page IDs of topic-resolved notifications

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

Change 295435 merged by jenkins-bot:
Add maintenance script to update page IDs of topic-resolved notifications

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

I'm guessing this task had been left open because the script was never run. I just ran the script successfully on all wikis. It only updated a couple hundred events overall.

@SBisson running the script in beta, I got the following:
etonkovidova@deployment-tin:/srv/mediawiki/php-master$ foreachwiki extensions/Flow/maintenance/FlowUpdateResolvedNotifTitles.php

enwiki
-----------------------------------------------------------------
enwiki:  Retitling flow-topic-resolved events...
Fatal error: Call to a member function getNamespace() on a non-object in 
/mnt/srv/mediawiki/php-master/extensions/Flow/maintenance/FlowUpdateResolvedNotifTitles.php on line 62

The only other error was:

enwikiversity
-----------------------------------------------------------------
Fatal error: Class 'Flow\Container' not found in 
/mnt/srv/mediawiki/php-master/extensions/Flow/maintenance/FlowUpdateResolvedNotifTitles.php on line 54

The enwikiversity error is normal, it happens for wikis that don't have Flow installed. I should have probably told you to run foreachwikiindblist flow.dblist or something like that, but I was lazy.

The enwiki error is a bug, probably because the script doesn't expect to process rows that point to deleted pages. That would be easily fixed by changing the if condition on line 62 from $title->getNamespace === NS_TOPIC to !$title || $title->getNamespace() === NS_TOPIC.

Change 303117 had a related patch set uploaded (by Mattflaschen):
Skip deleted boards in FlowUpdateResolvedNotifTitles

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

Change 303117 merged by jenkins-bot:
Skip deleted boards in FlowUpdateResolvedNotifTitles

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

Re-run the updated script - no errors.

jmatazzoni closed this task as Resolved.Aug 18 2016, 11:59 PM