From bbda74f26e416afe7f04d25927dd5d0fdfebb50a Mon Sep 17 00:00:00 2001 From: Adrien Saurat Date: Tue, 19 Mar 2013 13:51:37 +0100 Subject: [PATCH] Bug 9842: SIP tests, fix for 07hold.t on Cancel Hold Fixes tests resulting in a "No such hold on patron record" error when it was not true. When running the SIP tests, 07hold.t returns errors. ERROR 1: patron (1X981)' line 204. DU ROI SALOMON|AOVIL|AY3AZEA76' doesn't match pattern '(?-xism:^161Y\d{8} {4}\d{6})' Here, the most important return value is the "1" after "16", indicating a success. And indeed, the hold is correctly recorded in Koha. The following character, indicating the "available" status (and which can be of "Y" or "N" value), is fed by the "available" method in C4/SIP/ILS/Item.pm which is not totally functionnal yet. So, the question is : - should the test gives a OK result by accepting N or Y as long as we get a 161 and not a 160? - should we keep this as it is? (seems safer and cleaner, if we want to respect the SIP protocol the item should be marked as available if it is... I'll go this way and won't touch this in this patch) ERROR 2: line 204. patron record.|AY6AZEA73' doesn't match pattern '(?-xism:^161[NY]\d{8} {4}\d{6})' Here, the hold actually exists, but the "No such hold on patron record" result is caused by an error in the perl script of ILS.pm Tests like "unless ($trans->ok)" should be written unless "($trans->ok(1))". ERROR 3: (1X999)' line 204. patron record.|AY0AZEA70' doesn't match pattern '(?-xism:^161[NY]\d{8} {4}\d{6})' Proposed patch : removes the ERRORS #2 and #3 Once these errors disappear, another one replaces them: line 204. SALOMON|AOVIL|AFHold Cancelled.|AY8AZE623' doesn't match pattern '(?-xism:^160N\d{8} {4}\d{6})' The lack of efficient return value when calling CancelReserve is causing this. I'd rather resolve this in another patch once this one has been validated. TEST PLAN : 1) In "C4/SIP/t", configure the SIPtest.pm file. 2) Configure your SIPconfig.xml if needed. 3) Start your SIP server. 4) In "C4/SIP/t", do a "make test" (see the README file for more informations) You can also run the 07hold.t only. Still, running everything can help to ensure that your SIPtest.pm is well configured. Before the patch, this plan should generate 3 errors for 7hold. After the patch it will generate 2 errors. Signed-off-by: Chris Cormack --- C4/SIP/ILS.pm | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/C4/SIP/ILS.pm b/C4/SIP/ILS.pm index 5d59850..cbe988b 100644 --- a/C4/SIP/ILS.pm +++ b/C4/SIP/ILS.pm @@ -302,14 +302,14 @@ sub cancel_hold { $trans->patron($patron); $trans->item($item); $trans->drop_hold; - unless ($trans->ok) { + unless ($trans->ok(1)) { $trans->screen_msg("Error with transaction drop_hold: " . $trans->screen_msg); return $trans; } # Remove the hold from the patron's record first $trans->ok($patron->drop_hold($item_id)); # different than the transaction drop! - unless ($trans->ok) { + unless ($trans->ok(1)) { # We didn't find it on the patron record $trans->screen_msg("No such hold on patron record."); return $trans; -- 1.7.10.4