joomla-projects / gsoc16_recording-action-logs

Recording actions logs, accessible by super admin
https://summerofcode.withgoogle.com/projects/#4547036649095168
GNU General Public License v2.0
4 stars 4 forks source link

Creating generic log messages #35

Closed muhakh closed 8 years ago

muhakh commented 8 years ago

I have a problem with that every event can be logged in more than one component and each component can give it different parameters, for example: onContentAfterSave event is triggered from the _comcontent (on new/edit article), _commedia (on new images), etc. The parameters in the com_content are (com_content.article, $article, $isNew) and the parameters in com_media are ('com_media.file', &$object_file, true).

My question is: If I want the log message to appear like: New Article created, its title is "Hello World" on creating new article and New image uploaded on uploading new image, Can I make this without specifying that every component that uses onContentAfterSave event has its own parameters that are stored to be translated later? For example: This is how I write the message for onContentAfterSave $message = '{"title":"'.$article->title.'","isNew":"'.$isNew_string.'", "event":"onContentAfterSave"}'; In the previous code, there is no title for the new image and for user notes it's called subject not title.

This is the plugin file where you can take a look on onContentAfterSave event https://github.com/joomla-projects/gsoc16_recording-action-logs/blob/staging/plugins/system/userlogs/userlogs.php#L109

and this is how it's translated https://github.com/joomla-projects/gsoc16_recording-action-logs/blob/staging/plugins/system/userlogs/userlogs.php#L381

Llewellynvdm commented 8 years ago

What is true

Seeing that the onContentAfterSave as you said has various signatures being passed to the method in each context. This would mean there is lots of polymorphism taking place here, as the context in each plugin will be used to trigger the behavior of the event per component and is relevant fields.

What I think

This would mean that it is hard to find a logarithmic way to solve the problem. You could firstly implement a whole new convention of uniform fields and therefore signatures per event across the board for all core components. But this idea is crazy, and would cause more issues then solving anything.

What seems more in scope and possible is a mapping convention like the history component is using. This will need to be done so other components can extend it based on their fields and expected behavior.

Take a look at the following: https://docs.joomla.org/Using_Content_History_in_your_Component

You will see that you should in your component map your fields to the available fields in the History Component. Look at this example: https://github.com/namibia/CBP-Joomla-3-Component/blob/master/script.php#L919

You can use a similar field (for title), event, context (per component) mapping.

Event better

You can use the object found in content_types(table)->field_mappings(column) that are already set in the common field to identify the title, and any other information you may need to build a well formed log message. Sorry for the run around the barn, hope it was fun...

So in short, use the component to get the content type, and then its mapped fields.

Let me know if you would need more information on this.

muhakh commented 8 years ago

Thanks @Llewellynvdm . That is a great idea which I think I can implement. My approach is instead of just having the names of components to be logged in the table #__user_logs_extensions, it can contain the following fields:

So, for example with onContentAfterSave has the middle parameter ($article for example). To get the title of the created or edited instance, I can write $article->get($title_holder) How about that?

Llewellynvdm commented 8 years ago

Great stuff!

muhakh commented 8 years ago

Now I have two questions: 1- Should I use the config.xml of all components to add an option like the one for the content history component https://docs.joomla.org/Using_Content_History_in_your_Component#Add_Component_Level_Options ?

2- The function addLogsToDb should be a helper function or should remain as a function in the plugin? https://github.com/joomla-projects/gsoc16_recording-action-logs/blob/staging/plugins/system/userlogs/userlogs.php#L50 As developers who want to extend the plugin can use it from this file https://github.com/joomla-projects/gsoc16_recording-action-logs/blob/staging/administrator/components/com_userlogs/helpers/userlogs.php instead of using the plugin file?

Robdebert commented 8 years ago

I think helpers are always good to separate some function from the core-logic. But it should not be a static class or called static (with ::)

If a helper can be overwritten by developers with own classes that contain more code, that would be more awesome, Because some Admins might want to track the actions not in the same database, but somewhere else, or receive an email for specific events.

Your plugin should not handle this in the first version, but developers may then be able to extend it to add this, and perhaps it can be integrated later.

yvesh commented 8 years ago

@muhakh I think there is no perfect answer to that topic, in my opinion you should try to have a generic logging (with the context you get, do the best out of it).

And then you should offer the possibility for extensions (and core) to improve / extend these, by having for example an helper file (we have that for com_categories in core) in their component directory with a certain function (which you check for if it exists)..

Still not ideal, but probably the most flexible way to go. I would not do an analysis von what you get in onContentAfterSave, else you are going to add a big ball of mud with endless ifs etc.

muhakh commented 8 years ago

@yvesh What about the config.xml part? Is it OK to add a boolean field in the config.xml file of all logable extensions (in the core) just like the one for content history https://docs.joomla.org/Using_Content_History_in_your_Component#Add_Component_Level_Options ?

yvesh commented 8 years ago

@muhakh hmm what do you think is the advantage of that? I would try to log every event (even if it's just based on context) and for more look for the helper. This has the advantage you don't need to touch core, except for the helper ;)

muhakh commented 8 years ago

This would be for the user if he wants to enable/disable logging for some extensions for his own reasons.

yvesh commented 8 years ago

If i see this correctly you hardcoded the list of extensions now, which imo has some disadvantages. I would try to catch every extensions event, at least as a generic one.. This gives more flexibilty. But you know the project and it's goals better then me :-)