-
Notifications
You must be signed in to change notification settings - Fork 17
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
Add a logger delegate class for logging. #2
base: 7.x-2.x
Are you sure you want to change the base?
Conversation
@darthsteven can you share a bit more about your use case - how is Message + Message notify serving you better? |
So I have a need to audit the sent messages and keep a history of what was sent, to whom and success/failure. The stack doesn't seem to support that sort of auditing, unless I'm mistaken. I will provide another module that will swap out the default logger delegate class so that sent messages are funneled into my audit trail. |
I believe this PR isn't needed, as you can already implement what you want (if I got it wrong feel free to re-open):
That you can already do -- see the example module
Since it's using |
Well, I am trying to do the second part there, and extend the logging beyond a simple watchdog on fail. I don't want to have to override all methods on all the classes we can configure for sending, I just want to log in a single place, and have any class that we configure for sending additionally have it's results logged in this way. |
So there's a example (advanced) logging module here: |
Ok, I better understand now -- you want to pipe all sent messages to be logged in one single place. Re-opening. |
@@ -140,4 +162,18 @@ abstract class MessageNotifierBase implements MessageNotifierInterface { | |||
public function access() { | |||
return TRUE; | |||
} | |||
|
|||
public function getLogger() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing PHPdocs.
I had only some minor comments. It would be great to add a simpleTest that verify a log using a dummy logger that will for example, log to a variable -- so it could be tested easily. |
I don't have time to do this right now, but it's on the list, and I'll get back to it, but if someone else wants to pick it up and run with it, feel free! |
Experience shows no one ever picks the tests ;) |
Add an original copy of the cloned message.
I'd like to be able to swap out the logger for something more exciting that just the watchdog, and I have the need to log success messages, so I figured that a nice way to supply this would be via a logger delegate class that can be easily swapped out.