[asterisk-dev] [Code Review] Prepend voicemail timeout bug

[mjordan]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/1327/#review3906
-----------------------------------------------------------



/branches/1.8/apps/app_voicemail.c
<https://reviewboard.asterisk.org/r/1327/#comment7733>

    Two quick comments:
    1. The sound 'then-press-pound' may be useful in a number of places in app_voicemail, and not just when prepend times out.  You may want to rename the variable and its corresponding configuration value to reflect that.
    
    2. Include the 'vm-prepend-timeout' variable in voicemail.conf.samples for documentation purposes



/branches/1.8/main/app.c
<https://reviewboard.asterisk.org/r/1327/#comment7734>

    I'm not sure we can make the assumption that we only say 'goodbye' during a prepend operation.  When a user leaves a voicemail, and presses the '#' key, ast_play_and_record_full is called, which will play the 'good-bye' message for them.
    
    Either this needs to be an option on each of the ast_play_and_record_* methods, and all of the places updated accordingly, or we should defer this change until we can properly pull the 'good-bye' message out of app.c


- mjordan


On July 21, 2011, 9:33 a.m., jrose wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1327/
> -----------------------------------------------------------
> 
> (Updated July 21, 2011, 9:33 a.m.)
> 
> 
> Review request for Asterisk Developers, David Vossel and mjordan.
> 
> 
> Summary
> -------
> 
> When recording a prepend message for voicemail, I think the following is the intended behavior:
> 
> 1.  User starts prepend recording
> 2.  User concludes prepend recording with a button in order to finish the recording.  Confirm success if the user stops the recording manually.
> 3.  If the user times out instead, confirm a failure and go back to the menu with no changes.
> 
> What was happening instead was...
> 
> The first time through, things were mostly as above, except if the user timed out, Allison would use the same message as if the call went through.
> The recording would not be made in this instance.
> 
> However, the second time and above through, a flag saying the call had already been recorded would be raised and this would trigger some file copying events on files that shouldn't have been being copied.  If the call timed out again, the prepend message would write over the inbox message and the whole message would be forwarded to the recipient.
> 
> This patch changes Allison's prompt for timing out while in voicemail mode when not entering the finished recording mode to say "Sorry, Please Try again" followed by "Press the pound key when finished" (I might be paraphrasing slightly).  Technically any DTMF works, but I haven't seen any prompt for that yet and I haven't settled on this as a permanent solution.  After this point, when the menu comes back around, we lower the flag for the recording having been made essentially reverting back to the original state of the menu.
> 
> If this seems appropriate but less than ideal, I am ready at to begin adding a post-prepend-recording menu that will allow the user to review the complete message as well as accept, re-record it, append it, cancel it, etc. before sending it on.  Naturally if something like this were in place, there wouldn't be any need to just outright discard prepend recordings after a time-out.  Depending on how many of those options we want to include, that could be some extra voice work, so we might consider holding off on that for now and just going with the patch as is, since this works and it informs the user of what is happening much more clearly than the current means does.
> 
> This is actually the second revision of the patch and moves the speech into app_voicemail's forwarding menu code and prepares it for customization.  Speech file customization isn't tested yet, but looks right.
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/apps/app_voicemail.c 329012 
>   /branches/1.8/main/app.c 329012 
> 
> Diff: https://reviewboard.asterisk.org/r/1327/diff
> 
> 
> Testing
> -------
> 
> Tested prepend with timeout and prepend with confirmation, as well as multiple timeouts and multiple timeouts followed by recording with confirmation.  In each case there is no residual backup message, and the call is processed as expected.  Also, the prepend message no longer gets tacked onto the original message.
> 
> 
> Thanks,
> 
> jrose
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20110721/c28bcf7e/attachment-0001.htm>