Bug 15221 - SIP server always sets the alert flag when item not returned
Summary: SIP server always sets the alert flag when item not returned
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: SIP2 (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low minor (vote)
Target Milestone: ---
Assignee: David Cook
QA Contact: Marcel de Rooy
URL:
Whiteboard:
Keywords:
Depends on: 13411
Blocks: 23057
  Show dependency treegraph
 
Reported: 2015-11-19 16:31 UTC by Kyle M Hall
Modified: 2019-06-20 16:18 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
19.05.00, 18.11.06


Attachments
Bug 15221 - SIP server always sets the alert flag when item not returned (2.66 KB, patch)
2019-05-21 01:23 UTC, David Cook
Details | Diff | Splinter Review
Bug 15221: SIP server always sets the alert flag when item not returned (2.70 KB, patch)
2019-05-22 17:00 UTC, Liz Rea
Details | Diff | Splinter Review
Bug 15221: SIP server always sets the alert flag when item not returned (2.80 KB, patch)
2019-05-23 08:47 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 15221: (QA follow-up) Fix Message.t (2.18 KB, patch)
2019-05-27 09:43 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall 2015-11-19 16:31:31 UTC
Here is a checkin with the item checked out to a patron
SEND: 09N20151015    07240220151015    072402APCPL|AOCPL|AB0140620125|ACstaff|BIN|
READ: 101YNY20151015    072402AOCPL|AB0140620125|AQMY|AJWuthering heights /|AA23529000035676|CSPZ3.B7902 W36|CV04|

Here is a checkin of the same item without it being checked out:
SEND: 09N20151015    07222220151015    072222APCPL|AOCPL|AB0140620125|ACstaff|BIN|
READ: 101YNY20151015    072222AOCPL|AB0140620125|AQMY|AJWuthering heights /|CSPZ3.B7902 W36|CV04|

Here is the same checkin with a checkout on master:
SEND: 09N20151015    07271820151015    072718APCPL|AOCPL|AB0140620125|ACstaff|BIN|
READ: 101YNY20151015    072719AOCPL|AB0140620125|AQMY|AJWuthering heights /|AA23529000035676|CSPZ3.B7902 W36|CV04|

And on master without a checkout:
SEND: 09N20151015    07280320151015    072803APCPL|AOCPL|AB0140620125|ACstaff|BIN|
READ: 100YNY20151015    072803AOCPL|AB0140620125|AQMY|AJWuthering heights /|CSPZ3.B7902 W36|CV04|AFItem not checked out|

So it appears that Koha always sets the alert flag no matter what!
Comment 1 Colin Campbell 2018-03-13 11:05:09 UTC
All of these responses have an alert type (CV) of 04 - 'ship to another branch', so the alert should be set
Comment 2 David Cook 2019-05-20 07:56:24 UTC
I think I'm having the same issue?

koha@9ec653ab82ce:~/koha$ perl ./misc/sip_cli_emulator.pl -l CPL -su staff -sp <password> --port=6001 --address=localhost --item test -m checkin --patron 51
Attempting socket connection to localhost:6001...connected!
SEND: 9300CNstaff|COStaff1|CPCPL|
READ: 941

Trying 'checkin'
SEND: 09N20190520    07432920190520    074329APCPL|AOCPL|ABtest|ACStaff1|BIN|
READ: 101YNY20190520    074329AOCPL|ABtest|AQCPL|AJTest|

According to http://multimedia.3m.com/mws/media/355361O/sip2-protocol.pdf, that means the alarm bit is set. 

I'm using checked_in_ok="1" for my account login in SIPconfig.xml, so I'm not sure what's going on here...
Comment 3 David Cook 2019-05-20 07:58:52 UTC
I don't have time to investigate this today but hopefully I'll take another look tomorrow...
Comment 4 David Cook 2019-05-21 00:29:26 UTC
Still looking but I'm guessing that these are the problematic lines:

   my ($return, $messages, $issue, $borrower) = AddReturn($barcode, $branch, undef, undef, $return_date);
    $self->alert(!$return);
Comment 6 David Cook 2019-05-21 01:00:17 UTC
According to http://multimedia.3m.com/mws/media/355361O/sip2-protocol.pdf, it seems like it's up to the ACS to decide when to raise an alert...
Comment 7 David Cook 2019-05-21 01:23:18 UTC
Created attachment 89928 [details] [review]
Bug 15221 - SIP server always sets the alert flag when item not returned

The checked in response from the ACS always has the alert bit set to
true, even when using the checked_in_ok option.

This patch sets the alert bit to 0 when checked_in_ok is used and
the item is checked in without a preceding checkout.

To test:

0) Create patron in web interface with a cardnumber and userid of "staff" with a password
that matches the account in SIPconfig.xml. Also set their branch to CPL
(also matching SIPconfig.xml).
1) Create an item with a barcode of 'test'
2) Choose a patron to check out to and record their borrowernumber

In one terminal:
3) cd to your git directory (e.g. /home/koha/koha)
4) perl ./C4/SIP/SIPServer.pm ~/koha-dev/etc/SIPconfig.xml

In another terminal:
5) cd to your git directory (e.g. /home/koha/koha)
6) perl ./misc/sip_cli_emulator.pl -l CPL -su staff -sp <password> --port=6001 --address=localhost --item test -m checkin --patron <borrowernumber>
NOTE: You need to replace <borrowernumber> with the borrowernumber from Step 2, and <password> with the password from Step 0.

Before patch:

koha@9ec653ab82ce:~/koha$ perl ./misc/sip_cli_emulator.pl -l CPL -su staff -sp <password> --port=6001 --address=localhost --item test -m checkin --patron <borrowernumber>
Attempting socket connection to localhost:6001...connected!
SEND: 9300CNstaff|COStaff1|CPCPL|
READ: 941

Trying 'checkin'
SEND: 09N20190521    00310320190521    003103APCPL|AOCPL|ABtest|AC<password>|BIN|
READ: 101YNY20190521    003106AOCPL|ABtest|AQCPL|AJTest|

After patch:
koha@9ec653ab82ce:~/koha$ perl ./misc/sip_cli_emulator.pl -l CPL -su staff -sp <password> --port=6001 --address=localhost --item test -m checkin --patron <borrowernumber>
Attempting socket connection to localhost:6001...connected!
SEND: 9300CNstaff|CO<password>|CPCPL|
READ: 941

Trying 'checkin'
SEND: 09N20190521    01000020190521    010000APCPL|AOCPL|ABtest|AC<password>|BIN|
READ: 101YNN20190521    010004AOCPL|ABtest|AQCPL|AJTest|
Comment 8 Liz Rea 2019-05-22 17:00:53 UTC
Created attachment 89970 [details] [review]
Bug 15221:  SIP server always sets the alert flag when item not returned

The checked in response from the ACS always has the alert bit set to
true, even when using the checked_in_ok option.

This patch sets the alert bit to 0 when checked_in_ok is used and
the item is checked in without a preceding checkout.

To test:

0) Create patron in web interface with a cardnumber and userid of "staff" with a password
that matches the account in SIPconfig.xml. Also set their branch to CPL
(also matching SIPconfig.xml).
1) Create an item with a barcode of 'test'
2) Choose a patron to check out to and record their borrowernumber

In one terminal:
3) cd to your git directory (e.g. /home/koha/koha)
4) perl ./C4/SIP/SIPServer.pm ~/koha-dev/etc/SIPconfig.xml

In another terminal:
5) cd to your git directory (e.g. /home/koha/koha)
6) perl ./misc/sip_cli_emulator.pl -l CPL -su staff -sp <password> --port=6001 --address=localhost --item test -m checkin --patron <borrowernumber>
NOTE: You need to replace <borrowernumber> with the borrowernumber from Step 2, and <password> with the password from Step 0.

Before patch:

koha@9ec653ab82ce:~/koha$ perl ./misc/sip_cli_emulator.pl -l CPL -su staff -sp <password> --port=6001 --address=localhost --item test -m checkin --patron <borrowernumber>
Attempting socket connection to localhost:6001...connected!
SEND: 9300CNstaff|COStaff1|CPCPL|
READ: 941

Trying 'checkin'
SEND: 09N20190521    00310320190521    003103APCPL|AOCPL|ABtest|AC<password>|BIN|
READ: 101YNY20190521    003106AOCPL|ABtest|AQCPL|AJTest|

After patch:
koha@9ec653ab82ce:~/koha$ perl ./misc/sip_cli_emulator.pl -l CPL -su staff -sp <password> --port=6001 --address=localhost --item test -m checkin --patron <borrowernumber>
Attempting socket connection to localhost:6001...connected!
SEND: 9300CNstaff|CO<password>|CPCPL|
READ: 941

Trying 'checkin'
SEND: 09N20190521    01000020190521    010000APCPL|AOCPL|ABtest|AC<password>|BIN|
READ: 101YNN20190521    010004AOCPL|ABtest|AQCPL|AJTest|

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 9 Marcel de Rooy 2019-05-23 08:47:04 UTC
Created attachment 90015 [details] [review]
Bug 15221: SIP server always sets the alert flag when item not returned

The checked in response from the ACS always has the alert bit set to
true, even when using the checked_in_ok option.

This patch sets the alert bit to 0 when checked_in_ok is used and
the item is checked in without a preceding checkout.

To test:

0) Create patron in web interface with a cardnumber and userid of "staff" with a password
that matches the account in SIPconfig.xml. Also set their branch to CPL
(also matching SIPconfig.xml).
1) Create an item with a barcode of 'test'
2) Choose a patron to check out to and record their borrowernumber

In one terminal:
3) cd to your git directory (e.g. /home/koha/koha)
4) perl ./C4/SIP/SIPServer.pm ~/koha-dev/etc/SIPconfig.xml

In another terminal:
5) cd to your git directory (e.g. /home/koha/koha)
6) perl ./misc/sip_cli_emulator.pl -l CPL -su staff -sp <password> --port=6001 --address=localhost --item test -m checkin --patron <borrowernumber>
NOTE: You need to replace <borrowernumber> with the borrowernumber from Step 2, and <password> with the password from Step 0.

Before patch:

koha@9ec653ab82ce:~/koha$ perl ./misc/sip_cli_emulator.pl -l CPL -su staff -sp <password> --port=6001 --address=localhost --item test -m checkin --patron <borrowernumber>
Attempting socket connection to localhost:6001...connected!
SEND: 9300CNstaff|COStaff1|CPCPL|
READ: 941

Trying 'checkin'
SEND: 09N20190521    00310320190521    003103APCPL|AOCPL|ABtest|AC<password>|BIN|
READ: 101YNY20190521    003106AOCPL|ABtest|AQCPL|AJTest|

After patch:
koha@9ec653ab82ce:~/koha$ perl ./misc/sip_cli_emulator.pl -l CPL -su staff -sp <password> --port=6001 --address=localhost --item test -m checkin --patron <borrowernumber>
Attempting socket connection to localhost:6001...connected!
SEND: 9300CNstaff|CO<password>|CPCPL|
READ: 941

Trying 'checkin'
SEND: 09N20190521    01000020190521    010000APCPL|AOCPL|ABtest|AC<password>|BIN|
READ: 101YNN20190521    010004AOCPL|ABtest|AQCPL|AJTest|

Signed-off-by: Liz Rea <wizzyrea@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 10 Nick Clemens 2019-05-23 15:01:09 UTC
Awesome work all!

Pushed to master for 19.05
Comment 11 Jonathan Druart 2019-05-24 20:13:31 UTC
Tests are failing.

t/db_dependent/SIP/Message.t .. 2/4 
    #   Failed test 'Alert flag is set'
    #   at t/db_dependent/SIP/Message.t line 361.
    #          got: 'N'
    #     expected: 'Y'

    #   Failed test 'Checkin without CV triggers alert flag when cv_triggers_alert is off'
    #   at t/db_dependent/SIP/Message.t line 388.
    #          got: 'N'
    #     expected: 'Y'
    # Looks like you failed 2 tests of 27.
t/db_dependent/SIP/Message.t .. 3/4 
#   Failed test 'Checkin V2'
#   at t/db_dependent/SIP/Message.t line 74.
# Looks like you failed 1 test of 4.
Comment 12 Marcel de Rooy 2019-05-27 09:43:50 UTC
Created attachment 90112 [details] [review]
Bug 15221: (QA follow-up) Fix Message.t

As Jonathan reported on comment11, SIP/Message.t fails now.

This is a trivial fix for that. In the first case we should just no longer
expect the alert flag. In the second case it makes more sense to toggle
the value of the checked_in_ok setting.

Test plan:
Run t/db_dependent/SIP/Message.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 13 Marcel de Rooy 2019-05-27 09:48:51 UTC
RM: Only last follow-up
Comment 14 Nick Clemens 2019-05-30 10:32:22 UTC
last follow-up pushed to master for 19.05
Comment 15 Martin Renvoize 2019-05-30 19:56:21 UTC
Pushed to 18.11.x for 18.11.06
Comment 16 David Cook 2019-06-06 00:22:56 UTC
Kyle's found a bug with this patch (https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23057). 

I'm going to provide a patch on that new report, but I'm under time pressure at the moment, so I'd be happy for someone else to take it over, or just revert this patch as it's probably causing more problems than it's fixing.
Comment 17 Liz Rea 2019-06-18 19:35:56 UTC
Declining this for 18.05 as it causes quite a bad bug in SIP for users with Automated Materials Handlers.