This patch modifies _Findgroupreserve so that its one caller,
CheckReserves(), would include the reserve_id field in the
hold request it returns.
Failure to include reserve_id in every circumstance resulted
in bug 11947. This patch is therefore a complementary fix for
that bug, but is not meant to preempt the direct fix for
that bug.
To test:
[1] Verify that t/db_dependent/Reserves.t passes.
[2] Verify that the following test plan taken from
the patch for bug 11947 works for this patch
*without* applying the patch for 11947:
* have a few borrowers, say 4.
* have a biblio with a single item (you can scale this up, it should
work just the same.)
* issue the item to borrower A
* have borrowers B, C, and D place a hold on the item
* return the item, acknowledge that it'll be put aside for B.
* view the holds on the item.
Without the patch:
* the hold priorities in the UI end up being "waiting, 2, 1" when they
should be "waiting, 1, 2".
* in the database "reserves" table, they're really "0, 2, 3" when they
should be "0, 1, 2".
With the patch:
* the hold priorities in the UI end up being "waiting, 1, 2"
* in the database, they're "0, 1, 2"
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Work as described. No koha-qa errors. Test pass
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Currently when a reserve is moved to "waiting" status because it's
acknowledged on checkin, the reserve priorities aren't renumbered. This
causes things to go a bit haywire in the UI, in particular, some
reserves can unjustly end up with priority 1 when they shouldn't. It
also seemed to mess with the logic of who should get it next, but I
didn't look too closely at that.
This patch forces a renumbering so that all the priorities remain
copacetic.
Test plan:
* have a few borrowers, say 4.
* have a biblio with a single item (you can scale this up, it should
work just the same.)
* issue the item to borrower A
* have borrowers B, C, and D place a hold on the item
* return the item, acknowledge that it'll be put aside for B.
* view the holds on the item.
Without the patch:
* the hold priorities in the UI end up being "waiting, 2, 1" when they
should be "waiting, 1, 2".
* in the database "reserves" table, they're really "0, 2, 3" when they
should be "0, 1, 2".
With the patch:
* the hold priorities in the UI end up being "waiting, 1, 2"
* in the database, they're "0, 1, 2"
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Test plan confirms that the problem exists and that the patch corrects
it.
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Passes all tests and QA script, especially t/db_dependent/Reserves.t.
Improves priority calculation.
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Changed title and author field for UNIMARC.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Tested for MARC21, NORMARC and UNIMARC by adding temporary set_preference..
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Set marcflavour to MARC21 to make tests pass.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Works for MARC21. But I would prefer a better fix for UNIMARC.
Will send a follow-up for that.
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Adding a few unit tests, including the following situations:
Placing a hold when there is a wait.
Placing a hold when there is a future hold.
Calculating priority with future date.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Rebased on January 29, 2014 (marcelr)
Added text on the two 'is'-statements.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Adds three tests to Reserves.t for GetReservesFromItemnumber.
We test if this routine does not return a future next available hold,
a future item level hold. And if it does return a future wait (that is:
a confirmed future hold, using ConfirmFutureHolds).
Note that Holds.t does also contains some basic tests for this routine,
but the additional tests seem to better located in the direct context of
tests for bug 9761 for ConfirmFutureHolds.
Test plan:
Run both t/db_dependent/Holds.t and t/db_dependent/Reserves.t.
Verify if both tests do not fail.
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
This patch implements a regression test for verifying that
duplicate hold notifications aren't sent if ModReserveAffect() is
called repeatedly (as might happen if a circ operator accidentally
checks in an item and confirms its hold more than once).
Note that the test depends on the fact that _koha_notify_reserve()
defaults to sending a HOLD_PRINT letter if the borrower has not
specified an email or SMS hold notification.
To test:
[1] Run prove -v t/db_dependent/Reserves.t
[2] The 'patron not notified a second time (bug 11445)' test
should fail.
[3] Apply the main patch and run prove -v t/db_dependent/Reserves.t
again. This time all tests should pass.
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
This patch adds unit tests for two parts of DelItemCheck: checking
if the item is on loan, and checking if it is waiting on the hold
shelf.
To test:
[1] Verify that prove -v t/db_dependent/Circulation/IsItemIssued.t
is successful.
[2] Verify that prove -v t/db_dependent/Reserves.t is *not*
successful -- as it turns out, there was a latent bug where items
waiting on the hold shelf or in transit to fill a hold could still
be deleted without any warning.
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
This fixes a regression introduced by the patch for bug
9394 -- when printing a hold slip using the 'print and confirm'
button, the slip would contain only the text 'reserve not found',
not a full hold slip.
This patch also adds a regression test.
To test:
[1] Check in an item that would fill a hold. Use the 'print
and confirm button' to confirm the hold.
[2] The printout will only contain text to the effect of
'reserve not found'.
[3] Apply the patch.
[4] Repeat step 1. This time, a full hold slip should be printed.
[5] Verify that prove -v t/db_dependent/Reserves.t passes.
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Pass all tests, new and old, and QA script.
Verified wrong and corrected behaviour.
Note: Sometimes there will not be the message 'reserve not found'
showing up, but hold information for a different record. This happens
when there exists a reserve_id with the borrowernumber of the patron
in question in your database.
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Adds tests for CheckReserves with lookahead parameter.
Adds tests for AddReturn with regard to future reserve messages.
The following test cases are added, resulting in 8 new tests:
a) Add a reserve without date, CheckReserve should return it
b) Add a reserve with future date, CheckReserve should not return it
c) Add a reserve with future date, CheckReserve should return it if lookahead
is high enough
d) Check ResFound message of AddReturn for future hold
Test plan:
Run the test. No fails?
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Optionally add some branches and categories that may not exist.
Test plan:
Run the test with or without CPL branch or S (staff) category.
Verify that the test does not fail.
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
This patch adds a regression test for this bug, effectively
implementing the manual test plan in the previous patch.
To test:
[1] Verify that prove -v t/db_dependent/Reserves.t passes.
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
CheckReserves was using the CircControl system preference to determine what
patrons an item can fill a hold for. It should be using ReservesControlBranch
instead.
Test Plan:
1) Set ReservesControlBranch to "item's home library".
2) Create an item at Library A, place holds for it for patrons at
Library B, Library C, and Library A in that order,
for pickup at the patrons home library.
3) Make sure the holds policy for Library A is set to
Hold Policy = "From home library" and
Return Policy = "Item returns home".
Make sure the holds policies for the other libraries are set to
Hold Policy = "From any library".
4) Check the item in at Library C, the hold for the patron at Library B
should pop up, even though it's in violation of the circulation rules.
Don't click the confirm button!
5) Apply this patch, and reload the page,
now the hold listed should be for the last hold,
the hold for the patron at Library A, which is correct.
This patch adds the subroutine C4::Reserves::GetReservesControlBranch as
an equivilent to C4::Circulation::_GetCircControlBranch.
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Fixed POD so that arguments and explanation match (C<$item>).
Also tested opac-reserves.pl for regressions.
Passes all tests, QA script, and Reserves.t.
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Since the patron barcode is nullable, and since the tests using
the test patron don't care what the barcode is, don't set it. This
avoids the tests failing if the test database happens to already
have a patron record with the hard-coded barcode that used
to be supplied.
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Works better now, creats biblio records, and cleans up after itself as
well.
Signed-off-by: Paul Poulain <paul.poulain@biblibre.com>
To test:
Create 4 holds on a bib, for patrons A, B, C, and D,
Check in the item to mark hold as waiting for patron A
Check out the item to patron B -> reserve for patron B should be removed
Check in the item to mark hold as waiting for patron A
Check out the item to Patron A, hold should complete normally
Check in the item to mark hold as waiting for patron C
Check out the item to patron D -> reserve for patron D should be removed.
Check in the item to mark hold as waiting for patron C
Check out the item to patron C, hold should complete normally
Check in the item -> there should be no more reserves.
We also tested:
Created 4 holds on a bib with two items, for patrons A, B, C, and D
All worked as expected.
Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Signed-off-by: Paul Poulain <paul.poulain@biblibre.com>
