Page MenuHomePhabricator

Update NewUserMessage to use AuthManager
Closed, ResolvedPublic

Description

See parent task and T110414#1578206.

NewUserMessage: uses AuthPluginAutoCreate to post welcome messages. Should use LocalUserCreated instead.

Related Objects

Event Timeline

Tgr created this task.Aug 27 2015, 2:23 AM
Tgr updated the task description. (Show Details)
Tgr raised the priority of this task from to Needs Triage.
Tgr added subscribers: Liuxinyu970226, Krenair, Billinghurst and 2 others.
Restricted Application added a subscriber: StudiesWorld. · View Herald TranscriptDec 14 2015, 11:28 AM
Dereckson updated the task description. (Show Details)Jan 20 2016, 3:22 AM
Dereckson set Security to None.

@Tgr Could both AddNewAccount and LocalUserCreated be fired in some scenario?

Dereckson triaged this task as Normal priority.

Change 265210 had a related patch set uploaded (by Dereckson):
Support LocalUserCreated hook

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

Anomie added a subscriber: Anomie.Jan 21 2016, 5:19 PM

@Tgr Could both AddNewAccount and LocalUserCreated be fired in some scenario?

At the moment, I'm thinking probably not outside of the craziness done by the Translate extension's ApiTranslateSandbox.php. But it all depends on what exactly we decide to do about AddNewAccount.

Tgr added a comment.Jan 21 2016, 5:43 PM

It seems good form to call old hooks for a B/C time period. IMO the extension has an easier time making sure no work is duplicated (either use a global flag or just don't use AddNewAccount if the AuthManager class exists) than core making sure the right hook is called on each extension.

Thank you both for the input, I've added a check for AuthManager class, to avoid to process AddNewAccount callback when it's present.

I see the extension still uses the 'AddNewAccount' and 'AuthPluginAutoCreate' hooks, which are both deprecated in favor of 'LocalUserCreated', and does not use the 'LocalUserCreated' hook.

Note that LocalUserCreated has been in MediaWiki since 1.26wmf24, so you should be able to just switch to using the new hook by now. The old hooks will still be called alongside the new hook in some cases:

  • Pre-AuthManager, or when it's not enabled.
  • From web UI account creations under AuthManager.
  • Auto-creations under AuthManager will still call AuthPluginAutoCreate, but will raise a deprecation warning.

Another thing to consider: You can use User::newSystemUser() in NewUserMessage::fetchEditor() to account for the possibility that someone somehow created the messaging account before your extension had a chance to do it. Unless, of course, you want to allow for the possibility that these automatic messages are seen coming from some human's account rather than a bot-like account owned by the extension.

demon raised the priority of this task from Normal to High.May 31 2016, 4:16 PM
Anomie closed this task as Resolved.May 31 2016, 5:21 PM

Change 265210 merged by jenkins-bot:
Support LocalUserCreated hook

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

Change 292168 had a related patch set uploaded (by Gergő Tisza):
Support LocalUserCreated hook

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

Change 292168 merged by jenkins-bot:
Support LocalUserCreated hook

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

Change 296516 had a related patch set uploaded (by Gergő Tisza):
Support LocalUserCreated hook

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

Change 296516 merged by jenkins-bot:
Support LocalUserCreated hook

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