Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Admin controllers #280

Merged
merged 77 commits into from Mar 31, 2013
Merged

Admin controllers #280

merged 77 commits into from Mar 31, 2013

Conversation

norv
Copy link
Contributor

@norv norv commented Mar 29, 2013

This PR has a higher-level (file/class/function) redesign of most admin controllers, to object oriented controllers, and a few rewrites of methods. It's another step (or a few of them) in preparation for the MVCish code design we target.

I'm not sure we want it in as it is, since some aspects are very basic (and it looks terrible :P).
Update0: well not that bad anymore.

Note: the issue of forwarding directly to actions is still unsolved in this PR, updates of the menu and dispatching mechanism will be necessary for that, apart from more fully OOP controllers. (more than this)

On yet the brighter side, most of these is what will stay (tm). :)

Update1: I added the previous branch on @emanuele45 's Settings... Which I'm not happy with... yet.
Update2: admin is almost finished with the current changes. Settings_Form is being used in admin, for settings pages. The class is used through its interface thereby allows for internal changes to improve it or replace it completely, as long as it behaves the same on the outside.

Update3: I added Action class. This is a small class used to 'know' the actions and subactions of the current request, the permissions for them if any specified, the default sub-action to execute, and potentially a few other things like this. The class is meant to get all common code of the sub-actions handling, in one place, and forward to the appropriate method for the current request. (a dispatcher/router at the level of controller, as the dispatcher class is at the level of the entire site).

norv added 30 commits March 29, 2013 00:59
Signed-off-by: Norv <a.w.norv@gmail.com>
Signed-off-by: Norv <a.w.norv@gmail.com>
Signed-off-by: Norv <a.w.norv@gmail.com>
Signed-off-by: Norv <a.w.norv@gmail.com>
Signed-off-by: Norv <a.w.norv@gmail.com>
Signed-off-by: Norv <a.w.norv@gmail.com>
Signed-off-by: Norv <a.w.norv@gmail.com>
…t logging.

Signed-off-by: Norv <a.w.norv@gmail.com>
Signed-off-by: Norv <a.w.norv@gmail.com>
Utility functions in the file stay as functions.

Signed-off-by: Norv <a.w.norv@gmail.com>
Signed-off-by: Norv <a.w.norv@gmail.com>
Signed-off-by: Norv <a.w.norv@gmail.com>
(otherwise known as - how to be worse before being better.)

Signed-off-by: Norv <a.w.norv@gmail.com>
Signed-off-by: Norv <a.w.norv@gmail.com>
Signed-off-by: Norv <a.w.norv@gmail.com>
…, for settings.

Signed-off-by: Norv <a.w.norv@gmail.com>
Signed-off-by: Norv <a.w.norv@gmail.com>
Signed-off-by: Norv <a.w.norv@gmail.com>
Signed-off-by: Norv <a.w.norv@gmail.com>
… oriented controller.

Signed-off-by: Norv <a.w.norv@gmail.com>
Signed-off-by: Norv <a.w.norv@gmail.com>
Used action_main() as main dispatcher.

Signed-off-by: Norv <a.w.norv@gmail.com>
Signed-off-by: Norv <a.w.norv@gmail.com>
Signed-off-by: Norv <a.w.norv@gmail.com>
Signed-off-by: Norv <a.w.norv@gmail.com>
Signed-off-by: Norv <a.w.norv@gmail.com>
norv added 26 commits March 29, 2013 15:49
Fixes. Tweak Settings to start instantiating it and use it normally.

Signed-off-by: Norv <a.w.norv@gmail.com>
Signed-off-by: Norv <a.w.norv@gmail.com>
…with one of these.

Signed-off-by: Norv <a.w.norv@gmail.com>
Signed-off-by: Norv <a.w.norv@gmail.com>
Signed-off-by: Norv <a.w.norv@gmail.com>
Signed-off-by: Norv <a.w.norv@gmail.com>
Signed-off-by: Norv <a.w.norv@gmail.com>
Fix admin search. (as usual :P)

Signed-off-by: Norv <a.w.norv@gmail.com>
…, rework avatars and attach to forms.

Also added action_balancingSettings_save(), currently unused.
Introduced some redundancy (meant as temporary, during refactoring/rewriting stuff).
Deprecated a few settings() methods. (because some are replaced with form initialization,
_initXxxForm; apart from initializing forms with them, they seem only used for admin search,
I'm not sure yet if it's worth keeping settings() in each controller only for that - will see later.)

Signed-off-by: Norv <a.w.norv@gmail.com>
…xes.

Signed-off-by: Norv <a.w.norv@gmail.com>
Signed-off-by: Norv <a.w.norv@gmail.com>
Signed-off-by: Norv <a.w.norv@gmail.com>
…and call it.

Rework the existing "form-based" controllers to use it: get rid of some redundancy in subactions calls
and make some of the existing arrays more 'alike'.

Signed-off-by: Norv <a.w.norv@gmail.com>
…nd some to Actions.

Signed-off-by: Norv <a.w.norv@gmail.com>
Signed-off-by: Norv <a.w.norv@gmail.com>
Signed-off-by: Norv <a.w.norv@gmail.com>
Update News, subscriptions, membergroups admin settings implementation to the "settings forms".

Signed-off-by: Norv <a.w.norv@gmail.com>
Signed-off-by: Norv <a.w.norv@gmail.com>
…re any, settings forms placeholders.

Signed-off-by: Norv <a.w.norv@gmail.com>
Signed-off-by: Norv <a.w.norv@gmail.com>
Signed-off-by: Norv <a.w.norv@gmail.com>
…on for action class.

Signed-off-by: Norv <a.w.norv@gmail.com>
… each action/subaction.

Added more permission checks (redundant atm, but that may change.)

Signed-off-by: Norv <a.w.norv@gmail.com>
…ontroller and add ManageAddonSettings.

An unfixed trick remains: a load method.

Signed-off-by: Norv <a.w.norv@gmail.com>
norv added a commit that referenced this pull request Mar 31, 2013
@norv norv merged commit 02d80fe into elkarte:master Mar 31, 2013
@norv
Copy link
Contributor Author

norv commented Mar 31, 2013

I'm going to be shot for this, but there we go: the branch is stable as far as I can tell, I'm merging it, before making another round of changes.
It contains some redundancies now (I removed some, I added others!), which are probably documented (I hope) and more importantly:

  • the general patterns of admin controllers are stable
  • the naming patterns are mostly established and respected (there are some exceptions, notably the files themselves)
  • the Action class is handling the subactions, permissions, potentially other conditions, with the purpose to forward to the right subaction, or, if it is the case, to the right 'activity' (see maintenance). The dispatcher forwards usually to the main method, the action object of each controller takes over and forwards to the appropriate method. (they might also be combined at some point)
  • the Settings_Form class is underdeveloped, it offers the basic interface for save/prepare configuration variables. I'm looking to replace it with a proper form class.
  • Controller classes should ideally have the action_xxx() methods (almost exclusively, they process a user action, and that's almost all they need to do). The functions along their side, in the file, are utility functions for their use. Some may (should) be moved to .subs.
  • Queries have to disappear from action_xxx() methods. (they might remain in list_xxx() or such methods, but not action handlers. Those only control the flow, don't work with databases.)
  • This PR has no changes to templates. There are several templates that should probably be updated, at the level of functions/new files (not their code). To keep inline with the /admin controller files. i.e. mostly, ManageXxx should "have" a corresponding ManageXxx template, and action_xxx() a corresponding template_xxx().
  • Similar for language files. Makes things easier to find.
  • A couple of refactorings such as creation of a new Manage file have been included. I split the bigger files which were managing more than one area: ideally Manage files are per area, and some even per admin screen.

@Spuds
Copy link
Contributor

Spuds commented Mar 31, 2013

I'm going to be shot for this, but there we go: the branch is stable as far as I can tell, I'm merging it, before making another round of changes.

I did go thought this a couple of times and did not see anything really amiss 😃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants