Commit graph

167 commits

Author SHA1 Message Date
8d5b4306e0 Bug 17835: Replace GetItemTypes with Koha::ItemTypes
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Lari Taskula <lari.taskula@jns.fi>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-04-14 10:43:51 -04:00
71f34c7b34 Bug 17737: [QA Follow-up] Remove unused reservedfor variable
The changes in this patch set obsolete this variable.
Remove confusing comment about reserve via host record.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-04-13 08:49:12 -04:00
b01fe1a666 Bug 17737: Rename holds_placed_before_today with current_holds
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-04-13 08:49:12 -04:00
95c96f823d Bug 17737: Replace GetReservesFromItemnumber with Koha::Item->get_holds_placed_before_today
On the same way of Koha::Biblio->get_holds,
Koha::Biblio->get_holds_placed_before_today and Koha::Patron->get_holds,
this new subroutin will permit to retrieve the holds placed on a
specific item.
Note that at the moment we do not need a Koha::Item->get_holds method:
we do not want to display future holds placed in the future.

Test plan:
I would suggest to test this patch with patches from bug 17736 and bug 17738,
to place different kind of holds (biblio and item level, future and
past).
Then do a whole workflow to detect bug, view a record, delete record,
order, place a hold on an item which has been ordered, etc.
The hold's informations should always be the same without or without
these patches.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-04-13 08:49:11 -04:00
87afa5142b Bug 17736: Replace GetReservesFromBiblionumber with Koha::Biblio->holds
The C4::Reserve::GetReservesFromBiblionumber took 3 parameters, the
biblionumber, an optional itemnumber and a "all_dates" flag.
If set, the subroutine returned all the holds placed on a given bibliographic
record, even the ones placed in the future. Almost all of the calls had this
flag set, they will be replaced with a call to Koha::Biblio->holds.

But 5 did not have it:
- C4::Biblio::DelBiblio
-tools/batch_delete_records.pl
=> These 2 were wrong, we want to retrieve the holds to cancel them
before deleting the record. We need to get all the holds, even the ones
placed in the future /!\ CHANGE IN THE BEHAVIOR

- acqui/parcel.pl
=> 1 call per item were made to this subroutine. They have been replaced
with only 1 call to the new method Koha::Biblios->holds_placed_before_today
Then we filter on the itemnumbers.
I think this is wrong: we need the number of holds to know if the record
can be deleted, so even if future holds exist, the deletion should not
be possible.

- serials/routing-preview.pl
- C4::ILSDI::Services::GetRecords
- C4::SIP::ILS::Item->new
=> Seems ok, we just one to display holds placed before today

Test plan:
I would suggest to test this patch with patches from bug 17737 and bug 17738,
to place different kind of holds (biblio and item level, future and
past).
Then do a whole workflow to detect bug, view a record, delete record,
order, place a hold on an item which has been ordered, etc.
The hold's informations should always be the same without or without
these patches.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-03-31 12:02:14 +00:00
d420e6ae21 Bug 17844: Replace C4::Koha::get_notforloan_label_of with Koha::AuthorisedValues
This patch is more a bugfix than a refactoring.
Indeed the C4::Koha::get_notforloan_label_of behaviors were buggy:
1/ It does not display the opac description at the OPAC, but always the
staff description
2/ It does not care of the framework of the biblio, but retrieve the
first row of the marc_subfield_structure mapped with items.notforloan

These 2 bugs can easily be fixed using the
Koha::AuthorisedValues->search_by_koha_field

Steps to recreate the issues:
- Create 2 authorised value categories for not for loan (NFL1 and NFL2)
with the same values. Define a different description for the OPAC.
- Define link 952$7 to NFL1 for the default framework and to NFL2 for
the BK framework
- Create 2 bibliographic records (B1 using NFL1 and B2 using NFL2) with
2 items each (1 item should have a not for loan value)
- Go to the "Place a hold" view for this record.
- In the item list, you should see the not for loan value
=> The staff description of NFL1 will always be used, even for the OPAC

Test plan:
- Recreate the issues without this patchset
- Apply this patchset
- Recreate the steps to recreate the issues
=> The staff description of NFL2 should be displayed for the B2 item
=> The opac description of NFL2 should be displayed for the B2 item at
the OPAC

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-03-31 10:11:08 +00:00
9b92a494e6 Bug 18037: Hold notes template cleanup (from 15545)
From the second patch of bug 15545:
Removing some unused template code related to a former approach.
Adding some changes for future extension by bug 15545.

This patch was tested by Liz Rea when the routine IsHoldNoteRequired was
called by opac-reserve.pl. The only change here is that we do not yet
call this routine; so leaving her original signoff.

Test plan:
[1] Enable OPACHoldNotes.
[2] Place a hold on a serial record. No behavior change.

Signed-off-by: Liz Rea  <liz@catalyst.net.nz>
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: Kyle M Hall <kyle@bywatersolutions.com>
2017-03-03 18:11:00 +00:00
61c752a98b Bug 17453: Take into account items that are lost or damaged
If all the items are either lost, damaged or checked out, then pickup
should be allowed.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

https://bugs.koha-community.org/show_bug.cgi?id=14753

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-02-17 17:15:07 +00:00
0d55faa9a9 Bug 17453: Allow pickup at a library where all items are checked out
If all items are checked out then it should be possible to select the pickup
library for that record.

Signed-off-by: Janet McGowan <janet.mcgowan@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

https://bugs.koha-community.org/show_bug.cgi?id=14753

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-02-17 17:15:06 +00:00
d1d12fc770 Bug 17453: Add exceptions
This patch adds the ability to define patron categories not affected by
the behavior of OPACHoldsIfAvailableAtPickup.
The new pref OPACHoldsIfAvailableAtPickupExceptions get a list of patron
categories (separated by pipes |).

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

https://bugs.koha-community.org/show_bug.cgi?id=14753

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-02-17 17:15:06 +00:00
00c5929c1a Bug 17453: Inter-site holds improvement
At the moment users can reserve items and choose any library as a pick up
location, but there is no mechanism to prevent users from reserving items that
are available on the shelf at any given location from reserving the item at the
same location, essentially creating a Fetch and Collect scenario.
This has an impact on staff workloads as they are having to process reservations
and check shelves for items that students can already come and collect from the
open library shelves.
The aim of this enhancement is to decrease the impact on staff workload there
should be a restriction in place that prevents users from requesting items for
collection at a library where the item is currently available.

Implementation:
We first tried to add a new circulation rule adding a 4th
“NotIfAvailableAtPickupLibrary” option to "On shelf holds allowed".
That would make the development more flexible.
But in that case we quickly faced non-trivial problematics:
Let's say you have 3 items I1, I2 and I3. The first one has onshelfholds
set to Yes and 2 others has it set to “NotIfAvailableAtPickupLibrary”.
What would be the expected behavior if a hold is placed at biblio level?
And if a hold is placed at item level for I1?
This second point could be answered by reworking the interface to move
the libraries dropdown list elsewhere (1 list per item) or by adding a
lot of JS code to handle the different situation. But it would be
much more complicated to implement.
So finally I moved back to the simple approach and added a new pref to
handle the behavior globally.

Test plan:
0/ Switch off OPACHoldsIfAvailableAtPickup
1/ Let's say you have 3 libraries L1, L2, L3, create 2 items owned by L1
and L2
2/ Place a biblio level hold. You should only be able to pick it up at
L3
2/ Place a item level hold. You should only be able to pick it up at
L3
3/ Create a third items owned by L3
4/ Now you should not be able to place a hold on this record anymore

Sponsored-by: University of the Arts London

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

https://bugs.koha-community.org/show_bug.cgi?id=14753

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-02-17 17:15:05 +00:00
631d3006bc But 17578: (followup) amountoutstanding
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-12-16 13:12:45 +00:00
b59df2bce7 Bug 17578: GetMemberDetails - Remove GetMemberDetails
All the values different from the ones GetMember returned has been
managed outside of GetMemberDetails.
It looks safe to replace all the occurrences of GetMemberDetails with
GetMember.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-12-16 13:12:44 +00:00
43dda64381 Bug 17578: GetMemberDetails - Remove reservefee
Same as other patches, reservefee is only used in opac-reserve.pl

Test plan;
Set reserve fee for a patron category
Place a hold at the OPAC with one of these patrons.
You must get a message about the reserve fee.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-12-16 13:12:43 +00:00
5a0a2ce584 Bug 17578: GetMemberDetails - Remove is_expired
The is_expired value is used in 2 places, let's use
Koha::Patron->is_expired instead.

Test plan:
Depending on the different value of BlockExpiredPatronOpacActions for
the patron category, a patron must be blocked if he has expired.
Confirm that behavior from opac-renew and opac-reserve scripts

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-12-16 13:12:43 +00:00
4e78f1000d Bug 17578: GetMemberDetails - Remove amountoutstanding
The amountoutstanding value set by GetMemberDetails was only used in a
few places. In that case it makes sense to only retrieve it when needed.

Test plan:
1/ Add fines to a patron, on the OPAC patron info page, you should see a
"Fines" tab
2/ Add credit to a patron, you should see the credit displayed
3/ Set the pref maxoutstanding to 3
4/ Add a fine of 4 to a patron
5/ Try to place an hold for this patron
=> You should get a "too much oweing" message

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-12-16 13:12:41 +00:00
492280102f Bug 17578: GetMemberDetails - Remove BlockExpiredPatronOpacActions
The correct way to get the value of BlockExpiredPatronOpacActions from a
patron object is to get the patron category then call the
effective_BlockExpiredPatronOpacActions:
  $patron->category->effective_BlockExpiredPatronOpacActions

So this patch applies this change and remove this value from the
GetMemberDetails subroutine.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-12-16 13:12:40 +00:00
df97814f30 Bug 15758: Koha::Libraries - Remove GetBranches
Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-09-08 14:36:03 +00:00
9b9803b69c Bug 15758: Koha::Libraries - Remove GetBranchesLoop
Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-09-08 14:36:02 +00:00
bc39f0392b Bug 14695 - Add ability to place multiple item holds on a given record per patron
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jason M. Burds <JBurds@dubuque.lib.ia.us>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
2016-09-03 00:17:56 +00:00
d1eb706153 Bug 14642: Add logging for Holds
This patch adds logging for several holds actions. Specifically for:

- CREATE
- CANCEL
- DELETE
- RESUME
- SUSPEND
- MODIFY

To test:
- Enable the HoldsLog syspref
- Add a hold on a record/item
=> SUCCESS: The log view shows the CREATE action
- Click on the <Suspend> button
=> SUCCESS: The log view shows the SUSPEND action
- Click on the <Unsuspend> button
=> SUCCESS: The log view shows the RESUME action
- Click on the red cross, to delete the hold
=> SUCCESS: The log view shows the CANCEL action

Note: The DELETE action is logged when DelMember is called, with bug 16819 patches applied.

Sponsored-by: NEKLS
Signed-off-by: Liz Rea <liz@catalyst.net.nz>
I also wonder about this going in defaulted on, but since the other logs are as well it seems ok to me.

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-08-17 18:43:13 +00:00
e1e38896bb Bug 16849: Move IsDebarred to Koha::Patron->is_debarred
In order to move IsMemberBlocked to Koha::Patron it makes sense to move
the code from Koha::Patron::Debarments::IsDebarred to
Koha::Patron->is_debarred.

Test plan:
1/ Add a restriction to a patron
2/ make sure he is not able to checkout items any more
3/ Make sure he cannot get a discharge
4/ Put a hold and make sure you get "Patron has restrictions"

Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-07-15 18:08:14 +00:00
b87af43c47 Bug 15533 [QA Followup] - All itemtypes for all items showing in OPAC multi-hold
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-04-29 10:26:05 +00:00
fc81ee5004 Bug 15533 - Allow patrons and librarians to select itemtype when placing hold
Some libraries would like the ability to select the itemtype to request
when placing holds. For example, if a record has 3 copies of BookA and 3
copies of BookA in large print, this feature would allow a person to
place a hold on the record, but still be able to target only the Large
Print edition so that the first Large Print copy that becomes available
is targeted, rather than forcing the patron to select a particular copy
to hold.

Test Plan:
1) Apply this patch
2) Run updatedatabase.pl
3) Create a record with items of two or more itemtypes
4) Place a record level hold on the record while choosing one particular
   itemtype
5) Check in an item from the record that is not of that itemtype
6) Notee it is not trapped for the hold
7) Check in an item from the record that does match the selected itemtype
8) Note the item is trapped for the hold

Signed-off-by: Andreas Hedström Mace <andreas.hedstrom.mace@sub.su.se>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-04-29 10:26:03 +00:00
3691bd8419 Bug 15548: Move new patron related code to Patron*
The 'borrower' should not be used anymore, especially for new code.
This patch move files and rename variables newly pushed (i.e. in the Koha
namespace).

Test plan:
1/
  git grep Koha::Borrower
should not return code in use.

2/
Prove the different modified test files

3/ Do some clicks in the member^Wpatron module to be sure there is not
an obvious error.

Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>
Works as described. Tested with Circulation, Members/Patrons, Discharge,
Restrictions modules and the must common functionalities

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

Signed-off-by: Jesse Weaver <jweaver@bywatersolutions.com>
2016-03-03 14:38:26 -07:00
8627ec5f6a Bug 4941: Remove the singleBranchMode system preference
The singleBranchMode system preference does not make sense.
Either the install has only 1 library defined or several. In both case,
we can easily guess the behavior to follow.

So the idea of this patch is to replace the fetch of this syspref with a
call to count the number of libraries defined in DB.

Test plan:
1/ From a fresh Koha install, execute the DB entry to remove the pref.
2/ Define only 1 library
3/ Confirm that Koha behaves the same as before (try to change your
library, look at the facets)
4/ Create another library (or more) and reinsert the pref and set it:
  insert into systempreferences (variable, value)
    values('singleBranchMode', 1);
5/ Execute the DB entry
You should get a warning message.
6/ Repeat 3.

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>

Does what it says, but will change behaviour for any Koha install that
has 2 branches defined, One circulation, and this preference set.
If that is an acceptable change, we might need to make sure this is noted well in the
release notes.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-02-26 12:13:09 +00:00
4369486767 Bug 15375 [QA Followup] - Fix non-functional restriction message and date
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
2016-01-27 05:25:04 +00:00
Marc Véron
7a46b1599e Bug 14956: C4::Dates from files opac/*.pl
Remove C4::Dates from files:
-  opac/opac-memberentry.pl
-  opac/opac-reserve.pl
-  opac/opac-search-history.pl
-  opac/opac-showreviews.pl
-  opac/opac-suggestions.pl
-  opac/opac-serial-issues.pl
-  opac/opac-alert-subscribe.pl
-  opac/opac-ics.pl

To test:
- Apply patch
- Verify, that self registration and holds work as before
- Verify that tabs in catalog item detail work and display
  as before
- For serials: Verify that subscriptions work as before. It is a
  little bit hidden, in tab Subscriptions, then 'More details', then
  tab 'Brief history', button 'Subscribe to email notificatin on
  new issues'
- For ics: Can not be tested at the moment, not yet used (Bug 5456),
  pls. have a look at the code changes

(Amended following comment #2)

Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>
Works as advertised

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2015-11-06 15:01:28 -03:00
22068e3950 Bug 14100: Fix 3 occurrences more
- opac-suggestions.pl
- opac-readingrecord.pl
- opac-reserve.pl

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2015-10-27 12:34:07 -03:00
011f439740 Bug 5144: Don't display the failed_holds param in the url if not needed
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2015-10-06 10:02:06 -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
542ab0bce9 Bug 5371: Force no caching for private pages at the OPAC
In order no to slow too much the browsing, it is certainly not a good
idea to add this cache-control value for all pages at the OPAC.

This patch just adds where the author found it could be useful.

Test plan:
1/ Login at the OPAC
2/ Go on the account page (opac/opac-account.pl)
3/ Click log out
4/ Use the back button of your browser
Without this patch you will see the previous page.
With this patch, the previous page will be reloaded and you will be
redirected to the login form.

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>
2015-10-02 11:06:17 -03:00
b711984885 Bug 9809: [QA Follow-up] Remove an erroneous call to GetReserveFee
The call to GetReserveFee in opac-reserve.pl is useless in its current
form. The first parameter undef takes care of receiving 0.
But note that the user is warned correctly for the charge via param
variable RESERVE_CHARGE on the opac form.

When the hold is placed, AddReserve calls GetReserveFee. So if the routine
would work correctly, we would not need this extra call in opac-reserve
in the whole place. Unfortunately, the routine is not working correctly.

I will submit a fix for GetReserveFee under a new report (14702).

Test plan:
[1] Add a hold fee to some category.
[2] Check the warn for placing a hold on such a book in OPAC.
[3] Observe that the actual fee is not charged. This is a current bug and
    it will be addressed on report 14702.

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:54 -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
Jonathan Druart
baea0a79d5 Bug 7976: Remove the borrow permission
The borrow permission was used but uselessly.
For instance, at the opac, the flagsrequired parameter was set to
'borrow' but the 'authnotrequired' was set also (which means no auth
required).
At the end, this permission was used at only 1 place: for the basket,
intranet side.
This can be replaced with the catalogue permission (which is used to
search).

Test plan:
1/ Confirm that you are able to show/download/sent the cart (intranet side)
with the catalogue permission.
2/ At the OPAC, you should be able to access the same pages as before
with any other permissions.

Concretely it is quite difficult to test this patch, you should have a
look at the code.

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2015-06-05 13:43:34 -03:00
Indranil Das Gupta
642e6012cd Bug 14186 [QA Followup]: Undefined $reservedfor causes warn in opac-reserve.pl
This is a followup for Bug 14186 that removes the extraneous tab
char on line 470, so that the patch can clear QA tools.

This patch sets $reservedfor to an empty string.

Test plan
=========

1/ in a terminal, run `tail -f ` on your instance's opac-error.log
2/ go to the opac and search from an item that exists on the Koha
   instance.
3/ Select the title (if more than one title is returned) and click on
   'Place hold' link to go to opac-reserve.pl
4/ notice the warning - "opac-reserve.pl: Use of uninitialized value
   $reservedfor" appear in the `tail`ed opac-error.log
5/ apply the patch
6/ reload the page (opac-reserve.pl)
7/ page works but the warning in step #4 is no longer thrown up
8/ run qa test (i.e. koha-qa.pl -c 1 -v 2), there should be no error

Remarks: Testing result match expected test plan output. The QA tests
         pass with "OK" for the commit.

Signed-off-by: Indranil Das Gupta (L2C2 Technologies) <indradg@gmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2015-05-26 10:42:05 -03:00
Aleisha
8fdd352bc9 Bug 14186: Undefined $reservedfor causes warn in opac-reserve.pl
This patch sets $reservedfor to an empty string.

Test plan
=========

1/ in a terminal, run `tail -f ` on your instance's opac-error.log
2/ go to the opac and search from an item that exists on the Koha
   instance.
3/ Select the title (if more than one title is returned) and click on
   'Place hold' link to go to opac-reserve.pl
4/ notice the warning - "opac-reserve.pl: Use of uninitialized value
   $reservedfor" appear in the `tail`ed opac-error.log
5/ apply the patch
6/ reload the page (opac-reserve.pl)
7/ page works but the warning in step #4 is no longer thrown up
8/ run qa test (i.e. koha-qa.pl -c 1 -v 2), there should be no error

Remarks: The QA test failed - "forbidden pattern: tab char (line 470)".

         Marking this as 'FAILED QA'

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2015-05-26 10:42:05 -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
Marc Véron
20dc9ce5a1 Bug 13917: OPAC: Putting holds still possible by URL even if turned off by system preference
To test:

1)
Prepare an URL that put's a hold in OPAC (see comment #1), e.g.
[YOUR SERVER]/cgi-bin/koha/opac-reserve.pl?biblionumbers=1

2)
Set system preference 'RequestOnOpac' to 'Don't allow patrons to place holds on items from the OPAC.'

3)
Go diectly to this URL.
Result: The page displays to confirm hold (wrong)

4)
Apply patch

5)
Repeat step 3
Result: Redirect to 404 page (OK)

6)
Set system preference to "Allow"

7)
Repeat step 3
Result: The page displays to confirm hold (OK), like it would be from link in OPAC.

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>
2015-04-20 10:18:26 -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
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
295fb70f62 Bug 13335 - Holds and priority display via OPACShowHoldQueueDetails confusing
No matter what the selection of OPACShowHoldQueueDetails is, if it is
enabled it displays a line "Holds and priority:" even if you've opted to
hide one of those!

Test Plan:
1) Apply this patch
2) Test each setting of OPACShowHoldQueueDetails
3) Ensure each setting displays the correct fields ( or lack thereof )

Signed-off-by: Christopher Brannon <cbrannon@debian.localdomain>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2015-02-12 15:20:59 -03:00
Jonathan Druart
e20270fec4 Bug 11944: use CGI( -utf8 ) everywhere
Signed-off-by: Paola Rossi <paola.rossi@cineca.it>
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Signed-off-by: Dobrica Pavlinusic <dpavlin@rot13.org>

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2015-01-13 13:07:21 -03:00
a317f7d62f Bug 13108 - Members are restricted even if the debarment is ended - opac-reserve
This is the same issue as bug 12134.

Test Plan:
1) Add a manual restriction to a patron with expiration date in the past
2) Go on the OPAC and connect
3) Try to add an hold on a record (opac-reserve.pl)
3) Note the warning message
    "Sorry, you cannot place holds because your account has been frozen ..."
4) Apply this patch
5) Repeat step 2
6) Note the warning message does not appear anymore

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Makes code cleaner, also works as described.

NOTE: If you set a debarment with date in the past in the GUI,
there will be no entry in borrowers.debarred and you won't be
able to see the problem. Set one with a date in the future and
then alter the date in borrower_debarments and borrowers.debarred.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-19 11:45:01 -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
13c93259bd Bug 10433 - colspan calculation done by opac/opac-reserve.pl should be moved to template
Since the bootstrap OPAC's place hold interface doesn't use tables
there is no use for the colspan calculation being done in the script.
This patch removes it.

To test, place one or more holds in the OPAC and confirm that there are
no display problems. The OPAC error logs should show no associated
error.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
No display problems, no related errors on log, no koha-qa errors

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Passes tests and QA script, no regressions found.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-10-29 22:11:44 -03:00
bbfc908fd8 Bug 4045 - No check for maximum number of allowed holds.
Re-add the system preference maxreserves.

All the code using maxreserves is still in place. Though it
is not used in the Reserves module, it is used in all the
scripts where holds are placed.

Also adds a check so that a borrower cannot exceed the maximum
number of allowed holds by using the multi-hold feature via
the opac.

Test Plan:
1) Apply this patch
2) Run updatedatabase
3) Set maxreserves to 3, set opactheme to bootstrap
4) Log into the opac as a patron
5) Place 3 holds
6) Attempt to place a 4th hold
7) Note you get an error message and cannot place a forth hold
8) Delete two of those holds
9) Attempt to place 3 or more holds as a multi-hold
10) You should see a warning that you cannot place this many holds
11) Try to anyway
12) You should see an alert to tell you to reduce the number of holds
    you are placing.
13) Reduce the number for holds you are placing to 2
14) Your holds should now be placed

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-08-07 11:47:08 -03:00
Olli-Antti Kivilahti
2db8ba555d Bug 12668 - Stray dollar ($) -sign in opac-reserve.pl
A dollar sign is hard-coded in opac-reserve.pl and becomes apparent when trying to place a reservation
 when one has "too_much_oweing" or too much fines.

Removing the dollar sign so we just get

<"Käyttömaksujen katto ylitetty. Et voi tehdä varauksia. Sinulla on maksamattomia maksuja 9.50.">

instead of

<"Käyttömaksujen katto ylitetty. Et voi tehdä varauksia. Sinulla on maksamattomia maksuja $9.50.">

Patch removes hard coded $ sign.
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
The dollar sign is gone and the message still displays correctly.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-08-03 16:49:47 -03:00
afd2418d73 Bug 11349: Change .tmpl -> .tt in scripts using templates
Since we switched to Template Toolkit we don't need to stick with the
sufix we used for HTML::Template::Pro.

This patch changes the occurences of '.tmpl' in favour of '.tt'.

To test:
- Apply the patch
- Install koha, and verify that every page can be accesed

Regards
To+

P.S. a followup will remove the glue code.

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-07-17 11:05:49 -03:00