Bug 16284

Summary: With CheckPrevCheckout used, check only the item for previous checkout if biblio is serial
Product: Koha Reporter: Alex Sassmannshausen <alex>
Component: CirculationAssignee: Josef Moravec <josef.moravec>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: black23, gmcharlt, josef.moravec, katrin.fischer, kyle.m.hall, martin.renvoize, victor
Version: unspecified   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Prior to this patch the `CheckPrevCheckout` functionality errantly included serial type records.
Version(s) released in:
19.11.00
Bug Depends on: 6906    
Bug Blocks:    
Attachments: Bug 16284: Check only item for previous checkout if biblio is serial
Bug 16284: Add test
Bug 16284: Check only item for previous checkout if biblio is serial
Bug 16284: Add test
Bug 16284: Add Koha::Biblio->is_serial method
Bug 16284: Use new method to determine serials in check previous checkouts
Bug 16284: Check only item for previous checkout if biblio is serial
Bug 16284: Add test
Bug 16284: Add Koha::Biblio->is_serial method
Bug 16284: Use new method to determine serials in check previous checkouts
Bug 16284: Check only item for previous checkout if biblio is serial
Bug 16284: Add test
Bug 16284: Add Koha::Biblio->is_serial method
Bug 16284: Use new method to determine serials in check previous checkouts
Bug 16284: Check only item for previous checkout if biblio is serial
Bug 16284: Add test
Bug 16284: Add Koha::Biblio->is_serial method
Bug 16284: Use new method to determine serials in check previous checkouts

Description Alex Sassmannshausen 2016-04-18 15:25:05 UTC
In discussion of Bug 6909, Radek Šiman raised the following additional concerns around CheckPreviousCheckouts functionality: 

"Inspecting the last attachment 50286 [details] [review] [review] I cannot find if periodical biblios are well processed. This special case concerns a scenario where a borrower is about to get another item of the same periodical title. In such a situation we need to check if the same item was already issued and not the same biblio only. There are borrowers ckecking-out several items if the same title at once. It seems to me that the current solution would ask for N-1 items to "Checkout anyway?". We should ask for repeating items of periodicals only."

This bug is for the discussion and resolution of this additional feature.

Best wishes,

Alex
Comment 2 Victor Grousset/tuxayo 2017-11-29 11:33:33 UTC
Just some info about the context:

«In discussion of Bug 6909, Radek Šiman raised the following additional concerns around CheckPreviousCheckouts functionality»

It was actually in bug 6906
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6906#c27
Comment 3 Josef Moravec 2019-04-16 11:01:16 UTC
Created attachment 88050 [details] [review]
Bug 16284: Check only item for previous checkout if biblio is serial

Test plan:
1) Set 'CheckPrevCheckout' system preference to 'Do'
2) Make a checkout with item from serial record (942$s is 1 on default
    settings)
3) Return that checkout
4) Try to check it out again, you should see an alert, that the patron
has thi item checked out previously
5) Try to checkout another item from same bibliographic record, you
sholud not see the alert and item should be checked out
6) prove t/db_dependent/Patron/Borrower_PrevCheckout.t
Comment 4 Josef Moravec 2019-04-16 11:01:19 UTC
Created attachment 88051 [details] [review]
Bug 16284: Add test
Comment 5 Michal Denar 2019-04-16 19:48:20 UTC
Sorry Josef,
but here is some bug.

Test Summary Report
-------------------
t/db_dependent/Patron/Borrower_PrevCheckout.t (Wstat: 65280 Tests: 55 Failed: 0)
  Non-zero exit status: 255
  Parse errors: Bad plan.  You planned 60 tests but ran 55.
Files=1, Tests=55,  7 wallclock secs ( 0.03 usr  0.00 sys +  5.54 cusr  1.20 csys =  6.77 CPU)
Result: FAIL

I tested patch with newspaper record krom default kohadevbox with title "Perthshire advertiser". I follow test plan, but anothemr item start alert too.
Comment 6 Michal Denar 2019-04-17 19:51:36 UTC
Hi,
test t/db_dependent/Patron/Borrower_PrevCheckout.t fail just with ES. With Zebra passed.

Result with ES as engine:
t/db_dependent/Patron/Borrower_PrevCheckout.t .. 38/60 Can't call method "leader" on an undefined value at /home/vagrant/kohaclone/Koha/SearchEngine/Elasticsearch.pm line 429.
# Looks like your test exited with 255 just after 55.
t/db_dependent/Patron/Borrower_PrevCheckout.t .. Dubious, test returned 255 (wstat 65280, 0xff00)
Failed 5/60 subtests

So, I geve this back to Need Singed Off.
Comment 7 Michal Denar 2019-04-17 19:56:15 UTC
Created attachment 88236 [details] [review]
Bug 16284: Check only item for previous checkout if biblio is serial

Test plan:
1) Set 'CheckPrevCheckout' system preference to 'Do'
2) Make a checkout with item from serial record (942$s is 1 on default
    settings)
3) Return that checkout
4) Try to check it out again, you should see an alert, that the patron
has thi item checked out previously
5) Try to checkout another item from same bibliographic record, you
sholud not see the alert and item should be checked out
6) prove t/db_dependent/Patron/Borrower_PrevCheckout.t

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 8 Michal Denar 2019-04-17 19:56:19 UTC
Created attachment 88237 [details] [review]
Bug 16284: Add test

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 9 Katrin Fischer 2019-04-17 20:17:15 UTC
Can you explain why the 942$s is used here? 
Why not use the LDR pos. 7 = s or check for an existing subscription?
How is 942$s set?

Setting Failed QA to get attention to my questions! :)
Comment 10 Josef Moravec 2019-04-18 05:31:24 UTC
(In reply to Katrin Fischer from comment #9)
> Can you explain why the 942$s is used here? 
> Why not use the LDR pos. 7 = s or check for an existing subscription?
> How is 942$s set?

Subscription is not a solution I think, sometimes library don't have subscription for all serials - for example the ones not currently subscribed, but imported from legacy system and circulated.

But using leader does make sense, I will add it, we could check both - leader and biblio.serial.

As 942$s is by default mapped to biblio.serial, i just used it. There was some discussion some time ago (year or two?) I think, but I can't find it now...

942$s need's to be added by cataloguer, or as default value in framework used for serials...

> 
> Setting Failed QA to get attention to my questions! :)
Comment 11 Katrin Fischer 2019-04-18 05:53:34 UTC
Hi Josef, I think you are right about subscription, thx for taking my other suggestion into account!
Comment 12 Josef Moravec 2019-04-18 07:24:36 UTC
Created attachment 88258 [details] [review]
Bug 16284: Add Koha::Biblio->is_serial method

Test plan:
prove t/db_dependent/Koha/Biblio.t

should return green
Comment 13 Josef Moravec 2019-04-18 07:24:39 UTC
Created attachment 88259 [details] [review]
Bug 16284: Use new method to determine serials in check previous checkouts
Comment 14 Josef Moravec 2019-04-18 07:27:24 UTC
Test plan remains same, but now there is possible to just change leader position 7, and it detect the biblio is serial ;)
Comment 15 Michal Denar 2019-06-13 19:56:06 UTC
Josef, can You rebase this on master? 

Thank You.


error: Failed to merge in the changes.
Patch failed at 0001 Bug 16284: Add Koha::Biblio->is_serial method
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-16284-Add-KohaBiblio-isserial-method-BxRAK4.patch
Comment 16 Josef Moravec 2019-06-17 09:14:33 UTC
Created attachment 90642 [details] [review]
Bug 16284: Check only item for previous checkout if biblio is serial

Test plan:
1) Set 'CheckPrevCheckout' system preference to 'Do'
2) Make a checkout with item from serial record (942$s is 1 on default
    settings)
3) Return that checkout
4) Try to check it out again, you should see an alert, that the patron
has thi item checked out previously
5) Try to checkout another item from same bibliographic record, you
sholud not see the alert and item should be checked out
6) prove t/db_dependent/Patron/Borrower_PrevCheckout.t

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 17 Josef Moravec 2019-06-17 09:14:36 UTC
Created attachment 90643 [details] [review]
Bug 16284: Add test

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 18 Josef Moravec 2019-06-17 09:14:39 UTC
Created attachment 90644 [details] [review]
Bug 16284: Add Koha::Biblio->is_serial method

Test plan:
prove t/db_dependent/Koha/Biblio.t

should return green
Comment 19 Josef Moravec 2019-06-17 09:14:42 UTC
Created attachment 90645 [details] [review]
Bug 16284: Use new method to determine serials in check previous checkouts
Comment 20 Josef Moravec 2019-06-17 09:15:04 UTC
(In reply to Michal Denar from comment #15)
> Josef, can You rebase this on master? 
> 
> Thank You.
> 

Rebased, thanks for testing
Comment 21 Michal Denar 2019-06-17 10:24:05 UTC
Created attachment 90646 [details] [review]
Bug 16284: Check only item for previous checkout if biblio is serial

Test plan:
1) Set 'CheckPrevCheckout' system preference to 'Do'
2) Make a checkout with item from serial record (942$s is 1 on default
    settings)
3) Return that checkout
4) Try to check it out again, you should see an alert, that the patron
has thi item checked out previously
5) Try to checkout another item from same bibliographic record, you
sholud not see the alert and item should be checked out
6) prove t/db_dependent/Patron/Borrower_PrevCheckout.t

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 22 Michal Denar 2019-06-17 10:24:10 UTC
Created attachment 90647 [details] [review]
Bug 16284: Add test

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 23 Michal Denar 2019-06-17 10:24:15 UTC
Created attachment 90648 [details] [review]
Bug 16284: Add Koha::Biblio->is_serial method

Test plan:
prove t/db_dependent/Koha/Biblio.t

should return green

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 24 Michal Denar 2019-06-17 10:24:19 UTC
Created attachment 90649 [details] [review]
Bug 16284: Use new method to determine serials in check previous checkouts

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 25 Katrin Fischer 2019-08-10 12:38:38 UTC
Created attachment 92130 [details] [review]
Bug 16284: Check only item for previous checkout if biblio is serial

Test plan:
1) Set 'CheckPrevCheckout' system preference to 'Do'
2) Make a checkout with item from serial record (942$s is 1 on default
    settings)
3) Return that checkout
4) Try to check it out again, you should see an alert, that the patron
has thi item checked out previously
5) Try to checkout another item from same bibliographic record, you
sholud not see the alert and item should be checked out
6) prove t/db_dependent/Patron/Borrower_PrevCheckout.t

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 26 Katrin Fischer 2019-08-10 12:38:42 UTC
Created attachment 92131 [details] [review]
Bug 16284: Add test

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 27 Katrin Fischer 2019-08-10 12:38:47 UTC
Created attachment 92132 [details] [review]
Bug 16284: Add Koha::Biblio->is_serial method

Test plan:
prove t/db_dependent/Koha/Biblio.t

should return green

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 28 Katrin Fischer 2019-08-10 12:38:51 UTC
Created attachment 92133 [details] [review]
Bug 16284: Use new method to determine serials in check previous checkouts

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 29 Katrin Fischer 2019-08-10 12:39:31 UTC
I've verified that UNIMARC and MARC21 are the same in regard to marking serials.
https://archive.ifla.org/VI/8/unimarc-concise-bibliographic-format-2008.pdf
Comment 30 Martin Renvoize 2019-08-13 13:37:37 UTC
Nice work!

Pushed to master for 19.11.00