Description
Alex Sassmannshausen
2016-04-18 15:25:05 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 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 Created attachment 88051 [details] [review] Bug 16284: Add test 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. 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. 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> 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> 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! :) (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! :) Hi Josef, I think you are right about subscription, thx for taking my other suggestion into account! 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 Created attachment 88259 [details] [review] Bug 16284: Use new method to determine serials in check previous checkouts Test plan remains same, but now there is possible to just change leader position 7, and it detect the biblio is serial ;) 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 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> 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> 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 Created attachment 90645 [details] [review] Bug 16284: Use new method to determine serials in check previous checkouts (In reply to Michal Denar from comment #15) > Josef, can You rebase this on master? > > Thank You. > Rebased, thanks for testing 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> 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> 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> 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> 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> 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> 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> 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> 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 Nice work! Pushed to master for 19.11.00 |