Page MenuHomePhabricator

Homepage: If a talk page containing a question is deleted, it causes fatal error for all newbies
Closed, ResolvedPublic

Description

If a newbie adds a nonsense-question to a mentor, and I decide to delete the mentor's talk page, it causes a fatal to be displayed to the newbie at Special:Homepage. See screencast at https://martin.urbanec.cz/files/growth_screen_recordings/delete_talkpage_causes_harm.webm.

Traceback:

[51a559f23531639ddda6cc18] /mw/index.php?title=Special:Homepage&source=personaltoolslink&namespace=0 Error from line 216 of /mnt/c/Users/urban/unsynced/gerrit/mediawiki/extensions/GrowthExperiments/includes/HelpPanel/QuestionStore.php: Call to a member function getVisibility() on null

Backtrace:

#0 /mnt/c/Users/urban/unsynced/gerrit/mediawiki/extensions/GrowthExperiments/includes/HelpPanel/QuestionStore.php(90): GrowthExperiments\HelpPanel\QuestionStore->isRevisionVisible(GrowthExperiments\HelpPanel\QuestionRecord)
#1 /mnt/c/Users/urban/unsynced/gerrit/mediawiki/extensions/GrowthExperiments/includes/HelpPanel/QuestionStore.php(101): GrowthExperiments\HelpPanel\QuestionStore->loadQuestions()
#2 /mnt/c/Users/urban/unsynced/gerrit/mediawiki/extensions/GrowthExperiments/includes/HomepageModules/Mentorship.php(273): GrowthExperiments\HelpPanel\QuestionStore->loadQuestionsAndUpdate()
#3 /mnt/c/Users/urban/unsynced/gerrit/mediawiki/extensions/GrowthExperiments/includes/HomepageModules/Mentorship.php(111): GrowthExperiments\HomepageModules\Mentorship->getRecentQuestions()
#4 /mnt/c/Users/urban/unsynced/gerrit/mediawiki/extensions/GrowthExperiments/includes/HomepageModules/BaseModule.php(59): GrowthExperiments\HomepageModules\Mentorship->getActionData()
#5 /mnt/c/Users/urban/unsynced/gerrit/mediawiki/extensions/GrowthExperiments/includes/Specials/SpecialHomepage.php(89): GrowthExperiments\HomepageModules\BaseModule->render()
#6 /mnt/c/Users/urban/unsynced/gerrit/mediawiki/core/includes/specialpage/SpecialPage.php(569): GrowthExperiments\Specials\SpecialHomepage->execute(NULL)
#7 /mnt/c/Users/urban/unsynced/gerrit/mediawiki/core/includes/specialpage/SpecialPageFactory.php(578): SpecialPage->run(NULL)
#8 /mnt/c/Users/urban/unsynced/gerrit/mediawiki/core/includes/MediaWiki.php(288): MediaWiki\Special\SpecialPageFactory->executePath(Title, RequestContext)
#9 /mnt/c/Users/urban/unsynced/gerrit/mediawiki/core/includes/MediaWiki.php(865): MediaWiki->performRequest()
#10 /mnt/c/Users/urban/unsynced/gerrit/mediawiki/core/includes/MediaWiki.php(515): MediaWiki->main()
#11 /mnt/c/Users/urban/unsynced/gerrit/mediawiki/core/index.php(42): MediaWiki->run()
#12 {main}

Event Timeline

Urbanecm created this task.Apr 30 2019, 4:49 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptApr 30 2019, 4:49 PM

Change 507589 had a related patch set uploaded (by Sbisson; owner: Sbisson):
[mediawiki/extensions/GrowthExperiments@master] Recent questions: handle deleted page

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

SBisson claimed this task.May 1 2019, 2:52 PM
SBisson moved this task from Incoming to Code Review on the Growth-Team (Current Sprint) board.
kostajh added a subscriber: kostajh.May 1 2019, 5:34 PM

@MMiller_WMF I suggested on the patch that we should prevent the mentorship module from rendering at all in this case (also, same for help module if the help desk title is deleted). Otherwise, if a user goes to post a question on a deleted page, that page will be newly created, and that doesn't seem like desired behavior. Can you let us know what you'd like to have happen?

Ehm, in my point of view, not showing the modules if the target is deleted doesn't look like anything I would expect as a consequence of deleting a page, in my sysop experience.

Change 507845 had a related patch set uploaded (by Kosta Harlan; owner: Sbisson):
[mediawiki/extensions/GrowthExperiments@wmf/1.34.0-wmf.3] Recent questions: handle deleted page

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

Change 507589 merged by jenkins-bot:
[mediawiki/extensions/GrowthExperiments@master] Recent questions: handle deleted page

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

Change 507845 merged by jenkins-bot:
[mediawiki/extensions/GrowthExperiments@wmf/1.34.0-wmf.3] Recent questions: handle deleted page

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

Mentioned in SAL (#wikimedia-operations) [2019-05-02T18:33:55Z] <catrope@deploy1001> Synchronized php-1.34.0-wmf.3/extensions/GrowthExperiments/: Don't fatal on deleted pages in 'recent questions' (T222206) (duration: 01m 01s)

Etonkovidova closed this task as Resolved.May 3 2019, 10:15 PM
Etonkovidova added a subscriber: Etonkovidova.

@MMiller_WMF please review the following:

No fatal error is displayed on Homepage when a mentor's User talk page is deleted -which was a scope for the fix for this ticket. Done.

Quesitons:

(1) Per comment form @kostajh

@MMiller_WMF I suggested on the patch that we should prevent the mentorship module from rendering at all in this case (also, same for help module if the help desk title is deleted). Otherwise, if a user goes to post a question on a deleted page, that page will be newly created, and that doesn't seem like desired behavior. Can you let us know what you'd like to have happen?

In the screenshot, ET13 goes on to delete mentor's User talk pages, and mentees by posting the questions revive the pages.

(2) If a deleted mentor's User talk page gets restored after additional questions were posted, it's restored in a weird state when only questions posted after the page was deleted are displayed.

(3) If a mentor's page is moved - the posted questions will re-created the page (just as in the above case), disregarding redirect link on a page. So, it would look like this - ET10 is a mentor and ET203 is a mentee.

Etonkovidova reopened this task as Open.May 3 2019, 10:16 PM
Etonkovidova moved this task from QA to Needs PM Review on the Growth-Team (Current Sprint) board.

(2) If a deleted mentor's User talk page gets restored after additional questions were posted, it's restored in a weird state when only questions posted after the page was deleted are displayed.

That's normal, isn't it? It's caused by the fact that revisions are actually snapshots of the page, not diffs. Deletion is just moving revisions away to an archive table (restoring is the opposite process). If I ask a question, a snapshot is stored. Then, I delete the page, which causes the revision to be moved away. Then, I ask another question - create another snapshot. This snapshot doesn't contain anything from the previous, the page was empty at this point. Then, if I restore the page, the previous snapshot (with a different question), is moved back, which makes it show like the first question that was asked after deletion deletes the previous one. It's something that's happening by-design. Changing the behaviour would probably require a lot of extra work, that might require a lot of lengthy discussions - probably not worth the effort.

(3) If a mentor's page is moved - the posted questions will re-created the page (just as in the above case), disregarding redirect link on a page. So, it would look like this - ET10 is a mentor and ET203 is a mentee.

Isn't this a scope of T222208: Homepage: If a talkpage is archived by moving, it is not considered as archivation by the recent questions feature?

@Urbanecm I think the main problem is that mentor's User page (deleted or moved) gets re-created automatically when a new question is posted.
Agree that what you described is a normal workflow for deleted pages. However, for mentor's User talk page the workflow is interrupted.

  1. Mentee asks a question - Mentor's User page has now most recent questions+previous quesitons
  2. Mentor's User page is deleted.
  3. Mentee asks a question again - Mentor's User page gets recreated with just one question.
  4. Special:Undelete warns that "If a new page with the same name has been created since the deletion, the restored revisions will be merged with the new page's history, and the current revision of the live page will not be automatically replaced. Be careful not to do this unless you specifically intend to merge the histories of the two pages."

As a result of restoring, I'll have User talk page with the most recent question displayed (the one that was posted after deletion) and a history with all previous records.

Regarding the following

(3) If a mentor's page is moved - the posted questions will re-created the page (just as in the above case), disregarding redirect link on a page. So, it would look like this - ET10 is a mentor and ET203 is a mentee.

Isn't this a scope of T222208: Homepage: If a talkpage is archived by moving, it is not considered as archivation by the recent questions feature?

The issue of moved Mentor User page is more general, in my opinion. Having questions archived when Mentor User talk page is moved could be one of the solutions (although there might be different reasons for moving User talk pages not only for purposes of archiving User talk pages). Different wikis (and different mentors) may have different approaches to it.

@Etonkovidova @Urbanecm @kostajh -- I think it is fine for the page to be newly created if a user tries to post a question on a deleted User Talk page. I can't think of a reason why a user would want their User Talk page to be permanently deleted. Do those situations exist (@Urbanecm and @Trizek-WMF)? If the issue is that the mentor is deleting their page because of what the newcomer posted, then the solution is to block that newcomer, not leave their page permanently deleted.

Taking all this together, I think that the change as implemented on this task is right. Is there something I'm missing?

@MMiller_WMF User talk pages are usually not deleted, because we want to have talk pages permanently archived. Deletion happens in cases a new talk page is a vandalism. My point is that talk page _can_ be deleted, and homepage probably shouldn't throw fatal when that happens :-).

BTW, I just tested this at test.wikipedia.org, and it appears to be resolved. Is there a reason to keep this task opened?

MMiller_WMF closed this task as Resolved.Jun 6 2019, 6:07 PM

Okay, so I think we have implemented and deployed the right change. I'm resolving this.