|
Description
Kyle M Hall (khall)
2020-06-18 17:21:46 UTC
Created attachment 106028 [details] [review] Bug 25808: Renewal via the SIP 'checkout' message gives incorrect message If a renewal via SIP cannot be made because the patron has reached the maximum number of renewals, the AF screen message should read "Item has reached maximum renewals!", instead we get "Item checked out to another patron". Test Plan: 1) Check out an item to a patron 2) Using the SIP CLI tool, run checkout messages until the checkout has reached the maximum number of renewals 3) Note the incorrect message in the AF field 4) Apply this patch 5) Restart the SIP server 6) Run another SIP checkout message 7) Note the message is now correct! Created attachment 109188 [details] [review] Bug 25808: Renewal via the SIP 'checkout' message gives incorrect message If a renewal via SIP cannot be made because the patron has reached the maximum number of renewals, the AF screen message should read "Item has reached maximum renewals!", instead we get "Item checked out to another patron". Test Plan: 1) Check out an item to a patron 2) Using the SIP CLI tool, run checkout messages until the checkout has reached the maximum number of renewals 3) Note the incorrect message in the AF field 4) Apply this patch 5) Restart the SIP server 6) Run another SIP checkout message 7) Note the message is now correct! Created attachment 114289 [details] [review] Bug 25808: Renewal via the SIP 'checkout' message gives incorrect message If a renewal via SIP cannot be made because the patron has reached the maximum number of renewals, the AF screen message should read "Item has reached maximum renewals!", instead we get "Item checked out to another patron". Test Plan: 1) Check out an item to a patron 2) Using the SIP CLI tool, run checkout messages until the checkout has reached the maximum number of renewals 3) Note the incorrect message in the AF field 4) Apply this patch 5) Restart the SIP server 6) Run another SIP checkout message 7) Note the message is now correct! Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> It works :) Testing notes: https://wiki.koha-community.org/wiki/Koha_SIP2_server_setup#Example ./misc/sip_cli_emulator.pl -a 127.0.0.1 -p 6001 -su term1 -sp term1 \ -l CPL --patron 23529001000463 -m checkout --item 39999000001259 Same command for 1) and 2) and 6) Created attachment 114898 [details] [review] Bug 25808: Renewal via the SIP 'checkout' message gives incorrect message If a renewal via SIP cannot be made because the patron has reached the maximum number of renewals, the AF screen message should read "Item has reached maximum renewals!", instead we get "Item checked out to another patron". Test Plan: 1) Check out an item to a patron 2) Using the SIP CLI tool, run checkout messages until the checkout has reached the maximum number of renewals 3) Note the incorrect message in the AF field 4) Apply this patch 5) Restart the SIP server 6) Run another SIP checkout message 7) Note the message is now correct! Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> We're early in the cycle.. whilst this works.. I'd really like to see an accompanying unit test... t/db_dependant/SIP/ILS.t exists and there's prior are for adding tests for a SIP function in there 'cancel_hold'... any chance you could add a subtest for the basics of 'checkout' there to prevent future regressions like this Kyle? Failing for now to get feedback on the above. Created attachment 115161 [details] [review] Bug 25808: (QA follow-up) Add regression test This patch adds a basic regression test for the bug. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> I came back and wrote the test for you.. Passing QA *** Bug 26375 has been marked as a duplicate of this bug. *** The test is failing for me
t/db_dependent/SIP/ILS.t .. 2/11
# Failed test 'Renewals has been reduced'
# at t/db_dependent/SIP/ILS.t line 173.
# got: '0'
# expected: '1'
# Looks like you planned 1 test but ran 4.
# Looks like you failed 1 test of 4 run.
t/db_dependent/SIP/ILS.t .. 11/11
Also QA failure
FAIL t/db_dependent/SIP/ILS.t
FAIL forbidden patterns
forbidden pattern: trailing space char (line 166)
Created attachment 115436 [details] [review] Bug 25808: (QA follow-up) Fix unit tests Pushed to master for 21.05, thanks to everybody involved! Pushed to 20.11.x for 20.11.03 Pushed to 20.05.x for 20.05.09 Not backported to oldoldstable (19.11.x), tests don't pass.
If needed for 19.11, please send a patch. Note that it's not a major bug so it's not a big deal if it's not backported. Just tried to backport since I already knew the test plan.
t/db_dependent/SIP/ILS.t .. 2/11
# Failed test 'We get a success message when issue is renewed'
# at t/db_dependent/SIP/ILS.t line 170.
# got: 'Issue failed : NO_MORE_RENEWALS'
# expected: 'Item already checked out to you: renewing item.'
# Failed test 'Renewals has been reduced'
# at t/db_dependent/SIP/ILS.t line 173.
# got: '0'
# expected: '1'
# Looks like you failed 2 tests of 4.
t/db_dependent/SIP/ILS.t .. 11/11
# Failed test 'checkout'
# at t/db_dependent/SIP/ILS.t line 174.
# Looks like you failed 1 test of 11.
For the first failure, it seems the messages where simpler (NO_MORE_RENEWALS) in 19.11 which can be fixed in the test, however this is not the same semantic between the actual and expected message.
|