Modify

Opened 6 years ago

Closed 3 years ago

Last modified 8 months ago

#3739 closed enhancement (fixed)

[Patch] Add option for mail notification

Reported by: gannis Owned by: CuriousCurmudgeon
Priority: normal Component: BatchModifyPlugin
Severity: normal Keywords:
Cc: bernd@…, trachacks@… 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@… 4 years ago.
Patch that sends notifications for tickets that are modified through plugin.
batchmod-3739-notify.patch (4.6 KB) - added by gregmac 4 years ago.
Updated patch (from r8446) to support sending notifications, controllable via checkbox
batchmod-3739-notify.2.patch (4.1 KB) - added by gregmac 4 years ago.
patch applied against r8451

Download all attachments as: .zip

Change History (22)

comment:1 Changed 5 years ago by bernd@…

  • Cc bernd@… added

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 4 years ago by anonymous

Is there any update regarding this feature request ?

comment:3 Changed 4 years ago by rjollos

This feature request is somewhat similar to #5495.

comment:4 Changed 4 years ago by CuriousCurmudgeon

  • Type changed from defect to enhancement

Changed 4 years ago by bernd@…

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

comment:5 follow-up: Changed 4 years ago by rjollos

  • Summary changed from Add option for mail notification to [Patch] Add option for mail notification

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

comment:6 Changed 4 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 4 years ago by gregmac

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

comment:7 Changed 4 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 4 years ago by CuriousCurmudgeon

  • Owner changed from ashwin_phatak to CuriousCurmudgeon
  • Status changed from new to assigned

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 4 years ago by CuriousCurmudgeon

  • Trac Release changed from 0.11 to 0.12

Changed 4 years ago by gregmac

patch applied against r8451

comment:10 Changed 4 years ago by CuriousCurmudgeon

(In [8818]) refs #3739

  • Patch applied. Bumped the version to 0.8.0a1.

comment:11 Changed 4 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 4 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 4 years ago by CuriousCurmudgeon

(In [8826]) refs #3739

  • Send notifications message only appears if smtp_enabled is true.

comment:14 Changed 3 years ago by CuriousCurmudgeon

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

comment:15 Changed 3 years ago by CuriousCurmudgeon

  • Resolution set to fixed
  • Status changed from assigned to closed

comment:16 in reply to: ↑ 5 ; follow-up: Changed 3 years ago by robguttman

  • Resolution fixed deleted
  • Status changed from closed to reopened

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 3 years ago by robguttman

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 follow-up: Changed 3 years ago by robguttman

  • Resolution set to fixed
  • Status changed from reopened to closed

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 8 months ago by bof

  • Cc trachacks@… 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 8 months ago by rjollos (previous) (diff)

Add Comment

Modify Ticket

Action
as closed .
as The resolution will be set. Next status will be 'closed'.
to The owner will be changed from CuriousCurmudgeon. Next status will be 'closed'.
The resolution will be deleted. Next status will be 'reopened'.
Author


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

 
Note: See TracTickets for help on using tickets.