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

Controllers (part ...) #576

Merged
merged 34 commits into from Jun 26, 2013
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
2eb810f
Add abstract base class Action_Controller.
norv Jun 21, 2013
52c0728
Derive all admin controllers from Action_Controller.
norv Jun 21, 2013
5c95d4b
Make two dozen user/mod controllers extend Action_Controller.
norv Jun 21, 2013
ac269a7
Update more user controllers to extend the base class.
norv Jun 21, 2013
b6aa853
Refactor to action controller subclasses. Tweak modcenter.
norv Jun 21, 2013
dda2464
Rework createList() callbacks in moderation center controller:
norv Jun 21, 2013
8384a69
Tweaks for list_ callbacks, add methods visibility.
norv Jun 21, 2013
a024408
Fix inclusion with TESTDIR.
norv Jun 21, 2013
a3e36cf
Move Action_Controller to core files loaded by default.. SSI too.
norv Jun 22, 2013
3c43d17
Another batch of user controllers, updated to inherit Action_Controller.
norv Jun 22, 2013
ae92594
ProfileHistory controller. Make it a class; main methods visibility,
norv Jun 22, 2013
4e795f9
ProfileInfo, into a class. (wrapping methods only)
norv Jun 22, 2013
0fd1ffa
Extend ProfileInfo controller from Action_Controller and add missing
norv Jun 22, 2013
ca07460
action_activateaccount() was supposed to be in ProfileAccount..
norv Jun 22, 2013
c7aee79
ProfileSubscriptions controller, make into a class, extend Action_Con…
norv Jun 22, 2013
cef6e3d
Rework ProfileOptions controller as a class, initial pass.
norv Jun 22, 2013
60a2643
Rework the list_ callbacks for ProfileOptions controller:
norv Jun 22, 2013
805f16c
Rework Xml requests: XmlPreview controller handles previews.
norv Jun 22, 2013
9288b92
Xml controllers extend Action_Controller. Methods visibility.
norv Jun 22, 2013
2771278
Batch of method visibilities updates. (mostly public for actions hand…
norv Jun 22, 2013
4c8b519
Attachment controller extends Action_Controller (looks it was missed.)
norv Jun 22, 2013
22d4727
Tweak dispatcher.
norv Jun 22, 2013
cd823bc
Oops, list_ () callbacks are public.
norv Jun 23, 2013
d2fc184
Topic controller.
norv Jun 23, 2013
8b5d1f1
MoveTopic controller.
norv Jun 23, 2013
4de7c7b
MoveTopic controller. The last query gone. Clean-up useless $db.
norv Jun 23, 2013
0f9acb5
Fix Xml controllers.
norv Jun 24, 2013
d2b0a8b
Themes controller inherits Action_Controller.
norv Jun 24, 2013
8965a2f
Adapt mod blocks functions, as helper methods in the controller. Naming.
norv Jun 24, 2013
9b0ac97
Moderator blocks code redesigned.
norv Jun 24, 2013
8357325
Make moderatorNotice() function also in moderation subs,
norv Jun 24, 2013
992c56c
Adapt list_ callbacks as controller methods, and respective subs func…
norv Jun 25, 2013
fb83016
Adapt a couple more createList() callbacks in profile.
norv Jun 25, 2013
e2ee56f
Adapt list_ callbacks in AdminDebug and ManageAddonSettings controllers.
norv Jun 25, 2013
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion SSI.php
Expand Up @@ -96,14 +96,15 @@
require_once(SOURCEDIR . '/Errors.class.php');
require_once(SUBSDIR . '/Util.class.php');
require_once(SUBSDIR . '/TemplateLayers.class.php');
require_once(SOURCEDIR . '/Action.controller.php');

// Clean the request variables.
cleanRequest();

// Create a variable to store some specific functions in.
$smcFunc = array();

// Initate the database connection and define some database functions to use.
// Initiate the database connection and define some database functions to use.
loadDatabase();

// Load settings from the database.
Expand Down
1 change: 1 addition & 0 deletions index.php
Expand Up @@ -79,6 +79,7 @@
require_once(SOURCEDIR . '/Errors.class.php');
require_once(SUBSDIR . '/Util.class.php');
require_once(SUBSDIR . '/TemplateLayers.class.php');
require_once(SOURCEDIR . '/Action.controller.php');

// Forum in extended maintenance mode? Our trip ends here with a bland message.
if (!empty($maintenance) && $maintenance == 2)
Expand Down
41 changes: 41 additions & 0 deletions sources/Action.controller.php
@@ -0,0 +1,41 @@
<?php

/**
* @name ElkArte Forum
* @copyright ElkArte Forum contributors
* @license BSD http://opensource.org/licenses/BSD-3-Clause
*
* @version 1.0 Alpha
*/

if (!defined('ELKARTE'))
die('No access...');

/**
* Abstract base class for controllers.
* Requires a default action handler, action_index().
* Defines an empty implementation for pre_dispatch()
* method.
*/
abstract class Action_Controller
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question of the day: Is this class designed to do something in the future? The current implemantation would be more of an interface instead of an abstract class...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few thoughts:

  • if it were an interface, it'd force implementation of all methods - pre_dispatch() - for all controllers. Otherwise they remain abstract and PHP will bail. Some of them have pre_dispatch(), but it doesn't seem a must for all, atm?
  • same goes for eventually settings() in admin controllers. Many have it, but not all.
    With an interface all are forced to write something (be it something empty), with a base abstract class, we can add an empty implementation in the base class, and who cares and needs to override it, will; the rest will just inherit it with no change needed.
  • sometimes, during Ema's template_layers experiments, I've seen the possibility that an object of the template layers class could be an instance var in any controller, initialized when execution of action_something() starts, and used to add layers to. It'd have a place as protected in the base class. To note, this isn't an intention, it's a just a thought, but having a base class already will save us from making it then.
  • sometimes, during earlier (well, middle-late, lol) work on Action class, I've been playing with an isAllowed in the base controller class, or a dispatch(). I'm not happy with the results, and I'd rather keep things plain and explicit, i.e. make in action_index manually what we need to make on subActions, using Action class. But in case a need appears more clearly than before, perhaps we can reconsider using the base class for some default implementation of controller dispatch.
  • for Elk, templates and languages are simply procedural files. Even the 'engine' loading them, is our core functions in Load.php. With Ema's template layers, and perhaps with language files loading (Arantor had interesting comments on potential improvements), we might want a template engine object, and a language engine object, which - as above - a base controller could always initialize and be done with; all will just add/call stuff on them.

That said, I don't feel there is any issue at all to add an interface. Or, an extra-interface 😸 I'm just tempted to keep the object oriented model to its simplest; simplest which does the job with no burden.
An abstract base class is an interface conceptually, after all; but, it also allows defaults.
We could give up pre_dispatch(), or implement it in all controllers; or we can add an extra-interface (Action_Controller implements Controller). I'm just curious the other way around, why it'd be a "actual" interface?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the late response...

I can see the benefits of an abstract parent class now .. the current scaffolding design reminds me of an interface (primarily because the functions are emtpy ) but a parent class (abstract or not) has definitely some benefits, at least it allows to easily extend all childs functionality at once ..

{
/**
* Default action handler. This will be called
* by the dispatcher in many cases. It may set up a menu,
* sub-dispatch at its turn to the method matching ?sa= parameter
* or simply forward the request to a known default method.
*/
abstract public function action_index();

/**
* Called before any other action method in this class.
* Allows for initializations, such as default values or
* loading templates or language files.
*/
public function pre_dispatch()
{
// By default, do nothing.
// Sub-classes may implement their prerequisite loading,
// such as load the template, load the language(s) file(s)
}
}
21 changes: 6 additions & 15 deletions sources/Dispatcher.class.php
Expand Up @@ -132,36 +132,30 @@ public function __construct()
// $_GET['action'] => array($file, $class, $method)
$actionArray = array(
'activate' => array('Register.controller.php', 'Register_Controller', 'action_activate'),
'admin' => array('Admin.php', 'AdminMain'),
// 'announce' => array('Announce.controller.php', 'Announce_Controller', 'action_index'),
'admin' => array('Admin.php', 'Admin_Controller', 'action_index'),
'attachapprove' => array('ModerateAttachments.controller.php', 'ModerateAttachments_Controller', 'action_attachapprove'),
'buddy' => array('Members.controller.php', 'Members_Controller', 'action_buddy'),
// 'calendar' => array('Calendar.controller.php', 'Calendar_Controller', 'action_calendar'),
'collapse' => array('BoardIndex.controller.php', 'BoardIndex_Controller', 'action_collapse'),
'contact' => array('Register.controller.php', 'Register_Controller', 'action_contact'),
'coppa' => array('Register.controller.php', 'Register_Controller', 'action_coppa'),
'deletemsg' => array('RemoveTopic.controller.php', 'RemoveTopic_Controller', 'action_deletemsg'),
'dlattach' => array('Attachment.controller.php', 'Attachment_Controller', 'action_dlattach'),
'dlattach' => array('Attachment.controller.php', 'Attachment_Controller', 'action_index'),
'disregardtopic' => array('Notify.controller.php', 'Notify_Controller', 'action_disregardtopic'),
'editpoll' => array('Poll.controller.php', 'Poll_Controller', 'action_editpoll'),
'editpoll2' => array('Poll.controller.php', 'Poll_Controller', 'action_editpoll2'),
// 'emailuser' => array('Emailuser.controller.php', 'Emailuser_Controller', 'action_emailuser'),
'findmember' => array('Members.controller.php', 'Members_Controller', 'action_findmember'),
// 'groups' => array('Groups.controller.php', 'Groups_Controller', 'action_list'),
// 'help' => array('Help.controller.php', 'Help_Controller', 'action_help'),
'quickhelp' => array('Help.controller.php', 'Help_Controller', 'action_quickhelp'),
'jsmodify' => array('Post.controller.php', 'Post_Controller', 'action_jsmodify'),
'jsoption' => array('Themes.php', 'Themes_Controller', 'action_jsoption'),
'loadeditorlocale' => array('subs/Editor.subs.php', 'action_loadlocale'),
// 'lock' => array('Topic.controller.php', 'Topic_Controller', 'action_lock'),
'lockvoting' => array('Poll.controller.php', 'Poll_Controller', 'action_lockvoting'),
'login' => array('Auth.controller.php', 'Auth_Controller', 'action_login'),
'login2' => array('Auth.controller.php', 'Auth_Controller', 'action_login2'),
'logout' => array('Auth.controller.php', 'Auth_Controller', 'action_logout'),
'markasread' => array('Markasread.controller.php', 'MarkRead_Controller', 'action_index'),
'mergetopics' => array('MergeTopics.controller.php', 'MergeTopics_Controller', 'action_index'),
'memberlist' => array('Memberlist.controller.php', 'Memberlist_Controller', 'action_index'),
'moderate' => array('ModerationCenter.controller.php', 'ModerationCenter_Controller', 'action_modcenter'),
'moderate' => array('ModerationCenter.controller.php', 'ModerationCenter_Controller', 'action_index'),
'karma' => array('Karma.controller.php', 'Karma_Controller', ''),
'movetopic' => array('MoveTopic.controller.php', 'MoveTopic_Controller', 'action_movetopic'),
'movetopic2' => array('MoveTopic.controller.php', 'MoveTopic_Controller', 'action_movetopic2'),
Expand All @@ -171,29 +165,25 @@ public function __construct()
'pm' => array('PersonalMessage.controller.php', 'PersonalMessage_Controller', 'action_index'),
'post' => array('Post.controller.php', 'Post_Controller', 'action_post'),
'post2' => array('Post.controller.php', 'Post_Controller', 'action_post2'),
// 'printpage' => array('Topic.controller.php', 'Topic_Controller', 'action_printpage'), // done
'profile' => array('Profile.controller.php', 'Profile_Controller', 'action_index'),
'quotefast' => array('Post.controller.php', 'Post_Controller', 'action_quotefast'),
'quickmod' => array('MessageIndex.controller.php', 'MessageIndex_Controller', 'action_quickmod'),
'quickmod2' => array('Display.controller.php', 'Display_Controller', 'action_quickmod2'),
'recent' => array('Recent.controller.php', 'Recent_Controller', 'action_recent'),
'register' => array('Register.controller.php', 'Register_Controller', 'action_register'),
'register2' => array('Register.controller.php', 'Register_Controller', 'action_register2'),
// 'reminder' => array('Reminder.controller.php', ''),
'removepoll' => array('Poll.controller.php', 'Poll_Controller', 'action_removepoll'),
'removetopic2' => array('RemoveTopic.controller.php', 'RemoveTopic_Controller', 'action_removetopic2'),
'reporttm' => array('Emailuser.controller.php', 'Emailuser_Controller', 'action_reporttm'),
'restoretopic' => array('RemoveTopic.controller.php', 'RemoveTopic_Controller', 'action_restoretopic'),
'search' => array('Search.controller.php', 'Search_Controller', 'action_plushsearch1'),
'search2' => array('Search.controller.php', 'Search_Controller', 'action_plushsearch2'),
// 'sendtopic' => array('Emailuser.controller.php', 'Emailuser_Controller', 'action_sendtopic'),
'suggest' => array('Suggest.controller.php', 'Suggest_Controller', 'action_suggest'),
'spellcheck' => array('Post.controller.php', 'Post_Controller', 'action_spellcheck'),
'splittopics' => array('SplitTopics.controller.php', 'SplitTopics_Controller', 'action_splittopics'),
'stats' => array('Stats.controller.php', 'Stats_Controller', 'action_stats'),
// 'sticky' => array('Topic.controller.php', 'Topic_Controller', 'action_sticky'), // done
'theme' => array('Themes.php', 'Themes_Controller', 'action_thememain'),
'trackip' => array('ProfileHistory.controller.php', 'action_trackip'),
'trackip' => array('ProfileHistory.controller.php', 'ProfileHistory_Controller', 'action_trackip'),
'unread' => array('Recent.controller.php', 'Recent_Controller', 'action_unread'),
'unreadreplies' => array('Recent.controller.php', 'Recent_Controller', 'action_unread'),
'verificationcode' => array('Register.controller.php', 'Register_Controller', 'action_verificationcode'),
Expand All @@ -202,7 +192,8 @@ public function __construct()
'viewquery' => array('AdminDebug.php', 'AdminDebug_Controller', 'action_viewquery'),
'viewadminfile' => array('AdminDebug.php', 'AdminDebug_Controller', 'action_viewadminfile'),
'.xml' => array('News.controller.php', 'News_Controller', 'action_showfeed'),
'xmlhttp' => array('Xml.controller.php', 'action_xmlhttp'),
'xmlhttp' => array('Xml.controller.php', 'Xml_Controller', 'action_index'),
'xmlpreview' => array('Xmlpreview.controller.php', 'XmlPreview_Controller', 'action_index'),
);

$adminActions = array ('admin', 'attachapprove', 'jsoption', 'theme', 'viewadminfile', 'viewquery');
Expand Down