Add trigger/action for e-mail notification
sethcohn - January 2, 2009 - 16:30
| Project: | Feedback 2.0 |
| Version: | 6.x-2.x-dev |
| Component: | Code |
| Category: | feature request |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs review |
Description
While storing the feedback in the DB table is important, in many use cases, the feedback needs to go to someone, and email is clearly the best answer. How about adding a 'feedback email address' admin setting (if empty, we won't send email, same as now), and then firing off a quick drupal email as part of feedback_add_entry or feedback_form_submit?
That way, if a user sees something (critically?) wrong, instead of the note sitting in the db waiting for someone to look for feedback, an admin can be proactively warned of the trouble, or if it's not important, ignore/delete the email, knowing that a copy of the feedback is in the db for later.
#1
Just saw #349288: Send an e-mail to admin? is marked as Won't Fix.
Will you take a patch that adds the above?
#2
Would you agree that this won't fix, if we would implement #351698: Use nodes for Feedback messages? New features go into 6.x first, and in 6.x, there are default actions for the "node creation" trigger. My point is that I would want to configure the recipient's mail address (and not bug the site owner, resp. client with feedback messages), but the whole mechanism would already be available in Drupal core if Feedback would use nodes to store feedback messages.
#3
I agree that would solve it for D6, and potentially for D5 (since there are similar means (actions/workflow) to do the same), but it seems _a lot_ of work for something that really is only about 20-30 lines of code to add otherwise (admin settings form with a variableset for the feedback_emailaddress, plus a drupal mail call if the variable is set). I agree, it should be a configurable email address, not the default site owner email.
As I asked, would you take such a patch? I see that both are valid approaches, and actually complementary (if other means for node notification exist, you can just not set the built in feedback email address, to turn off that functionality)
#4
Yes, if it is clean & solid, and does not become burden with me. ;) I'd rather like to invest time in the back-end abstraction / node based back-end...
#5
I would also be very interested in a patch that supports notification. Storing it in a database is awesome, but there should be a way to have it sent to someone, or at least notify them of a new entry.
You shouldn't need to constantly check a log, just to handle user feedback in a timely fashion.
#6
I agree that the node based / trigger approach would be a better end solution, but a quick and dirty configurable email address to send notifications to would be a nice added feature for now.
#7
me agree.
#8
I have a working version of this, and will post a cleaned up patch for 5.x shortly. I plan on a 6.x patch (the mail calls are different between 5.x and 6.x) but haven't had the time to work on it yet.
It'll use a themeable mail call, allowing for more or less custom mails if desired. For instance, I've added a tab delimited dump after the normal text, to allow a QA person to cut and paste the data as one line into an Excel sheet.
I attempted to test the browscap functionality, but browscap is currently broken.
#9
I have added a hook to my copy of this module, in the feedback_form_submit() function. This allows me to intercept the feedback submission in another module (our custom project module) and take whatever action we wish.
In the function feedback_form_submit(), I have added this as the first line:
module_invoke_all( 'feedback_form_submit', $form, $form_state );Now, in a module called 'foo', we can create a function foo_feedback_form_submit(), and it will be called whenever feedback is submitted.
Sun, any chance of adding this hook to the module? It is trivial to add, and provides an enormous increase in flexibility to developers.
Here is my custom module's implementation of this hook (with names changed to protect the innocent), which sends out a notification email to all users with a particular role ID:
<?php
// implementation of hook_feedback_form_submit
// sends notification email to all users with Admin role
function foo_feedback_form_submit( &$form, &$form_state ){
global $user;
$location = url( $form_state['values']['location'], array( 'absolute' => true ) );
$agent = $_SERVER['HTTP_USER_AGENT'];
$message = $form_state['values']['message'];
$body = "A feedback message has been received from My Site Name\n\n";
$body .= "User:\n$user->name ($user->mail)\n\n";
$body .= "Location:\n$location\n\n";
$body .= "User Agent:\n$agent\n\n";
$body .= "Message:\n$message";
// load all users with role ID = MY_ADMIN_ROLE_ID
$result = db_query("SELECT u.mail FROM {users} u INNER JOIN {users_roles} ur ON u.uid=ur.uid WHERE ur.rid = %d AND u.status = 1", MY_ADMIN_ROLE_ID );
$emails = array();
while ($u = db_fetch_object($result)) {
$emails[] = $u->mail;
}
$mail = array(
'id' => 'feedback',
'to' => implode( ', ', $emails ),
'subject' => 'My Site Name - User Feedback',
'body' => drupal_wrap_mail( $body ),
'headers' => array(
'From' => variable_get('site_mail', ini_get('sendmail_from')),
'MIME-Version' => '1.0',
'Content-Type' => 'text/plain; charset=UTF-8; format=flowed; delsp=yes',
'Content-Transfer-Encoding' => '8Bit',
'X-Mailer' => 'Drupal',
'Reply-To' => $user->mail
)
);
drupal_mail_send( $mail );
}
?>
Hope this helps someone.
#10
Sorry for delay... We've ended up using this with great success for multiple sites.
By using an admin setting form to set the destination email, and a themeable call for the mail itself, it's very customizable.
I agree, a form_submit handler like brian_c's patch would also work, but then it requires an extra module to do anything, meaning the out of the box experience is still lacking what so many agree is pretty basic functionality. The advantage of this patch is that it's self contained, yet still flexible enough to completely revamp or change. If both methods (theme call, and submit handler) are used, so much the better for maximum flexibility on the part of developers.
For example, I added a single line (which is in the patch, commented out, mostly as an example) that then includes a cut/paste version tab separated line of the data, to allow someone to easily paste everything into a spreadsheet with columns). Other future plans include adding other exporting/transmitting of the data, but now I could do that externally in the theme, or via brian's suggested hook.
#11
Absolutely, I would love to see an "out of the box" solution for notification as well. It sounds like Sun wants to implement this using Nodes though, so I'm thinking that won't be for a while.
In the meantime though, I'm happy with my one-liner hook addition... trivial to re-add if the module is updated.
Ideally, we could have both. Admin interface for ease of use, hook for maximum flexibility.
#12
Actually, in our current use case (client feedback post-development, prelaunch) we don't _want_ nodes, since we want the minimum impact on the site possible (ie a standalone table is perfect, that we can drop once the feedback phase is done with) We want to turn the module on/off with no noticeable impact on the db itself. Turning feedback items to nodes would end up a problem, in this use case, since we'd be adding content to the site itself and have to deal with that.
If Sun ends up putting that feature (http://drupal.org/node/351698) into the module, I hope it's optional.
#13
I thought if I attached a patch, sun might be more likely to commit :)
Patch adds brian_c's hook from #9
#14
Well...
1) This patch contains coding-style issues.
2) It does not add the requested functionality. Instead, it defers to a yet-unknown (any) module to take over the notification.
#15
sorry about that. sloppy copy and paste. also just realized it's just as easy to form alter and add a custom submit function.
#16
ok. So what I want to see here is an implementation of actions (and triggers), because that's a future-proof way of doing things. That way, the e-mail notification is configurable, can be toggled, and users could even assign other actions to Feedback's trigger (f.e. displaying a message or whatever).
#17
Implementing triggers / actions would be ideal.
#18
Agree. Subscribing.
#19
Attached Patch does the following.
Hope it helps.
#20
Thanks for your work antgiant.
Any chance of a 5.x patch?
#21
@davidlerin not from me. Sorry.
However, the Actions project (http://drupal.org/project/actions) may cause this patch to work in 5.x. (I can't verify as I don't have any 5.x installs.) Hope that helps.
#22
Note that the original issue was about sending an e-mail to a list of addresses (usually site admins or developers) while the latest patch sends an e-mail to the user who was giving the feedback. I've taken antgiant's patch from #19 and developed it a bit further:
- Added a configurable list of recipients to the action.
- The primary object passed to actions should not be a user object but an instance of the feedback object itself.
- Also feedback is NOT node-specific, so I've completely removed the node/taxonomy stuff used in the e-mail generation. Instead you have now access to almost all feedback object properties.
- If there can be found a way to #470146: Track title of current page, we could add another replacement token for it.
This patch is completely UNTESTED.
#23
@smk-ka You don't need to modify the action I provided to send it to a list. You can use the existing action provided by the system module to "send email...". I like the idea of sending the feedback object as the primary object. I just wasn't sure of its impact on existing user and system actions.
#24
Subscribing.
#25
subscribing
#26
In #492094: Rules integration: rules event when submitting feedback I have submitted a patch that provides integration with the Rules module, a more powerful approach to core actions/trigger
#27
Subscribing