Koha-community/Koha - Koha - Koha: The world's first free and open source library system

Commit Graph

Author	SHA1	Message	Date
Tomás Cohen Arazi	900670bd3b	Bug 13075: (followup) remove remaining warnings There's no point creating a MARC record with undef subfields for testing holds. This patch avoids that so no warnings are shown. To test: - Run $ prove t/db_dependent/Holds.t => FAIL: verify several warnings show - Apply the patch - Re-run => SUCCESS: no warnings showed. - Sign off :-D Regards NOTE: Not noticable under Ubuntu 12.04 LTS, but verifiable under Debian Wheezy. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>	10 years ago
Olli-Antti Kivilahti	51f0a0b722	Bug 13116 - Make it possible to propagate errors from C4::Reserves::CanItemBeReserved() to the web-templates. This patch changes the way CanBookBeReserved() and CanItemBeReserved() return error messages and how they are dealt with in the templates. This change makes it possible to distinguish between different types of reservation failure. Currently only two types of errors are handled, all the way to the user, from the CanItemBeReserved(): -ageRestricted -tooManyReserves which translates to maxreserves ############# - TEST PLAN - ############# ((-- AGE RESTRICTION --)) STAFF CLIENT 1. Find a Record with Items, update the MARC Subfield 521a to "PEGI 16". 2. Get a Borrower who is younger than 16 years. 3. Place a hold for the underage Borrower for the ageRestricted Record. 4. You get a notification, that placing a hold on ageRestricted material is forbidden. (previously you just got a notification about maximum amount of reserves reached) ((-- MAXIMUM RESERVES REACHED --)) 0. Set the maxreserves -syspref to 3 (or any low value) STAFF CLIENT AND OPAC 1. Make a ton of reserves for one borrower. 2. Observe the notification about maximum reserves reached blocking your reservations. ((-- MULTIPLE HOLDS STAFF CLIENT --)) 3. Observe the error notification "Cannot place hold on some items" ((-- MULTIPLE HOLDS OPAC --)) 1. Make a search with many results, of which atleast one is age restricted to the current borrower. 2. Select few results and "Place hold" from to result summary header element. (Not individual results "Place hold") 3. Observe individual Biblios getting the "age restricted"-notification, where others can be reserved just fine. Updated the unit tests to match the new method return values. t/db_dependent/Holds.t & Reserves.t Followed test plan. Works as expected and displays meaningful messages for the reason why placing a hold is not possible. Signed-off-by: Marc Veron <veron@veron.ch> Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>	10 years ago
Tomás Cohen Arazi	84b4ace746	Bug 12879: Remove unnecesary diags from Holds.t There are two unnecessary diag statements: - Creating biblio instance for testing. - Creating item instance for testing. TEST PLAN --------- 1) prove t/db_dependent/Holds.t t/db_dependent/Holds.t .. 1/38 # Creating biblio instance for testing. Use of uninitialized value in subroutine entry at /home/tcohen/git/koha-community-src/C4/Charset.pm line 181. # Creating item instance for testing. Use of uninitialized value in subroutine entry at /home/tcohen/git/koha-community-src/C4/Charset.pm line 181. Use of uninitialized value in subroutine entry at /home/tcohen/git/koha-community-src/C4/Charset.pm line 181. t/db_dependent/Holds.t .. ok All tests successful. Files=1, Tests=38, 1 wallclock secs ( 0.03 usr 0.01 sys + 1.13 cusr 0.11 csys = 1.28 CPU) Result: PASS -- They are in the first and fourth lines of this sample output 2) apply patch 3) prove t/db_dependent/Holds.t t/db_dependent/Holds.t .. 1/38 Use of uninitialized value in subroutine entry at /home/mtompset/kohaclone/C4/Charset.pm line 186. Use of uninitialized value in subroutine entry at /home/mtompset/kohaclone/C4/Charset.pm line 186. Use of uninitialized value in subroutine entry at /home/mtompset/kohaclone/C4/Charset.pm line 186. t/db_dependent/Holds.t .. ok All tests successful. Files=1, Tests=38, 1 wallclock secs ( 0.01 usr 0.00 sys + 0.78 cusr 0.09 csys = 0.88 CPU) Result: PASS -- They are no longer in the first and fourth lines of this sample output 4) run koha QA test tool Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>	10 years ago
Jonathan Druart	10622af499	Bug 10226: Add unit tests for GetReservesFromItemnumber Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>	10 years ago
Kyle Hall	e0e66612cc	Bug 8735 [QA Followup] - Add Unit Tests Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>	10 years ago
Galen Charlton	2afadcc358	Bug 9532: add regression test To test: [1] Run prove -v t/db_dependent/Holds.t. The last test should fail. [2] Apply the main patch. [3] Run step 1 again. This time the tests should all pass. Signed-off-by: Galen Charlton <gmc@esilibrary.com>	10 years ago
Kyle Hall	2e6d44bd32	Bug 10452: [QA Followup] - Unit tests Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Passes all tests and QA script. Also checked t/db_dependent/Holds.t t/db_dependent/HoldsQueue.t Tested holds triggering with different settings of AllowHoldsOnDamagedItems. Works as described. Signed-off-by: Galen Charlton <gmc@esilibrary.com>	10 years ago
Jonathan Druart	8b685c1e80	Bug 9823: Refactor return from GetReservesFromBiblionumber The return from GetReservesFromBiblionumber contains an unnecessary extra variable. In scalar context an array returns its element count. Maintaining a separate count can lead to unforeseen bugs and imposes ugly constructions on the subroutine's users. Remove the useless count variable from the return This patch also changes the parameters: now the routine takes a hashref. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Placed biblio holds, future holds and item holds. Works as expected. Tested Holds.t and Reserves.t. Pass. Tested /cgi-bin/koha/ilsdi.pl?service=GetRecords&id=999 with two holds on one item. Fine. C4/SIP/ILS/Item.pm: Looked for "whatever" and "arrayref" and could not find them anymore. Looks good. Handled a few unneeded calls in QA follow-up. Left only one point to-do for serials/routing-preview.pl. See Bugzilla. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Galen Charlton <gmc@esilibrary.com>	11 years ago
Jonathan Druart	78037c5573	Bug 11336: update hold queue priorities correctly when deleting holds In various places, deleting a hold request did not trigger recalculating the priority of the other holds on the bib: To reproduce the bug: - select or create 2 users U1 and U2 - select or create an holdable item - place on hold for both U1 and U2. U1 has priority 1 and U2 has priority 2. - delete the hold for U1 - go on circ/circulation.pl?borrowernumber=XXXX for U2 (or in the DB directly) and verify the priority has not been set to 1 The issue is repeatable (at least) on these 2 pages: * circ/circulation.pl?borrowernumber=XXXX (tab 'Holds', select "yes" in the dropdown list and submit the form) * reserve/request.pl?biblionumber=XXXX (click on the red cross) Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Reran my tests: Preparations: - Create holds for different patrons on a record: * 1st - title level hold * 2nd - item level hold * 3rd - title level hold * 4th - title level hold - AllowOnShelfHolds = On/Allow (items were not checked out) Tests: Deleted holds from various pages, confirming bugs first, then testing with applied patches. Reloading database after each test. 1) Cancel holds from OPAC patron account /cgi-bin/koha/opac-user.pl#opac-user-holds - Cancel 4th - ok, before and after applying the patch - Cancel 2nd - ok, after applying the patch 2) Cancel hold from holds tab on staff detail page /cgi-bin/koha/reserve/request.pl?biblionumber=7 a) Setting priority to 'del', submitting with 'Update holds' - Cancel first (1st) - ok, before and after - Cancel hold in the middle (was 3rd) - ok, before and after - Cancel last (was 4th) -ok, before and after b) Using red X - Repeating tests from a) - before the patch is applied holds get totally 'out of order' - after applying the patch, it works correctly Additional tests done on this page: - Change priority using up, down, to top, to bottom icons - Change priority with 'toggle to lowest' 3) Cancel hold from the patron's account a) Check out tab - Delete? Yes, 'Cancel marked holds' /cgi-bin/koha/circ/circulation.pl?borrowernumber=X - Cancel first (1st) - ok, after applying the patch - Cancel hold in the middle (was 3rd) - ok, after applying the patch - Cancel last (was 4th) - ok, after applying the patch b) Details tab - Delete? yes, 'Cancel marked holds' /cgi-bin/koha/members/moremember.pl?borrowernumber=X - Cancel first (1st) - ok, after applying the patch - Cancel hold in the middle (was 3rd) - ok, after applying the patch - Cancel last (was 4th) - ok, after applying the patch Without the patch, holds priorities get out of order. Additional tests done: - Check in one item to trigger first hold - Check in one item to trigger second hold - Check out first item Priorities are kept while the item is waiting, when it's checked out, priorities of remaining holds get reset correctly. Conclusion: Big improvement, no regressions found. Passes all tests in t, xt and QA script. Also: t/db_dependent/Holds.t t/db_dependent/HoldsQueue.t t/db_dependent/Reserves.t Signed-off-by: Galen Charlton <gmc@esilibrary.com>	11 years ago
Galen Charlton	23e302071e	bug 2394: regression test for canreservefromotherbranches If IndependentBranches is ON and canreservefromotherbranches is OFF, a patron is not permittedo to request an item whose homebranch (i.e., owner of the item) is different from the patron's own library. However, if canreservefromotherbranches is turned ON, the patron can create such hold requests. Note that canreservefromotherbranches has no effect if IndependentBranches is OFF. To test, run prove -v t/db_dependent/Holds.t. Without the bugfix patch for bug 2394, the last two tests should fail. Signed-off-by: Galen Charlton <gmc@esilibrary.com>	11 years ago
Galen Charlton	8d9e6f3710	Bug 9394: (follow-up) modernize test cases - wrap in a transaction - create the patron records needed for the test To test: [1] Run prove -v t/db_dependent/Holds.t [2] Verify that all tests have passed. [3] Verify that the additional patron records created by the test no longer exist in the database. Signed-off-by: Galen Charlton <gmc@esilibrary.com> Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> Signed-off-by: Galen Charlton <gmc@esilibrary.com>	11 years ago
Jonathan Druart	dd2185981d	Bug 9394: QA Followup * C4::Reserves::_FixPriority - The previous code checked the cancellationdate. If think you never pass in it with bad parameters, but in order to be sure I added the check on this value. - The reservedates array was never used. * circ/circulation.tt There was a bug: it was not possible to remove an hold from the circulation page. Passing reserve_id fixes the issue. * C4::Reserves::GetReserveId This subroutine did not have a unit test. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Galen Charlton <gmc@esilibrary.com> Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> Signed-off-by: Galen Charlton <gmc@esilibrary.com>	11 years ago
Kyle Hall	53fbfa2dde	Bug 9394: Use reserve_id where possible This patch switches from using a combination of biblionumber/borrowernumber to using reserve_id where possible. Test Plan: 1) Apply patch 2) Run t/db_dependent/Holds.t Signed-off-by: Maxime Pelletier <maxime.pelletier@libeo.com> Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Galen Charlton <gmc@esilibrary.com> Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> Signed-off-by: Galen Charlton <gmc@esilibrary.com>	11 years ago

13 Commits (900670bd3bd3d1984dbd23e86e78706ff15288f9)