---- Reported by jwagner@ptfs.com 2010-02-23 14:24:23 ---- Sponsored by Middletown Township Public Library, Middletown, NJ, USA. We have written a subroutine in Koha that will adds/appends data to a csv file when a notice (hold or overdue) is created. The format of this file was specified by Talking Tech. A cronned script uploads this file to the Talking Tech server where the phone calls are then generated. The script also runs a second time in the evening to grab the results of the automated phone calls. If patrons renew books etc. via the phone system, the update is loaded into Koha with a SIP transaction. ---- Additional Comments From jwagner@ptfs.com 2010-05-12 13:59:06 ---- The fix for this bug (in ptfs-harley) can be pulled from the public git repository at http://github.com/Koha-PTFS, branch http://bugs.koha.org/cgi-bin/bugzilla3/show_bug.cgi?id=4246 --- Bug imported by chris@bigballofwax.co.nz 2010-05-21 01:24 UTC --- This bug was previously known as _bug_ 4246 at http://bugs.koha.org/cgi-bin/bugzilla3/show_bug.cgi?id=4246 Unknown operating system Windows 2000. Setting to default OS "All". Actual time not defined. Setting to 0.0 The original reporter of this bug does not have an account here. Reassigning to the person who moved it here: chris@bigballofwax.co.nz. Previous reporter was jwagner@ptfs.com. The original assignee of this bug does not have an account here. Reassigning to the default assignee for the component, gmcharlt@gmail.com. Previous assignee was kohaprogrammers@ptfs.com. Setting qa contact to the default for this product. This bug either had no qa contact or an invalid one. Bug reassigned, setting status to "NEW". Previous status was "ASSIGNED". CC member jwagner@ptfs.com does not have an account here
Reassigning to myself, as ByWater has been commissioned to reimplement this feature for Koha. We're going to do this in two phases, and in a manner that alters the core of Koha as little as possible (no subroutines added to C4, no changes to data structure). The code pack will consist of two cronjobs (send and receive), the necessary messaging preference data, and a system preference to activate/deactivate the feature. Documentation will be added in-line in scripts, in a README file and in commit message of patch. In phase 1, phone notifications will be support for PREDUE, OVERDUE and HOLD notifications. Messages will be noted on the patron record, just like for email, and for PREDUE and HOLD notices, the patron messaging preferences will be used. Phase 2 will include SMS messging, more notice types, patron's preferred language and SIP2 CONNECT support.
Created attachment 7964 [details] [review] Implements support for Talking Tech I-tiva phone notification for OVERDUE, PREDUE and HOLD notifications. Overdues respect triggers as configured for the patron's branch. Predue and Holds notifications respect patron's messaging preference choices. A n Record of phone messages being sent to patrons is added to the patron's Notices tab; notice of success or failure can be retrieved from I-tiva. See the TalkingTech.README for installation and set-up instructions. Aside from the control system preference, and the necessary changes to Messaging Preferences forms to make use of phone notifications, the bulk of the code resides in external cronjobs. TalkingTech_itiva_outbound.pl generates the Spec C file to send to I-tiva. Actual transmission of the file must be handled by the system administrator. TalkingTech_itiva_inbound.pl processes the received Results file from I-tiva. Getting the file from I-tiva to Koha is the job of the system administrator, as well. Both scripts have a --help option with full documentation. The only necessary change to core Koha behavior is in C4::Letters::EnqueueLetter. The return value was changed from 0 or 1 (successful addition of letter to message_queue or not), to the actual insert ID of the letter. This was required by the outbound script to present a unique Transaction ID for the notice added to the patron's record (so a 'sent' or 'failed' status could be updated). Since the dbh and sth are not shared, and the last_insert_id() command is table-specific, this should be thread-safe. No changes are necessary to any parts of Koha, as all usage of EnqueueLetter currently ignores the return value. To Test: 1. Turn on TalkingTechItivaPhoneNotification system preference 2. Verify that 'phone' is now a valid notification option for patrons on both staff and OPAC side 3. Attempt to set a 'phone' preference for PREDUE or HOLD messaging; attempt should succeed 4. Set up the patron for notices to triggers: a. include checked out items due in a range of days, including the value set up in their messaging preferences. b. place several holds, some in position, others waiting for pickup, others in transit. c. set the patron up to have overdues, overdue by a range of days that includes the delay values for the patrons branch and categorycode 5. Run TalkingTech_itiva_outbound.pl --type=RESERVE --type=PREOVERDUE --type=OVERDUE --outfile=/tmp/talkingtechtest.csv The resulting talkingtechtest.csv file should include all the items due on X days (where X is the patrons' preference), and none of the ones due in other increments. Similarly, overdues messages should be added for each item due by a delay value as configured; overdues of other numbers of days should be ignore. Holds that are waiting pick up or in transit should have messages, those still pending should not. Messages should be added to the patron's notices tab for each issue sent. Verify these messages exist, and all Notices tokens are replaced with appropriate information. Repeat, this time with 4c making use of the default branch overdue triggers, instead of branch-specific triggers. To test the inbound script, create a CSV with rows in the format "<<Message_id>>","<<SUCCESS or FAIL>>" Message ID should correspond to the final column of the talkingtechtest.csv file (the transaction id) for the message.
Created attachment 7965 [details] [review] Talking Tech Support - Phase I Implements support for Talking Tech I-tiva phone notification for OVERDUE, PREDUE and HOLD notifications. Overdues respect triggers as configured for the patron's branch. Predue and Holds notifications respect patron's messaging preference choices. A new column for phone notification is added if the TalkingTechItivaPhoneNotification system preference is turned on Record of phone messages being sent to patrons is added to the patron's Notices tab; notice of success or failure can be retrieved from I-tiva. See the TalkingTech.README for installation and set-up instructions. Aside from the control system preference, and the necessary changes to Messaging Preferences forms to make use of phone notifications, the bulk of the code resides in external cronjobs. TalkingTech_itiva_outbound.pl generates the Spec C file to send to I-tiva. Actual transmission of the file must be handled by the system administrator. TalkingTech_itiva_inbound.pl processes the received Results file from I-tiva. Getting the file from I-tiva to Koha is the job of the system administrator, as well. Both scripts have a --help option with full documentation. The only necessary change to core Koha behavior is in C4::Letters::EnqueueLetter. The return value was changed from 0 or 1 (successful addition of letter to message_queue or not), to the actual insert ID of the letter. This was required by the outbound script to present a unique Transaction ID for the notice added to the patron's record (so a 'sent' or 'failed' status could be updated). Since the dbh and sth are not shared, and the last_insert_id() command is table-specific, this should be thread-safe. No changes are necessary to any parts of Koha, as all usage of EnqueueLetter currently ignores the return value. To Test: 1. Turn on TalkingTechItivaPhoneNotification system preference 2. Verify that 'phone' is now a valid notification option for patrons on both staff and OPAC side 3. Attempt to set a 'phone' preference for PREDUE or HOLD messaging; attempt should succeed 4. Set up the patron for notices to triggers: a. include checked out items due in a range of days, including the value set up in their messaging preferences. b. place several holds, some in position, others waiting for pickup, others in transit. c. set the patron up to have overdues, overdue by a range of days that includes the delay values for the patrons branch and categorycode 5. Run TalkingTech_itiva_outbound.pl --type=RESERVE --type=PREOVERDUE --type=OVERDUE --outfile=/tmp/talkingtechtest.csv The resulting talkingtechtest.csv file should include all the items due on X days (where X is the patrons' preference), and none of the ones due in other increments. Similarly, overdues messages should be added for each item due by a delay value as configured; overdues of other numbers of days should be ignore. Holds that are waiting pick up or in transit should have messages, those still pending should not. Messages should be added to the patron's notices tab for each issue sent. Verify these messages exist, and all Notices tokens are replaced with appropriate information. Repeat, this time with 4c making use of the default branch overdue triggers, instead of branch-specific triggers. To test the inbound script, create a CSV with rows in the format "<<Message_id>>","<<SUCCESS or FAIL>>" Message ID should correspond to the final column of the talkingtechtest.csv file (the transaction id) for the message.
Created attachment 8012 [details] [review] Talking Tech Support - Phase I Implements support for Talking Tech I-tiva phone notification for OVERDUE, PREDUE and HOLD notifications. Overdues respect triggers as configured for the patron's branch. Predue and Holds notifications respect patron's messaging preference choices. A new column for phone notification is added if the TalkingTechItivaPhoneNotification system preference is turned on Record of phone messages being sent to patrons is added to the patron's Notices tab; notice of success or failure can be retrieved from I-tiva. See the TalkingTech.README for installation and set-up instructions. Aside from the control system preference, and the necessary changes to Messaging Preferences forms to make use of phone notifications, the bulk of the code resides in external cronjobs. TalkingTech_itiva_outbound.pl generates the Spec C file to send to I-tiva. Actual transmission of the file must be handled by the system administrator. TalkingTech_itiva_inbound.pl processes the received Results file from I-tiva. Getting the file from I-tiva to Koha is the job of the system administrator, as well. Both scripts have a --help option with full documentation. The only necessary change to core Koha behavior is in C4::Letters::EnqueueLetter. The return value was changed from 0 or 1 (successful addition of letter to message_queue or not), to the actual insert ID of the letter. This was required by the outbound script to present a unique Transaction ID for the notice added to the patron's record (so a 'sent' or 'failed' status could be updated). Since the dbh and sth are not shared, and the last_insert_id() command is table-specific, this should be thread-safe. No changes are necessary to any parts of Koha, as all usage of EnqueueLetter currently ignores the return value. To Test: 1. Turn on TalkingTechItivaPhoneNotification system preference 2. Verify that 'phone' is now a valid notification option for patrons on both staff and OPAC side 3. Attempt to set a 'phone' preference for PREDUE or HOLD messaging; attempt should succeed 4. Set up the patron for notices to triggers: a. include checked out items due in a range of days, including the value set up in their messaging preferences. b. place several holds, some in position, others waiting for pickup, others in transit. c. set the patron up to have overdues, overdue by a range of days that includes the delay values for the patrons branch and categorycode 5. Run TalkingTech_itiva_outbound.pl --type=RESERVE --type=PREOVERDUE --type=OVERDUE --outfile=/tmp/talkingtechtest.csv The resulting talkingtechtest.csv file should include all the items due on X days (where X is the patrons' preference), and none of the ones due in other increments. Similarly, overdues messages should be added for each item due by a delay value as configured; overdues of other numbers of days should be ignore. Holds that are waiting pick up or in transit should have messages, those still pending should not. Messages should be added to the patron's notices tab for each issue sent. Verify these messages exist, and all Notices tokens are replaced with appropriate information. Repeat, this time with 4c making use of the default branch overdue triggers, instead of branch-specific triggers. To test the inbound script, create a CSV with rows in the format "<<Message_id>>","<<SUCCESS or FAIL>>" Message ID should correspond to the final column of the talkingtechtest.csv file (the transaction id) for the message. Primary Authorship: Ian Walls Additional modifications: Kyle M Hall
Created attachment 8014 [details] [review] Talking Tech Support - Phase I Implements support for Talking Tech I-tiva phone notification for OVERDUE, PREDUE and HOLD notifications. Overdues respect triggers as configured for the patron's branch. Predue and Holds notifications respect patron's messaging preference choices. A new column for phone notification is added if the TalkingTechItivaPhoneNotification system preference is turned on Record of phone messages being sent to patrons is added to the patron's Notices tab; notice of success or failure can be retrieved from I-tiva. See the TalkingTech.README for installation and set-up instructions. Aside from the control system preference, and the necessary changes to Messaging Preferences forms to make use of phone notifications, the bulk of the code resides in external cronjobs. TalkingTech_itiva_outbound.pl generates the Spec C file to send to I-tiva. Actual transmission of the file must be handled by the system administrator. TalkingTech_itiva_inbound.pl processes the received Results file from I-tiva. Getting the file from I-tiva to Koha is the job of the system administrator, as well. Both scripts have a --help option with full documentation. The only necessary change to core Koha behavior is in C4::Letters::EnqueueLetter. The return value was changed from 0 or 1 (successful addition of letter to message_queue or not), to the actual insert ID of the letter. This was required by the outbound script to present a unique Transaction ID for the notice added to the patron's record (so a 'sent' or 'failed' status could be updated). Since the dbh and sth are not shared, and the last_insert_id() command is table-specific, this should be thread-safe. No changes are necessary to any parts of Koha, as all usage of EnqueueLetter currently ignores the return value. To Test: 1. Turn on TalkingTechItivaPhoneNotification system preference 2. Verify that 'phone' is now a valid notification option for patrons on both staff and OPAC side 3. Attempt to set a 'phone' preference for PREDUE or HOLD messaging; attempt should succeed 4. Set up the patron for notices to triggers: a. include checked out items due in a range of days, including the value set up in their messaging preferences. b. place several holds, some in position, others waiting for pickup, others in transit. c. set the patron up to have overdues, overdue by a range of days that includes the delay values for the patrons branch and categorycode 5. Run TalkingTech_itiva_outbound.pl --type=RESERVE --type=PREOVERDUE --type=OVERDUE --outfile=/tmp/talkingtechtest.csv The resulting talkingtechtest.csv file should include all the items due on X days (where X is the patrons' preference), and none of the ones due in other increments. Similarly, overdues messages should be added for each item due by a delay value as configured; overdues of other numbers of days should be ignore. Holds that are waiting pick up or in transit should have messages, those still pending should not. Messages should be added to the patron's notices tab for each issue sent. Verify these messages exist, and all Notices tokens are replaced with appropriate information. Repeat, this time with 4c making use of the default branch overdue triggers, instead of branch-specific triggers. To test the inbound script, create a CSV with rows in the format "<<Message_id>>","<<SUCCESS or FAIL>>" Message ID should correspond to the final column of the talkingtechtest.csv file (the transaction id) for the message. Primary Authorship: Ian Walls Additional modifications: Kyle M Hall
Created attachment 8636 [details] [review] Talking Tech Support - Phase I Implements support for Talking Tech I-tiva phone notification for OVERDUE, PREDUE and HOLD notifications. Overdues respect triggers as configured for the patron's branch. Predue and Holds notifications respect patron's messaging preference choices. A new column for phone notification is added if the TalkingTechItivaPhoneNotification system preference is turned on Record of phone messages being sent to patrons is added to the patron's Notices tab; notice of success or failure can be retrieved from I-tiva. See the TalkingTech.README for installation and set-up instructions. Aside from the control system preference, and the necessary changes to Messaging Preferences forms to make use of phone notifications, the bulk of the code resides in external cronjobs. TalkingTech_itiva_outbound.pl generates the Spec C file to send to I-tiva. Actual transmission of the file must be handled by the system administrator. TalkingTech_itiva_inbound.pl processes the received Results file from I-tiva. Getting the file from I-tiva to Koha is the job of the system administrator, as well. Both scripts have a --help option with full documentation. The only necessary change to core Koha behavior is in C4::Letters::EnqueueLetter. The return value was changed from 0 or 1 (successful addition of letter to message_queue or not), to the actual insert ID of the letter. This was required by the outbound script to present a unique Transaction ID for the notice added to the patron's record (so a 'sent' or 'failed' status could be updated). Since the dbh and sth are not shared, and the last_insert_id() command is table-specific, this should be thread-safe. No changes are necessary to any parts of Koha, as all usage of EnqueueLetter currently ignores the return value. To Test: 1. Turn on TalkingTechItivaPhoneNotification system preference 2. Verify that 'phone' is now a valid notification option for patrons on both staff and OPAC side 3. Attempt to set a 'phone' preference for PREDUE or HOLD messaging; attempt should succeed 4. Set up the patron for notices to triggers: a. include checked out items due in a range of days, including the value set up in their messaging preferences. b. place several holds, some in position, others waiting for pickup, others in transit. c. set the patron up to have overdues, overdue by a range of days that includes the delay values for the patrons branch and categorycode 5. Run TalkingTech_itiva_outbound.pl --type=RESERVE --type=PREOVERDUE --type=OVERDUE --outfile=/tmp/talkingtechtest.csv The resulting talkingtechtest.csv file should include all the items due on X days (where X is the patrons' preference), and none of the ones due in other increments. Similarly, overdues messages should be added for each item due by a delay value as configured; overdues of other numbers of days should be ignore. Holds that are waiting pick up or in transit should have messages, those still pending should not. Messages should be added to the patron's notices tab for each issue sent. Verify these messages exist, and all Notices tokens are replaced with appropriate information. Repeat, this time with 4c making use of the default branch overdue triggers, instead of branch-specific triggers. To test the inbound script, create a CSV with rows in the format "<<Message_id>>","<<SUCCESS or FAIL>>" Message ID should correspond to the final column of the talkingtechtest.csv file (the transaction id) for the message. Primary Authorship: Ian Walls Additional modifications: Kyle M Hall
Created attachment 8637 [details] [review] Talking Tech Support - Phase I - Followup - Fix Messaging Preferences There is a flaw in C4::Members::Messaging::GetMessagingPreferences where the system assumes that every transport will use the same letter. This is not necessarily true. Even with the default preferences of just 'email' and 'sms', we should be able to have different letters for each, as one has a maximum character length ( sms ) and one does not. GetMessagingPreferences currently uses the letter code of the last result of its query as the letter code for every transport type. The returned data is a hashref with a key 'transport_types' that is an array of transport_types this borrower has selected for the given alert. This commit modifies GetMessagingPreferences such that the the 'transport_types' array is now a hash where the name of the transport type is now a key to the value of the letter code set for that transport type. It also modifies code calling GetMessagingPreferences where necessary, and as a side benefit will correctly get the letter codes for email and sms correctly, if they are defined differently.
Created attachment 8638 [details] [review] Talking Tech Support - Phase I - Followup - Fix Messaging Preferences There is a flaw in C4::Members::Messaging::GetMessagingPreferences where the system assumes that every transport will use the same letter. This is not necessarily true. Even with the default preferences of just 'email' and 'sms', we should be able to have different letters for each, as one has a maximum character length ( sms ) and one does not. GetMessagingPreferences currently uses the letter code of the last result of its query as the letter code for every transport type. The returned data is a hashref with a key 'transport_types' that is an array of transport_types this borrower has selected for the given alert. This commit modifies GetMessagingPreferences such that the the 'transport_types' array is now a hash where the name of the transport type is now a key to the value of the letter code set for that transport type. It also modifies code calling GetMessagingPreferences where necessary, and as a side benefit will correctly get the letter codes for email and sms correctly, if they are defined differently.
Created attachment 8639 [details] [review] 3.6.x - Talking Tech Support - Phase I - Followup - Fix Messaging Preferences There is a flaw in C4::Members::Messaging::GetMessagingPreferences where the system assumes that every transport will use the same letter. This is not necessarily true. Even with the default preferences of just 'email' and 'sms', we should be able to have different letters for each, as one has a maximum character length ( sms ) and one does not. GetMessagingPreferences currently uses the letter code of the last result of its query as the letter code for every transport type. The returned data is a hashref with a key 'transport_types' that is an array of transport_types this borrower has selected for the given alert. This commit modifies GetMessagingPreferences such that the the 'transport_types' array is now a hash where the name of the transport type is now a key to the value of the letter code set for that transport type. It also modifies code calling GetMessagingPreferences where necessary, and as a side benefit will correctly get the letter codes for email and sms correctly, if they are defined differently. http://bugs.koha-community.org/show_bug.cgi?id=4246 Conflicts: C4/Circulation.pm C4/Reserves.pm
Created attachment 9487 [details] [review] [SIGNED-OFF] Talking Tech Support - Phase I Implements support for Talking Tech I-tiva phone notification for OVERDUE, PREDUE and HOLD notifications. Overdues respect triggers as configured for the patron's branch. Predue and Holds notifications respect patron's messaging preference choices. A new column for phone notification is added if the TalkingTechItivaPhoneNotification system preference is turned on Record of phone messages being sent to patrons is added to the patron's Notices tab; notice of success or failure can be retrieved from I-tiva. See the TalkingTech.README for installation and set-up instructions. Aside from the control system preference, and the necessary changes to Messaging Preferences forms to make use of phone notifications, the bulk of the code resides in external cronjobs. TalkingTech_itiva_outbound.pl generates the Spec C file to send to I-tiva. Actual transmission of the file must be handled by the system administrator. TalkingTech_itiva_inbound.pl processes the received Results file from I-tiva. Getting the file from I-tiva to Koha is the job of the system administrator, as well. Both scripts have a --help option with full documentation. The only necessary change to core Koha behavior is in C4::Letters::EnqueueLetter. The return value was changed from 0 or 1 (successful addition of letter to message_queue or not), to the actual insert ID of the letter. This was required by the outbound script to present a unique Transaction ID for the notice added to the patron's record (so a 'sent' or 'failed' status could be updated). Since the dbh and sth are not shared, and the last_insert_id() command is table-specific, this should be thread-safe. No changes are necessary to any parts of Koha, as all usage of EnqueueLetter currently ignores the return value. To Test: 1. Turn on TalkingTechItivaPhoneNotification system preference 2. Verify that 'phone' is now a valid notification option for patrons on both staff and OPAC side 3. Attempt to set a 'phone' preference for PREDUE or HOLD messaging; attempt should succeed 4. Set up the patron for notices to triggers: a. include checked out items due in a range of days, including the value set up in their messaging preferences. b. place several holds, some in position, others waiting for pickup, others in transit. c. set the patron up to have overdues, overdue by a range of days that includes the delay values for the patrons branch and categorycode 5. Run TalkingTech_itiva_outbound.pl --type=RESERVE --type=PREOVERDUE --type=OVERDUE --outfile=/tmp/talkingtechtest.csv The resulting talkingtechtest.csv file should include all the items due on X days (where X is the patrons' preference), and none of the ones due in other increments. Similarly, overdues messages should be added for each item due by a delay value as configured; overdues of other numbers of days should be ignore. Holds that are waiting pick up or in transit should have messages, those still pending should not. Messages should be added to the patron's notices tab for each issue sent. Verify these messages exist, and all Notices tokens are replaced with appropriate information. Repeat, this time with 4c making use of the default branch overdue triggers, instead of branch-specific triggers. To test the inbound script, create a CSV with rows in the format "<<Message_id>>","<<SUCCESS or FAIL>>" Message ID should correspond to the final column of the talkingtechtest.csv file (the transaction id) for the message. Primary Authorship: Ian Walls Additional modifications: Kyle M Hall http://bugs.koha-community.org/show_bug.cgi?id=4246 Signed-off-by: Nicole C. Engard <nengard@bywatersolutions.com> Tested and in use in production by two public libraries : Middletown and Washoe. Both have given their sign off, but don't have git to actually sign off.
Created attachment 9488 [details] [review] [SIGNED-OFF] Talking Tech Support - Phase I - Followup - Fix Messaging Preferences There is a flaw in C4::Members::Messaging::GetMessagingPreferences where the system assumes that every transport will use the same letter. This is not necessarily true. Even with the default preferences of just 'email' and 'sms', we should be able to have different letters for each, as one has a maximum character length ( sms ) and one does not. GetMessagingPreferences currently uses the letter code of the last result of its query as the letter code for every transport type. The returned data is a hashref with a key 'transport_types' that is an array of transport_types this borrower has selected for the given alert. This commit modifies GetMessagingPreferences such that the the 'transport_types' array is now a hash where the name of the transport type is now a key to the value of the letter code set for that transport type. It also modifies code calling GetMessagingPreferences where necessary, and as a side benefit will correctly get the letter codes for email and sms correctly, if they are defined differently. http://bugs.koha-community.org/show_bug.cgi?id=4246 Signed-off-by: Nicole C. Engard <nengard@bywatersolutions.com> In use in production by two libraries: Middletown and Washoe who give their sign off but don't have git to do so.
This development is working fine and has been in use on two libraries' production servers (successfully) for several months.
C4::Form::MessagingPreferences::set_form_values still uses @{$pref->{'transports'}} syntax. As does opac-userupdate.pl : @{$pref->{'transports'}}. Changing the return value from array to hash will break these instances; marking Failed QA on followup patch. As for the first patch, could another member of the QA team take a look, as I have a large part in the authorship?
Created attachment 9501 [details] [review] Talking Tech Support - Phase I - Followup 2 - Bugfix
Changing status back to "signed off" because the first patch needs QA from someone other than Ian, please.
(In reply to comment #15) > Changing status back to "signed off" because the first patch needs QA from > someone other than Ian, please. Melia, when you say "1st patch", are you talking of 3.6.x or http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=9487 (Talking Tech Support - Phase I) ? QA comments (considering the 3.6.x patch is irrelevant) 1- the 3 patches (excludint 3.6.x one) apply smoothly on master 2- I tested with the syspref OFF (default), no noticeable change, as expected 3- the 2 new scripts does not pass perlcritic: misc/cronjobs/thirdparty/TalkingTech_itiva_inbound.pl: Two-argument "open" used at line 63, column 4. See page 207 of PBP. (Severity: 5) misc/cronjobs/thirdparty/TalkingTech_itiva_outbound.pl: Bareword file handle opened at line 87, column 4. See pages 202,204 of PBP. (Severity: 5) misc/cronjobs/thirdparty/TalkingTech_itiva_outbound.pl: Two-argument "open" used at line 87, column 4. See page 207 of PBP. (Severity: 5) misc/cronjobs/thirdparty/TalkingTech_itiva_outbound.pl: Bareword file handle opened at line 90, column 4. See pages 202,204 of PBP. (Severity: 5) 4- You changed the signature of the Enqueueletter sub: - return $result; + return $dbh->last_insert_id(undef,undef,'message_queue', undef); However, others use of this sub weren't managing any returned value, so that's not a problem. Please fix 3 and explain what should be done with the 3.6.x patch (obsoleted ?) to earn passed QA status
Created attachment 10062 [details] [review] Talking Tech Support - Phase I - Followup 3 - Follow PBP * Fixes violations of Perl Best Practices, where possible * perltidy both scripts
Setting to Passed QA as per Paul's comments.
Trying to apply this patch, I get a SQL constraint error: INSERT INTO message_transports -> (message_attribute_id, message_transport_type, is_digest, letter_module, letter_code) VALUES -> (4, 'phone', 0, 'reserves', 'HOLD_PHONE'), -> (2, 'phone', 0, 'circulation', 'PREDUE_PHONE') -> ; ERROR 1452 (23000): Cannot add or update a child row: a foreign key constraint fails (`lecannetdesmaures`.`message_transports`, CONSTRAINT `message_transports_ibfk_1` FOREIGN KEY (`message_attribute_id`) REFERENCES `message_attributes` (`message_attribute_id`) ON DELETE CASCADE) portable lecannetdesmaures>select * from message_attributes; Investigating a little show that the message_transports table does not contain message_attribute_id 4 and 2 Investigating more, it's my sample database that is wrong. Adding the values that I don't have fixes the problem will push now
Patch pushed, but untested as I don't have Talking Tech subscription. Please test again in case there's a problem !
[Mon Jun 18 09:43:38 2012] updatedatabase.pl: DBD::mysql::db do failed: Cannot add or update a child row: a foreign key constraint fails (`kohatest`.`message_transports`, CONSTRAINT `message_transports_ibfk_1` FOREIGN KEY (`message_attribute_id`) REFERENCES `message_attributes` (`message_attribute_id`) ON DELETE CASCADE ON UPDAT) at /usr/share/koha/testclone/installer/data/mysql/updatedatabase.pl line 5359. I checked and see that I do not have attribute 2 in my atts table? You refer to it in: (2, 'phone', 0, 'circulation', 'PREDUE_PHONE') Line 5362 insert on message_transports.
There have been no further reports of problems so I am marking this bug resolved.