<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://reviewboard.asterisk.org/r/1568/">https://reviewboard.asterisk.org/r/1568/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On November 5th, 2011, 8:08 p.m., <b>mjordan</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">The only real comment I have with this patch is that you duplicate the MAILBOX_EXISTS function. Since there's already a dedicated function for testing if a mailbox exists, I wouldn't duplicate it here. Also, if you get some time, it'd be great if this had a unit test associated with it - see the test_voicemail_msgcount unit test for an example of this (although I imagine a test for VM_INFO could be a bit less involved). You could certainly submit that as a separate patch if you'd like. Overall, looks like this would be a useful addition to get user information from the dialplan.</pre>
</blockquote>
<p>On November 6th, 2011, 9:17 a.m., <b>wdoekes</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">May I add that you have 'mailbox_folder', 'mbox' and 'mbxfolder' variables. I suggest you choose one name for 'mailbox' and stick with it.
The language, tz (timezone) and locale attributes might be worthy to add, although I don't have a strong opinion about that.
As far as the exists check, I second removing it. Having more ways than one to do things should be restricted to the perl language ;)</pre>
</blockquote>
<p>On November 7th, 2011, 9:27 a.m., <b>Tilghman Lesher</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I disagree. In fact, we should allow the 'exists' here and deprecated MAILBOX_EXISTS.</pre>
</blockquote>
<p>On November 7th, 2011, 9:41 a.m., <b>wdoekes</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Fine by me too. (Even better.. but I wasn't sure how lightly deprecation is taken.) As long as the end result is that there is only one canonical way to do things.</pre>
</blockquote>
<p>On November 7th, 2011, 9:48 a.m., <b>Tilghman Lesher</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">If past history is any guide, look at how the CHANNEL() dialplan function was created and what functions were deprecated as a result. We like dialplan functions that do multiple things, as long as the multiple things are all related in function.</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">'mailbox_context' (I assume you mean that and not mailbox_folder), 'mbox' and 'mbxfolder' mean different things in the code.
mailbox_context is the raw argument from the dialplan, such as 1234@default.
mbox is the parsed mailbox, such as 1234.
mbxfolder is the folder name, after the original folder name was checked for null. This would something like Inbox.
Are there standard names for these variables? I did look through app_voicemail.c and chose variable names that seemed similar to ones already in use in other functions for the same purpose.
</pre>
<br />
<p>- shawkris</p>
<br />
<p>On November 5th, 2011, 10:56 a.m., shawkris wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Asterisk Developers.</div>
<div>By shawkris.</div>
<p style="color: grey;"><i>Updated Nov. 5, 2011, 10:56 a.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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@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.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://issues.asterisk.org/jira/browse/ASTERISK-18634">ASTERISK-18634</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>trunk/apps/app_voicemail.c <span style="color: grey">(343488)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/1568/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>