Commit graph

199 commits

Author SHA1 Message Date
798d38e4c7 Bug 16011: $VERSION - Remove comments
perl -p -i -e 's/^.*set the version for version checking.*\n//' **/*.pm

+ manual adjustements

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
2016-03-24 17:20:29 +00:00
017699c345 Bug 16011: $VERSION - Remove the $VERSION init
Mainly a
  perl -p -i -e 's/^.*3.07.00.049.*\n//' **/*.pm
Then some adjustements

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
2016-03-24 17:20:28 +00:00
3830d78d46 Bug 16011: $VERSION - remove use vars $VERSION
perl -p -i -e 's/^(use vars .*)\$VERSION\s?(.*)/$1$2/' **/*.pm

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
2016-03-24 17:20:26 +00:00
4f5217314c Bug 15629: Koha::Libraries - Remove GetBranchDetail
C4::Branch::GetBranchDetail retrieved library infos, it could be easily
replaced with Koha::Libraries->find

When this change needs other big changes, the unblessed method is
called, to manipulate a hashref (as before) instead of a Koha::Library
object (for instance when $library is sent to GetPreparedLetter).

Test plan:
1/ Print a basket group, the library names should be correctly
displayed.
2/ Enable emailLibrarianWhenHoldIsPlaced and place a hold, a HOLDPLACED
notice will be generated (focus on the library name)
3/ Edit a patron and change his/her library
4/ Generate the advanced notices (misc/cronjobs/advance_notices.pl) and
have a look at the generated notices
5/ Same of overdues notices
6/ Set IndependentBranches and use a non superlibrarian user to place a
hold. The "pickup at" should be correctly filled.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Brendan Gallagher brendan@bywatersolutions.com
2016-02-24 03:55:06 +00:00
b86143b7b8 Bug 14310 [QA Follow] - Remove FIXME, add explanation comment instead
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
2016-01-27 06:20:20 +00:00
f87494a6e9 Bug 14310 [QA Followup] - Adapt existing code to use new methods
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
2016-01-27 06:20:19 +00:00
eed2c0d437 Bug 15443 - Re-code RESERVESLIP as HOLD_SLIP
It's a bit confusing to have RESERVESLIP named "Hold Slip", we should
finish the job and recode the slip.

Test Plan:
1) Apply this patch
2) Run updatedatabase.pl
3) Try printing a hold slip, you should note no difference
   from pre-patch behavior

Followed test plan. The slip is triggered if you want to check out
an item that is reserved / on hold for another patron. Works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2016-01-04 12:49:30 +00:00
06e372d0be Bug 13592: Add an option to charge for any hold placed
Currently the fee is applied on if all items for the record are issued
and at least one hold already exists on the record.
This patch does not give a complete answer to the problem (see
discussion on bug 13592 for the other user's expectations).
It only adds the ability to charge for any hold placed regardless of the
conditions.

Test plan:
1) Execute the updatedb entry to insert the new pref
2) Confirm that the behavior is the same as before applying this patch
3) Change the HoldFeeMode pref to 'always'
4) Note that the fee is applied for any hold placed

Signed-off-by: Sally Healey <sally.healey@cheshiresharedservices.gov.uk>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2015-12-31 15:59:05 +00:00
3ebf343af6 Bug 9303 [3] - relative's checkouts in the opac
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@quecheelibrary.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2015-12-31 13:08:06 +00:00
Barton Chittenden
f9b9c78231 Bug 14515: Add biblioitems to messages in C4/Reserves.pm
http://bugs.koha-community.org/show_bug.cgi?id=14515

Signed-off-by: Danielle Aloia <daloia@nyam.org>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
2015-12-29 19:43:36 +00:00
9180389045 Bug 14985: dbms expects a iso formatted date
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2015-11-13 11:24:55 -03:00
7aabe91749 Bug 14985: Simplify the date management in AddReserve
The code is much more readable using the str param of output_pref

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2015-11-13 11:24:54 -03:00
Marc Véron
b8b0b1370f Bug 14985: Remove C4::Dates from files in folder C4/*.pm (part one)
This patch removes C4::Dates from following files in folder C4:

- C4/Members.pm
- C4/Reserves.pm
- C4/Search.pm
- C4/Utils/DataTables.pm
- C4/Utils/DataTables/Members.pm
- C4/VirtualShelves/Page.pm

To test:
 -run tests as appropriate,
- have a close look at the code changes
- try to find regressions

http://bugs.koha-community.org/show_bug.cgi?id=14985

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>

Bug 14985: (followup) Remove eval if dates come from database

This patch removes some evals from date-formatting where the dates come
from the database.

See comments #7 - #9

Additionaly, C4/VirtualShelves/Page.pm is removed from the patches (obsolete).

Bug 14985: (followup) Remove C4::Dates from C4/Overdues.pm

Ths patch removes a stray C4::Dates from C4/Overdues.pm

- To test got to a patron who has overdues
  (Home > Circulation > Checkouts > [Patron])
- Print overdues

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2015-11-13 11:24:54 -03:00
4f4c5e67ef Bug 5144: Duplicate holds allowed if patron clicks back button after placing hold
Koha is currently not engineered to handle multiple holds per record.
Until such time that is does, we should not allow them to be created.

Test Plan:
1) Apply this patch
2) Log in to the opac
3) Place a hold
4) Hit the back button on your browser
5) Place the hold again
6) Note the new message

Signed-off-by: David Kuhn <kuhn@monterey.org>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2015-10-06 10:01:55 -03:00
37ad2d7867 Bug 14733: Prevent a record from having holds with duplicate priorities
It is possible to create holds with duplicate priorities.

The reason for this is that typically the priority is calculated before
placing the hold. When the hold is placed the priority is calculated.
This can easily be shown by opening up two browser windows and starting
to place a hold for a record in each one. You'll see that both list the
same priority. If you than place the hold in each window, both holds
will have the same priority!

Test Plan:
1) Run unit tests pre-patch, note they fail
2) Run unit tests post-patch, note they succeed

Signed-off-by: Heather Braum <hbraum@nekls.org>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2015-09-25 11:25:27 -03:00
a37b3bb7f7 Bug 14640: 'Cancel Hold' check box on check-out confirmation does not cancel the hold when item is checked out.
This bug is dealing with the situation where an item is checked out to a
patron that is not the next in line hold-wise for an item. In this case,
Koha will warn the librarian that there are holds on the item and
show the first person in line. Again, I want to stress that this
is the case where the item *is not waiting* for a patron. The
hold for the patron listed will just have a priority of 1.

The only situation where the "Cancel hold" checkbox will function
is when the priority 1 hold is an item level hold. This is due to
the fact that CancelReserve is being passed the trio of
biblionumber, borrowernumber, and itemnumber rather than the
singular reserve_id.

1) place biblio level hold on a book to borrower A.
2) check out an item of the book to borrower B.
3) When confirming checkout, check the 'Cancel hold' check-box, and
   click the "Yes, check out" button.
4) Note the hold was not canceled
5) Apply this patch
6) Repeat steps 1 through 3
7) Note the hold was indeed canceled

Signed-off-by: Joonas Kylmälä <j.kylmala@gmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>
2015-09-16 11:00:19 -03:00
70133ef343 Bug 14801: Fix Reserves.t -- Follow-up for ChargeReserveFee
The problem making some tests fail, actually was the unneeded addition
of zero accountline records by ChargeReserveFee, called by AddReserve.
The balance is still zero, but a test like !$var responds differently
when var is 0.00 instead of 0 or undef.

This patch adjusts the test in ChargeReserveFee in order to prevent
adding these records with 0.00.
The first patch that adjusts the tests in Reserves.t is not strictly
needed anymore, but can stay.

Test plan:
[1] Run t/db_dependent/Reserves.t
[2] Run t/db_dependent/Reserves/GetReserveFee.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>
2015-09-16 10:19:38 -03:00
e2a87c54c0 Bug 14702: [QA Follow-up] More readable variable names, less queries
The names are much better now :)
Combined the queries for items and issues.
Only check the number of holds when needed.

Test plan:
Verify the changes here by running the unit test again.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Joonas Kylmälä <j.kylmala@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2015-09-07 12:04:49 -03:00
44a4e043a5 Bug 14702: Refactor GetReserveFee
The code of GetReserveFee was not very clear.
What it did was: check if there are some items not issued. If so and there
are no holds, calculate no fee.

While doing so, I moved the code to charge the fee (in AddReserve) to a small
new sub ChargeReserveFee.

There is no change in behavior.
The follow-up patch adds unit tests.

Test plan:
[1] Make sure that a patron category (X) includes a hold fee.
[2] Select a biblio with 2 items.
[3] Issue one item to another patron.
[4] Place a hold on this biblio by patron with category X. No charge?
[5] Cancel the hold from the previous step.
[6] Use another patron to place another hold on this biblio.
[7] Place hold again by patron with category X. Is it charged?
[8] Cancel that hold again. Issue the second item to another patron.
[9] Place hold again by patron with category X. Is it charged again?

Signed-off-by: Joonas Kylmälä <j.kylmala@gmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2015-09-07 12:04:48 -03:00
Joonas Kylmälä
198e273530 Bug 14526: (follow-up) add a space before equals sign
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2015-09-02 14:53:42 -03:00
ea6c4f5b8a Bug 14526: MoveReserve should look at future holds too
At checkout a hold for the same borrower is considered to be filled.
It is consistent to do the same for holds of the same borrower within
[ConfirmFutureHolds] days (if non-zero).

This goal is achieved by adjusting the CheckReserves call within
MoveReserve, adding the lookahead parameter.
I used this occasion to revisit other calls of CheckReserves:
- transferbook: no need to add lookahead; a future hold should not block
  a transfer;
- CanBookBeIssued: no lookahead; future hold does not block an issue;
- CanBookBeRenewed: idem.
- GetOtherReserves (only used in circ/returns): this call might be a
  candidate for lookahead too, but I leave that for another report. It is
  in the context of checkin and transfer, not checkout.

Test plan:
[1] Set ConfirmFutureHolds to zero days. (You may also need to enable
    AllowHoldDateInFuture.)
[2] Place a hold with borrower A on biblio X for tomorrow. Also place a hold
    with borrower B on X for today. (Use biblio level holds.)
[3] Check out item Y of X to borrower A. Ignore the warning for borrower B
    and do not cancel the hold of B (so: confirm checkout).
    Verify that X has still two holds.
[4] Check in Y (without confirming a hold).
[5] Enable ConfirmFutureHolds, say 2 days.
[6] Check out Y to A again. Ignore the warning for B (no cancel). Verify that
    X now only has one hold for borrower B (the hold for A was filled).

Signed-off-by: Joonas Kylmälä <j.kylmala@gmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2015-09-02 14:53:42 -03:00
6f95acd1a6 Bug 12632: Hold limits ignored for record level holds with item level itemtypes
The crux of the issue is that if you are using item level itemtypes, but
are allowing biblio levels holds, those holds do not have items.

So, in CanItemBeReserved, when Koha counts the number of holds to
compare against the given rule, it will always give 0 ( except of course
for found holds, and the occasional item-level hold ).

So the query is saying "link each of these reserves to the reserved
item, and count the number of reserves this patron where the itemtype is
DVD". However, since these are all record level reserves, there are no
items to link to, and so when it looks for all reserves this and item
whose itemtype is DVD, it finds zero reserves!

This patch solves the problem by looking first at the item level
itemtype, and if it does not exist, then it looks at the record
level itemtype. For installations using record level itemtypes, the
behavior remains unchanged.

Test plan:
1) Enable item level itemtypes
2) Create two records with one item each of a given itemtype
3) Create a single issuing rule and limit the holds allowed for that
   itemtype to 1
4) Place a record level hold on your first record
5) Attempt to place a record level hold for the same patron on your
   second record. You should not be able to but you can!
6) Apply this patch
7) Repeat step 5, note you can no longer place the hold!

Signed-off-by: Paola Rossi <paola.rossi@cineca.it>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2015-08-26 10:42:04 -03:00
64fcab9e51 Bug 9809: Fix pod errors
FAIL   C4/Reserves.pm
   FAIL   pod
                 in file C4/Reserves.pm
                *** ERROR:
                Spurious =cut command

Test plan:
perl -e "use Pod::Checker;podchecker('C4/Reserves.pm');"
Should not return any errors.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2015-08-26 10:27:00 -03:00
ad3239479d Bug 9809: Update AddReserve prototype to remove constraint parameter
Test Plan:
1) Apply this patch set
2) prove t/db_dependent/Circulation.t
3) prove t/db_dependent/Holds.t
4) prove t/db_dependent/Holds/LocalHoldsPriority.t
5) prove t/db_dependent/Holds/RevertWaitingStatus.t
6) prove t/db_dependent/HoldsQueue.t
7) prove t/db_dependent/Reserves.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

AMENDED: An else branch in reserve/placerequest.pl was removed. This had
the effect of making it no longer possible to place an any hold in the
staff client.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Verified placing a biblio level and an item level hold.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2015-08-26 10:26:43 -03:00
a379525086 Bug 9809: Remove reserveconstraints references from C4::Reserves
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2015-08-26 10:26:36 -03:00
Jesse Weaver
4ce29452ad Bug 14464: (QA followup) add unit tests
This followup adds several tests to t/db_dependent/Reserves.t.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>
2015-08-21 09:20:22 -03:00
Jesse Weaver
bb6277ffcc Bug 14464: Add ability to cancel waiting holds from checkin screen
Test plan:

    1. Ensure that ExpireReservesMaxPickUpDelayCharge is set to 0.
    2. Place a hold (doesn't matter whether it's a bib/item-level hold),
       then confirm the hold by checking it in.
    3. Check in the item again, and hit Cancel.
    4. The reserve in question should be cancelled.
    5. Repeat steps 2-4 twice, once after setting
       ExpireReservesMaxPickUpDelayCharge to a nonzero value and again
       after clicking the "Forgive fees for manually expired holds"
       checkbox.

A fine should only be applied when the syspref is enabled and the
checkbox is not checked. Also, the checkbox should only appear after
enabling the syspref. And finally, the checkbox should remember whether
it is checked across multiple checkins, same as the "Forgive overdue
charges" and "Book drop mode" checkboxes.

Signed-off-by: Jason Burds <jburds@dubuque.lib.ia.us>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Amended patch: Removed 2 debugging lines.
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>
2015-08-21 09:20:11 -03:00
Stefan Weil
64925f7522 Bug 14383: C4: Fix some typos (mostly in comments and documentation)
Most of them were found and fixed using codespell.
Fix also some related grammar issues.

In C4/Serials.pm a variable was renamed to make future codespelling
checks easier.

Signed-off-by: Stefan Weil <sw@weilnetz.de>

http://bugs.koha-community.org/show_bug.cgi?id=14383
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2015-06-22 17:34:45 -03:00
Julian Maurice
1970b245f5 Bug 13687: Move hold policy check into CanItemBeReserved
This way ILS-DI HoldItem and HoldTitle services also benefit from this
check

Test plan:

1/ Define some default holds policies by item type in
/admin/smart-rules.pl
2/ Use ILS-DI HoldItem service and check that those rules are respected
3/ Check that staff and opac hold behaviour is unchanged regarding
these rules.

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Passes tests and QA script. No regressions found,
improves the ILS-DI HoldItem response.
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2015-05-19 12:05:50 -03:00
Jonathan Druart
a6c9bd0eb5 Bug 9978: Replace license header with the correct license (GPLv3+)
Signed-off-by: Chris Nighswonger <cnighswonger@foundations.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>

http://bugs.koha-community.org/show_bug.cgi?id=9987

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2015-04-20 09:59:38 -03:00
ebccf4099f Bug 5786 [QA Followup]
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2015-03-25 10:33:31 -03:00
Srdjan
1802aa9153 Bug 5786 - Move AllowOnShelfHolds and OPACItemHolds system prefs to the Circulation Matrix
C4::Reserves:
* Added OnShelfHoldsAllowed() to check issuingrules
* Added OPACItemHoldsAllowed() to check issuingrules
* IsAvailableForItemLevelRequest() changed interface, now takes
  $item_record,$borrower_record; calls OnShelfHoldsAllowed()

opac/opac-reserve.pl and opac/opac-search.pl:
* rewrote hold allowed rule to use OPACItemHoldsAllowed()
* also use OnShelfHoldsAllowed() through
* IsAvailableForItemLevelRequest()

templates:
* Removed AllowOnShelfHolds and OPACItemHolds global flags, they now
  only have meaning per item type

Signed-off-by: Nicole C. Engard <nengard@bywatersolutions.com>

I have tested this patch left, right and upside down for the last
several months. All tests have passed.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2015-03-25 10:33:14 -03:00
fcaa6f35c0 Bug 13636 - Staff search results item status incorrect for holds
Imagine this scenario: we have one record with four items. Two of those
items are checked out, one of those items is a waiting hold, and one of
those items is available. We would expect to see this on the search
results page. Instead, we will see both non-checked out items as
unavailable due to waiting holds.

This is due to a semantic issue GetReserveStatus.
C4::Search::searchResults uses GetReserveStatus to get the reserve
status of each item, but unlike all other calls to the sub, this one
passes in not only itemnumber, but biblionumber.

When no reserve is found for the available item, the subroutine uses the
biblionumber to grab what is essentially an arbitrary reserve to use for
the status. This makes no sense and this functionality should be
entirely removed from the subroutine so regressions like this will be
prevented in the future.

Test Plan:
1) Create one record with 4 items
   a) check two of the items out to patrons
   b) set one of the items as a waiting hold
   c) leave the fourth item as available
2) Run a search where this record will be in the results list
3) Note that the results list 2 items on loan, two unavailable
4) Apply this patch, reload the search results
5) Note that the results list 1 available, 2 on loan, 1 unavailable

Signed-off-by: John Andrews <jandrews@washoecounty.us>
Signed-off-by: Sheila Kearns <sheila.kearns@state.vt.us>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Note: This is for the staff search result list!

Works as expected.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2015-02-11 10:20:35 -03:00
Jonathan Druart
0f406a840a Bug 12792: C4::Reserves breaks my vim syntax color
C4/Reserves.pm is unreadable with my vim configuration.
It appears I am the only one having this problem.
For an incomprehensible reason, a string constructs with
  qq/my string/;
completely breaks the syntax color for all the rest of the file (~2300l).
If I replace it with
  qq{my string};
all is fine!

Test plan:
launch
  git show HEAD
and verify this patch won't break anything.

Additionally, prove t/db_dependent/Reserves.t
This will trigger the three functions that were modified.
The prove currently fails on test 8, but the other succeeding
tests prove that this change is fine.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
All tests pass on my installation.
No problems found.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2015-01-14 12:42:32 -03:00
Chris Cormack
ef6bc21b2c Bug 13368 Holds priority messed up on checkout
To Test

1/ Create 3 (or more holds) on one biblionumber, make sure at least
one item is not on loan
2/ Check out the not on loan item to a borrower (maybe number 2 in the
queue)
3/ Look in the database (or on the holds tab on the moredetail.pl)
notice the priorities have not been reordered
4/ Apply patch and try again, notice now they have

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Confirmed the problem without the patch, and confirmed that the patch
corrects it.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-12-17 19:59:44 -03:00
e3fe0bdb31 Bug 13152 - Duplicate phone hold notices when using Talking Tech
If a library is using Talking Tech for phone notices, any waiting hold
phone notice will show up twice!

This is because Koha generates on at the time the hold is set to
waiting, and then the cronjob TalkingTech_itiva_outbound.pl generates
it's own notice as well.

The former notice will always have a status of 'pending', as the
TalkingTech_itiva_inbound.pl script will update the notice the outbound
script created.

The solution is to prevent Koha from creating a phone notice for waiting
holds if TT is enabled, and let the cron script do it.

Test Plan:
1) Enable Talking Tech from the system preferences
2) Set a hold waiting phone notice in the notices and slips editor
3) Choose a patron, enable hold phone notices for that patron
4) Place a hold for a patron, and check it in so it's marked as waiting
5) Note the phone notice generated for the patron
6) Apply this patch
7) Repeat step 4
8) Note that this time, a phone hold waiting notice is not generated

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Amends condition with an additional or statement. Shoudn't affect
anything but phone notices. Change appears logical.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-25 17:33:56 -03:00
3b300146b2 Bug 11634 [QA Followup 3] - Found holds should be considered unavailable
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-12 11:27:42 -03:00
ebf4350735 Bug 11634 - Allow renewal of item with unfilled holds if other available items can fill those holds
The current holds behavior in Koha allows a situation like this:
- Patron A has an item currently checked out.
- Patron B places a hold on the next available copy of that title.
- Then Patron A will not be able to renew his item, even if there are
  other available copies of that title that could potentially fill Patron
  B's hold.

Since this seems unfair to Patron A, we should allow renewal of items
even if there are unfilled holds, but those holds could all be filled
with currently available items.

Test Plan:
1) Apply this patch
2) Create a record with two items
3) Check out the item to a patron
4) Place a hold on the record
5) Note you cannot renew the item for the patron
6) Enable the new system preference AllowRenewalIfOtherItemsAvailable
7) Note you can now renew the item, as all the holds can be satisfied
   by available items.
8) Place a second hold on the record
9) Note you can no longer renew the item, as all the holds *cannot*
   be filled by currently available items

Signed-off-by: Holger Meissner <h.meissner.82@web.de>
Signed-off-by: Chris Rohde <crohde@roseville.ca.us>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-12 11:27:31 -03:00
3bc14af683 Bug 13116 [QA Followup] - Remove tabs, use unless instead of if
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-12 11:23:45 -03:00
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>
2014-11-12 11:23:41 -03:00
Olli-Antti Kivilahti
7767f2e53b Bug 13113 - Prevent juvenile/children from reserving ageRestricted material
There is no reason for underage borrowers to reserve ageRestricted material and
then be denied it's check-out due to ageRestriction.

This patch prevents reserving material for borrowers not suitably aged.

 # # # # # #
 # A PRIORI #
 # # # # # #
BOTH THE STAFF CLIENT AND THE OPAC
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 can reserve an ageRestricted Record with ease.
STAFF CLIENT ONLY
5. Check-in an Item from the ageRestricted Record and catch the reservation.
6. Check-out the ageRestricted Item for this underage Borrower.
7. You get a notification about being unable to check-out due to age restriction.
   How lame is that for a 12 year old?

 # # # # # # # #
 # A POSTERIORI #
 # # # # # # # #
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. Check-out an ageRestricted Item for this underage Borrower.
4. You get a notification about having the maximum amount of reserves.
5. Place a hold for the underage Borrower for the ageRestricted Record.
6. You get a notification, that placing a hold on ageRestricted material is
   forbidden.

Includes Unit tests.

Followed test plan. Patch behaves as expected. (Note: Propagating error messages to template will be handled in Bug 13116 or 11999)

Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-11 09:53:55 -03:00
Jonathan Druart
659f7cd097 Bug 11126: qa follow-up
- use Modern::Perl;
- fix a typo
- remove an old comment

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-04 18:53:51 -03:00
3463dd2449 Bug 11126 [QA Followup] - Make reserves returned by _Findgroupreserve sorted by priority
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-04 18:53:43 -03:00
6b2d2abb19 Bug 11126 - Make the holds system optionally give precedence to local holds
This feature will allow libraries to specify that, when an item is returned,
a local hold may be given priority for fulfillment even though it is
of lower priority in the list of unfilled holds.

This feature has three settings:
* LocalHoldsPriority, which enables the feature
* LocalHoldsPriorityPatronControl, which selects for either tha patron's
  home library, or the patron's pickup library for the hold
* LocalHoldsPriorityItemControl, which selects for either the item's
  holding library, or home library.

So, this feature can "give priority for filling holds to
patrons whose (home library|pickup library) matches the item's
(home library|holding library)"

Test Plan:
1) Apply this patch
2) Run t/db_dependent/Holds/LocalHoldsPriority.t

Signed-off-by: Joel Sasse <jsasse@plumcreeklibrary.net>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-04 18:53:37 -03:00
Jonathan Druart
9daef6fb53 Bug 12876: Improve unit tests for CanReserveBeCanceledFromOpac
This patch fix the subroutine name and add a restriction on the
arguments: both argument are mandatory!

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-09-23 20:46:57 -03:00
Rafal Kopaczka
e9eef04b95 Bug 12876 - Reserve in waiting/transfer status may be cancelled by user
User may cancel his own reservation at waiting or in transit status
through calling opac-modrequest.pl. Cancel button is disabled in
interface but possibility to cancel should be checked also in
opac-moderequest.pl, before calling CancelReserve().
Similar situation is with opac-modrequest-suspend.pl

This patch provides new soubroutine to chceck if user can cancel given
reserve. It's possible only when he's owner of hold and hold isn't in
transfer or waiting status.

Additionaly there are new test for this function in Reserves.t

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Passes all tests, QA script and new tests.
Works as described, tested with:
.../cgi-bin/koha/opac-modrequest.pl?reserve_id=XXX

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-09-23 20:46:50 -03:00
Julian Maurice
45f474abdf Bug 8868: ILS-DI: CancelHold needs to take a reserve_id
CancelHold takes two parameters: patron_id and item_id.
If item_id is considered as an itemnumber, holds on title can't be
canceled.
If item_id is considered as a biblionumber, all holds on this
biblionumber (for a borrower) will be canceled.

So CancelHold have to consider item_id as a reserve_id.

- Added subroutine C4::Reserves::GetReserve
- C4::ILSDI::Services::GetRecords now returns the reserve_id
- Fix the text in the ilsdi.pl?service=Describe&verb=CancelHold page
- Unit tests for CancelReserved and GetReserve
- Do not delete row in reserves table if insert in old_reserves fails

Signed-off-by: Leila and Sonia <koha.aixmarseille@gmail.com>
Signed-off-by: Benjamin Rokseth <bensinober@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>

Signing off, while noting a style issue in the patch review

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Passes tests and QA script.
Placed and cancelled a hold using ILS-DI successfully.
Adding a follow-up to also update the ils-di documentation
page in the bootstrap theme.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

EDIT: I removed the changes it did to the prog theme.
2014-09-18 09:48:41 -03:00
3e78a908f0 Bug 10226 - suspended holds still show not available
If you suspend a hold, the item does not show Available.  It still shows
the person next in line, who isn't eligible for the hold yet because of
the suspension.  This is not the case for a delayed hold, where you
originally place the hold and tell it not to start until a future date.
If you do that, it shows as Available.  This is confusing and
inconsistent.

Test Plan:
1) Create an item level suspended hold for a record with no other holds
2) Note in the record details that the hold shows an item level hold
3) Apply this patch
4) Refresh the record details page, note the item is "Available"
5) Optional: prove t/db_dependent/Holds.t t/db_dependent/Reserves.t

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Works as described, passes all tests and QA script.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-09-14 02:02:51 -03:00
3c1f7dae0a Bug 8735 - Expire holds waiting only on days the library is open - Followup - Switch from C4::Calendar to Koha::Calendar
Test Plan:
 1) Set ExpireReservesMaxPickUpDelay
 2) Set ReservesMaxPickUpDelay to 1
 3) Place a hold, set it to waiting
 4) Using the MySQL console, modify the waiting date and set it to the
    day before yesterday.
 5) Set today as a holiday for the pickup branch in question.
 6) Run misc/cronjobs/holds/cancel_expired_holds.pl
 7) The hold should remain unchanged
 8) Remove today as a holiday
 9) Run misc/cronjobs/holds/cancel_expired_holds.pl again
10) The hold should now be canceled

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-09-05 11:51:01 -03:00
3b092a899f Bug 8735 - Expire holds waiting only on days the library is open
Signed-off-by: Leila <koha.aixmarseille@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-09-05 11:50:58 -03:00