Add `preprocess()` logic for `InstallPluginsPage.php`

[lgedgar]

• cleanup logic so easier to read/maintain
• make sure settings are only saved upon form POST

this also tweaks how plugin settings are retrieved from config file generally, to better handle namespace if present

This of course should be vetted thoroughly.. I believe the only behavioral change should be how a v1 plugin settings are got from config file, as mentioned above. And, settings are no longer saved just by visiting the page.

My end goal here is to make sure DB settings are "overwritten" but I want to avoid truncating that table, so as to allow "ad-hoc" settings to be added, and this page should not touch them if present.

Anyway wanted to make sure this was an okay direction first etc. Let me know what you think.

[lgedgar]

I haven't really thought about this in quite a while, and e.g. I might even code it differently now, who knows. But it'd still be nice to get some changes made; this PR may or may not be the best way.

Mentioned above sort of but I want a settings table that is not going to be "messed with" - some settings I would want to add directly e.g. via SQL as opposed to crafting an admin UI page for them (for e.g. testing with feature flags). Others though may warrant UI exposure. The plugin settings could either remain a separate table, or the "global settings" table could contain plugin settings along with others. As long as there is some prefix convention for namespace etc.

This PR was an attempt I think to "convert" the existing plugin settings table, to be the global table. Main sticking point with that being, the Plugins page needs to stop destroying the settings table for each page load.

So wondering what you think here generally. No hurry still at this point though.

[gohanman]

I had forgot about this. The changes look pretty reasonable.

I have mixed feelings about settings without a UI. I can see the case for it with something like enabling SFTP for SPINS. Once it's enabled, it's highly unlikely there would ever be a reason to change it back. But my inclination would be to clean up the UI by eventually dropping the old FTP functionality and eliminating the setting at that point.

[lgedgar]

Thanks for looking. Agreed that many settings warrant UI exposure, but I find that some/many never do. For instance:

When first developing something, it may need some production testing before an upstream PR happens. I'd use a feature flag setting, no UI for it, so I know the setting will only be enabled if someone knows what they're doing. Once the feature is complete, fully vetted/tested and PR lands upstream then the setting goes away forever.

OTOH if a setting turns out to be a "good idea" generally, then by all means incorporate it in the admin UI when that happens.

I'd also want to write arbitrary settings as needed, from the running code. I use this approach a fair bit in Rattail, main example being to record a "last run time" for some utility, which when it runs will check the last run time to affect behavior etc.

In CORE it seems this could even be used to get away from the COOP_ID magic setting. So instead of this:

if ($config->get('COOP_ID') == 'WFC') {
    // hard-coded magic
}

We would do this:

if ($config->getSetting('core.some_very_unique_use_case') == 'true') {
    // generic magic code
    $foo = $config->getSetting('core.some_very_unique_use_case.foo');
    $bar = $config->getSetting('core.some_very_unique_use_case.bar');
    // now do something based on foo/bar values
}

This would require a different setting for each such check, but the setting names could be descriptive etc. Note the use of core. namespace which probably CORE should reserve for itself. A given org would use its own namespace (e.g. wfc.) to stay clear of everything else, for any settings which are only ever meant for that org.

Anyway I could go on and on about this. But the key is, it's all made possible with a very simple "settings"table which stores only string values using a flat namespace. Any admin UI must take care to only modify/remove settings for which it is responsible. Which is pretty straightforward as long as it sticks to records within a given namespace.

I still haven't looked close at this PR again yet though, so more interested in getting the overall idea hashed out at this point. Let me know..

[lgedgar]

BTW it's possible there are PHP libs out there which might be useful. I suspect maybe they're overkill and may not do exactly what we'd want anyway. I know in Rattail my Config class/logic is custom even though I tried to use existing libs where possible. But thought I'd mention just in case, my first search found:

• https://packagist.org/packages/hassankhan/config
• https://packagist.org/packages/league/config
• https://packagist.org/packages/symfony/config

[gohanman]

This sounds reasonable enough to move forward with. I'd suggest adding a column to the settings table schema like hidden or dbOnly. It'd serve both as a way for a person to quickly assess what's going on under the hood and for the UI to know which records it shouldn't touch.

[lgedgar]

Okay great..will do another PR when I get around to it. This one probably will get abandoned but I'll leave it open for now.