Summary:	ASTERISK-06538: [patch][post 1.4] voicemail external notification not working
Reporter:	Ryan Hulsker (rhulsker)	Labels:
Date Opened:	2006-03-13 17:03:41.000-0600	Date Closed:	2011-06-07 14:10:46
Priority:	Minor	Regression?	No
Status:	Closed/Complete	Components:	Applications/app_voicemail
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) voicemail_changes.diff
Description:	Due to a typo in app_voicemail.c external notification scripts never get run. ****** ADDITIONAL INFORMATION ****** Line 2350 in app_voicemail.c reads if (messagecount(ext_context, &newvoicemails, &oldvoicemails)) { When it should read if (!messagecount(ext_context, &newvoicemails, &oldvoicemails)) { Because messagecount() returns 0 on failure and 1 on success.
Comments:	By: Olle Johansson (oej) 2006-03-14 01:07:54.000-0600 Messagecount returns 0 on success, -1 on some failures By: Ryan Hulsker (rhulsker) 2006-03-14 11:47:56.000-0600 Well, that explains alot. It looks like the version of messgagecount() that is used with ODBC_STORAGE is different from the messagecount() used without ODBC_STORAGE. under ODBC_STORAGE messagecount returns 0 on failure, and 1 on success. without ODBC_STORAGE messgaecount returns -1 on failure and 0 on success. except in the case where no mailbox is passed in which case they both return 0. The other difference is that the ODBC_STORAGE messagecount does not handle the case where multiple comma separated mailboxes are passed to the function. I will work on a patch. By: Ryan Hulsker (rhulsker) 2006-03-14 12:38:21.000-0600 Patch contains several simple fixes to messagecount() when used with ODBC_STORAGE. ODBC_STORAGE version of messagecount() now has the same return behavior as the non ODBC_STORAGE version (ie 0=ok, -1=fail). ODBC_STORAGE version of messagecount() now properly handles comma separated list of mailboxes to check (as in non ODBC_STORAGE) ODBC_STORAGE version of messagecount() now properly handles NULL inputs for *newmsgs and *oldmsgs ie. don't check (as in non ODBC_STORAGE) Sorry for the large amount of whitespace changes, this is due to wrapping significant portions of the code in if(){} blocks to handle the NULL parameters. By: Olle Johansson (oej) 2006-03-14 14:52:21.000-0600 I can't test this patch, but it looks like you got it right. Thanks for taking time to fix this! By: Jason Parker (jparker) 2006-03-16 20:26:02.000-0600 Please use tabs instead of spaces in your patch, per the coding guidelines. Also...why are you calling messagecount recursively?  You could probably do it in a cleaner way - instead of calling 2 queries per call.  It appears that you're just returning a newmsg count and an oldmsg count. By: Olle Johansson (oej) 2006-03-17 01:24:10.000-0600 north: Look at the other non-odbc messagecount function. We need to implement them the same way. If we change one to be non-recursive, we need to change both. The recursive part is for supporting multiple mailboxes in one mailbox=1234,4556,5555 line. By: Jason Parker (jparker) 2006-03-17 23:16:16.000-0600 I guess I know why it's being called recursively.  I'm just thinking there might be a better way to go about it.  2 queries (setups and teardowns) per mailbox seems like...a lot. By: Olle Johansson (oej) 2006-03-18 03:22:29.000-0600 north: Right, I see what you mean. Agree. Please try to come up with a better way to do it! Thanks. By: Olle Johansson (oej) 2006-04-04 08:57:27 Ok, seems like we need to stick with the current code since we don't get any clever new code. There's still time for that. By: Olle Johansson (oej) 2006-04-04 08:58:25 We need an updated patch with formatting according to coding guidelines and applicable to current trunk version, thanks, rhulsker. I'm including this in test-this-branch as soon as I get that. By: Serge Vecher (serge-v) 2006-05-04 10:00:44 please update the patch as per OEJ, so that this has a chance to make it into 1.4. Thanks! By: Serge Vecher (serge-v) 2006-05-26 11:38:08 last try: rhulsker please update the patch very soon for it to make it into 1.4. thank you By: Tilghman Lesher (tilghman) 2006-09-21 17:43:02 This appears to have been already fixed, in a different way.  Please ask a bug marshal to reopen this bug if it does not appear to be working for you.