Bug 32602 - SIP Server doesn't respect IssuingInProcess system preference
Summary: SIP Server doesn't respect IssuingInProcess system preference
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: SIP2 (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Aleisha Amohia
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-01-10 04:18 UTC by Alex Buckley
Modified: 2024-02-07 21:43 UTC (History)
3 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
This fix ensures that the IssuingInProcess system preference is considered when circulating via SIP. If the system preference is enabled, checkouts via SIP should be allowed if the patron's fines are not exceeding the noissuescharge system preference at the start of the transaction, and the AllFinesNeedOverride system preference is disabled.
Version(s) released in:


Attachments
Bug 32602: SIP server doesn't respect IssuingInProcess system preference (1.70 KB, patch)
2023-01-10 04:29 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 32602: Unit tests (1.48 KB, patch)
2023-01-13 05:21 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 32602: SIP server doesn't respect IssuingInProcess system preference (3.99 KB, patch)
2023-04-05 02:00 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 32602: Unit tests (995 bytes, patch)
2023-04-05 02:00 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 32602: SIP server doesn't respect IssuingInProcess system preference (4.88 KB, patch)
2023-10-10 23:55 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 32602: Unit tests (996 bytes, patch)
2023-10-10 23:55 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 32602: SIP server doesn't respect IssuingInProcess system preference (2.18 KB, patch)
2023-10-18 01:00 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 32602: Unit tests (4.32 KB, patch)
2023-10-18 01:00 UTC, Aleisha Amohia
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Buckley 2023-01-10 04:18:44 UTC
The system preference 'IssuingInProcess' when set to "Don't prevent" means patrons will still be allowed to issue items if their rental charge took the patron over the noissuescharge system preference.

However, SIP returns that a patron is fine blocked (even if 'IssuingInProcess' equals "Don't prevent") when they try to issue items that take them over the noissuecharge amount.
Comment 1 Alex Buckley 2023-01-10 04:29:36 UTC
Created attachment 145170 [details] [review]
Bug 32602: SIP server doesn't respect IssuingInProcess system preference

The system preference 'IssuingInProcess' when set to "Don't prevent" means patrons will still be allowed to issue items if their rental charge took the patron over the noissuescharge system preference.

However, SIP returns that a patron is fine blocked (even if 'IssuingInProcess' equals "Don't prevent") when they try to issue items that take them over the noissuecharge amount.

Test plan:
1. Configure the BK item type to have a rental charge of $2
2. Configure system preferences:
- noissuescharge = $5
- IssuingInProcess = Don't prevent
3. From a SIP2 device issue 3 BK items to a borrower and notice after the 2nd item you
cannot issue the third item

4. Apply patches
5. Restart your SIP2 server
6. From a SIP2 device issue 3 BK items to a different patron and notice you can issue all
three items

Sponsored-by: Horowhenua District Council, New Zealand
Comment 2 Alex Buckley 2023-01-13 05:21:50 UTC
Created attachment 145249 [details] [review]
Bug 32602: Unit tests

Test plan:
1. sudo koha-shell <instance>
2. prove -v t/db_dependent/SIP/Transaction.t
3. Confirm all tests pass

Sponsored-by: Horowhenua District Council, New Zealand
Comment 3 David Nind 2023-01-17 02:05:15 UTC
The tests failed for me after apply the patch, restarting everything (including the SIP server), and after running through the test plan:

prove t/db_dependent/SIP/Transaction.t
t/db_dependent/SIP/Transaction.t .. 9/16 
    #   Failed test 'Got correct fines screen message'
    #   at t/db_dependent/SIP/Transaction.t line 496.
    #          got: ''
    #     expected: 'Patron has fines'
    # Looks like you failed 1 test of 5.
t/db_dependent/SIP/Transaction.t .. 10/16 
#   Failed test 'do_checkout_with_patron_blocked'
#   at t/db_dependent/SIP/Transaction.t line 534.
t/db_dependent/SIP/Transaction.t .. 16/16 # Looks like you failed 1 test of 16.
t/db_dependent/SIP/Transaction.t .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/16 subtests 

Test Summary Report
-------------------
t/db_dependent/SIP/Transaction.t (Wstat: 256 Tests: 16 Failed: 1)
  Failed test:  10
  Non-zero exit status: 1
Files=1, Tests=16,  8 wallclock secs ( 0.04 usr  0.01 sys +  5.13 cusr  0.82 csys =  6.00 CPU)
Result: FAIL
Comment 4 David Nind 2023-01-17 02:15:06 UTC
Testing notes (using KTD):

1. For step 3, this is how I set things up to test:

   - Edited SIPconfig.xml so that the koha institution is "CPL" instead of "kohalibrary"
   - Restarted the SIP server: koha-sip --restart -v kohadev
   - Installed telnet in KTD: apt-get install telnet
   - Telnet to 6001: telnet 6001
   - Send this command: 9300CNkoha|COkoha|CPCPL|
   - Commands to checkout items:*!
      11YY20220711    16350220250711    163502AOCPL|AA23529000080862|AB39999000011111|ACkoha|BON|BIN|
      11YY20220711    16350220250711    163502AOCPL|AA23529000080862|AB39999000000498|ACkoha|BON|BIN|
      11YY20220711    16350220250711    163502AOCPL|AA23529000080862|AB39999000012392|ACkoha|BON|BIN|
   - To exit: press enter

* Initial patron used was Henry: 23529000035676
  After the patches were applied, the patron used was Bobbie: 23529000080862
  The patron number is entered after |AA

! Need to change the items to checkout (after |AB), or check them back in so that they are available
Comment 5 Alex Buckley 2023-01-17 02:19:54 UTC
Thanks for testing David. I will take this back and have a go at fixing it up!
Comment 6 David Nind 2023-01-17 02:22:16 UTC
(In reply to David Nind from comment #3)
> The tests failed for me after apply the patch, restarting everything
> (including the SIP server), and after running through the test plan:

However, the tests pass if I just apply the patch, restart everything (including the SIP server) - without setting the system preferences and configuration in steps 1 and 2:

prove t/db_dependent/SIP/Transaction.t
t/db_dependent/SIP/Transaction.t .. ok     
All tests successful.
Files=1, Tests=16,  7 wallclock secs ( 0.04 usr  0.01 sys +  5.07 cusr  0.79 csys =  5.91 CPU)
Result: PASS
Comment 7 Alex Buckley 2023-04-05 02:00:02 UTC
Created attachment 149152 [details] [review]
Bug 32602: SIP server doesn't respect IssuingInProcess system preference

The system preference 'IssuingInProcess' when set to "Don't prevent" means patrons will still be allowed to issue items if their rental charge took the patron over the noissuescharge system preference.

However, SIP returns that a patron is fine blocked (even if 'IssuingInProcess' equals "Don't prevent") when they try to issue items that take them over the noissuecharge amount.

Test plan:
1. Configure the BK item type to have a rental charge of $2
2. Configure system preferences:
- noissuescharge = $5
- IssuingInProcess = Don't prevent
3. From a SIP2 device issue 3 BK items to a borrower and notice after the 2nd item you
cannot issue the third item

4. Apply patches
5. Restart your SIP2 server
6. From a SIP2 device issue 3 BK items to a different patron and notice you can issue all
three items

Sponsored-by: Horowhenua District Council, New Zealand
Comment 8 Alex Buckley 2023-04-05 02:00:17 UTC
Created attachment 149153 [details] [review]
Bug 32602: Unit tests

Test plan:
1. sudo koha-shell <instance>
2. prove -v t/db_dependent/SIP/Transaction.t
3. Confirm all tests pass

Sponsored-by: Horowhenua District Council, New Zealand
Comment 9 David Nind 2023-04-07 01:36:40 UTC
Just checking whether the status for this bug should still be assigned or changed to needs signoff.
Comment 10 Aleisha Amohia 2023-10-10 23:55:00 UTC
Created attachment 156808 [details] [review]
Bug 32602: SIP server doesn't respect IssuingInProcess system preference

The system preference 'IssuingInProcess' when set to "Don't prevent" means patrons will still be allowed to issue items if their rental charge took the patron over the noissuescharge system preference.

However, SIP returns that a patron is fine blocked (even if 'IssuingInProcess' equals "Don't prevent") when they try to issue items that take them over the noissuecharge amount.

Test plan:
1. Configure the BK item type to have a rental charge of $2
2. Configure system preferences:
- noissuescharge = $5
- IssuingInProcess = Don't prevent
3. From a SIP2 device issue 3 BK items to a borrower and notice after the 2nd item you
cannot issue the third item

4. Apply patches
5. Restart your SIP2 server
6. From a SIP2 device issue 3 BK items to a different patron and notice you can issue all
three items

Sponsored-by: Horowhenua District Council, New Zealand
Comment 11 Aleisha Amohia 2023-10-10 23:55:03 UTC
Created attachment 156809 [details] [review]
Bug 32602: Unit tests

Test plan:
1. sudo koha-shell <instance>
2. prove -v t/db_dependent/SIP/Transaction.t
3. Confirm all tests pass

Sponsored-by: Horowhenua District Council, New Zealand
Comment 12 Aleisha Amohia 2023-10-18 01:00:21 UTC
Created attachment 157296 [details] [review]
Bug 32602: SIP server doesn't respect IssuingInProcess system preference

The system preference 'IssuingInProcess' when set to "Don't prevent" means patrons will still be allowed to issue items if their rental charge took the patron over the noissuescharge system preference.

However, SIP returns that a patron is fine blocked (even if 'IssuingInProcess' equals "Don't prevent") when they try to issue items that take them over the noissuecharge amount.

Test plan:
1. Configure the BK item type to have a rental charge of $2
2. Configure system preferences:
- noissuescharge = $5
- IssuingInProcess = Don't prevent
3. From a SIP2 device issue 3 BK items to a borrower and notice after the 2nd item you
cannot issue the third item

4. Apply patches
5. Restart your SIP2 server
6. From a SIP2 device issue 3 BK items to a different patron and notice you can issue all
three items

Sponsored-by: Horowhenua District Council, New Zealand
Comment 13 Aleisha Amohia 2023-10-18 01:00:24 UTC
Created attachment 157297 [details] [review]
Bug 32602: Unit tests

Test plan:
1. sudo koha-shell <instance>
2. prove -v t/db_dependent/SIP/Transaction.t
3. Confirm all tests pass

Sponsored-by: Horowhenua District Council, New Zealand
Comment 14 David Nind 2023-10-24 00:12:50 UTC
I've tested again, but I can't replicate the issue before the patch is applied - the behavour before and after the patch is applied seems the same to me. 

Not sure what I've done wrong here....


Testing notes (using KTD):

1. For step 3, this is how I set things up to test:

   - Edited SIPconfig.xml so that the koha institution is "CPL" instead of "kohalibrary"
   - Restarted the SIP server: koha-sip --restart -v kohadev
   - Installed telnet in KTD: apt-get install telnet
   - Telnet to 6001: telnet 127.0.0.1 6001
   - Send this command: 9300CNkoha|COkoha|CPCPL|
   - Commands to checkout items:*!
      11YY20220711    16350220250711    163502AOCPL|AA23529000080862|AB39999000011111|ACkoha|BON|BIN|
      11YY20220711    16350220250711    163502AOCPL|AA23529000080862|AB39999000000498|ACkoha|BON|BIN|
      11YY20220711    16350220250711    163502AOCPL|AA23529000080862|AB39999000012392|ACkoha|BON|BIN|
   - To exit: press enter

* Initial patron used was Bobbie: 23529000080862 
  After the patches were applied, the patron used was Henry: 23529000035676
  The patron number is entered after |AA

! Need to change the items to checkout (after |AB), or check them back in so that they are available


Output and results before the patch was applied
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

kohadev-koha@kohadevbox:koha(bz32602)$ telnet 127.0.0.1 6001
Trying 127.0.0.1...
Connected to 127.0.0.1.
Escape character is '^]'.
9300CNkoha|COkoha|CPCPL|
941
11YY20220711    16350220250711    163502AOCPL|AA23529000080862|AB39999000011111|ACkoha|BON|BIN|
121NNY20231023    234841AOCPL|AA23529000080862|AB39999000011111|AJPerl template toolkit /|AH|BV2.00|BHUSD|BT06|
11YY20220711    16350220250711    163502AOCPL|AA23529000080862|AB39999000000498|ACkoha|BON|BIN
121NNY20231023    234859AOCPL|AA23529000080862|AB39999000000498|AJCGI programming with Perl /|AH|AFOutstanding Fines block issue|BV2.00|BHUSD|BT06|
11YY20220711    16350220250711    163502AOCPL|AA23529000080862|AB39999000012392|ACkoha|BON|BIN|
121NNY20231023    234924AOCPL|AA23529000080862|AB39999000012392|AJLady Susan ; The Watsons ; Sanditon /|AH|AFOutstanding Fines block issue|BV2.00|BHUSD|BT06|

The three items were issued to Bobbie, with no issuing blocked.

The tests pass: prove t/db_dependent/SIP/Transaction.t


Output and results after the patch was applied
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

kohadev-koha@kohadevbox:koha(bz32602)$ telnet 127.0.0.1 6001
Trying 127.0.0.1...
Connected to 127.0.0.1.
Escape character is '^]'.
9300CNkoha|COkoha|CPCPL|
941
11YY20220711    16350220250711    163502AOCPL|AA23529000035676|AB39999000011111|ACkoha|BON|BIN|
121NNY20231024    000235AOCPL|AA23529000035676|AB39999000011111|AJPerl template toolkit /|AH|BV2.00|BHUSD|BT06|
11YY20220711    16350220250711    163502AOCPL|AA23529000035676|AB39999000000498|ACkoha|BON|BIN|
121NNY20231024    000302AOCPL|AA23529000035676|AB39999000000498|AJCGI programming with Perl /|AH|AFOutstanding Fines block issue|BV2.00|BHUSD|BT06|
11YY20220711    16350220250711    163502AOCPL|AA23529000035676|AB39999000012392|ACkoha|BON|BIN|
121NNY20231024    000328AOCPL|AA23529000035676|AB39999000012392|AJLady Susan ; The Watsons ; Sanditon /|AH|AFOutstanding Fines block issue|BV2.00|BHUSD|BT06|

The three items were issued to Henry, with no issuing blocked.

The tests pass: prove t/db_dependent/SIP/Transaction.t
Comment 15 Magnus Enger 2024-01-26 13:27:25 UTC
I get the same result as David, before applying the patches: 

9300CNkoha|COkoha|CPCPL|
941
11YY20220711    16350220250711    163502AOCPL|AA23529000080862|AB39999000011111|ACkoha|BON|BIN|
121NNY20240126    130933AOCPL|AA23529000080862|AB39999000011111|AJPerl template toolkit /|AH|BV2.00|BHUSD|BT06|

The first issue was OK, and 2$ "Rental fee" was added to the patrons's accounting. 

11YY20220711    16350220250711    163502AOCPL|AA23529000080862|AB39999000000498|ACkoha|BON|BIN|
121NNY20240126    130948AOCPL|AA23529000080862|AB39999000000498|AJCGI programming with Perl /|AH|AFOutstanding Fines block issue|BV2.00|BHUSD|BT06|

The second issue worked OK too, the loan and the "Rental fee" was added, but the SIP2 response says "Outstanding Fines block issue", which seems weird? The patron is not over noissuescharge yet, and the issue was not blocked. 

11YY20220711    16350220250711    163502AOCPL|AA23529000080862|AB39999000012392|ACkoha|BON|BIN|
121NNY20240126    131027AOCPL|AA23529000080862|AB39999000012392|AJLady Susan ; The Watsons ; Sanditon /|AH|AFOutstanding Fines block issue|BV2.00|BHUSD|BT06|

Same SIP2 message: "Outstanding Fines block issue", but the loan was made to the patron and the rental fee was added, bringing the patron above the noissuescharge limit. 

If I try to make a fourth loan, I get a different message from SIP2: 

11YY20220711    16350220250711    163502AOCPL|AA23529000080862|AB39999000001334|ASkoha|BON|BIN|                                
120NUN20240126    131908AOCPL|AA23529000080862|AB39999000001334|AJA companion to Heidegger's Introduction to metaphysics /|AH|AFPatron has fines|BLY|

And the loan is not added to the patron's account. 

Looks to me like IssuingInProcess works the way it should. Could it have been fixed by some other patch? 

But the AF-messages of "Outstanding Fines block issue" when the patron is below the noissuescharge limit do seem weird.
Comment 16 Aleisha Amohia 2024-02-07 21:43:22 UTC
(In reply to Magnus Enger from comment #15)
> 
> If I try to make a fourth loan, I get a different message from SIP2: 
> 
> 11YY20220711    16350220250711   
> 163502AOCPL|AA23529000080862|AB39999000001334|ASkoha|BON|BIN|               
> 
> 120NUN20240126    131908AOCPL|AA23529000080862|AB39999000001334|AJA
> companion to Heidegger's Introduction to metaphysics /|AH|AFPatron has
> fines|BLY|
> 
> And the loan is not added to the patron's account. 
> 
> Looks to me like IssuingInProcess works the way it should. Could it have
> been fixed by some other patch? 
> 

I think you're actually right that this is working the way we want. I misunderstood the preference and thought a patron could KEEP issuing if they started their SIP session without block-worthy debt, but it makes sense that it would allow one issue to take them over the limit, then stop.