« git problems for non-gits.
Is the admin area in urgent need of an overhaul?
Need opinions on a variable name... »

Nao

  • Dadman with a boy
  • Posts: 16,082
Is the admin area in urgent need of an overhaul?
« on November 20th, 2013, 11:21 PM »
This was Pete's domain. I barely ever touched it in the 3 years I worked on Wedge.

Ever since I decided to remove theme support, I've had to cope with admin. It's just horrible.

Perhaps the worst offender is the 'general settings' page, which uses a completely different format for option lists. As far as I can tell, the ONLY reason for this difference is that these settings can either be saved to the database (as usual), or to a 'file' (i.e. Settings.php), but I don't really see why this can't be put into a special setting in prepareDBSettingContext, e.g. array('check', 'my_variable', 'where' => 'file'),... And voilà, the setting is going to be saved to the file instead.

What do you think? Can anyone think of a reason why it would fail?

Also, this special list has up to FIVE different non-keyed items. That's madness. name, label, target (file/db), type, size and help. It also accepts a keyed item, 'subtext'. It really, really doesn't make sense. The regular system has type, name/label/help (they're all linked together), and size/select-box content (depending on whether it's an int or an array.) It's much simpler to me, because usually all of these variables use the same $txt, $helptxt etc. for their descriptions.

Pages that use the 'weird' system:
- General settings
- Mail settings (webmaster e-mail goes to file)
- Server settings (maintenance status goes to file)
- Database settings (obviously)
- Cookie settings (cookie name goes to file)
- Debug settings (db_show_debug goes to file)

Apart from these, the rest uses prepareDBSettingContext, which is simpler.

Example:

array('autoFixDatabase', $txt['autoFixDatabase'], 'db', 'check', false, 'autoFixDatabase'),

Would become:

array('check', 'autoFixDatabase'),

Well, sure. That's a bit better...


Also, on another note. I'm currently in the mood for getting rid of 'small' admin pages with a reduced number of settings. The 'thing' with SMF 2.0 was to split long pages of settings to multiple pages. As a result, I tend to take some time looking for an option, when I could just hit the general settings page and hope that the option is in there (ctrl+F on the page). I've never been into the custom search engine for the admin area.
One thing that I like about FluxBB is that its entire admin area is split over a limited number of pages; it doesn't have a LOT of options of course, but I appreciated that my test setup of FluxBB was done by simply going to the admin area, looking at the general settings page, going through each option, and selecting what I wanted. In SMF 2.0/Wedge, I have to specifically click to multiple pages just to get the thing started (e.g. instead of general settings > minify JS/CSS, I have to also go to server settings...)
I took the Likes page (2 settings!) and moved it to general settings. I also took general member settings (enable contact lists, etc.), and moved them to the general settings as well.

I'm aware that it's a question of method. There are people who'll prefer the SMF 2 way, and others who'll prefer to have a long page with everything in it, as long as sections are clearly separated (in Wedge, I'm adding headers and big icons next to these headers, that should be more than enough.)
I'm interested in seeing who are the most vocal, though. Personally, I'll be vocal about avoiding multiple pages when they could be merged together. ;)

live627

  • Should five per cent appear too small / Be thankful I don't take it all / 'Cause I'm the taxman, yeah I'm the taxman
  • Posts: 1,670
A confident man keeps quiet.whereas a frightened man keeps talking, hiding his fear.

Nao

  • Dadman with a boy
  • Posts: 16,082
Re: Is the admin area in urgent need of an overhaul?
« Reply #3, on November 21st, 2013, 08:57 PM »
That's pretty much it; prepareServerContextSettings (or something like that) is no longer. It's now merged with the 'regular' version, which I also tweaked to be easier to understand (seriously, some parts of that function were just too complicated for their own good. Had that developer never heard about assigning temp variables anyway..?!)

As a result, all pages can now freely save any variables to the Settings.php file, they just need to specify 'file' => true in their array declaration.
Also, I removed some extra code that didn't make sense to me. $db_show_debug is supposed to be either 'true' or 'false' (not really 'false', anything other than 'true'), but not '1' for instance. I traced this implementation back to SMF 1.1 beta 1 or 2 (I don't have earlier code for the 1.1 branch, and 1.0.x doesn't have $db_show_debug at all), it was really written like that from the start.

Pete had written some check-up code that ensured the variable was set to true or false, rather than 0 or 1. It probably would have been simpler to just go through the whole codebase, as I did, and rewrite all those 'exact' tests ($db_show_debug === true). I'm pretty sure the original code was written like that to avoid having people complain that $db_show_debug = 'false'; in the Settings.php would still show the debug code. Well, Wedge has this in the admin area, you don't need to access the file and update it yourself, so who cares..?

Finally, what I expect to be some 'security' code... Wedge/SMF check all variables that should be written to the file, and ensure they're of the correct format. But there's absolutely no reason they could be modified otherwise. It seems like a waste of time, to me... Maybe it was security code for an older version of PHP, I don't know.

That's pretty much all for today, I think.
Oh, and just for fun, I did a run of unused-globals.php on SMF 2.1, and it fixed over 900 variables automatically. I compared the files, and it's just fine. The only bug I had was because the SMF 2.1 repo is still badly handled. It made me laugh (and I rewrote my script to account for that), but I don't know, it just screams 'fix me!!!', and I don't want to.

Pandos

  • Living on the edge of Wedge
  • Posts: 635
Re: Is the admin area in urgent need of an overhaul?
« Reply #4, on November 22nd, 2013, 07:11 PM »
YouI know I'll like General Settings :)
So for the new look & feel i think that the new Admin-Center should look like this:

General Settings:
All settings that are important to adminstrate the forum. This includes Maintenance, Caching, Timezone, Prettyurl's, Plugins, Themes/Skins, Search Engine-Tracking, News, Likes, Languages, Embedding and Plugins.

Forum:
All related stuff (leave it untouched instead of Antispam, Attachments, Avatars and Embedding).

Media:
Leave as it is. Plus Avatars and Attachments

Permssions:
Including Members, Registration, Infractions, Bans, Antispam, Permissions and Member Options

Server:
Only Server related things as DB, Load Balancing, Proxy Settings and Sessions.
# dpkg-reconfigure brain
error: brain is not installed or configured

Nao

  • Dadman with a boy
  • Posts: 16,082
Re: Is the admin area in urgent need of an overhaul?
« Reply #5, on November 24th, 2013, 01:37 AM »
I moved SE tracking to Maintenance because I feel that anything related to 'logs' should be around the 'log' area. It's not just spider logs, but to me it's where I would expect to find this kind of thing.
Plugins should stay keep their own menu entry because they can have individual menu items for each plugin.

Your Permissions area sounds, to me, a lot like the current Members area... ;)

I may still move data around, eventually. But the #1 problem is with how admin stuff is handled internally. Sometimes, you need to have stuff inside a submenu, other times inside a main menu entry. Just moving Add Plugins out of the submenu and into the main menu was a PITA to implement, and I'm actually thinking of, hmm... Rewriting that stuff, to ensure that one can do what they want and put it where they want, at least for 90% of the current admin data (and 100% of all $settings manipulations, which to me is more important and urgent to 'fix' than other areas, like plugins, maintenance etc.)
Probably not realistic, but I'll give it a try. I have a few ideas on how to do that, of course.
Re: Is the admin area in urgent need of an overhaul?
« Reply #6, on November 25th, 2013, 12:03 AM »
So... Would this layout make sense?

- A 'Settings' drop menu,
- And other drop menus similar to what we have now.

The 'Settings' menu would have ALL settings pages from all other features, allowing you to: (1) quickly find where to enable/disable a setting, (2) determine if a setting can be overridden by users, (3) maybe have 'quick links' (next to each setting section) pointing to other pages in the menu that deal with related items, e.g. the Aeva embedding setting section could have a link to the Site list.

All settings would be, if possible, in as few pages as possible. They'd all be abstracted, i.e. each section would be built the same war ($config_vars), but individual entries could also specify 'prepare' and 'process' (or 'save') array items, which would hold a function name. These would, of course, be callbacks, that tell Wedge to execute some extra code on page load, or when saving the item if it has changed.

This would require/allow merging $settings, $theme and $options together (mostly?), although I don't know if this has been that much of a problem to SMF users so far. It has been to me, but I'm not everyone.

Is it a bit too ambitious? Or uncalled for..? I have a feeling it might be. This rewrite could take weeks (at best days) to complete, maybe this time could be better used on finishing contact lists or privacy, I don't know. :-/
Re: Is the admin area in urgent need of an overhaul?
« Reply #7, on November 25th, 2013, 08:27 AM »
I only got a 'like' on the post above, and I don't know if it's for the former part ('settings menu'), or latter ('time consuming, might be best to work on other things'), and no 'real words' comments, so it's hard to guess.

For now I guess I'll just focus on finishing what's on the grill right now, hmm.

Sara

  • Walking Contradiction
  • Posts: 41
Re: Is the admin area in urgent need of an overhaul?
« Reply #8, on November 25th, 2013, 09:43 PM »
Quote
All settings would be, if possible, in as few pages as possible.
This what stood out at me the most, and I agree to this. 

But I think as of right now just deal with what's on your plate.  The admin center is important, yes.  But I think it can wait.  (my opinion, but one in many)

Nao

  • Dadman with a boy
  • Posts: 16,082
Re: Is the admin area in urgent need of an overhaul?
« Reply #9, on November 25th, 2013, 09:57 PM »
It's just that it's all related...

1- Settings all in one place: can easily mark a variable as 'overridable', meaning I can store everything in $settings and then have users override these settings with their own preference.
2- This allows me to remove $theme entirely, and thus makes it easier to remove theme support entirely.
3- Which then allows me to properly redo the file structure change...
4- And then it flags it as 'publicly viewable', since it's a pain for anyone who's installed a beta before, to see all files being moved, and having to reupload everything and clean up their unused folders.

2 can be done without 1, though. I could, for instance, keep using the theme table, and then when retrieving $themeData, instead of moving options to either $theme or $options[1], a first step could be to merge ALL three arrays together: $options overrides $theme, which overrides $settings. So, basically, $settings = array_merge($settings, $themeData[0], $themeData[$member]) should work just fine. I don't know exactly, since I haven't even tried it, but it should make all things simpler already.
With this half-arsed method, I still have to deal with that strange little 'theme' table that is separate from the 'settings' table, but it's only an internal problem. I also can't retrieve 'overridable' $settings and automatically build a list of them for users to modify in their profile. That's the big annoyance to me, but... Well, I don't know, maybe I'm just too anal about it.

What do you think..?

(I know I'm reaching for opinions a little too obviously, but the relative silence these days in this forum is deafening to me, because I really rely on 'regular user' opinions to determine whether I'm doing things right, or I'm doing changes that are useless to most people, and thus a waste of time that would better be spent elsewhere. But where, ah ah, don't ask me.)
 1. To avoid confusion, Wedge has $settings, $theme and $options, where SMF has respectively $modSettings, $settings and $options, which is one of the strangest ways of doing things because $settings are 'higher up' than $modSettings, in all logic, but in SMF's logic, they aren't, so...

Pandos

  • Living on the edge of Wedge
  • Posts: 635
Re: Is the admin area in urgent need of an overhaul?
« Reply #10, on November 25th, 2013, 11:19 PM »Last edited on November 25th, 2013, 11:27 PM
Sounds cool and I'm a friend of a clean Admin-Center. :)

But I'm also with Sara, because this overhaul will postpone everything again a bit.
But...
There's so much done in the meantime and it's half the way done :)
So go for it.

It's very complicated to contribute something at this stage....

Nao

  • Dadman with a boy
  • Posts: 16,082
Re: Is the admin area in urgent need of an overhaul?
« Reply #11, on November 27th, 2013, 05:04 PM »
Possibly, yes.

Okay, I'll leave this aside for now, with the understanding that if future plugins are broken by a later rewrite, its not my fault, I wanted to do it first :P

emanuele

  • Posts: 125
Re: Is the admin area in urgent need of an overhaul?
« Reply #12, on December 20th, 2013, 10:21 AM »
Quote from Nao on November 21st, 2013, 08:57 PM
Finally, what I expect to be some 'security' code... Wedge/SMF check all variables that should be written to the file, and ensure they're of the correct format. But there's absolutely no reason they could be modified otherwise. It seems like a waste of time, to me... Maybe it was security code for an older version of PHP, I don't know.
Of course not knowing the code, I'm not sure which one is it or how it was modified, though when you write anything to a file you have to be 100% sure what you are writing is exactly what you expect it to be. ;)
A couple of similar things were reported a while ago as security issues in SMF and were in the 2.0.2 and 2.0.3 patches (IIRC).

Nao

  • Dadman with a boy
  • Posts: 16,082
Re: Is the admin area in urgent need of an overhaul?
« Reply #13, on December 20th, 2013, 03:25 PM »
AFAIK Pete updated Wedge to SMF 2.0.5 along the years, so these patches should be in, if need was.
Regarding what I was talking about, let's say you have an admin form with ('check', 'MyValue'), where Wedge then generates an <input type="checkbox" name="MyValue">, etc. Once it gets submitted to the server, SMF (and previously Wedge) would:
1- build a list of variable types, including 'MyValue', so let's say this one is in $config_bools, corresponding to a checkbox type.
2- go through all variables, and test their recorded type.
3- When it reaches the one that interests us, it makes sure that it belongs to the $config_bools.
4- if yes, then it records it.

So, basically, it ensures that MyValue (likely defined in the same file AND SAME PAGE LOAD as ('check', 'MyValue') is part of what the same file defines as $config_bools. Which means, it's a complete waste of time, because $config_var is first defined, then immediately tested against.
The only thing that could pose a security risk, is if you have a mod that changes the definition to something else, or changes the $config_bools array to something else. Which, (1) they'd have no reason to do, (2) would likely have absolutely zero impact on security.

See what I mean..?

emanuele

  • Posts: 125
Re: Is the admin area in urgent need of an overhaul?
« Reply #14, on December 21st, 2013, 01:50 AM »
I think.
If you already know the variable is a check and it is "cast" accordingly (or if you verify is one of the allowed types) then there should be no need for a second variable with a repetition of the type, yes, I agree. :)
« git problems for non-gits.
Need opinions on a variable name... »