Skip to main content ↑ ↓
Something like prepareDBSettingContext and saveDBSettings... Started by emanuele · · Read 11833 times 0 Members and 1 Guest are viewing this topic. previous topic - next topic

Something like prepareDBSettingContext and saveDBSettings...

...that can be used for any kind of thing?

 emanuele dreams something like that, so that we could get rid of the last few templates in the admin panel in favour of just one... O:-)

I imagine the validation class is part of it, am I wrong?
Bugs creator.
Features destroyer.
Template killer.

Re: Something like prepareDBSettingContext and saveDBSettings...

Reply #1 –

I have no idea what you're dreaming about, but I like the idea of fewer admin templates. :D
Master of Expletives: Now with improved family f@&king friendliness! :D

Sources code: making easy front end changes difficult since 1873. :P

Re: Something like prepareDBSettingContext and saveDBSettings...

Reply #2 –

Meh, unless you guys have done serious changes, prepareDBSettingContext and saveDBSettings are geared totally towards pages of settings that fit into the show_settings template scenario. Anything that doesn't likely needs not only its own template but also a lot of handling and logic.

(It's not like there aren't places where fudging is done to make it fit into the prepareDBSettingsContext mindset, e.g. warning system combining multiple values into one $modSettings entry)

Having one uber template is cool but it can get very warty doing more than it originally intended.

Re: Something like prepareDBSettingContext and saveDBSettings...

Reply #3 –

I did some kind of hacking (though the result was quite ugly) a while ago:
https://github.com/emanuele45/playpen/commits/general_settings
but nothing that I would be proud of. O:-)

But the topic is not about what has been already done, but more what it could be done in "the future".
Some templates are indeed difficult to remove (permissions).
Others are much easier (see themes that I did in my experiments and I'm pretty sure new/edit boards too would be easy to replace and more).
I don't expect to cover everything, but at least those that are already based on a default style, but for some reason or another using a custom one.
Bugs creator.
Features destroyer.
Template killer.

Re: Something like prepareDBSettingContext and saveDBSettings...

Reply #4 –

I did one twist I on what saveDBSettings does in a new function, that basically allows you to use a standard confg_vars array, followed by a prepareDBSettingContext and then our buddy show_settings for display as normal. 

The difference is that instead of adding rows per config var it instead interprets the config vars as the table cols ... so it adds/edits a single row to a specified table, with the supplied data as the insert col values.  Could be lots more flexible but suited a need I had at the time and helped me avoid creating more templates.

So its yet another in the line of prepareDBSettingContext and saveDBSettings

Re: Something like prepareDBSettingContext and saveDBSettings...

Reply #5 –

QuoteBut the topic is not about what has been already done, but more what it could be done in "the future".

Sure, but One Function To Rule Them All will generally end up creating more and more exceptions as you find them. If something fits, use it, if not, accept the extra template. Extra templates aren't necessarily a curse ;)

 Arantor is tackling the new permissions UI... is... interesting...

Re: Something like prepareDBSettingContext and saveDBSettings...

Reply #6 –

Quote from: Arantor – Sure, but One Function To Rule Them All will generally end up creating more and more exceptions as you find them. If something fits, use it, if not, accept the extra template. Extra templates aren't necessarily a curse ;)
 emanuele is template killer 8)
Bugs creator.
Features destroyer.
Template killer.

Re: Something like prepareDBSettingContext and saveDBSettings...

Reply #7 –

I took a look at this, and started to refactor it:
https://github.com/norv/elkarte/compare/settings
Please note that those static methods shouldn't really be static, but for quick refactoring I just did them this way.

Next: instance methods, initialize the instance with $config_vars in the callers, and go from there (prepare() or save() or whatnot.)
Also, probably: add "standard" validation if possible instead of hard-coding it.
Also, could make it handle more variations of this form (but as pointed out by Arantor, I think we should be careful because this is a set of very specialized operations, and generalizing them might go right or wrong).

I think both points of view are correct, but one thing is, I think the route taken in the general_settings branch is the wrong way to go. It adds "complexity" to a specialized set of functions, instead of simplifying things. Small example: this call to saveDBSettings() is not understandable, you have to go re-read the function to find a special case on ===true for a parameter called $callback, and it actually means something else unrelated to callback. Granted, this bit is minor, the parameter override array and even subtemplate reuse are somehow stranger (to the client code, aka reader/customizer).

Lets put it this way: we have those 4 functions for settings stuff (plus template stuff). They do what they're supposed to do, for every area in admin. Others seem to "get close" to do a somewhat similar thing. They're all about settings and permissions in settings screens.
Ok. What about taking things step by step. The above is a take at that. It wants to go towards some real handling of those forms (and basically only those with a load of settings), in specialized classes.
The best moment for testing your PR is right after you merge it. Can't miss with that one.
 

Re: Something like prepareDBSettingContext and saveDBSettings...

Reply #8 –

I think the validation class and some other tricks cover pretty much all of that.
Bugs creator.
Features destroyer.
Template killer.