Page MenuHomePhabricator

Fixup EchoAttributeManager::ATTR_LOCATORS usages to use better callables
Closed, ResolvedPublic

Description

There's various cases of not great looking callables when EchoAttributeManager::ATTR_LOCATORS is used.

For example, in OAuth there is EchoAttributeManager::ATTR_LOCATORS => [ Utils::class . '::locateUsersToNotify' ],.

AbuseFilter has (more brackets than I'd expect), but it works:

			EchoAttributeManager::ATTR_LOCATORS => [
				[
					[ EchoUserLocator::class, 'locateFromEventExtra' ],
					[ 'user' ]
				]
			],

So the OAuth example should be updated to EchoAttributeManager::ATTR_LOCATORS => [ [ [ Utils::class, 'locateUsersToNotify' ] ] ],

From what I can see, the format isn't well documented. Not in code, or on MediaWiki.org...

Other usages around are fully stringified...

			EchoAttributeManager::ATTR_LOCATORS => [
				'EchoUserLocator::locateEventAgent'
			],

Would be nice to update most (all?) of these to use ::class or similar.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

Change 871912 had a related patch set uploaded (by Reedy; author: Reedy):

[mediawiki/extensions/ContentTranslation@master] Hooks: Use better callable with EchoAttributeManager::ATTR_LOCATORS

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

Change 871913 had a related patch set uploaded (by Reedy; author: Reedy):

[mediawiki/extensions/Wikibase@master] EchoSetupHookHandler: Use better callable with EchoAttributeManager::ATTR_LOCATORS

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

Change 871914 had a related patch set uploaded (by Reedy; author: Reedy):

[mediawiki/extensions/TheWikipediaLibrary@master] Hooks: Use better callable with EchoAttributeManager::ATTR_LOCATORS

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

Change 871915 had a related patch set uploaded (by Reedy; author: Reedy):

[mediawiki/extensions/LoginNotify@master] Hooks: Use better callable with EchoAttributeManager::ATTR_LOCATORS

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

Change 871916 had a related patch set uploaded (by Reedy; author: Reedy):

[mediawiki/extensions/GrowthExperiments@master] MentorHooks: Use better callable with EchoAttributeManager::ATTR_LOCATORS

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

IMO we should rather fix evaluateUserCallable which messes up callables which are arrays (which I suppose is why such callables have to be packaged in an extra array).

IMO we should rather fix evaluateUserCallable which messes up callables which are arrays (which I suppose is why such callables have to be packaged in an extra array).

I'd definitely still like to fix up 'EchoUserLocator::locateEventAgent'/ Utils::class . '::locateUsersToNotify' type anyway (for the usual reasons; searchability in IDE's etc). Would be good if we could do that without having to add an extra nested array though (and therefore work more like we would expect based on other callables in MW)!

Change 871914 merged by jenkins-bot:

[mediawiki/extensions/TheWikipediaLibrary@master] Hooks: Use better callable with EchoAttributeManager::ATTR_LOCATORS

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

Change 871912 merged by jenkins-bot:

[mediawiki/extensions/ContentTranslation@master] Hooks: Use better callable with EchoAttributeManager::ATTR_LOCATORS

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

Change 871916 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] MentorHooks: Use better callable with EchoAttributeManager::ATTR_LOCATORS

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

Change 871915 merged by jenkins-bot:

[mediawiki/extensions/LoginNotify@master] Hooks: Use better callable with EchoAttributeManager::ATTR_LOCATORS

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

Change 871913 merged by jenkins-bot:

[mediawiki/extensions/Wikibase@master] EchoSetupHookHandler: Use better callable with EchoAttributeManager::ATTR_LOCATORS

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

Tgr assigned this task to Reedy.

Thank you!
This is finished, right?