Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
This patch tackles a very specific scenario. Calling split(...,
CGI::param) makes it be called in list context. The split docs say:
split /PATTERN/,EXPR,LIMIT
this means the first CGI param will be used as EXPR and the second one
as LIMIT, which is wrong anyway. So the fix is to just force scalar
context.
To test:
1. Not sure, just make sure nothing breaks when using the scripts in the
browser.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
It excepts to receive a real array and not a list, in order to be able
to generate the query parameters.
Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
This removes the need to handle single and multiple cases separately,
thus removing bunch if-else cases and simplifying our code. This
coding style is also in line with our other .pl scripts.
To test:
1) Make sure placing a hold still works from the following pages:
/cgi-bin/koha/catalogue/detail.pl?biblionumber=XXX
/cgi-bin/koha/catalogue/search.pl?q=a
/cgi-bin/koha/virtualshelves/shelves.pl?op=view&shelfnumber=XXXX
/cgi-bin/koha/clubs/clubs.pl (create a new club and add a patron
there and through the clubs.pl create a hold to a bib)
Signed-off-by: Hayley Pelham <hayleypelham@catalyst.net.nz>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
It seems that hiddencount didn't work prior to this patch (to confirm)
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
If you cancel holds in bulk, the list is not updated as we enqueued the
task. But the "Update hold(s)" button will explode if clicked.
Test plan:
Place several holds on a bib record
Use the "Cancel selected" link to cancel holds in bulk
The job is enqueued and the hold list still show the holds you cancelled
Click "Update holds"
=> Without this patch you get an ugly 500
Can't call method "found" on an undefined value at /kohadevbox/koha/C4/Reserves.pm line 1060
=> With this patch applied the table is refresh, no crash (and there is
a warning in the log, that may not be necessary)
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
and some more...
There are lot of inconsistencies in our ->search calls. We could
simplify some of them, but not in this patch. Here we want to prevent
regressions as much as possible and so don't add unecessary changes.
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Test plan:
Try modrequest with a user having only 'catalogue' perms and the following URLs:
[1] /cgi-bin/koha/reserve/modrequest.pl?reserve_id=XX&CancelBorrowerNumber=XX&CancelItemnumber=XX&biblionumber=XX
Fill the XXs with correct identifiers for some item level hold.
[2] /cgi-bin/koha/reserve/modrequest_suspendall.pl?suspend=1&suspend_until=2021-12-01&borrowernumber=XX
Fill the XX with borrowernumber for borrower that has pending holds.
You should see: Error: You do not have permission to access this page.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
[AMENDED] More consensus for using reserveforothers than circulate_remaining.
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Most of the changes here are simple, this can be read to view the changes
Testing that holds can be placed via staff client, and opac, and are disallowed
when expected is the best test plan, beyond running the unit tests
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
This patch adds a new cookie 'awalys_show_holds' and utlises it on request.pl
to determine if the existing holds table should be loaded.
If the cookie does not exist, the table is loaded to preserve exiting behaviour.
If the cookie exists and is set to 'DONT' a new button 'Show holds' is added to
the page to allow force loading the table
To test:
1 - Place some holds on a record
2 - Click 'Holds' tab or 'Place a hold' from details page
3 - Note search box to place hold and table of existing holds
4 - Apply patch
5 - Reload the holds page and note new checkbox 'Always show holds'
6 - Confirm the box is checked
7 - Uncheck it
8 - Reload the page and confirm holds do not automatically load
9 - Click the new 'Show holds' button
10 - Confirm the existing holds load, and that 'Always show holds' is unchecked
11 - Check 'Always show holds'
12 - Reload page and confirm holds don't show
13 - Uncheck the box, load holds for another record, confirm it remains unchecked
14 - Bring up browser console (F12)
15 - In 'Storage' tab, under 'Cookies', delete the cookie for 'always_show_holds'
16 - Reload holds page, confirm holds load and check box is checked
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Bug 29058: (follow-up) Add hold count and don't display show options if no holds
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
If we haven't picked a patron or a club yet we don't need to fetch
biblios, items and holdability infos
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Without this patch, the list will always display all clubs.
To test:
1. Have two clubs, with enrollemnts:
- Cthulhu fans
- The Shadow Out of Time fans
2. Search for the letter c
=> FAIL: You get both results
3. Apply this patch
4. Repeat 2
=> SUCCESS: Only Cthulhu is returned
5. Sign off :-D
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
There is an error when placing a hold for a club without members:
Uncaught TypeError: err.responseJSON.error is undefined
It seems that we should remove clubs without members from the search.
Test plan:
Create 1 club xx with 2 patrons
Create 1 club xxx with 1 patron and cancel their enrolment
Create 1 club xxxx without patron
Place a hold for club "x", only the first one should be returned with
this patch.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
The original code for pickup locations when placing item-level holds
picked the currently logged-in library.
We made things more robust, as the logged-in library might not be a
valid pickup location for the patron and item. But it was wrongly chosen
to use the holding branch as the default.
A more robust approach is needed, and this precedence is picked this
time (it could be configuration-driven in the future):
- Logged-in library
- Empty
To test:
1. Pick a biblio with various valid pickup locations, some not including
the logged-in library.
2. Pick a patron for placing the hold
=> FAIL: Notice that (when valid pickup location) the holding branch is
always chosen
3. Apply this patch
4. Repeat 2
=> SUCCESS: If valid pickup location, the logged-in branch is picked as
default for item-type level. When it is not, an empty dropdown is used
as a fallback.
5. Sign off :-D
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
The code didn't previously correctly do the checks for patron
debarment status and whether the patron has fees, this caused the
intended warnings messages to not appear when they should have.
To test:
1) Create a new club and add a patron to it who has manual debarment
and a manual charge of over $5 (more than the maxoutstanding syspref).
2) Try to create a new club hold for the previously created club and
notice no warnings are listed for the patron.
3) Apply patch
4) Try to create now the club hold again and notice the warnings
being shown in the hold page.
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
We can use the Koha::Patron::is_expired accessor instead of re-writing
it in request.pl.
To test:
1) Make that a warning is being shown with and without this patch
when placing a hold to an expired patron account
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
The logic in request.pl only passes pickup locations if a hold is allowed,
they are not calculated when the hold is overridden.
This patch copies the pickup locations code into the override conditional,
adjusting to mark the item overridden
This also highlights an condition of override:
If a hold is allowed, but there are no valid pickup locations the override
is not allowed.
To test:
1 - Set a holds rule to allow on-shelf holds only if all unavailable
2 - Set AllowHoldPolicyOverride to 'Allow'
3 - Find a record with several items available
4 - Attempt to place a hold
5 - The items have a yellow triangle, but no pickup locations
6 - Place an item level hold
7 - Note the hold has no pickup location
8 - Cancel the hold
9 - Apply patch and restart all
10 - Attempt hold again
11 - Items still have yellow triangle, but there are dropdowns for pickup location
12 - Place an item level hold, verify a pickup location is set
13 - Cancel the hold
14 - Alter circ rule to allow on shelf holds
15 - Change 'Default checkout, hold and return policy' - 'Hold pickup library match' to "item's hold group"
16 - Ensure there is no group for the item
17 - Attempt to place hold
18 - Override is not allowed 'No valid pickup locations'
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch allows staff patrons to cancel multiple holds in bulk.
To test:
1. Apply this patch
2. restart_all
3. In cataloge go to a book and place many holds
CHECK => Holds table shows a column of checkboxes
4. Play with checkboxes (have some fun ;-P)
CHECK => When you manually check all checkboxes, the checkbox in the
header also gets checked.
=> When you uncheck one of the checkboxes, the one in the header also gets unchecked.
=> If no checkbox is checked and you check the one in the header,
all checkboxes get checked.
=> If there are some checkboxes that are checked and others are
not, when you click on the checkbox in the header all checkboxes get
unchecked.
=> If all checkboxes are checked, when you uncheck the one in the
header, all checkboxes get unchecked.
=> Every time you play with checkboxes, the number in the button
"Cancel selected" changes.
5. Check some of the checkboxes and click on cancel selected.
SUCCESS => A background job gets fired to cancel all selected holds.
=> A message should appear with a link to the job.
6. Wait a few seconds and click on the link
SUCCESS => A message appears with the report of the execution of the
background job.
7. Grab a patron and search to hold
8. Select multiple biblios and click on "place hold for <patron>"
CHECK => After holds are confirmed, multiple holds table are shown.. one for
each record. Checkboxes work exactly the same as before, but scoped
for each individual table. Checkboxes from one table will not affect
checkboxes from other tables.
9. Repeat steps 4 to 6.
10. Check In some of the items so the get in Waiting state.
11. Update expirationdate os some of those holds and set it to
ReservesMaxPickUpDelay + 1 days earlier
NOTE => ReservesMaxPickUpDelay = 7 days by default, so sql syntax to update would be
=> update reserves set expirationdate = date_sub(expirationdate, interval 8 day) where reserve_id in (...)
12. Repeat steps 4 to 6 but in waitingreserves.pl, in both tabs.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Bug 23678: (QA follow-up) Add missing template filter
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Bug 23678: (QA follow-up) Add missing filters
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Bug 23678: (QA follow-up) Use correct indentation
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
JD amended patch: also Koha/BackgroundJob/BatchCancelHold.pm
JD Amended patch: Full rebase and adjustements made on top of bug 26080.
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch makes a few changes:
1 - If we are not splitting the queue ( HoldsSplitQueue == 'nothing' ) then
there are no changes for 'virtual' vs 'actual' numbering
2 - If we are splitting the queue you cannot use the dropdown, so we do not need to process the options
3 - If the hold is 'found' we do not need to process the options
4 - We can simply use the 'last priority' to build the options, we do not need to process in the script
5 - We can use a block to build the options
6 - Remove a stray holds.index
To test:
1 - Place 5 holds on a bib
2 - 'Find' two of the holds i.e. check them in to set status to 'transit' or 'waiting'
3 - View the holds page for the biblio
4 - Inspect the priorty dropdown for the found holds, note they have options for 1-5 but are disabled
5 - Note the dropdowns for other holds have options 1-5
6 - Note options 4&5 in the dropdowns have no effect
7 - Apply patch
8 - Inspect the priority dopdowns on the found holds
9 - Note they only show their found status
10 - Note the other dropdowns only show options 1-3
11 - Test with other HoldsSplitQueue options and HoldsSplitQueueNumbering
12 - When holds queue is split, no dropdowns should be active and should show only their current priority
13 - When HoldsSplitQueueNumbering is 'virtual' the dropdown should be disabled and correctly count the number of unfound holds in the list
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
JK: Fixed grammar in commit title
Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>
JD: Remove trailing 'i'
Bug 29049: (QA follow-up) Remove excessive whitespace
Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Revised test plan from Owen:
This patch modifies the hold process so that if one of the titles in a
multi-hold process has no items the process doesn't abort completely.
To test, apply the patch and perform a search in the catalog which will
return one or more records with no items attached.
- Check checkboxes for multiple results, some of which have items and
at least one of which has no items.
- Click "Place hold."
- You should be taken to the page for placing multiple holds, with a
heading, "Cannot place hold on some items."
- Note: You will not be able to complete the holds process without the
next patch.
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Just checking with a regex that whether an input looks like a biblionumber
is not enough, we need to also verify there is a biblio really existing
in the database and skip processing of hold request for non-existent
biblionumbers.
To test:
1) Go to page /cgi-bin/koha/reserve/request.pl?biblionumbers=XXXXX
where XXXXX is non-existent biblionumber, notice internal server error
2) Apply patch
3) Repeat step 1 and notice we cannot place a hold
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
When we fetch the biblioitems we use a select to limit the columns fetched,
we must include the biblionumber as well
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
The loop here gets items from the record, plus analytic items. Because of this
we need to check more than 1 record - we decide to do this via biblioitems.
We need to preserve that, but when checking ItemsAnyAvailableAndNotRestricted we
cannot assume that the biblionumber and biblioitemnumber are the same (they should be
but this may not be the best of all possible worlds)
I simply switch the call here
To test:
1 - Apply patch
2 - Test placing holds on single bibs and multiple bibs
3 - Confirm it works as expected
Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Currently we send $biblionumbers as the parameter, but this is just apassthrough form when a list of biblios is selected for placing a hold
If passed a single biblionumber we push it into @biblionumbers and use that for building the biblio loop
This patch uses @biblionumbers to avoid sending a blank variable in the URL
To test:
1 - On the staff client click 'place hold' for an individual record
2 - Use the form to find a patron
3 - Note the url is:
http://localhost:8081/cgi-bin/koha/reserve/request.pl?biblionumbers=
4 - Apply patch
5 - Repeat
6 - The url is now like:
http://localhost:8081/cgi-bin/koha/reserve/request.pl?biblionumbers=248
(but with whatever biblionumber you chose)
7 - Perform a search and select multiple biblios and confirm you can place holds as before
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
JD Amended patch: Add missing space
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
On bug 17591 we discovered that there was something weird going on with
the way we export and use subroutines/modules.
This patch tries to standardize our EXPORT to use EXPORT_OK only.
That way we will need to explicitely define the subroutine we want to
use from a module.
This patch is a squashed version of:
Bug 17600: After export.pl
Bug 17600: After perlimport
Bug 17600: Manual changes
Bug 17600: Other manual changes after second perlimports run
Bug 17600: Fix tests
And a lot of other manual changes.
export.pl is a dirty script that can be found on bug 17600.
"perlimport" is:
git clone https://github.com/oalders/App-perlimports.git
cd App-perlimports/
cpanm --installdeps .
export PERL5LIB="$PERL5LIB:/kohadevbox/koha/App-perlimports/lib"
find . \( -name "*.pl" -o -name "*.pm" \) -exec perl App-perlimports/script/perlimports --inplace-edit --no-preserve-unused --filename {} \;
The ideas of this patch are to:
* use EXPORT_OK instead of EXPORT
* perltidy the EXPORT_OK list
* remove '&' before the subroutine names
* remove some uneeded use statements
* explicitely import the subroutines we need within the controllers or
modules
Note that the private subroutines (starting with _) should not be
exported (and not used from outside of the module except from tests).
EXPORT vs EXPORT_OK (from
https://www.thegeekstuff.com/2010/06/perl-exporter-examples/)
"""
Export allows to export the functions and variables of modules to user’s namespace using the standard import method. This way, we don’t need to create the objects for the modules to access it’s members.
@EXPORT and @EXPORT_OK are the two main variables used during export operation.
@EXPORT contains list of symbols (subroutines and variables) of the module to be exported into the caller namespace.
@EXPORT_OK does export of symbols on demand basis.
"""
If this patch caused a conflict with a patch you wrote prior to its
push:
* Make sure you are not reintroducing a "use" statement that has been
removed
* "$subroutine" is not exported by the C4::$MODULE module
means that you need to add the subroutine to the @EXPORT_OK list
* Bareword "$subroutine" not allowed while "strict subs"
means that you didn't imported the subroutine from the module:
- use $MODULE qw( $subroutine list );
You can also use the fully qualified namespace: C4::$MODULE::$subroutine
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
There is a "debug" parameter we are passing from the controller scripts
to C4::Auth::get_template_and_user, but it's not actually used!
Test plan:
Confirm the assumption
Review the changes from this patch
Generated with:
perl -p -i -e 's#\s*debug\s*=\>\s*(0|1),?\s*##gms' **/*.pl
git checkout misc/devel/update_dbix_class_files.pl # Wrong catch
+ Manual fix in acqui/neworderempty.pl
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch makes request.pl pass the holding library object to the
template, if it is a valid pickup location for the item. This way, the
template can set a good default to save clicks.
To test:
1. Have "Hold pickup library match" set to "Item's home library"
2. Have a record with items in three different branches. For example:
- item1: homebranch: MPL, holdingbranch: MPL
- item2: homebranch: FPL, holdingbranch: FPL
- item3: homebranch: CPL, holdingbranch: IPT
3. Have FPL marked as 'No' for pickup location
4. On the record, open the page for placing a hold for a patron
(acevedo?)
=> SUCCESS: You are presented the regular hold placing page, with an
extra column on the items for pickup location setting
=> SUCCESS: The item2 (on FPL) cannot be selected, there's a clear message
about not having valid pickup locations
=> FAIL: The other ones don't have anything pre-selected on the
dropdowns
5. Apply this patch
6. Repeat 4 (go back to the record, etc)
=> SUCCESS: Nothing changed BUT the item with holding branch = MPL has
it set by default in the dropdown.
=> SUCCESS: IPT is not a valid pickup location for item3, so not set by
default in this case.
7. Sign off :-D
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Besides the commit subject, this patch does much more:
- It makes request.pl stop passing a pickup location to
CanItemBeReserved
- It makes the page use the API to render a dropdown for each item, with
their valid pickup locations
- Items with no valid pickup locations have a nice message about why
they are disabled for selection
To test:
1. Apply this patch
2. Choose a biblio for placing a hold
3. Choose a patron
=> SUCCESS: You are presented with a new layout, that includes a
dropdown for choosing each item's pickup location. If an item is not
holdable, it still isn't.
4. Try having an item whose home branch is not marked as a pickup
location
=> SUCCESS: Notice you cannot choose that item
5. CHoose an item, but do not choose a branch, and click 'Place hold'
=> SUCCESS: It shows an alert about the need to choose a pickup location
6. Choose one of the (only possible) pickup locations for the specific
item
7. Place the item level hold
=> SUCCESS: All goes as expected!
8. Sign off :D
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch makes the multi-hold page offer only valid pickup locations
for the selected biblios. Prior to this, all system-wide pickup
locations were offered.
To test:
1. Set 'Hold pickup library match' to 'Item's home branch' so we put a
constraint on the valid pickup locations for easier testing.
2. Choose two or more biblios from a search, which contain in total 2
or 3 item home branches.
3. Click 'Place hold'
4. Choose a patron
=> FAIL: The dropdown offers all system's pickup locations
5. Apply this patches
6. Reload the page
=> SUCCESS: Only valid pickup locations are offered
7. Sign off :-D
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
We don't need to count the number of clubs if we selected a patron or
club already
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
1. Have no existing clubs
2. Apply patch
3. Go to request.tt and you will not see the club tab or any mention of clubs
4. Create at least 1 club
5. Go back to request.tt and now see the tab for clubs
6. Make sure you can place holds as an individual with and without clubs.
7. Make sure you can place holds for clubs.
8. rejoice and sign-off
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch alters C4/Reserves.pm to pass back 'noReservesAllowed' when
allowedreserves=0. This allows passing to the user an appropriate
message about the availability of items for holds
This patch also fixes a FIXME about using effective_itemtype to fetch item rules
To test:
1 - Set one itemtype to allow no holds
2 - Set 'Holds per record' to 0 for another itemtype/patron combination
3 - Create or find 2 records, each with items only of the itemtypes above
3 - Attempt to place a hold for a patron on each record above
4 - The message will be 'Too many holds'
5 - Apply patch and repeat
6 - Message should be "Cannot place hold: no item are available to be placed on hold"
7 - Try placing a multihold with either record above and a holdable record,
message should end "Cannot place hold on some items'
8 - prove -v t/db_dependent/Holds.t
Rebase - Fix test expectation
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
During bug 27071 development, this line got inadvertedly changed. This
patch restores the original behaviour.
To test:
1. Search for a title
2. Go try place a hold
=> FAIL: Look at the HTML, there's no pickup location with
selected="selected"
3. Switch branch and repeat 2
=> FAIL: Still the same
4. Apply this patch
5. Repeat 2 and 3
=> SUCCESS: Branch is selected, chosen current branch is picked
6. Sign off :-D
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Doing
$ git grep pickup_locations_code
shows there's some calculated data that is not actually used anywhere.
We can get rid of it.
This patch also reuses $item_object (which is in the same loop scope) to
avoid an extra DB call.
To test:
1. Run:
$ git grep pickup_locations_code
=> FAIL: It is only used/set as a comma separated string, inside
request.pl
2. Apply this patch
3. Repeat 1
=> SUCCESS: The unused stuff is not there anymore
4. Open the page for placing some holds
=> SUCCESS: It doesn't explode
5. Sign off :-D
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch makes the 'Any library' string translatable, by converting it
into a flag and using it accordingly on the request.tt template.
To test:
1. Have the 'Hold pickup library match' set to 'any library' on the
circultation rules.
2. Open the page to place a hold on a biblio with some items
=> SUCCESS: The item says 'Any library' on the 'Allowed pickup
locations' column.
3. Apply this patch
4. Repeat 2
=> SUCCESS: No behavior change
=> SUCCESS: The string is on the template
5. Sign off :-D
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
We should remove all SQL queries that contain 0000-00-00 and finally
assume we do not longer have such value in our DB (for date type)
We already dealt with such values in previous update DB entries.
The 2 added by this one haven't been replaced already.
The code will now assume that either a valid date exist, or NULL/undef.
Test plan:
QA review is needed and test of the different places where code is
modified.
Not sure about the change from reports/issues_avg_stats.pl
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Pretty much like the opac-reserve.pl does, this patch makes the staff
request.pl script get the branch from the specialized routine from
C4::Reserves instead of falling back to userenv.
To test:
1. Follow the original test plan:
- Create two local hold groups containing distinct lists of libraries.
- In default rules for all libraries, set Hold Policy = "From local hold group" and Hold pickup library match to "Patron's hold group"
- Make sure AllowHoldPolicyOverride is set to Don't Allow
- Make sure you're logged in at a library in Group 1
- Find a bib with only 1 item from Group 1. Confirm you can place a hold on this title for a patron in Group 1 (correct), but not for a patron in Group 2 (correct).
- Find a bib with only 1 item from Group 2. Confirm you cannot place a hold for a patron from Group 1 (correct), BUT you also cannot place a hold for a patron from Group 2 (incorrect) -- Koha gives the erroneous message "pickupNotInHoldGroup"
- Change your library to a something in Group 2
- Find a bib with only 1 item from Group 1. Confirm you cannot place a hold on this title for a patron in Group 1 (incorrect), and not for a patron in Group 2 (correct).
- Find a bib with only 1 item from Group 2. Confirm you cannot place a hold for a patron from Group 1 (correct), but you can place a hold for a patron from Group 2 (correct)
=> FAIL: Things expected to fail on the plan, fail.
2. Apply this patches
3. Repeat 1
=> SUCCESS: It now works!
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
