For single bibs we use an ajax select2 dropdown, and we don't use this
value
To test:
1 - Place some holds on single bibs
2 - Confirm dropdowns for pickup location work as expected
3 - Do the same for multiple bibs selected from search results
4 - Apply patch
5 - Confirm there is no change after the patch
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
(cherry picked from commit 63772a9cb2)
Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
(cherry picked from commit dd7b6e528a)
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit d98074a747)
Signed-off-by: Arthur Suzuki <arthur.suzuki@biblibre.com>
When placing an item-level hold in the Staff interface,
the table at the bottom of the screen (under the "Place
a hold on a specific item" heading) should display the
icon associated with the item type of each item in the
'Item type' column.
This feature broke in v22.05.00 (for Debian package installs):
instead of the item type icon you get the default 'broken
image URL' image of your web browser.
This patch fixes that.
Test plan:
1) (if needed) Configure the following System preferences:
- item-level_itypes => 'specific item'
- noItemTypeImages => 'Show'
2) (if needed) In Koha Administration > Item types, create
an item type and associate it with an icon
3) (if needed) Create a test bibliographic record, and add
an item to it that has an item type with an icon
4) Visit the bibliographic record's details page in the
Staff interface. Select 'Holds' on the left hand side,
then pick a patron. The "Place a hold on ..." page should
load. In the 'Item type' column of the table at the bottom
of the screen notice that instead of seeing the icon
associated with the item type of each item you get the
default 'broken image URL' image of your web browser [*]
[*] In non-Debian package installs, the 'href' property
of the item type icon's <img> tag should start with
'/opac-tmpl/bootstrap/itemtypeimg'
5) Apply this patch, restart Plack and refresh the page.
The item type icon should now be displayed correctly.
KTD and Koha Sandboxes users: To verify the patch has
worked, you should right-click on the item type icon and
click 'Inspect': the 'href' property of the <img> tag
should now start with: '/intranet-tmpl/prog/img/itemtypeimg'
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
(cherry picked from commit ed84cfd43b)
Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
(cherry picked from commit c206321d2f)
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 2849807f08)
Signed-off-by: Arthur Suzuki <arthur.suzuki@biblibre.com>
If some holds are overridable, and AllowHoldPolicyOverride is enabled, Koha
used to "flag" each holdable item to let librarians know the hold could not
normally be placed. In 22.05 up to current master as of this writing, no
warning is displayed.
Test Plan:
1) Set one circ rule to allow 0 holds
2) For Library A, set "Hold pickup library match" to "item's hold group"
3) Enable AllowHoldPolicyOverride
4) Set ReservesControlBranch to "item's home branch"
5) Create a record with an item for Library A, one for Library B, and one for Library C
This should create a scenario where A's item cannot be held, and B and C's items must be overridden
6) Note you get the "Cannot place hold" but can place those holds anyway without a "warning" sign
7) Apply this patch
8) Restart all the things!
9) Reload the page, you should now see the warning icons!
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
(cherry picked from commit 30eb30b3c1)
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 124d54c957)
Signed-off-by: Arthur Suzuki <arthur.suzuki@biblibre.com>
Bug 29540 increased the flags required to for modreserve.pl, but in doing so created a situation where a library can select a different pickup location for a hold, but upon clicking the "Update hold(s)" button, will be logged out due to having insufficient permissions for modreserve.pl.
Test Plan:
1) Create a librarian with "place_holds" permissions, but not "modify_holds_priority".
2) Place a hold for a patron
3) Attempt to change the pickup location from request.pl
4) Note you get logged out with a permissions error
5) Apply this patch
6) Restart all the things!
7) Attempt to change the pickup location from request.pl
8) This time it should succeed!
Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit 3a13667ad3)
Signed-off-by: Arthur Suzuki <arthur.suzuki@biblibre.com>
To test:
1) Grep for placerequest.pl in the source code and check that
request.tt is the only place where the type parameter is passed.
Signed-off-by: David Nind <david@davidnind.com>
JD amended patch: adjust commit title
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
(cherry picked from commit 54a6c73c6d)
(cherry picked from commit 88f257ba55)
Signed-off-by: Arthur Suzuki <arthur.suzuki@biblibre.com>
Rebeased/rewritten patch - includes fix from 30892
This patch does a few things:
1 - Adds itemtype not for loan status to display
2 - Adds a conditional to display notforlaon status as the reason a hold cannot be placed
3 - Seperates the lower 'Place hold(s)' buttons for single and multi holds into two template sections
4 - Handles the case where all bibs in a multi hold have no items available
5 - Disables the button for single hlds when all items are unavailable
To test:
1 - Find or create a record with all items of itemtype marked 'notforloan'
2 - Attempt to place single hold on this record from staff client
3 - See one disab;ed button, one enabled 'Place holds' button
4 - Click 'Place holds' - hold placed
5 - Cancel hold
6 - Place multiple holds with some bibs that can be held, and this one that cannot
7 - Notice message that 'Cannot place hold on some items'
8 - Click 'Place holds' - hold is generated for the notforloan bib
9 - Apply patch
10 - Place single hold
11 - Note you now see not for loan status on items
12 - Note the red x also includes message abnout not for loan status
13 - Note the 'Place hold' button is disabled
14 - Attempt multi hold
15 - Message now includes "No items available: One or more records have no items that can be held"
16 - Click 'Place holds'
17 - Above still places the hold - this is for a followup patch
Currently place request gets a list of bad_bibs that is created via javascript on the
template. It ignores this list
Ths patch instead doesn't add info for bad bibs, and provides a list of the bibs that
can be held
To test:
1 - Attempt multi hold with some items that can be held, and one that cannot due to notforloan
2 - Fill in pickup locations and place hold
3 - Note hold is place on bib with no avilable items and hsows twice in results
4 - Apply patch
5 - repeat with another patron
6 - Note no aidditonal hold on record with notforloan items
7 - Note with with not for loan items appears only once in results
Bug 30892
To test:
1. Try placing hold
2. Everything seems to work but no hold gets placed.
3. Apply patch
4. Verify holds are no placed correctly.
Signed-off-by: Arthur Suzuki <arthur.suzuki@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>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.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>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.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: Kyle M Hall <kyle@bywatersolutions.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>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
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>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.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: Kyle M Hall <kyle@bywatersolutions.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>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.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>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.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>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.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>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.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>
There is a missing parameter to CanItemBeReserved, we need to know if
the hold can be placed at a given pickup library.
Test plan:
1 - Set preferences:
UseBranchTransferLimits: enforce
BranchTransferLimitsType: itype
AllowHoldPolicyOverride: Don't allow
2 - Set Centerville 'Book' items to not be allowed to transfer to any other library
3 - Make sure Hold policy is set to 'any library'
4 - Find a record with a Centerville item and other items
5 - Attempt to place an item level hold on the Centerville item (with a
pickup library different than Centerville)
=> Without this patch you are taken to the holds list, but your hold is not placed
Nothing indicates why hold has failed
=> With this patch you cannot select the item from Centerville
"Cannot be transferred to pickup library"
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
When no desk is defined request.pl returns an internal server
error. Check a desk is defined before asking for reservation deskname.
Plan test:
1. log in with a library with no desk
2. check in a reserved book so it'll be switch to “waiting reserve”
3. go to the book notice and click the Holds tab (request.pl page)
4. boum
5. apply patch, restart plack
6. refresh page
7. now loading properly
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
When an item is checked in and marked 'Waiting' or already 'Waiting'
and there is a desk attached to the session, the item is marked
waiting at the current desk of the current library.
The information is displayed on the OPAC and on the intranet. The
patron can then know at which desk he can retrieve his document.
Desk Management (Bug 13881) is now useful.
Test plan :
1. apply Bug 24201
2. $KOHA_PATH/installer/data/mysql/updatedatabase.pl
3. Check out some document to someone
4. make another one reserve this document
5. check in the document
6. you can see the document is attach to the current library
7. create some desks and attach one to your session (see Bug 13881 and
Bug 24201)
8. cancel the preceding reserve and redo steps 3 to 5
9. you should see the document is waiting at the current library and
current desk on:
a. the intranet document request page
b. the intranet borrower holds tab
c. the item list where the document is listed on the bibliographic
details
d. the borrower's OPAC holds tab.
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Bug 24412: (follow-up) QA
Following Josef Moravec QA comments :
- rewrite Koha::Hold->desk according to Object Oriented Koha
Guidelines and use it to fetch desk name in various templates
- remove unused Desks.GetName
- Check for columns existence in db update
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Bug 24412: (follow-up) QA: useless change
Maybe it was a relic of something usefull... anyway
not anymore.
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Bug 24412: (follow-up) Fix POD
Koha::Desk and not Koha::Library...
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Nick Clemens
Andreas Roussos
Kyle Hall
Joonas Kylmälä
Tomas Cohen Arazi
Jonathan Druart
Marcel de Rooy
Agustin Moyano
Joonas Kylmälä
Lucas Gass
Nicolas Legrand
Arthur Suzuki
Julian Maurice