[asterisk-dev] [Code Review] Added VM_INFO dialplan function to return voicemail user information

[wdoekes]

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



trunk/apps/app_voicemail.c
<https://reviewboard.asterisk.org/r/1568/#comment8899>

    Wrap a <note> around the <para/> for extra visibility.



trunk/apps/app_voicemail.c
<https://reviewboard.asterisk.org/r/1568/#comment8900>

    An extra tab please.



trunk/apps/app_voicemail.c
<https://reviewboard.asterisk.org/r/1568/#comment8901>

    Another <note>.



trunk/apps/app_voicemail.c
<https://reviewboard.asterisk.org/r/1568/#comment8903>

    Drop the cast (char*) cast. ast_log does not need to modify the contents.
    
    (Yea.. it was here before you came. The whole file is littered with bad casts. But since you're here, you might as well fix it.)



trunk/apps/app_voicemail.c
<https://reviewboard.asterisk.org/r/1568/#comment8902>

    I would've preferred static variable declarations first, but you're consistent with the rest of the source. So it's good where it is. 



trunk/apps/app_voicemail.c
<https://reviewboard.asterisk.org/r/1568/#comment8904>

    Another (char*) to remove.



trunk/apps/app_voicemail.c
<https://reviewboard.asterisk.org/r/1568/#comment8905>

    messagecount takes a const char*, so you don't need the ast_strdupa.
    
    Furthermore, it already does an ast_strlen_zero(folder), so you don't need the S_OR either.
    
    If anything, you should do S_OR(arg.mailbox_folder, NULL) as the ODBC_STORAGE messagecount() doesn't do an ast_strlen_zero, but a boolean test. But you should probably fix that instead.


You are right about the locale settings. populate_defaults is called on all new mailboxes before *all* of the config settings are read.

populate_defaults sets these: globalflags, passwordlocation, saydurationminfo, callcontext, dialcontext, exitcontext, zonetag, locale, vmminsecs, vmmaxsecs, maxmsg, maxdeletedmsg, imapfolder

But this is done in the "if (ucfg)" and "while (cat)" blocks. The following of those are set first afterwards: globalflags (VM_PBXSKIP flag), zonetag, locale

That's a bug and should be fixed in a different patch.

- wdoekes


On Nov. 7, 2011, 7:04 p.m., shawkris wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1568/
> -----------------------------------------------------------
> 
> (Updated Nov. 7, 2011, 7:04 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> Creted VM_INFO dialplan function which can return information about a user defined in the standard voicemail application.
> 
> Supported attributes are: email, exists, fullname, pager, password, count.
> 
> e.g. from the dialplan, ${VM_INFO(1234 at test,email)} would return the email address for voicemail user 1234 in the test context.
> 
> The 'count' attribute can also take a folder parameter and will return the number of messages in that folder, or by default the Inbox.
> 
> 
> This addresses bug ASTERISK-18634.
>     https://issues.asterisk.org/jira/browse/ASTERISK-18634
> 
> 
> Diffs
> -----
> 
>   trunk/CHANGES 343636 
>   trunk/UPGRADE.txt 343636 
>   trunk/apps/app_voicemail.c 343636 
> 
> Diff: https://reviewboard.asterisk.org/r/1568/diff
> 
> 
> Testing
> -------
> 
> Tested with a local dialplan calling the VM_INFO function with each parameter to make sure the expected result was returned. Tested that calling with an invalid attribute parameter produced an error message.
> 
> 
> Thanks,
> 
> shawkris
> 
>

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