[Home]

Summary:ASTERISK-11150: [patch] Notification email should use the voicemail's metadata
Reporter:James McCoy (jamessan)Labels:
Date Opened:2008-01-04 10:15:45.000-0600Date Closed:2009-04-03 15:05:58
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Applications/app_voicemail/NewFeature
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) 20090104__bug11678.diff.txt
( 1) 20090327__bug11678.diff.txt
( 2) 20090330__bug11678.diff.txt
Description:When a user has voicemail notification enabled, the email that is sent builds various variables (VM_DATE, VM_CALLERID, VM_CIDNAME, VM_CIDNUM) on the fly instead of using the information from msgXXXX.txt.  This isn't normally that different for a notification email triggered by someone directly leaving a voicemail but it can differ significantly when it is for a voicemail that is being forwarded.  Also, using the voicemail metadata would make the notification email consistent with what is presented to the user when they access the voicemail via dialing in to the voicemail app.  For example, A leaves a voicemail for B.  B forwards the voicemail to C.  C's notification email will say that they received an email from B whereas when they listen to the voicemail envelope it will be listed as originating from A.
Comments:By: Tilghman Lesher (tilghman) 2008-01-04 12:57:08.000-0600

Actually, it seems like what we really ought to be doing is changing the envelope, since it is otherwise confusing to hear a message recorded earlier after one originally left later.  Thoughts?
By: James McCoy (jamessan) 2008-01-04 13:06:35.000-0600

I had thought of proposing additional fields for the envelope specific to forwarded messages.  This gives you the additional info of who forwarded it and when but you retain who the original message is from.  The original callerid info from the notification email is important information which we don't currently have.
By: BJ Weschke (bweschke) 2008-01-04 13:32:28.000-0600

I agree with jamessan that supporting the original callerid fields as well as having the correct originating information in a new envelope would be a good "nice to have". Those additional fields I would consider to be new functionality.
However, the way it is now, it almost seems a breach of confidentiality for C to know that A originated the voicemail message when it was B that did the forwarding. This falls more into the "bug" category in my mind.
By: Tilghman Lesher (tilghman) 2008-02-05 13:15:05.000-0600

Given the multitude of backends, though, this is fairly difficult to change and get right in a release branch, so I'm treating this as a feature add to trunk.
By: Tilghman Lesher (tilghman) 2008-02-05 13:16:18.000-0600

Specifically, to accomodate all of the fields needed, you would need to alter the ODBC table structure, which is not a nice change to make in the middle of a release cycle.
By: Greg Merriweather (shido6) 2008-05-19 15:26:25

The Email notification is fine, this is great. When using the phone and replying to the forwarded message it sends the message to the ORIGINAL sender instead of the IMMEDIATE sender.

Why is this important?

John sends a message to Sally complaining. Sally forwards the message with a "prepend" to Jim a colleague who then replies and says, "Yeah John is an a-hole." The message gets sent to John and Jim gets fired for his comment. The message should be sent to Sally and not John.
By: Digium Subversion (svnbot) 2008-06-10 16:08:23

Repository: asterisk
Revision: 121683

U   trunk/include/asterisk/res_odbc.h
U   trunk/res/res_config_odbc.c
U   trunk/res/res_odbc.c

------------------------------------------------------------------------
r121683 | tilghman | 2008-06-10 16:08:20 -0500 (Tue, 10 Jun 2008) | 4 lines

Move the table cache routines to res_odbc, so they can be used from other
places (app_voicemail, for example).
(Related to bug ASTERISK-11150)

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=121683
By: Digium Subversion (svnbot) 2008-06-10 16:09:19

Repository: asterisk
Revision: 121684

_U  branches/1.6.0/

------------------------------------------------------------------------
r121684 | tilghman | 2008-06-10 16:09:18 -0500 (Tue, 10 Jun 2008) | 11 lines

Blocked revisions 121683 via svnmerge

........
r121683 | tilghman | 2008-06-10 16:14:58 -0500 (Tue, 10 Jun 2008) | 4 lines

Move the table cache routines to res_odbc, so they can be used from other
places (app_voicemail, for example).
(Related to bug ASTERISK-11150)

........

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=121684
By: Tilghman Lesher (tilghman) 2008-10-09 19:10:00

My current thinking is that we're going to specify a completely separate format for the email notifications of forwarded voicemails, to accommodate for this change.  The prerequisite changes are in place; all that is needed now is for someone to make the changes, so they can be tested.
By: Tilghman Lesher (tilghman) 2009-03-03 15:40:07.000-0600

I rethought the problem and instead created 4 additional variables for your use.  Note that you can use the IFNULL() dialplan function to optionally select either one, depending on if the others are blank.

The existing variables remain the same, for backwards compatibility.  However, in addition to the existing variables, ORIG_VM_DATE, ORIG_VM_CALLERID, ORIG_VM_CIDNUM, and ORIG_VM_CIDNAME correspond to the values within the message information file.

This patch should apply cleanly against current 1.4 SVN.


By: Leif Madsen (lmadsen) 2009-03-13 14:49:58

I think s/IFNULL()/ISNULL()/ right?
By: Tilghman Lesher (tilghman) 2009-03-13 16:02:19

lmadsen: yes, that sounds right
By: Leif Madsen (lmadsen) 2009-03-23 16:37:02

Sorry, backtrace attached -- I tested this, and it crashed.
By: Tilghman Lesher (tilghman) 2009-03-23 16:43:38

Stupid bang key...
By: Leif Madsen (lmadsen) 2009-03-27 12:56:52

OK, I tested again. No crashing!

However, when I forward a message, I don't get the ORIG_* variables populated it seems. Here is the template I'm using to send:

emailbody=Current variables for this message are:\n\n* ${VM_NAME}\n* ${VM_DUR}\n* ${VM_MSGNUM}\n* ${VM_MAILBOX}\n* ${VM_CALLERID}\n* ${VM_CIDNUM}\n* ${VM_CIDNAME}\n* ${VM_DATE}\n\n* ${ORIG_VM_DATE}\n* ${ORIG_VM_CALLERID}\n* ${ORIG_VM_CIDNUM}\n* ${ORIG_VM_CIDNAME}

And here is the result of a forwarded message, left for Bob, and Bob forwarded to Alice (600 and 601 respectively):

* Alice
* 0:06
* 1
* 601
* "Leif Madsen" <4164790259>
* 4164790259
* Leif Madsen
* Friday, March 27, 2009 at 01:52:47 PM

*
*
*
*
By: Leif Madsen (lmadsen) 2009-03-27 13:20:38

If you can layout some information on all the things that need to be tested here that are expected to be resolved, then I can make sure I get through all of them as well. Thanks!
By: Tilghman Lesher (tilghman) 2009-03-27 17:35:47

Okay, tested and tweaked.  I'm mostly happy with this one.
By: Tilghman Lesher (tilghman) 2009-03-30 11:59:00

New patch uploaded.  The main difference with this one is that I changed the default emailbody to have a slightly different message when a forward is being done.  This should mostly match the example message.  The example message has been fairly well tested.
By: Leif Madsen (lmadsen) 2009-04-02 11:00:44

* Left a voicemail at 600
* 600 forwards the voicemail to 601 with a prepend
* 601 forwards the voicemail to 602 with a prepend
* 602 then tries to reply; voicemailmain() does not know who sent the message, and is unable to reply.
By: Leif Madsen (lmadsen) 2009-04-02 12:19:06

OK, this looks good to go. The other issue with the replying will need to be opened in a separate issue (I found it as well), but the issue reported here has been resolved by Tilghman, with a slight tweak by myself.
By: Digium Subversion (svnbot) 2009-04-03 14:06:59

Repository: asterisk
Revision: 186415

U   branches/1.4/apps/app_voicemail.c
U   branches/1.4/configs/voicemail.conf.sample

------------------------------------------------------------------------
r186415 | tilghman | 2009-04-03 14:06:58 -0500 (Fri, 03 Apr 2009) | 7 lines

Distinguish in a sent email between simple sends and forwards.
(closes issue ASTERISK-11150)
Reported by: jamessan
Patches:
      20090330__bug11678.diff.txt uploaded by tilghman (license 14)
Tested by: tilghman, lmadsen

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=186415
By: Digium Subversion (svnbot) 2009-04-03 14:30:35

Repository: asterisk
Revision: 186444

_U  trunk/
U   trunk/apps/app_voicemail.c
U   trunk/configs/voicemail.conf.sample

------------------------------------------------------------------------
r186444 | tilghman | 2009-04-03 14:30:35 -0500 (Fri, 03 Apr 2009) | 14 lines

Merged revisions 186415 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
 r186415 | tilghman | 2009-04-03 14:06:58 -0500 (Fri, 03 Apr 2009) | 7 lines
 
 Distinguish in a sent email between simple sends and forwards.
 (closes issue ASTERISK-11150)
  Reported by: jamessan
  Patches:
        20090330__bug11678.diff.txt uploaded by tilghman (license 14)
  Tested by: tilghman, lmadsen
........

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=186444
By: Digium Subversion (svnbot) 2009-04-03 14:58:22

Repository: asterisk
Revision: 186446

_U  branches/1.6.0/
U   branches/1.6.0/apps/app_voicemail.c
U   branches/1.6.0/configs/voicemail.conf.sample

------------------------------------------------------------------------
r186446 | tilghman | 2009-04-03 14:58:22 -0500 (Fri, 03 Apr 2009) | 21 lines

Merged revisions 186444 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

................
 r186444 | tilghman | 2009-04-03 14:30:34 -0500 (Fri, 03 Apr 2009) | 14 lines
 
 Merged revisions 186415 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
   r186415 | tilghman | 2009-04-03 14:06:58 -0500 (Fri, 03 Apr 2009) | 7 lines
   
   Distinguish in a sent email between simple sends and forwards.
   (closes issue ASTERISK-11150)
    Reported by: jamessan
    Patches:
          20090330__bug11678.diff.txt uploaded by tilghman (license 14)
    Tested by: tilghman, lmadsen
 ........
................

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=186446
By: Digium Subversion (svnbot) 2009-04-03 15:04:17

Repository: asterisk
Revision: 186448

_U  branches/1.6.1/
U   branches/1.6.1/apps/app_voicemail.c
U   branches/1.6.1/configs/voicemail.conf.sample

------------------------------------------------------------------------
r186448 | tilghman | 2009-04-03 15:04:17 -0500 (Fri, 03 Apr 2009) | 32 lines

Merged revisions 186444,186447 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

................
 r186444 | tilghman | 2009-04-03 14:30:34 -0500 (Fri, 03 Apr 2009) | 14 lines
 
 Merged revisions 186415 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
   r186415 | tilghman | 2009-04-03 14:06:58 -0500 (Fri, 03 Apr 2009) | 7 lines
   
   Distinguish in a sent email between simple sends and forwards.
   (closes issue ASTERISK-11150)
    Reported by: jamessan
    Patches:
          20090330__bug11678.diff.txt uploaded by tilghman (license 14)
    Tested by: tilghman, lmadsen
 ........
................
 r186447 | tilghman | 2009-04-03 14:59:55 -0500 (Fri, 03 Apr 2009) | 9 lines
 
 Merged revisions 186445 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
   r186445 | tilghman | 2009-04-03 14:56:48 -0500 (Fri, 03 Apr 2009) | 2 lines
   
   Found a conflict in the last commit, due to multiple targets
 ........
................

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=186448
By: Digium Subversion (svnbot) 2009-04-03 15:05:57

Repository: asterisk
Revision: 186449

_U  branches/1.6.2/
U   branches/1.6.2/apps/app_voicemail.c
U   branches/1.6.2/configs/voicemail.conf.sample

------------------------------------------------------------------------
r186449 | tilghman | 2009-04-03 15:05:57 -0500 (Fri, 03 Apr 2009) | 32 lines

Merged revisions 186444,186447 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

................
 r186444 | tilghman | 2009-04-03 14:30:34 -0500 (Fri, 03 Apr 2009) | 14 lines
 
 Merged revisions 186415 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
   r186415 | tilghman | 2009-04-03 14:06:58 -0500 (Fri, 03 Apr 2009) | 7 lines
   
   Distinguish in a sent email between simple sends and forwards.
   (closes issue ASTERISK-11150)
    Reported by: jamessan
    Patches:
          20090330__bug11678.diff.txt uploaded by tilghman (license 14)
    Tested by: tilghman, lmadsen
 ........
................
 r186447 | tilghman | 2009-04-03 14:59:55 -0500 (Fri, 03 Apr 2009) | 9 lines
 
 Merged revisions 186445 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
   r186445 | tilghman | 2009-04-03 14:56:48 -0500 (Fri, 03 Apr 2009) | 2 lines
   
   Found a conflict in the last commit, due to multiple targets
 ........
................

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=186449