Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
This patch deals with the fact that high-level circualtion methods like
`AddIssue`, `AddReturn` and `ModDateLastSeen` all eventually call
lower-level methods like ModBiblio, Koha::Item->store of
UpdateTotalIssues which are expected to trigger holds queue updates (for
the object CRUD operations use cases). As the circulation methods need
to trigger holds queue update as well, duplicate updates were being
requested which is suboptimal, of course.
In order to prevent this, and because circulation methdos could trigger
holds queue updates several times, actually, I added a new parameter
*skip_holds_queue* to the low-level methods, so when they are called
from circulation, the trigger is skipped and we have greater control on
when and how holds queue updates are scheduled.
This patch introduces the `skip_holds_queue` parameter to the following
methods:
* C4::Biblio::ModBiblio
* C4::Biblio::UpdateTotalIssues
* Koha::Item->store
Calls to those methods from the following methods will include the new
parameter, and thus duplicated holds queue updates avoided:
* C4::Circulation::AddIssue
* C4::Circulation::AddReturn
* C4::Items::ModDateLastSeen
Tests are added, to verify that the (mocked) BatchUpdateBiblioHoldsQueue
task is only scheduled once when they are called.
To test:
1. Apply up to the previous patch
2. Run:
$ kshell
k$ prove t/db_dependent/Biblio.t \
t/db_dependent/Biblio_holdsqueue.t \
t/db_dependent/Circulation_holdsqueue.t
=> FAIL: Tests fail!
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!
5. Sign off :-D
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
This patch introduces triggers for real-time updating the holds queue at
check out and check in.
The following high-level methods are involved:
- C4::Circulation::AddIssue
- C4::Circulation::AddReturn
To test:
1. Apply this patch
2. Run:
$ kshell
k$ prove t/db_dependent/Circulation_holdsqueue.t
=> SUCCESS: Tests pass! Triggers are triggered
3. Sign off :-D
Note: I put the tests on a separate file because the other one was too
big already.
Sponsored-by: Montgomery County Public Libraries
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
This patch adds an 'info' return param to CanBookBeRenewed and passes
the soonest renewal date when returning too_soon errors
To test:
1 - prove -v t/db_dependent/Circulation.t
Fix whitespace
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
GetSoonestRenewDate() calculates the renewal due date now also
correctly for auto-renewals after the fixes from bug 29476 so we don't
have to add any if-else checks because of it being buggy anymore.
To test:
1) prove t/db_dependent/Circulation.t
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
This patch adds a few tests to cover more cases, and to highlight current functionality.
The script only allows renewal if all outstanding holds can be filled by available items.
This means we can return as soon as we have determined that not all holds can be filled.
I add FIXME and some explanatory comments - I will file a follow-up bug for those, but
I feel we can accept these improvements to the performance and deal with the issues
of how it 'should' work versus how it does work on another report.
To test:
1 - prove -v t/db_dependent/Circulation.t
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
ItemsAnyAvailableAndNotRestricted can take a long time and create nested loops.
We can check it once per patron, however, this requires us to flip the loops.
Since an item can only be used once, we now add a check to see if this item has already
been assigned to a borrower.
To test:
1 - Find or create a biblio with 100 items
2 - Place ten 'Next available' holds on a biblio
3 - Set preference 'AllowRenewalIfOtherItemsAvailable' to 'Allow'
Set circ rules 'On shelf holds allowed' to 'If any unavailable'
4 - Checkout one of the items to a patron, backdated to be overdue
5 - Note a long loading time for the patron's checkouts
6 - Apply patch, restart_all
7 - Patron loads much faster
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
If a record has holds on it, and, using offline circulation, an item on
that is checked out to a patron that did not have a hold on the record,
the next hold that would have trapped that item will be canceled.
1) Place two bib level holds on an item for two patrons
2) Find a third patron, note his or her cardnumber
3) Create the file test.koc by copying and pasting the following into a
text editor
Version=1.0 Generator=kocTest GeneratorVersion=0.7
2008-06-11 12:24:11 547 issue PATRONCARDNUMBER ITEMBARCODE
note the fields are tab delemited, ensure the tabs do not get lost
when copying and pasting!
4) Upload the offline circ action, and commit the action
5) Note the item was checked out to the third patron
6) Note one of the other patron's holds were canceled
7) Reset your database
8) Apply this patch
9) Test again, no hold should be canceled!
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
There are several occurrences of `my $var += ` or .= in the code.
It should not cause problems but it's confusing.
Test plan:
Read the patch and confirm that the changes make sense.
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
If a maximum number of checkouts allowed is defined in circulation
rules, C4::Circulation::TooMany will loop over all patron's checkouts.
When a patron has several hundreds of checkouts, it can really slow down
the checkout process by several seconds (or even tens of seconds)
This patch does two things:
- Always prefetch item data so that `$c->item` does not execute an
additional SQL query at every iteration of the loop. Item data is
always needed at the first line of the loop, so there is really no
downside for doing this.
- Build the `@types` array only once, out of the checkouts loop. Since
it does not depend at all on patron's checkouts data, it does not make
sense to build it inside the loop.
Test plan:
1. Before applying the patch, create a patron with a lot of checkouts.
I tested with 1000 checkouts, but the slowness should be noticeable
with less.
2. Make sure you have a circulation rule (one that apply to your patron
and the item(s) you will check out for testing) with a maximum number
of checkouts allowed
3. Check out an item for the patron with a lot of checkouts. Measure the
time it takes.
4. Apply the patch
5. Check out another item (or check in and then check out the same item
used in step 3). Measure the time it takes and compare it to step 3.
It should be faster now.
6. Run `prove t/db_dependent/Circulation/TooMany.t`
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
When system preference is off, call no code related to Koha::Recalls.
Also add some missing module import.
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
This patch makes the different ->recalls accessors implemented on this
bug be more standard. This means:
- They don't do special things like default sorting or stripping out
special parameters. That's all left to the caller and the methods are
clean: they just return the related objects
- Useful filtering methods for Koha::Recalls resultsets are added. The
only used one (in the end) was ->filter_by_current. It seems like a
better approach, because it gives devs more control on how they want
to chain things, and there's a single place in which to maintain the
criteria of what is 'current' or 'finished'. This clearly makes the
'old' column obsolete IMHO, at least in the use cases I found. This is
covered by tests as well.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
This patch makes the status attribute an ENUM, setting the default value
as 'requested' as well. The chosen names are easier to read than single
letters. Also, renamed F into fulfilled (this impacts methods names as
well). This is because 'finished' or 'completed' is more a synonym for
old => 1...
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Koha::Old::Checkout->anonymize takes care of checking the syspref and
raises an exception if not set. So no we can now leverage on it, instead
of checking manually and then tweaking the checkout object manually as
well.
To test:
1. Apply this patch
2. Run:
$ kshell
k$ prove t/db_dependent/Circulation/MarkIssueReturned.t
=> SUCCESS: Tests pass!
3. Sign off :-D
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Use the newly introduced Koha::Object::Messages system to pass
additional information provided by object methods internally in the
objects themselves.
This patch updates the existing bespoke passing scheme to something
we've formally agreed to adopt going forward.
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>
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>
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>
If we simply return "no" immediately from the function when the
checkout is not an autorenewed checkout we can drop one layer of
indendation and the code becomes much easier to read.
To test:
1) prove t/db_dependent/Circulation.t
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
The too_soon check does not relate at all to auto-renewed issues and
should be moved outside the _CanBookBeAutoRenewed() function.
To test:
1) prove t/db_dependent/Circulation.t
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
This patch allows us to avoid checking reserves when the issue is
too_soon and we are running the cron
Code is covered by existing tests
To test:
1 - prove -v t/db_dependent/Circulation.t
Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>
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: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
If a checkout with auto-renewal enabled doesn't have a
"norenewalbefore" circulation rule set the code in CanBookBeRenewed()
falls back to using due date (to verify this please look for the
string "auto_too_soon" in C4/Circulation.pm), the calculation result
of GetSoonestRenewDate() however didn't do this, though luckily it was
not used in CanBookBeRenewed so we didn't get any issues
there. However, GetSoonestRenewDate() is used for displaying the
soonest renewal date in the staff interface on the circ/renew.pl page
so you would have gotten wrong results there.
This patch moves additionally the tests made for Bug 14395 under a new
subtest for GetSoonestRenewDate() as they should have been like that
already before.
To test:
1) prove t/db_dependent/Circulation.t
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
2132 $messages->{'LostItemFeeRefunded'} = $updated_item->{_refunded};
But, in returns.pl:
495 elsif ( $code eq 'LostItemFeeRefunded' ) {
496 $template->param( LostItemFeeRefunded => 1 );
497 }
1. Set BlockReturnOfLostItems to 'don't block'
2. Make sure your 'Default lost item fee refund on return policy' circ rule is set to 'Leave lost item charge'
3. Check something out and then mark it as lost.
4. Check the item in
=> No message to tell there was a refund
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This is based on Julian's idea on bug 28026 where we could get rid of call_recursive by passing refs as arguments to call.
Test Plan:
1) Apply this patch
2) prove t/db_dependent/Koha/Plugins/Plugins.t
3) prove t/db_dependent/Koha/Plugins/Barcode_transform_hooks.t
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Wrapped it in a TODO ;)
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Some of our partners have unusual barcode requirements that have
required us to transform scanned barcodes using javascript. This is not
the most reliable method. It would make more sense to have Koha
transform the barcodes on the backend using a plugin. We should add
hooks to transform and generate new item and patron barcodes.
Test Plan:
1) Apply this patch
2) Download and install the Barcode Transformer plugin
https://github.com/bywatersolutions/koha-plugin-barcode-transformer/releases/
3) Go to the plugin configuration page, set the configuration to the example configuration from the same page
4) In the item barcode field on the checkin and checkout pages,
and anywhere else you can scan an item barcode, type in some
valid barcodes, but prefix them with X and postfix them with
Y, e.g. X123456Y
5) Note the letters are removed by Koha!
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Bug 26351: (QA follow-up) Fix QA script issue
* Fixes issue with barcode generate stub so perlcritic is happy
* Removes extra semicolon from return call in configure method
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Bug 26351: Add unit tests
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Bug 26351: (QA follow-up) Remove unused method barcode_transform
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Bug 26351: (QA follow-up) Rename barcode_transform to item_barcode_transform
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Bug 26351: (QA follow-up) Barcodes inputted into Koha should always pass though barcodedecode
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Bug 26351: (QA follow-up) Catch one last case of itemBarcodeInputFilter
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Bug 26351: (QA follow-up) Fix Checkouts.t
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Bug 26351: Use call_recursive() as a replacement for call()
The method `call()` is not sufficient for barcode transformations. It's
possible that more than one barcode transformation plugin will be
installed. The `call_recursive()` method takes the output of the first
plugin and uses it as the input for the next plugin and so on. This allowes
each plugin to see the current version of the barcode and modify it if
necessary.
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Bug 26351: Fix t/db_dependent/Koha/Plugins/Circulation_hooks.t
Bug 26351: Revert improper change to unit test, fix number of tests
Bug 26351: Remove uneeded use Koha::Plugins statements
Left over from previous changes
Bug 26351: Add missing barcodedecode import
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This moves the checks related to whether auto renewal can be done on
the checkout to its own function _CanBookBeAutoRenewed(). This makes
it more clear which parts of the code are related to auto renewal
checking and which are not.
To test:
1) Make sure prove t/db_dependent/Circulation.t still passes
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
The UpdateItemLocationOnCheckin System Preference can be set to update the
location of ALL items during check in, regardless of their shelving location.
However, this does not currently work 100% as it is excluding items with no
shelving location (i.e. value of NULL in the corresponding database field).
This patch, based on the comment made by Nick Clemens, fixes that.
Test plan (based on the original Bug Description by Andrew Fuerste-Henry):
1) Have a shelving location CART
2) In UpdateItemLocationOnCheckin, enter "_ALL_: CART" (without the quotes)
3) Check in an item that has a shelving location, confirm it changes to CART
4) Check in an item with a NULL shelving location, confirm it doesn't go to CART
5) Apply this patch
6) Repeat step 4): this time the item should move to the CART shelving location
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch adds 'can_be_blank => 0' for the rentaldiscount rule to prevent
storing blank values in the database
Additionally, if there is no charge we do not need to check for a discount
and can simply return
To test:
1 - Set rental discount to "" to a rule in circulation rules
2 - Checkout an item that will follow this rule
3 - Check the intranet log:
[WARN] Argument "" isn't numeric in subtraction (-) at /kohadevbox/koha/C4/Circulation.pm line 3385.
4 - Apply patch and restart all
5 - Update database
6 - Set the rule to "" again
7 - Check the DB, no rule is stored
SELECT * FROM circulation_rules WHERE rule_name = 'rentaldiscount';
8 - Checkout the item again
9 - No warns in log
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
No idea why UpdateStats in C4::Circulation needs the fully qualified
namespace!
I kept getting
Undefined subroutine &C4::Circulation::UpdateStats called at /kohadevbox/koha/C4/Circulation.pm line 1643.
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>
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
We are using Koha::Logger when it makes sense to keep the info,
otherwise we simply remove it
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Bug 28572: Replace missing occurrence in misc/admin/koha-preferences
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This fixes regression caused by "Bug 12362: Cancel transfer with hold
cancelation" where cancelled hold's transfer didn't show up in
intranet and opac because it create a new transfer that was not yet
put in in-transit state. The original idea of bug 12362 was to be able
to trigger transfer back home if a hold was cancelled (a regression
caused by bug 26078). However, we can do it more simply by setting the
$validTransfer variable true in the item check-in code when we are
dealing with Reserve transfers. More down in the AddReturn() code
there is also a check "and !$resfound" to make sure we only try to
trigger the transfer back home automatically if there is no hold
waiting at the current location the item arrived in.
It should be noted however that now we only display generic message
for the automatic transfer reason. Bug 12362 made the return display
as the reason "Transfer was cancelled whilst in transit". However,
since this fixes the original regressions caused by bug 26078 and
restores similar behaviour to that I think giving a more descriptive
message for example regarding a hold being cancelled can be considered
a further enhancement.
To test:
1) Apply patch
1) Have biblio with item in branch A
2) Create a new hold with a pickup library to branch B
3) Check-in the item at branch A and confirm the hold and transfer
4) Cancel the hold
5) Check-in the hold at branch B and notice it prompt to return it to
branch A
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch re-instates the call to updateWrongTransfer to ensure the
transfer line is updated to reflect the new frombranch.
Test plan
note: when applying the patches, also update the database
1/ Check an item out from it's home library
2/ Check the item in at another library
(with AutomaticItemReturn system preferences enabled)
2a/ Accept the transfer and note we now have a transfer present from the
items check-in library to it's home library
3/ Check the item in at a third library
3a/ Note you are asked to return the item to it's home library.
3b/ With the patch applied, the modal title should highlight that a
'Wrong transfer' was detected.
4/ Go to the item record and note the holding library has been updated
to reflect where the item was most recently checked in.
4a/ With the patch applied the item status should reflect the last
checked in branch as the 'from' branch of the transfer.
5/ check-in from a 4th library, but use the 'Print slip' option for
accepting the transfer and confirm that also works.
6/ check-in from a 5th library, but use the 'Cancel' option and check
that this results in the item staying at it's current location and the
final transfer having been cancelled.
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch prevents the cancellations we were seeing. Transfers were
being created pre-emtively for reserves in branchtransfers before the
librarian had selected their prefered action. As such, we were getting
double lines in the branchtransfers table.
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This simply adds a JOIN to the collections table - if the collection is gone
then the item is not considered to be in a rotating collection
Also remove variable to store the return that is not used later
To test:
Create a rotating collection
Add an item
Delete the rotating collection
View the db (SELECT * FROM collections_tracking;)
The item is still in the collection
Enable AutomaticItemReturn
Checkin the item at a branch not its home
The item does not get transferred home
Apply patch
Restart all the things
Check in the item, it is transferred home
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch restores the skip_record_index optional parameter for
LostItem and passes it through to MarkIssueReturned as required.
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
With bug 27281 we removed a call to reset an items holdingbranch back to
the 'frombranch' of the transfer. This is correct, as with bug 23129
(and friends) we ensure that the holdingbranch was not changed at
transfer request time and instead would only be updated at the reciept
of transfer. As such, it's safe to remove the update here as it would be
just setting the same branch as is already present.
However, when removing this ModItem call, I neglected to realise that
also remoes the requirement for passing around the defered indexing bit
as we no longer need to trigger a re-index as there's not any change to
the item.
This patch therefore cleans up the POD for LostItem, removes the no
longer references 'params' parameter and drops the corresponding index
test.
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Aleisha Amohia
Julian Maurice
Joonas Kylmälä
Joonas Kylmälä
Andreas Roussos