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>
This patch removes the last mentions of DeleteTransfer
Test plan
1/ Grep the codebase for DeleteTransfer
2/ It should no longer exist
Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Bug 27896: (QA follow-up) Fix number of unit tests
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch updates C4::Circulation::LostItem to use the Koha::Item and
Koha::Item::Transfer methods to cancel transfers when an item is marked
as lost.
Test plan
1/ Confirm t/db_dependant/Circulation.t passes prior to applying the
patches
2/ Apply the patch and run updatedatabase.pl
3/ Confirm that t/db_dependant/Circulation.t still passes
4/ Signoff
Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
To test:
1 - Apply all other patches and dependencies
2 - prove -v t/db_dependent/Circulation.t
3 - It fails
4 - Apply this patch
5 - It passes
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
The current call checks for truth against a returned hash, we need
to check the 'status' value
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
In the case of 'AllowRenewalIfOtherItemsAvailable' we check all existing reserves against
all existing items. This patchset reduces the number of DB/subroutine calls
To test:
1 - Apply patch
2 - prove -v t/db_dependent/Circulation.t
Signed-off-by: Amit Gupta <amitddng135@gmail.com>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
PROCESSED gave the apearance that the item processing had been completed
whereas in reading the code it appears to actually signify that the item
is awaiting/in proessing state.
This patch updates the variable to be PROCESSING consistently throughout
the codebase.
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This prevents checking out to a patron an item with hold to someone
else in the In Processing state via staff interface.
Also the checkout error message via SIP is now a more clearer one: "Item is
on hold for another patron." Before it was "Item cannot be issued:
$confirmation".
Also the branch transfer and batch checkout pages are adapted to this new
confirmation message as well.
To test:
1) Create bib level hold to an item for patron A
2) Check-in that item via SIP2, now the hold state should be "In processing"
3) Apply patch
4) Try to checkout the item to patron B via staff interface and
notice we get now confirmation prompt do we really want to do it
because it is in processing.
In order to not have to setup SIP2 server, alternatively steps 1) and
2) can be done so that you check-in the item in staff interface and
make it Waiting, and then with SQL change it to "In processing":
UPDATE reserves SET found = "P" WHERE reserve_id = XXX;
UPDATE reserves SET waitingdate = NULL WHERE reserve_id = XX
UPDATE reserves SET expirationdate = NULL WHERE reserve_id = XXX;
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
The patch "Bug 19116: Hold not set to waiting after transfer" added a
new meaning to 'Reserved' return value of C4::Reserves::CheckReserves
function. Let's remove double usage and have separate Transferred
return value so we can differentiate between attached and non-attached
holds. This will come useful in future refactorings.
This patch does no changes to the logic except in the
/cgi-bin/koha/circ/branchtransfers.pl and circulation.pl we now give
similarly to waiting state notice about hold being transferred.
To test:
1) Apply this patch
2) Create a new item level hold so that pickup library is different
than where the item is currently. Then return the item so that hold
is being attached and transferred.
3) Go to branchtransfers.pl and try to create a new transfer: it
should prompt you with message "Item is attached to a hold and
being transferred for XXX" and provide you with option to cancel
the hold or to ignore the transfer.
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
With the first uses of Koha::Item->request_transfer and
Koha::Item::Transfer->cancel having been added, we need to update
GetTransferFromTo to respect transfers that have been requested but
not yet sent and not cancelled.
Test plan
1/ Run the included unit test
2/ Confirm the "Transfers to recieve" page loads
3/ Signoff
Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Update C4::Circulation::AddReturn to use Koha::Item->get_transfer to
find requested transfers and use Koha::Item::Transfer->receipt to complete
transfer requests if they have arrived at their destination or return the
relevant 'WrongTransfer', 'WasTransfered' and 'TransferTrigger' messages
to the end user.
Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch changes the original implementation so the item object is
refreshed altogether instead of explicitly pinpointing a specific field
we identified an edge case can leave out from ->store. I propose this
alterate implementation because what this bug highlights is the fact we
don't code thinking calls to things can have side-effects (like this
case, with AddReturn updating the onloan status (and maybe other
things?).
To test:
1. Make sure circ tests pass with and without this patch
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
If any item is currently checked out to a patron, and the item is then checked out directly to another patron without manually checking the item in first, the items.onloan column will remain NULL.
This will only happen if the new checkout will be due the same day as the previous checked.
This is caused by the item being returned without updating the item object from storage afterward. Even though AddIssue will call AddReturn which sets the value of onloan to NULL in the database, we are not passing in the item object by reference, so it's onloan value remains set to a date. Then we set the onloan value to the same date. Because the value does not change in the object, the column does not get marked dirty.
We could update the object from storage first, but it seems more efficient to mark the column as dirty manually to avoid that otherwise unnecessary fetch.
Test Plan:
1) Apply these patches
2) prove t/db_dependent/Circulation.t
Signed-off-by: Lisette Scheer <lisettes@latahlibrary.org>
Signed-off-by: Marti Fuerst <mfuerst@hmcpl.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
The deadlock reports tell us that multiple transactions are
waiting for a X lock on a record but using a secondary index
on borrowernumber and itemnumber. Since we have the issue_id
at hand already, we should use that and benefit from the clustered
index (on PK) instead of using a secondary index.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
While this won't prevent the deadlock, it should catch the case where
a deadlock causes the DB update to fail and provide feedback to the user
and rollback the transaction
I don't know how to trigger the deadlock, I can only confirm that we see it, and
that this should catch it.
To test:
1 - Apply patches
2 - Checkout several items to a patron
3 - Confirm that 'Renew all' feature continues to work as expected and all items are renewed
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
To prevent conflict with Encode::encode
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
There is a difference between YAML::Load and YAML::XS::Load
From YAML::XS pod:
"YAML::XS only deals with streams of utf8 octets"
Test plan:
We are going to test 1 occurence and QA will confirm others don't
contain typos.
0. Don't apply the patches
1. Create a new itemtype with code=❤️
2. Create a new item using this itemtype (to biblionumber=1 will work)
3. Fill OpacHiddenItems with
itype: [❤️]
4. Search for "street shuffle" or any terms that will return the biblio
Notice that the item is there (there is an error in logs)
5. Apply the patches
6. Repeat 4 and confirm that the item is now hidden
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
From tht YAML pod:
"""
This module has been released to CPAN as YAML::Old, and soon YAML.pm will be changed to just be a frontend interface module for all the various Perl YAML implementation modules, including YAML::Old.
If you want robust and fast YAML processing using the normal Dump/Load API, please consider switching to YAML::XS. It is by far the best Perl module for YAML at this time. It requires that you have a C compiler, since it is written in C.
"""
See also
https://gitlab.com/koha-community/qa-test-tools/-/merge_requests/35
Test plan:
Try some place where YAML::XS is not used and confirm that it works
correctly
QA note: This patch removes some uses of YAML that were not useful
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Since bug 26639 we have auto savepoint enabled and the LOCK TABLE query
in C4::Circulation::SendCirculationAlert is not correctly handled.
From the MySQL doc that is copied few lines before, "LOCK TABLE will
commit any transactions", but here we don't have a savepoint and the
release for a non-existent savepoint will throw a DBI exception.
This patch removes the unecessary transaction and prevent the following
error when a renewal is done:
> DBIx::Class::Storage::DBI::mysql::_exec_svp_release(): DBI Exception: DBD::mysql::db do failed: SAVEPOINT savepoint_0 does not exist [for Statement "RELEASE SAVEPOINT savepoint_0"] at /usr/share/koha/lib/C4/Circulation.pm line 3590
Test plan:
1. Enable RenewalSendNotice
2. Add some email address to patron and select the email box from the message preference "Item checkout and renewal" in order to receive renewal emails.
3. Check 1 item out to a patron
4. Renew it
Signed-off-by: Andrew Nugged <nugged@gmail.com>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
In the previous patch you may have noticed many warns when running the tests
We add guarantor charges to a variable to determine if over the limit, but we don't initialize that value
We should
To test:
1 - Apply first patch and follow test plan
2 - Note warns when proving test
3 - Apply this patch
4 - prove
5 - No more warns
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
On bug 15494, the same method "get_yaml_pref_hash" has been added to two modules,
Koha/Config/SysPref.pm and Koha/Util/SystemPreferences.pm
We only need the one from Koha::Config::SysPref and remove the whole
Koha::Util::SystemPreferences module.
Test plan:
prove t/db_dependent/Circulation/issue.t
must return green
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
1. Check using git grep command that addDate does not exist
2. Check the patch that addDuration is spelt right
3. Check that tests pass t/Calendar.t
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Currently checkins can fail for one of two known reasons:
1 - Duplicate issue ids
2 - Anonymous borrower configuration issues
In the first case we refer to the 'data problems' or 'data corrupted' section of the about page. All errors are on the 'System information' tab and this is what we should mention
In the second case we refer to the data problems section, but information is in another section on the tab. Additionally, during checkin we issue an 'Error' but on the about page we call it a 'Warning' we should upgrade to error and specify what is broken
Additionally this patch fixes the warning that should display while issuing to a patron if anonymous is not set and patron has selected privacy
To test:
1 - Enable system preference OPACPrivacy
2 - Set system preference AnonymousPatron to '0'
3 - Log in a a patron on OPAC and set your privacy to never
4 - Issue a book to that patron in the staff client
5 - Note there is no warning
6 - Return the book
7 - Note the circulation page displays an error and the checkin fails and refers to 'data problems'
8 - Apply patch
9 - Issue another book to patron
10 - Note warning on the circulation page
11 - Try to check in a book again
12 - Check the new message makes sense
13 - Click the link to 'about.pl' check that errors are displayed
14 - Check the logs
15 - Confirm updated error message makes sense
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch remove the private sub used in GetIssuingCharges in favor
of get_effective_rule
It corrects the wrong precedence for rules and adds tests to cover this
subroutine
NOTE: the 'branch' for the discount will be determined by the signed in branch,
this is a bug to be fixed in the future
To test:
1 - Define a rentalcharge for an itemtype
2 - Define a 10% discount for library A, category A, all itemtypes
3 - Define a 50% discount for all libraries, category A, same itemtype
4 - Attempt to checkout an item from library A of the matching itemtype
5 - The 50% discount is applied
6 - Apply patches
7 - Attempt to checkout an item from library A of the matching itemtype
8 - The 10% discount is applied
prove -v t/db_dependent/Circulation.t
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Actually, the GetRecords service gives many information :
bibliographic data (marcxml field)
item data (items field)
issue data (issues field)
The issue data should not be sent by these service, its a privacy problem.
Biblio title and author are redundant.
This patch changes to use Koha::Checkouts to get issues informations with only the addition of item barcode.
Test plan :
1) Enable ILS-DI webservice
2) Checkout an item
3) Go to {opac}/cgi-bin/koha/ilsdi.pl?service=GetRecords&id={biblionumber}
4) Check the issues tag content
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
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>
The SQL code was duplicated, I combine them here
frombranch needs to take input
switch owning to frombranch in second script
initialize hash as empty list, not a reference
add a newline after printed output if not mailing notices
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch fixes a perlcritic issue and also replaces tabs with four
spaces where appropriate.
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
The provided patch adds the following functionality:
* Add --owning to both advance_notice.pl and overdue_notice.pl
* Add --library to advance_notice.pl like the way overdue_notice.pl already works
When specifying "--owning" both scripts will use items.homebranch instead of
issues.branchcode to determine sending library.
For advance_notice.pl this affects non-digest DUE and PREDUE, but not digest notices.
To test:
1. Have a patron who wants advance notices as email with 2 days in advance (not digest)
2. Have the first overdue date set 3 days past due date
3. Have issue for that patron where date_due is 2 days away where the item homebranch differs from the issuing branch
4. Have issue for that patron where date_due is 3 days old
5. Run advance_notices.pl without --owning
6. Run overdue_notices.pl without --owning
7. Confirm that two messages were created for that patron with the sender being the issuing branch
8. Delete messages or create two more issues according to (3) and (4)
9. Run advance_notices.pl with --owning
10. Run overdue_notices.pl with --owning
11. Confirm that the two messages created has the item homebranch as sender
Extra feature for advance_notices.pl is that it adds "--library" the same way overdue_notices.pl has.
Adding variants of that flag in steps (5) and (9) above can confirm this option as well.
Sponsored-by: Gothenburg University Library
Signed-off-by: Björn Nylén <bjorn.nylen@ub.lu.se>
Signed-off-by: Timothy Alexis Vass <timothy_alexis.vass@ub.lu.se>
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>
Marcel de Rooy
Nick Clemens
Kyle M Hall
Joonas Kylmälä
Andreas Roussos
Martin Renvoize
Jonathan Druart
Tomas Cohen Arazi
Eden Bacani
Fridolin Somers
Stefan Berndtsson