Welcome Guest, Not a member yet? Register   Sign In
Controller + Model vs Ultra-slim controller that barely does anything + Library
#1

(I'll apologize in advance, because the number of problems increased from the basic one mentioned in the title to some more while writing this post...)

Hey guys.

I've been trying to implement a small action-logging system directly coupled with a notification system today.
What I want(ed) to achieve: Every failure or success of a specific action (Logging in) corresponds to an "Action": For example, you could have "LoggedIn", "LoggedOut", "LoginFail", "CreatePost" or whatever really as action. (I spent some time thinking about if it would make sense to have a different action for failure and success, but I came to the conclusion, that, while messier, it would be easier to implement that way)
Also, most actions will be coupled with notifications - I guess just like flash messages, just without the session-coupled part (I wanted to be able to manage everything from one place) - that will be displayed to the user until (s)he deletes them (Which, I just noticed, may be quite annoying for simple notifications like "Successfully logged in!", but letting some be displayed until deleted and others only once like a flash message would be quite bothersome to implement... I need to think about that.)

At first, I implemented everything as a straight-forward library, with methods such as "logAction" and "makeNotification", and the following database structure:

Code:
ActionTypes
-----------
ATID -> ActionType ID
Name -> Action Type name
UserMessage -> Message displayed to user for that type of action, with %1 and %2 being replaced by specific data
AdminMessage -> Same for admins
-----------

Actions
-------
AID -> Action ID
ActionType -> Type of this action
UserID -> User the action applies to
Data1 -> Specific action data
Data2 -> Specific action data (I was too lazy to couple that stuff with yet another table to make the amount of data variable)
-------

Notifications
-------------
NID -> Notification ID
UserID -> User the notification should be displayed to
ActionID -> The action mapped to that specific notification
Apart from the main problem of this thread, there are already a few within the above database design.
1. I have a maximal amount of two action specific data fields, instead of a variable one. Not really important, but something I should maybe reconsider.
2. While the Action and Notification system is one and the same, the Action system should still be usable without notifications. Which is currently not the case, as the messages shown in the notifications are defined in "ActionTypes". I honestly do not really know how I would go about this while keeping the amount of tables to a minimum.

But, while writing the library, which's code I'll include below, I noticed that if I want users to click away the notification to get rid of it, I'll need some kind of supplementary controller that will be called by JavaScript code. Which would render a library a little redundant, since a controller with a model would also do the job. On the other hand, always needing to call HMVC's Modules::run('Notification/create/arguments') in my own code to create a notification or action instead of simply calling the library $this->notifications->create(arguments) seems annoying too.
So I am currently undecided between using a reaaaally slim controller that barely does anything than forward a call + library or a real controller with a model - Or even a library for the action system (Since that one does not need any user input), plus a controller and model for notifications.

In addition to this "main" problem, there are a few more secondary ones: Mostly, guests.
I can't really save a notification with a guest as target user, since it would get displayed to lots of other guests too. And it would stay and need to be deleted, too. Which basically leads me back to flash_messages and a lot of if and elses in my library. So, yeah, I'm a little confused on how to approach this.

Here's the unfinished code I currently have:
PHP Code:
<?php
defined
('BASEPATH') OR exit('No direct script access allowed');

class 
Notifications {

    private 
$CI;

    function 
__construct(){
        
$this->CI = &get_instance();

        
//Load all action types and store them as variables for easier action creation outside of the library
        
$res $this->CI->db->select('ATID, Name, UserMessage, AdminMessage')->from('actiontypes')->get()->result_array();
        foreach(
$res as $actionType){
            
$actionName $actionType['Name'];
            
$this->$actionName = new ActionType($actionType['ID'], $actionName$actionType['UserMessage'], $actionType['AdminMessage']);
        }
    }

    
//TODO CACHE actions and notifications
    //TODO Finish this

    
function getUserNotifications($userID){
        
$notifications = array();
        
$res = array();

        if(
$userID == $this->CI->userystem->GUEST_ID){ //Get actions by ActionID saved for this specific guest instead of by User ID
            
$res $this->CI->db->select('actions.ActionType, actions.Data1, actions.Data2, actiontypes.UserMessage')->from('actions')
            ->
join('actiontypes''actions.ActionType = actiontypes.ATID')->where("actions.AID IN ({$this->CI->session->flashdata('light_notifs')})")->get()->result_array();
        }else{ 
//Get notifications by user ID
            
$res $this->CI->db->select('notifications.NID, actions.ActionType, actions.Data1, actions.Data2, actiontypes.UserMessage')
            ->
from('notifications')->join('actions''actions.AID = notifications.ActionID')->join('actiontypes''actions.ActionType = actiontypes.ATID')
            ->
where('notifications.UserID', (int) $userID)->get()->result_array();
        
//TODO Use the already loaded Action Types instead of joining that shit again
                
}

        
        
//$res = $this->CI->db->select('notifications.NID, actions.ActionType, actions.Data1, actions.Data2')
        //->from('notifications')->join('actions', 'actions.AID = notifications.ActionID')->where('notifications.UserID', (int) $userID)->get()->result_array();

        
foreach($res as &$notification)
            
array_push($notificationsstr_replace(array('%1''%2'), array($notification['Data1'], $notification['Data2']), $notification['UserMessage']));

        return 
$notifications;
    }

    function 
logAction($actionType$actioner$data1 ''$data2 ''){
        if(! 
$this->CI->db->insert('actions', array('ActionType' => $actionType->ID'UserID' => $actioner'IP' => $_SERVER['REMOTE_ADDR'], 'Data1' => $data1'Data2' => $data2)) ){
            
// $this->db->error(); TODO Log error
            
return false;
        }
        return 
$this->CI->db->insert_id();
    }

    
/**
     * If user is a guest, save the ActionID in a temporary array
    **/
    
function makeNotification($actionID$targetUser){
        if(
$targetUser == $this->CI->usersystem->GUEST_ID){
            
array_push($this->lightNotifications$actionID);
            
$this->CI->session->set_flashdata('light_notifs'$this->lightNotifications);
            return 
false//TODO I don't know about this... Damned be the guests
        
}
        if(! 
$this->CI->db->insert('notifications', array('UserID' => $targetUser'ActionID' => $actionID))){
            return 
false;
        }
        return 
$this->CI->db->insert_id();
    }

    function 
deleteNotification($NID){
        
//TODO controller?
    
}

    function 
getAdminNotificationsByUser($userID){

    }



    private 
$lightNotifications = array();

}

class 
ActionType {
    public 
$ID$name$userMesage$adminMessage;

    function 
__construct($ID$name$userMessage$adminMessage){
            
$this->ID $ID;
            
$this->name $name;
            
$this->userMessage $userMessage;
            
$this->adminMessage $adminMessage;
    }



Thank you very much for reading! 
Help is greatly appreciated Smile


Kind regards
Reply
#2

(This post was last modified: 08-19-2016, 10:29 AM by PaulD. Edit Reason: Added PPPPS )

There are lots of different ways you can do this. I would do this:

First you need at least two tables, one for actions, and then one for action_log

Code:
actions
-------
 action_id        
 action_name    // ie logout, login, not allowed, whatever
 action_code    // for convenience so logout = LO, login=LI etc (unique)

action log
----------
 log_id
 log_action_id
 log_date
 log_user_id

Then a library for logging actions. Something to use like this:

PHP Code:
public function log_action($action_code$user_id) {
 
  ....


As for redirects or flash messages or how you deal with a certain circumstance, that is entirely separate from the action logs and should not be part of the action logging. I would autoload the action library, and then you are free to call it from wherever you need to.

But if you really want to directly couple it together, you could add a column or two to the action table, one to indicate a logout url to implement a redirect, one to indicate a message field for a standard message to be returned.

Or, you can add to the library with a set method something like:
PHP Code:
$this->login_library->set_redirect('my/link');
$this->login_library->set_message('You are not allowed to do that.');
$this->login_library->log_action($action_code$user_id); 

Or just put it into the call
PHP Code:
$this->login_library->log_action($action_code$user_id'my/link''You are not allowed to do that.'); 

It really all depends on your app design and how you are implementing your messages/notifications etc. For instance, in your common header file you might have a:
PHP Code:
<?php if (!empty($notification)) { ?>
   ... show notification html absolutely positioned on page at top right or something
<?php ?>
And your action log would return a suitably formatted notification. You could even, in your actions table, indicate if an action should send a red error message, a blue info message, or a green success message, etc etc.

Hope that helps in some way

Paul.

PS In terms of guests, you can set the user_id to 0, which could, for example, return the notificatons as per normal but not log in the database the actions done.

PPS Since notifications would be pretty standard piece of js, it would be in your site.js loaded on every page. Even if not used, it is a few lines at most.

PPPS You definitely do not need an entire module for this. But if you are into HMVC, then I suppose you should do a notifications module rather than a library.

PPPPS (Last one) - It does not matter how tiny a module or a library or a model is. It is about the right place to put it to keep your site consistently built. Even if a model has a single database call in it, if it serves for proper seperation of concerns, you should do it. It will almost certainly fill up later as other functions pop up as being needed. And even if not, it still makes sense to keep to whatever pattern you are using.
Reply
#3

Thanks for your answer!

(08-19-2016, 10:15 AM)PaulD Wrote: There are lots of different ways you can do this. I would do this:

First you need at least two tables, one for actions, and then one for action_log

Code:
actions
-------
 action_id        
 action_name    // ie logout, login, not allowed, whatever
 action_code    // for convenience so logout = LO, login=LI etc (unique)

action log
----------
 log_id
 log_action_id
 log_date
 log_user_id
Then a library for logging actions. Something to use like this:

PHP Code:
public function log_action($action_code$user_id) {
 
  ....

Yup, this is pretty much what I have right now (+ A little bit of action specific data)

Also, since I want the "Action" part of the system be independent of the notification part, I'm not entirely sure if a "Type"/"actions" table would even make sense... I mean, sure, it stores all types of actions, but if I only have the name of the type then there's no real point in having a table for that. Same problem with my own code right now, though.


Quote:As for redirects or flash messages or how you deal with a certain circumstance, that is entirely separate from the action logs and should not be part of the action logging. I would autoload the action library, and then you are free to call it from wherever you need to.
Yes, this is what I want to achieve. Redirects shouldn't have anything to do with it and simply be called by the controller itself, but I want the notification (Not *only* flash messages, as described in my post above) part to rely on the action part without the action part having to depend on the notification part.

Quote:But if you really want to directly couple it together, you could add a column or two to the action table, one to indicate a logout url to implement a redirect, one to indicate a message field for a standard message to be returned.

Or, you can add to the library with a set method something like:
PHP Code:
$this->login_library->set_redirect('my/link');
$this->login_library->set_message('You are not allowed to do that.');
$this->login_library->log_action($action_code$user_id); 

Or just put it into the call
PHP Code:
$this->login_library->log_action($action_code$user_id'my/link''You are not allowed to do that.'); 
Actually this is exactly what I want to avoid. While the notification part can and should depend on the action part, it should really not be the other way around - Meaning that I want to get rid of the "Message" fields I currently have in my ActionTypes table. I also do not want to code in the message in my code though: I like it much better in my database. Maybe I should remove the "actiontypes"/"actions" table altogether and instead use an "NotificationTypes" table that sets a message for each type of action?

Quote:It really all depends on your app design and how you are implementing your messages/notifications etc. For instance, in your common header file you might have a:
PHP Code:
<?php if (!empty($notification)) { ?>
   ... show notification html absolutely positioned on page at top right or something
<?php ?>
And your action log would return a suitably formatted notification. You could even, in your actions table, indicate if an action should send a red error message, a blue info message, or a green success message, etc etc.
Yeah, exactly. But I'm simply not sure where exactly I should put those individual fields. In my old actiontypes table? In a new notificationtypes table? I just don't want to somehow repeat myself somewhere where it doesn't need to be repeated (Which is why I don't want to include the message in the code, for example, but only once in the database with placeholders for specific data)

Quote:PS In terms of guests, you can set the user_id to 0, which could, for example, return the notificatons as per normal but not log in the database the actions done.
Yeah, my usersystem has a constant variable $GUEST_ID which serves that purpose. The problem with my current notification system is that, for guests, notifications should only act as a simple flash message - Whereas for real users, it sometimes can act like a flash message, and sometimes like a persistent message. And I don't want to mix both everywhere in my code.

Quote:PPPS You definitely do not need an entire module for this. But if you are into HMVC, then I suppose you should do a notifications module rather than a library.
I do need a notifications controller though, so users are able to delete notifications. The question is: Should my controller act as a true module and do most of the job, excluding database querying by a mode, or should I simply forward the call to my library? Because in both cases, I will need a controller (For user related actions on notifications, like deleting. Actually, pretty much only deleting.), a library (Either the whole thing or simply the action system, which doesn't need user feedback), and a model (Or not, depending on wether I do everything in the library or not). Which is the question I've called "Main problem" in my post :/

Quote:PPPPS (Last one) - It does not matter how tiny a module or a library or a model is. It is about the right place to put it to keep your site consistently built. Even if a model has a single database call in it, if it serves for proper seperation of concerns, you should do it. It will almost certainly fill up later as other functions pop up as being needed. And even if not, it still makes sense to keep to whatever pattern you are using.
Yeah, but it's exactly that "dilemma" that puts me back to square one. I can't decide on what would be the best way to do it.

Thanks again for your answer!
Reply
#4

lol - it seems it was not much use.

You said you wanted to couple action logging with notifications. I would separate those out, they really do not have much to do with each other. That step alone makes the whole thing easier to approach.
Reply
#5

(08-19-2016, 01:16 PM)PaulD Wrote: lol - it seems it was not much use.

You said you wanted to couple action logging with notifications. I would separate those out, they really do not have much to do with each other. That step alone makes the whole thing easier to approach.

Oh, sorry, I expressed myself in a really bad way there. By coupling (Wrong word, sorry for that =S) I actually meant that the Action System itself should work on it's own, while a Notification system will rely on the Action system: Every action will be logged with specific data (Blog example: If an user creates a blog, a blog creation will be logged with the corresponding UserID and blog post ID), and a notification ca be created with at its base an action (Blog example: The notification at its base contains a target user - the user the notification will be shown to - a message with placeholders, and the action ID the notification corresponds to. Then, the placeholders will be filled with the specific data from the "coupled" action.)
Reply
#6

Sounds like a good job for jQuery.
What did you Try? What did you Get? What did you Expect?

Joined CodeIgniter Community 2009.  ( Skype: insitfx )
Reply
#7

@InsiteFX
Thanks for your answer!
It doesn't really matter to me right now what I will be using client side. jQuery, "pure" JavaScript, or even no JavaScript at all and simple site refreshes instead. What I have problems with is my back-end structure, where I can't decide between multiple ways of doing it and am not even sure if any of my ways could be considered "good".
Reply
#8

To me, this seems like you need to take a step back and think about each step and figure out what you're trying to do. It sounds like you really have three or four systems you're trying to implement at once. You have a message queue (actions), message logging, notifications, and possibly some user-level interactions with these systems (beyond simply displaying a notification to the user in reaction to an event).

The message queue itself is probably the simplest step, and the one you're probably the closest to having complete. The hardest part about implementing a message queue is usually keeping everything else separate. You should think about what information you really need in the message queue, and what isn't really relevant. Usually you just need the message (action) and some metadata (common metadata might be a message type/category, date/time the message was inserted, time to live/expiration, a delay value to prevent the queue from processing the message until a certain amount of time has passed since insertion, and an ID to aid in managing the queue).

A basic queue would just be a simple CRUD system. You can create, read, update, and delete messages as you see fit.

A slightly more complicated system to implement, but significantly more powerful to use, would be a pub/sub system, where any code can publish a message, and code subscribes to receive certain types of messages. When the queue is processed (maybe you process it on every visit, in response to certain events, manually from a control panel, or via the CLI so the system can trigger it from a cron job), each message is sent to all of the subscribers to handle as they see fit, then removed from the queue.

In the pub/sub system, if you want your message logger to log every message that goes through the system, you make it a subscriber to all of the message types. If you want notifications for certain types of messages, you make the notification system a subscriber to those message types.

Your notification system might even insert new messages into the queue which are only processed when the specific user who is intended to see those notifications logs in. In that case, you may want messages removed from the queue only when they are processed by subscribers, and you would not subscribe your message logger to these types of events. Alternatively, you could setup your queue system to allow multiple queues, and give each user their own notification queue which is processed when that user logs in (or maybe you allow the user to set a time for a daily or weekly notification and send their notifications in a batch via email).

One more complication I might add to this system would be to encapsulate the storage of the messages so you could change it out in the future. You may find the database is not the ideal way to store a message queue. You might decide to move your queue to Redis, Memcached, a NoSQL database, or a cloud-based message queue. Maybe you don't have access to any of these options now, or you don't want to throw out the work you've done on the database interaction, but you should isolate that code from the rest of the message queue code so you can replace it, even if it's just to try some of the alternatives in development/test environments to see if they would improve performance enough to be worth investing in a different hosting option for your production environment.
Reply




Theme © iAndrew 2016 - Forum software by © MyBB