Right now, GlobalPreferences is a bit hacky and could probably benefit from some core changes. Figure out how we could make the code less fragile and more future-proof, but without rewriting the whole preferences infrastructure.
|Resolved||Niharika||T139145 global disabling of compact language list|
|Resolved||None||T16950 [Epic] Support global preferences on Wikimedia wikis|
|Resolved||Samwilson||T178267 Dynamically enable/disable the related pref when toggling "Set a local exception for this global preference"|
|Resolved||Samwilson||T178044 Add local exceptions to global prefs|
|Resolved||Samwilson||T68869 Update Global preferences UI|
|Resolved||Samwilson||T173476 Investigation: Figure out what needs to be rewritten in GlobalPreferences|
|Resolved||Samwilson||T178449 RFC: How to modify all preferences?|
- Mentioned In
- T178449: RFC: How to modify all preferences?
T172029: Fix a gotcha with hook handlers order
T173474: Investigation: How will extensions opt specific preferences out of GlobalPreferences?
- Mentioned Here
- T68869: Update Global preferences UI
rMW05a7239ab382: Make it possible for subclasses to provide a different form
T172029: Fix a gotcha with hook handlers order
My initial thoughts:
- Create a service in the core, called e.g. PreferencesProvider
- Move some stuff into it:
- Stuff from User class that operate on preferences and are contributing to it being a god class: getDefaultOptions(), loadOptions(), saveOptions(), etc.
- Make Preferences an instantiable class and make PreferencesProvider return it.
- Make core factory for PreferencesProvider overridable.
- Have an override in GlobalPreferences
- Less reliance on hooks
- This extension avoids becoming an annoying analog of CentralAuth that embeds itself into too many things
- Way more stable unit tests
- This is a relatively large rewrite, though it still can be argued whether it falls within this ticket's requirements
without rewriting the whole preferences infrastructure
That sounds reasonable to me. How would fallback work to local preferences if the user doesn't have a global preference set for that option?
The other hacky part is the way that Special:GlobalPreferences is created, and how it uses the late hook and tries to add checkboxes for everything. I'd rather we subclass PreferencesForm, and create a custom HTMLFormField that adds the "use this globally" checkbox. I think Sam was looking into something similar? (given rMW05a7239ab382: Make it possible for subclasses to provide a different form)
Sounds good Max.
And yeah, as part of T68869: Update Global preferences UI I've added GlobalPreferencesForm which so far doesn't do very much other than add a section header and change the form layout from table-based to div-based. I've experimented with overriding all the HTMLFormField types, in order to add a common parent to them which would then insert the extra checkbox.
The idea would be to have two PreferencesForm subclasses in this extension wouldn't it? GlobalPreferencesForm and LocalPreferencesForm? The former to make the form fields all use new custom subclasses of themselves, and the latter to add the links and help texts about what's global?
If we do customize the form class, then we can get rid of GlobalPreferences\Hooks::onGetPreferences() and don't need to introduce a new hook for processing the fully-built preferences array. Shall I do that in T68869?
As the first part of this, I've added a new hook to core, called PreferenceFactory. This will mean we can replace the Preferences object that's used by SpecialPreferences with our own LocalPreferences object, and so do whatever we want (i.e. modify the form and the preferences list appropriately).
Do you think this is an okay direction?
I realise that it's a bit of an odd way to treat the Preferences class, which is never (as it stands) instantiated. But I'd suggest that it should be, and that its static methods should be refctored (e.g. almost all of them use a context and/or a user, and these should be injected to the class). I've not done any great refactoring here, though, because I'm just doing the minimum required for the task at hand.
The patch I've got for GlobalPreferences https://gerrit.wikimedia.org/r/#/c/370152/ uses this new hook, so have a look at that to see what I'm on about.
This patch is ready for review: https://gerrit.wikimedia.org/r/#/c/370152
and is now live on the commtech test wiki: http://localhost/wikimedia/wiki1/index.php/Special:GlobalPreferences
There are a bunch of things lacking still, but they're recorded under separate tickets. This one just focuses on switching to the new PreferencesFactory, and fixing for T68869: Update Global preferences UI.