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

[David Vossel]

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


This is a much better approach!


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

    While you are familiar with this code, can you comment about the return values of this function in app.h please.



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

    you can reduce indentation.
    
    if ((outmsg == 2) && !skip_confirmation_sound)) {
    
    ....
    }



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

    I made a comment about documenting the return code of ast_play_and_record... Its probably worth while to document the behavior a little better of all of the ast_play_and_* functions now that you have been in the code.  Particularly the behavior of playing back the "thankyou" sound file.  It would not be obvious to me looking at the documentation that this would even occur.


- David


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/6f2f02d5/attachment-0001.htm>