Modify

Opened 16 years ago

Closed 13 years ago

Last modified 11 years ago

#3739 closed enhancement (fixed)

[Patch] Add option for mail notification

Reported by: Johan Risberg Owned by: CuriousCurmudgeon
Priority: normal Component: BatchModifyPlugin
Severity: normal Keywords:
Cc: bernd@…, Patrick Schaaf Trac Release: 0.12

Description

No mail notifications are sent which might be what you want if handling lots of tickets, but it would be nice to have the option to enable such.

Attachments (3)

Notifications.patch (3.4 KB) - added by bernd@… 14 years ago.
Patch that sends notifications for tickets that are modified through plugin.
batchmod-3739-notify.patch (4.6 KB) - added by gregmac 14 years ago.
Updated patch (from r8446) to support sending notifications, controllable via checkbox
batchmod-3739-notify.2.patch (4.1 KB) - added by gregmac 14 years ago.
patch applied against r8451

Download all attachments as: .zip

Change History (22)

comment:1 Changed 14 years ago by bernd@…

Cc: bernd@… added; anonymous removed

I'd like to see this option as well. I sometimes would like to use the batch modify but don't because I want individual emails to go out about the ticket updates. Having the option to send these emails out in a batch modify operation would be ideal.

comment:2 Changed 14 years ago by anonymous

Is there any update regarding this feature request ?

comment:3 Changed 14 years ago by Ryan J Ollos

This feature request is somewhat similar to #5495.

comment:4 Changed 14 years ago by CuriousCurmudgeon

Type: defectenhancement

Changed 14 years ago by bernd@…

Attachment: Notifications.patch added

Patch that sends notifications for tickets that are modified through plugin.

comment:5 Changed 14 years ago by Ryan J Ollos

Summary: Add option for mail notification[Patch] Add option for mail notification

I'd be interested to know if the notification will work when using the AnnouncerPlugin.

comment:6 Changed 14 years ago by Olaf Meeuwissen <olaf.meeuwissen@…>

I'd like to have such an option as well. I occasionally find my self doing work that affects a pile of tickets and would like to have everyone notified. Using BatchModify to write the feedback greatly relieves me of copy-pasting the same comment.

I had a quick look at the patch and it seems to only support unconditional notification and contain some unrelated changes as well.

For my use cases I think I'd like a check box so I can activate notification on a per BatchModify basis. Just an idea.

Changed 14 years ago by gregmac

Attachment: batchmod-3739-notify.patch added

Updated patch (from r8446) to support sending notifications, controllable via checkbox

comment:7 Changed 14 years ago by gregmac

For some reason I can't see the contents of the patch? so just in case:

Index: batchmod/web_ui.py
===================================================================
--- batchmod/web_ui.py  (revision 8446)
+++ batchmod/web_ui.py  (working copy)
@@ -9,12 +9,14 @@
 from trac.perm import IPermissionRequestor
 from trac.ticket import TicketSystem, Ticket
 from trac.ticket.query import QueryModule
+from trac.ticket.notification import TicketNotifyEmail
 from trac.web.api import ITemplateStreamFilter
 from trac.web.chrome import ITemplateProvider, Chrome, \
                             add_script, add_stylesheet
 from trac.web.main import IRequestFilter
-from trac.util.datefmt import to_datetime, to_utimestamp
+from trac.util.datefmt import to_datetime, to_utimestamp, utc
 from genshi.filters.transform import Transformer
+from datetime import datetime
 import re

 __all__ = ['BatchModifyModule']
@@ -135,6 +137,9 @@
         modify_changetime = bool(req.args.get(
                                               'batchmod_modify_changetime',
                                               False))
+        send_notifications = bool(req.args.get(
+                                              'batchmod_send_notifications',
+                                              False))

         values = self._get_new_ticket_values(req, env)
         self._check_for_resolution(values)
@@ -148,7 +153,7 @@
             raise TracError, 'No tickets selected'

         self._save_ticket_changes(req, env, log, selectedTickets, tickets,
-                                  values, comment, modify_changetime)
+                                  values, comment, modify_changetime, send_notifications)

     def _get_new_ticket_values(self, req, env):
         """Pull all of the new values out of the post data."""
@@ -174,27 +179,32 @@
             values['resolution'] = ''

     def _save_ticket_changes(self, req, env, log, selectedTickets, tickets,
-                             new_values, comment, modify_changetime):
+                             new_values, comment, modify_changetime, send_notifications):
         @with_transaction(env)
         def _implementation(db):
             for id in selectedTickets:
                 if id in tickets:
                     t = Ticket(env, int(id))
-
+                    new_changetime = datetime.now(utc)
+
                     log_msg = ""
                     if not modify_changetime:
                         original_changetime = to_utimestamp(t.time_changed)

-                    _values = values.copy()
-                    for field in [f for f in values.keys() \
+                    _values = new_values.copy()
+                    for field in [f for f in new_values.keys() \
                                   if f in self._fields_as_list]:
                         _values[field] = self._merge_keywords(t.values[field],
-                                                              values[field],
+                                                              new_values[field],
                                                               log)

                     t.populate(_values)
-                    t.save_changes(req.authname, comment)
+                    t.save_changes(req.authname, comment, when=new_changetime)

+                    if send_notifications:
+                        tn = TicketNotifyEmail(env)
+                        tn.notify(t, newticket=0, modtime=new_changetime)
+
                     if not modify_changetime:
                         self._reset_changetime(env, original_changetime, t)
                         log_msg = "(changetime not modified)"
@@ -234,4 +244,4 @@
         db = env.get_db_cnx()
         db.cursor().execute("UPDATE ticket set changetime=%s where id=%s"
                             % (original_changetime, ticket.id))
-        db.commit()
\ No newline at end of file
+        db.commit()
Index: batchmod/templates/batchmod.html
===================================================================
--- batchmod/templates/batchmod.html    (revision 8446)
+++ batchmod/templates/batchmod.html    (working copy)
@@ -35,6 +35,12 @@
                 <input type="checkbox" id="batchmod_modify_changetime" name="batchmod_modify_changetime" value="true" checked="checked" />
             </td>
         </tr>
+        <tr>
+            <td colspan="3">
+                <label class="batchmod_label" for="batchmod_send_notifications">Send E-mail Notifications</label>
+                <input type="checkbox" id="batchmod_send_notifications" name="batchmod_send_notifications" value="true" checked="checked" />
+            </td>
+       </tr>
     </table>

     <div>

Notes:

  • This includes the changes from #7553:comment:8 (plugin didn't work for me without it)
  • I added an checkbox using the same style as #7048 that says "Send E-Mail Notifications" (checked by default) to allow control of this function
  • Only tested on Trac 0.12

comment:8 Changed 14 years ago by CuriousCurmudgeon

Owner: changed from ashwin_phatak to CuriousCurmudgeon
Status: newassigned

I am having problems with the patch. I already applied the one in #7553, but even if I go back to the previous revision and try to apply this one I get errors.

$ patch -p0 -F 3 < batchmod-3739-notify.patch
patching file `batchmod/web_ui.py'
Hunk #2 succeeded at 137 with fuzz 3.
Hunk #3 succeeded at 153 with fuzz 3.
Hunk #4 FAILED at 179.
Hunk #5 succeeded at 244 with fuzz 2.
1 out of 5 hunks FAILED -- saving rejects to batchmod/web_ui.py.rej
patching file `batchmod/templates/batchmod.html'
Hunk #1 succeeded at 35 with fuzz 2.

comment:9 Changed 14 years ago by CuriousCurmudgeon

Trac Release: 0.110.12

Changed 14 years ago by gregmac

patch applied against r8451

comment:10 Changed 14 years ago by CuriousCurmudgeon

(In [8818]) refs #3739

  • Patch applied. Bumped the version to 0.8.0a1.

comment:11 Changed 14 years ago by CuriousCurmudgeon

Thanks for the patch! I will need to port this back 0.11 and then I will probably do a release.

comment:12 Changed 14 years ago by osimons

I agree with the need to support notifications. However, I don't think the checkbox should be rendered if the project doesn't have notifications enabled. Typically in a project with smtp_enabled = false, there should be no need to raise the issue at all - no notifications should be sent regardless.

The needed change would be do extend the data dict for rendering to include something like {'enable_notify': self.config.getbool('notification', 'smtp_enabled', False)} to the data dict for rendering, and then in template do a <tr py:if="enable_notify>.....</tr> test on including the row in output.

comment:13 Changed 14 years ago by CuriousCurmudgeon

(In [8826]) refs #3739

  • Send notifications message only appears if smtp_enabled is true.

comment:14 Changed 13 years ago by CuriousCurmudgeon

(In [9365] refs #3739
Merged r8818 and r8826 into the 0.11 branch. Not tested yet.

comment:15 Changed 13 years ago by CuriousCurmudgeon

Resolution: fixed
Status: assignedclosed

comment:16 in reply to:  5 ; Changed 13 years ago by Rob Guttman

Resolution: fixed
Status: closedreopened

Replying to rjollos:

I'd be interested to know if the notification will work when using the AnnouncerPlugin.

Agreed. In fact this patch does not work as expected when using the announcer plugin. It would need to also check whether the announcer had email enabled or not like this (web_ui.py line 100 in 0.12 trunk):

        batchFormData['notify_enabled'] = \
            self.config.getbool('notification', 'smtp_enabled', False) or \
            self.config.getbool('announcer', 'email_enabled', False)

The AnnouncerPlugin should take it from there. I just did a quick test and it worked.

comment:17 in reply to:  16 Changed 13 years ago by Rob Guttman

Replying to robguttman:

The AnnouncerPlugin should take it from there. I just did a quick test and it worked.

Err, I spoke too soon. Emails do get sent out via the AnnouncerPlugin, but the problem is that you can't disable email notifications. Looking into this...

comment:18 Changed 13 years ago by Rob Guttman

Resolution: fixed
Status: reopenedclosed

Dang, there's no easy way to fix this. I can't find any way to pass the send_notififcations data to the AnnouncerPlugin via the ITicketChangeListener. It seems one long-term solution is to add a **kw arg to the ITicketChangeListener.ticket_changed() api which can be either ignored or processed by each ticket listener. You could then pass the send_notififcations param which the AnnouncerPlugin could use to send notifications or not. But that's likely not going to happen anytime soon (if ever).

If anyone has any near-term ideas, that would be great. In the interim, I'm re-closing this ticket. Sorry for the noise.

comment:19 in reply to:  18 Changed 11 years ago by Patrick Schaaf

Cc: Patrick Schaaf added

Replying to robguttman:

If anyone has any near-term ideas, that would be great.

Just hacked something together that works, whatever that's worth. Don't know whether that warrants reopening this ticket, but I thought I'd write it down at least.

In announcer producers/ticket.py I added to TicketChangeProducer ticket_changed:

    def ticket_changed(self, ticket, comment, author, old_values):
        if getattr(ticket, 'announcer_suppress_announcement', False):
            return

And in batchmodify I applied (against 0.8.1_trac0.11dev_r11791):

--- /tmp/web_ui.py      2013-08-14 14:39:00.738226694 +0200
+++ web_ui.py   2013-08-14 14:28:21.263181156 +0200
@@ -34,6 +34,14 @@
                 default=' ',
                 doc="Connectr string for 'list' fields. Defaults to a space.")
 
+    notifications = None
+
+    def __init__(self):
+        if self.config.getbool('notification', 'smtp_enabled', False):
+            self.notifications = 'notification'
+        elif self.config.getbool('announcer', 'smtp_enabled', False):
+            self.notifications = 'announcer'
+ 
     # IPermissionRequestor methods
     def get_permission_actions(self):
         yield 'TICKET_BATCH_MODIFY'
@@ -66,7 +74,8 @@
             
             batch_modifier = BatchModifier(self.fields_as_list, 
                                            self.list_separator_regex, 
-                                           self.list_connector_string)
+                                           self.list_connector_string,
+                                           self.notifications)
             batch_modifier.process_request(req, self.env, self.log)
             # redirect to original Query
             # TODO: need better way to fake QueryModule...
@@ -96,8 +105,7 @@
         batchFormData = dict(data)
         batchFormData['query_href']= req.session['query_href'] \
                                      or req.href.query()
-        batchFormData['notify_enabled'] = self.config.getbool('notification', 
-                                                        'smtp_enabled', False)
+        batchFormData['notify_enabled'] = self.notifications is not None
         
         ticketSystem = TicketSystem(self.env)
         fields = []
@@ -125,11 +133,12 @@
     """Modifies a batch of tickets"""
     
     def __init__(self, fields_as_list, list_separator_regex, 
-                 list_connector_string):
+                 list_connector_string, notifications):
         """Pull all the config values in."""
         self._fields_as_list = fields_as_list
         self._list_separator_regex = list_separator_regex
         self._list_connector_string = list_connector_string
+        self._notifications = notifications
     
         # Internal methods 
     def process_request(self, req, env, log):
@@ -207,9 +216,11 @@
                                                           log)
                 
                 t.populate(_values)
+                if self._notifications == 'announcer' and not send_notification
s:
+                    t.announcer_suppress_announcement = True
                 t.save_changes(req.authname, comment, when=new_changetime)
   
-                if send_notifications:
+                if self._notifications == 'notification' and send_notifications
:
                     tn = TicketNotifyEmail(env)
                     tn.notify(t, newticket=0, modtime=new_changetime)
Last edited 11 years ago by Ryan J Ollos (previous) (diff)

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain CuriousCurmudgeon.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.