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 raised the priority of this task from to Needs Triage.
Tgr updated the task description. (Show Details)
Tgr added subscribers: Liuxinyu970226, Krenair, Billinghurst and 2 others.
Dereckson set Security to None.

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

Dereckson triaged this task as Medium priority.

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

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

@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.

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 Medium to High.May 31 2016, 4:16 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