<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/2064/">https://reviewboard.asterisk.org/r/2064/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On August 9th, 2012, 3:06 p.m., <b>Matt Jordan</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;">Bumping the review.</pre>
</blockquote>
<p>On August 9th, 2012, 3:17 p.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;">The problem is that you're only looking at the simplest case. Suppose a call comes in, they talk to a person, and then they get transferred to a Queue, and they hang up before the Queue is answered (in any of the scenarios above). Your essentially saying that the billsecs up until that time don't count and that the CDR should therefore both indicate "NO ANSWER", which is patently false, as well as recording some billsecs, because the call was, in fact, answered. That will confuse all sorts of billing systems, and thus is really of the type of fix that shouldn't go in unless you account for problems like this; well, no, shouldn't go in, period. There are too many call scenarios where trying to manipulate this value after the call was answered will be problematic and will break billing systems. In other words, don't touch it.</pre>
</blockquote>
<p>On August 9th, 2012, 3:27 p.m., <b>Paul Belanger</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;">This was the reason for my previous comment. I was under the impression that we no longer did any CDR fixes, and recommended people use CEL, simple because we could never ensure it would not break other scenarios.</pre>
</blockquote>
<p>On August 9th, 2012, 3:47 p.m., <b>Matt Jordan</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 assumption that we 'no longer did any CDR fixes' is incorrect. CDRs have not been deprecated. Would I like to deprecate them? Sure - but I don't think that's a valid option at this time. If I'm wrong in that then by all means, send an e-mail to the -users list telling everyone that CDRs are deprecated and see what response you get.
What we aren't going to attempt to do is define behavior that cannot be defined given the limitations imposed by CDRs.
So what behavior is undefined? Well, one area that is certainly undefined in CDRs is what happens in transfer scenarios. We've never claimed that the resulting CDR record in a blind or attended transfer is going to account for all of the actions that happen between the various channels.
So let's look at the case of an attended transfer as Tilghman outlined without this patch:
Phone A calls Phone B. Phone B transfers Phone A into a Queue with Phone C as a member (unpaused) and Phone D as a member (paused). Phone A hangs up.
Do you get a billsec value? Yup. Do you get a disposition? Yup - BUSY in this case. That hardly makes any sense, but - of course - CDR behavior in transfer scenarios is undefined.
Lets try this one. Phone A calls Phone B. Phone B transfers Phone A into a Queue with Phone C as a member and Phone D as a member, both unpaused.
Now its even worse: if there's an Answer() before the Queue, you'll get a CDR with a disposition of ANSWERED - if there isn't an Answer() before the Queue, you get a disposition of NOT ANSWERED. In both cases, however, you still have a billsec value.
So Tilghman's problem already exists without this patch.
I'm not claiming to fix the case of transfers, nor am I trying to 'fix' all issues with CDRs. In this case, however, the simple behavior of CDRs in Queue with multiple agents has multiple behaviors, none of which are predictable. I'm not sure how this patch doesn't at least make the simpler case consistent. Since that behavior has been defined with respect to a parallel Dial, matching that behavior with Queue makes some sense.</pre>
</blockquote>
<p>On August 9th, 2012, 3:51 p.m., <b>Matt Jordan</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 should amend my previous comment - in the case of the transfer (which as I said, is on its face undefined behavior), the call will be "ANSWERED" regardless of the Answer application before the Queue. That happens even if no one answers Phone B in the Queue (which is probably not desirable either).
It does record BUSY if one of the Queue members was paused.</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;">I do understand and commiserate with your interest in fixing legitimate bugs. But from my perspective and experience with prior "fixes" to the CDR subsystem, anything you do in this regard will be pilloried, when someone upgrades for security and gets your fix which breaks their billing system. It's why we tread very lightly around the CDR subsystem and why we encourage people to use the CEL subsystem when writing new billing systems.
Quite simply put, I don't think your desire for consistency (and especially not your proposed solution) outweighs the danger of breaking production systems in the middle of a release cycle. It would seem to me that the only desired consistency would be to make the disposition ANSWERED even when queue members are paused (or more correctly, never to backtrack from the ANSWERED disposition to NO ANSWER, BUSY, or CONGESTION, when it comes to setting disposition at the end of a Queue). I still think you should follow Paul's recommendation to only modify trunk, not any release branches.</pre>
<br />
<p>- Tilghman</p>
<br />
<p>On July 25th, 2012, 4:54 p.m., Matt Jordan 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 Matt Jordan.</div>
<p style="color: grey;"><i>Updated July 25, 2012, 4:54 p.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;">When a caller enters a queue and no queue member answers the call, some odd behavior can be reporter in the resulting CDR records, depending on the state of certain queue members.
* If no queue members are paused, the CDR result is whatever the result was prior to going into app_queue. If the call was answered, this is ANSWERED; otherwise, it is NO ANSWER.
* If some queue members are paused, the CDR result is BUSY. This patch changes this to NO ANSWER.
* If all queue members are paused, the CDR result is again whatever the result was prior to going into app_queue.
* If the caller hangs up, times out, or presses '*' with the 'h' option, the result is again not set.
Similar to app_dial, if the caller in the queue does not get answered, we should set the CDR record appropriately. In general, for the scenarios listed above, this is NO ANSWER.
This patch is a only slightly modified version of the one supplied by the issue reporter.
Since this is arguably a change in behavior, some tests were written to cover portions of this behavior in https://reviewboard.asterisk.org/r/2063.</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/AST-906">AST-906</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>/branches/1.8/apps/app_queue.c <span style="color: grey">(370418)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/2064/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>