Commit graph

8053 commits

Author SHA1 Message Date
Julian Maurice
4b65d099d7 Bug 28276: Do not fetch config ($KOHA_CONF) from memcached
memcached address and namespace are in $KOHA_CONF, so it is required to
read it before being able to access the cache. And after that,
configuration is kept in memory forever. Storing this in memcached is
useless and even counter-productive, since Koha reads both the file and
the cache

This patch addresses this issue by removing the cache-related code from
C4::Context->new.
It means that C4::Context->new will always read the configuration file,
so this patch also replaces inappropriate calls to
C4::Context->new->config by appropriate calls to C4::Context->config

It also fixes a bug where C4::Context->new would ignore the filepath
given in parameters if there was something in cache.

It also removes a problematic call to Koha::Caches->get_instance.
Because this call was outside of any subroutine, it would have happened
before the initialization of $C4::Context::context (which happen in
C4::Context::import)

Test plan:
1. Do not apply the patch yet
2. Add the following line at the beginning of Koha::Config::read_from_file
    warn "read_from_file($file)";
   This will allow you to check how many times the file is read.
3. Flush memcached and restart starman
4. Check the logs, you should see "read_from_file" a bunch of times
5. Apply the patch
6. Re-add the line from step 2
7. Flush memcached and restart starman
8. Check the logs, you should see "read_from_file" only once
9. Make sure the memcached config from $KOHA_CONF (memcached_servers,
   memcached_namespace) is taken into account by checking the About page

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>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-05-10 10:28:04 +02:00
Joonas Kylmälä
e2810dc2aa Bug 28281: Remove double decoding when importing installation data
The YAML::XS::Load and YAML::XS::LoadFile functions already decode the
YAML to Perl objects/strings, therefore decoding the output yet
another time is not needed and causes an error to happen.

To test:

1. Go through the following steps and notice without this patch the
error happens and with this patch it doesn't:

$ koha-mysql kohadev < installer/data/mysql/kohastructure.sql
$ restart_all
$ ./translate update pl-PL
$ ./translate install pl-PL
$ restart_all
 --> Go to the Koha web installer page and select pl-PL as the installer
 language, click through it and notice it gives following error at the
 MARC21 step:

 Cannot decode string with wide characters at /usr/lib/x86_64-linux-gnu/perl/5.24/Encode.pm line 202

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-05-07 14:44:00 +02:00
Emmi Takkinen
3ced2b3a4f Bug 14723: Use DATETIME instead DATE
Letters.t tests failed since time_queued didn't
include timecode. Use DATETIME when inserting
into message_queue.

To test prove t/db_dependent/Letters.t

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-05-07 13:48:55 +02:00
Emmi Takkinen
2405a21d6e Bug 14723: Add error codes and replace strings with them in template
This patch adds error codes for delivery notes and replaces English
strings with them in template.

To test confirm everything works as intended.

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-05-07 13:48:55 +02:00
Emmi Takkinen
5e58256d7f Bug 14723: Make delivery notes translatable
Delivery notes weren't translatable. This patch adds
them in notices.tt. If note is Mail::Sendmail error generic
'Error occured while sending email' is shown.

Also fixes MySQLisms and adds AFTER to atomicupdate file.

To test prove t/db_dependent/Letters.t

Sponsored-by: Koha-Suomi Oy

Signed-off-by: Stina Hallin <stina.hallin@ub.lu.se>

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-05-07 13:48:55 +02:00
Lari Taskula
aa45099442 Bug 14723: Additional delivery notes to messages
This patch adds additional delivery notes to messages in message queue as there
can be multiple reasons for a delivery to fail.

Currently in message_queue we are given only two delivery statuses for messages,
"sent" and "failed". When the status becomes failed, we have no idea why it fails.

This feature can be useful with SMS gateway providers. Many SMS gateways inform
the application the reason of SMS delivery failure. With this feature, this
information can now be stored. As well as for emails, instead of simply logging
failures, we can now store the reason of failure directly into the message row
of message_queue.

Test plan:

1. Enable EnhancedMessagingPreferences syspref
2. Find a borrower with notices at members/notices.pl
3. Observe that there is no column for Delivery notes
4. Apply patch and run the given database update
5. Repeat step 1.
6. Observe that there is now a column for Delivery notes

Sponsored-by: Hypernova Oy

Signed-off-by: Liz Rea <wizzyrea@gmail.com>

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-05-07 13:48:55 +02:00
b3baca4137 Bug 27577: Autolink biblio after biblionumber is generated
If BiblioAddsAuthorities and AutoCreateAuthorities are enabled, a new record is auto linked when saving and authorities generated with a 670 field that indicates which record generated the new authority.

The code to do this is called before the biblionumber is generated, so the variable that adds the biblionumber to the citation is empty

This patch moves the linking code to after the biblionumber is generated

To recreate:
1 - Enable BiblioAddsAuthorities and AutoCreateAuthorities
2 - Import or create a new record
3 - Ensure the record has controlled fields that do not have authorities before saving, i.e.:
    Add a 650 with "test_bug_27577_01"
4 - Search for the authority record
5 - Note the 670 field does not contain the biblionumber
6 - Apply patch, restart all the things
7 - Repeat 2-4
8 - Note the 670 now has the biblionumber

Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-05-06 15:52:38 +02:00
5a4d4b9478 Bug 27246: Remove obsolete BEGIN code from C4::Context
We don't longer enter the HTTP_USER_AGENT conditional in the BEGIN block
of C4::Context. It was used when apache only was used.

I am suggesting to remove it but keep the $CGI::VERSION < 4.08 condition
to keep support of all version (not sure it's needed but won't hurt).

I have not found a lot of occurrences of KOHA_BACKTRACES and I am
assuming not much people are using it, we should prefer the dev_install
flag instead (to set environment="development" when plack is started and
enabled the starman trace)

Test plan:
Regression test what you think is useful
Confirm the above statements

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-05-04 15:05:37 +02:00
e96c8439ff Bug 17202: Don't report an item in a collection if the collection doesn't exist
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>
2021-04-28 11:07:06 +02:00
7e6f30e60e Bug 27842: Update serial.biblionumber when a subscription is linked with another record
It seems that a librarian can edit the biblionumber for a given subscription, however, when receiving serials the biblionumber is pulled from the existing serials, so all new received issues are received on the old biblionumber.

This is problematic in light of bug 21901 - you can change the biblionumber, the serials still point to the old biblionumber - and deleting the biblio deletes the serials. Additionally, the update will delete serials where this was the case

Test plan:
1 - Create a new subscription
2 - Receive an issue
3 - Edit the subscription and change the biblionumber
4 - Receive another issue
5 - SELECT biblionumber FROM serial
=> the biblionumber link must have been updated

Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-04-28 11:07:06 +02:00
Joonas Kylmälä
eb1961f682 Bug 28230: Store C4::Message->{metadata} as Perl string always
When loading C4::Message->{metadata} we load it as perl string in all
other cases, see e.g. the subroutine find_last_message(). In this one
case we incorrectly populate the C4::Message->{metadata} as an UTF-8
octet instead of a Perl string. This causes a problem later on because
encode_utf8 later on in the line:

YAML::XS::Load(Encode::encode_utf8($self->{metadata}));

excepts a perl string and not a UTF-8 octet (please refer to the
functions perldoc). This breaks the encoding and causes an internal
server error:

To test the error is gone:
1. Create biblio with "AE" (bugzilla doesn't let me write the real
letter here, see it in the bugzilla comments) in title
2. Enable RenewalSendNotice
3. Enable "Item checkout and renewal" message preference for patron
4. Checkout an item from biblio "AE" to patron
5. Try to renew the patron's loan and notice the renewal fails
6. Apply patch and restart plack
7. Notice renewal works now and message h's the AE letter displayed correctly

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-04-28 11:07:06 +02:00
cff7ed4fd3 Bug 26734: Convert printfeercpt/printinvoice to use GetPreparedLetter
This patch updates C4::Letters to allow use of existing koha template
includes from notices, then updates the printfeercpt and printinvoice
slip print option to use GetPreparedLetter rather than calling getletter
directly.

As part of this work, we also add credits and debits handling to the
_parseletter_sth and _get_tt_params routines in C4::Letters to allow for
recognisable variable names in the notice template.

Test plan
1/ Update the ACCOUNT_CREDIT and ACCOUNT_DEBIT notices to use
'accounts.inc' include for account descriptions
1a/ Add `[% PROCESS 'accounts.inc' %]` to the top of the notice
1b/ Replace manual account descriptions with `[% PROCESS
account_type_description account=credit %]`
2/ Print one of each slip and check that descriptions are properly
substituted
3/ Clone the notice to another language
4/ Print the in the other language and note the translated descriptions
are properly substituted.
5/ Signoff

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-04-27 16:23:49 +02:00
bf13a36c8a Bug 28144: Check if issue_id defined
Historical OVERDUE fines may not have an issue_id - this causes
flooding of the logs when calling update fine

Trivial change, reading the code should be enough, but test plan below

To test:
1 - Create some fines without an issue_id
  - Either checkout an old version of Koha and add manually or
  - Checkout some items (backdated) to a patron, run fines.pl then
    UPDATE accountlines SET issue_id = NULL;
2 - Checkout a backdated item with fines in the circ rule to the same patron
3 - Run fines.pl
4 - Note the errors
5 - Apply patch
6 - Repeat and note no errors

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-04-27 16:23:49 +02:00
00883fc411 Bug 28210: Pass skip_record_index through LostItem
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>
2021-04-27 11:24:02 +02:00
7b15138278 Bug 27281: (QA follow-up) Fix Indexer Test
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>
2021-04-27 11:19:45 +02:00
6c9d15ba49 Bug 27896: Remove DeleteTransfer
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>
2021-04-23 11:58:49 +02:00
115ed5eb5b Bug 27281: Update LostItem to use Koha::Item[::Transfer] methods
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>
2021-04-23 11:58:49 +02:00
7e2679528e Bug 23971: Rename pref with AcquisitionLog
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-04-23 11:58:49 +02:00
Andrew Isherwood
9bd5a3c398 Bug 23971: (follow-up) Log entire objects
As requested by Tomás and Jonathan, we now log the entire basket object
when logging an action.

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-04-23 11:58:49 +02:00
Katrin Fischer
17670bd760 Bug 23971: (QA follow-up) Fix punctuation, doubled up return and syspref order
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>
2021-04-23 11:58:49 +02:00
Andrew Isherwood
79c2999c6e Bug 23971: (follow-up) Add additional basket logging
This patch adds additional basket logging as discussed in comment #14

- Basket reopening
- Basket users (manager) modification
- Basket header modification

Signed-off-by: Holly <hc@interleaf.ie>
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>
2021-04-23 11:58:49 +02:00
Andrew Isherwood
ac4ac649aa Bug 23971: (follow-up) Remove data duplication
In response to Séverine observations in comment #10, this patch removes
the duplicate logging of the borrowernumber

https://bugs.koha-community.org/show_bug.cgi?id=23971
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>
2021-04-23 11:58:49 +02:00
Andrew Isherwood
565726e556 Bug 23971: Add logging for basket related actions
This patch adds logging for the following Acq actions:

- Basket creation
- Basket editing
- Basket approval (via EDI)
- Basket closure

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>

Bug 23971: (QA follow-up) New DBrev syntax

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>
2021-04-23 11:58:47 +02:00
01f0e04172 Bug 27968: Fix framework import if isurl has an incorrect value
If isurl is set to undef in the DB, it will be exported as an empty
string, which is an invalid value for isurl (int)

Incorrect integer value: '' for column 'isurl'

Test plan:
Export framework structure in CSV and ODS, then reimport it and check that
all the subfields are important correctly

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-04-23 10:03:30 +02:00
83941d2ee1 Bug 23195: Always count shipping costs as 'spent'
As shipping costs are created when an invoice is created, and creation
of an invoice implies that items have been received, the shipment costs
are always assumed to be 'spent'

This logic is true in GetBudgetSpent/GetBudgetOrdered, however, GetBudgetHierarchy
treats open invoices shipping costs as 'ordered' and closed invoice shipping costs
as 'spent'

This leads to inconsistencies in acqui-home vs spent.pl and ordered.pl

To test:
 1 - Find a vendor
 2 - Click 'Receive shipments'
 3 - Create a new invoice with a shipping cost on budget A
 4 - Repeate and create a new invoice with a shipping cost on budget B
 5 - Close the second invoice
 6 - View acquisitions ome
 7 - Note budget A includes the shipping under ordered
 8 - Note budget B includes the shipping under spent
 9 - Click the 'ordered' column on budget A - no shipping is listed on ordered page
10 - Click the spent column on budget A - the shipping is listed here
11 - Apply patch
12 - Both budgets list the shipping as 'spent'
13 - Both 'spent' pages include the shipping
14 - Neither 'ordered' page includes shipping

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-04-22 14:50:11 +02:00
0f23622041 Bug 16787: 'Too many holds' message appears inappropriately and is missing data
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>
2021-04-22 14:37:14 +02:00
58c275e04b Bug 23583: Handle OpacHiddenItems with yaml_preference
This patch makes all places in Koha that rely on OpacHiddenItems
actually use C4::Context->yaml_preference instead of manually calling
the YAML libraries and handling it.

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Items* \
           t/db_dependent/Koha/Item* \
           t/db_dependent/Koha/Filter/EmbedItems.t
=> SUCCESS: Tests pass!
3. Try hiding things with opac-ISBDdetail.pl and opac-search.pl
=> SUCCESS: Things work the same!
4. Sign off :-D

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>
2021-04-21 15:25:07 +02:00
f6e0b04f48 Bug 23271: Replace search_limited with search_with_library_limits
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
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>
2021-04-21 15:25:07 +02:00
7c3c066714 Bug 20854: (QA follow-up) Improve readability
The logout_cas method does too little tasks. In this case, I consider we
should not split it into a separate method. As the CAS implementation
lacked tests, it made sense to do it like this, so it is testable. But
now we have higher-level tests for logout_cas, we can bake the behavior
inside of it, and it is testable.

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>
2021-04-21 10:51:29 +02:00
Matthias Meusburger
91307707ed Bug 20854: Allow correct redirect on logout for Cas servers 3.0 and superior.
Test plan:

 1) Apply the patch
 2) Set the system preference casLogout to "Yes"
 3) Set the new system preference CasServerVersion to "CAS 3 or superior"
 4) Check that you are redirected to Koha after a CAS logout from a CAS 3 server
 5) Set the new system preference CasServerVersion to "CAS 2 or inferior"
 6) Check that you are redirected to Koha after a CAS logout from a CAS 2 server

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>
2021-04-21 10:51:29 +02:00
bda9b9e122 Bug 27861: Warning in C4/XSLT.pm - use of uninitialized value in numeric eq (==)
In C4/XSLT.pm :
 elsif ( exists $itemtypes->{ $item->effective_itemtype }
            && $itemtypes->{ $item->effective_itemtype }->{notforloan} == 1 )

itemtype.notforloan is null by default, its generated a warning in logs.

Test plan :
1) Choose a record with itemtype.notforloan = NULL
2) Perform a search in the OPAC which will return results which
   include the record using XSLT
3) See patch removed warning

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>
2021-04-21 10:51:29 +02:00
f35571410b Bug 28069: (bug 27715 follow-up) Fix sort for lists
Sort functionality has been broken by bug 27715. We need to specify the
table.column in the datatable init

Note that "count" needs to be handled separately (which is a bit dirty
but cannot find something better)

Test plan:
Create some lists, add contents, change owner and dates
Sort using the different attributes and confirm that it's now working

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-04-16 16:20:50 +02:00
Didier Gautheron
d8f34430d1 Bug 26528: Z39.50/SRU ignore invalid replies
When searching for authorities if an authorities server reply has invalid records
none are displayed.

At least french BNF SRU server doesn't fully follow norm and can return an error
confusing Koha protocol handler which then returns an empty MARC record.

This patch silently removed bogus records.

To Test:
1- Add BNF SRU server
2- Go to authorities page
3- Add an authority
4- Search for keyword(any) droits de l'homme
5- No result (Internal Server Error)
6- Apply patch
7- restart starman
8- redo 4
9- Many records are displayed

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-04-16 16:20:42 +02:00
cb0f279390 Bug 18017: Add subdivisions to UNIMARC authorities zebra configuration
Like for MARC21, UNIMARC authorities has subdivisions form, general,
chronological and geographic.

In C4::Heading::UNIMARC, use subdivisions in _get_search_heading like in C4::Heading::MARC21.

Adds subdivisions variables into UNIMARC authorities zebra configuration.

Note that unlike MARC21 geographic is subfield $y and chronological is subfield $z.
See https://www.ifla.org/publications/unimarc-formats-and-related-documentation

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-04-16 15:12:45 +02:00
f719371662 Bug 15986: Add a script for sending hold waiting reminder notices
This patch adds a script for sending holds reminder notice to patrons.

We add a 'send_notice' routine to Koha::Patrons - this will either send using the patron's
email prefs, or allow forcing of a single method via the cron

To test:
 1 - Create an email hold reminder notice for a single library (Koha module: Holds, code HOLDREMINDER, branch: CPL)
 2 - Set some waiting holds today for patrons at CPL, ensure those patrons have 'email' as the transport for hold filled notices
 3 - perl misc/cronjobs/holds_reminder.pl -v -lettercode HOLDREMINDER -n -li CPL
 4 - You should see the patrons here would have received emails
 5 - perl misc/cronjobs/holds_reminder.pl -v -lettercode HOLDREMINDER -li CPL
 6 - You should see the emails that were sent
 7 - Check the patron notices tab to confirm
 8 - Note a ptron with two holds waiting receives only one notice
 9 - perl misc/cronjobs/holds_reminder.pl -v -lettercode HOLDREMINDER -li CPL -days 3
10 - No notices are sent
11 - Adjust the waiting date for the holds:
    UPDATE reserves SET waitingdate=DATE_SUB(CURDATE(), INTERVAL 3 DAY) WHERE waitingdate = CURDATE();
12 - perl misc/cronjobs/holds_reminder.pl -v -lettercode HOLDREMINDER -li CPL -days 3
13 - Confirm the holds are now reminded
14 - Set yesterday as a holiday for CPL
15 - perl misc/cronjobs/holds_reminder.pl -v -lettercode HOLDREMINDER -n -li CPL -holidays -days 3
16 - Notices should not be sent
17 - perl misc/cronjobs/holds_reminder.pl -v -lettercode HOLDREMINDER -n -li CPL -holidays -days 2
18 - Notices should be sent again
19 - perl misc/cronjobs/holds_reminder.pl -v -lettercode HOLDREMINDER -n -holidays -days 2
20 - Should get feedback that notice was not found for other libraries
21 - perl misc/cronjobs/holds_reminder.pl -v -lettercode HOLDREMINDER -n -holidays -days 2 -mtt sms
22 - Notice is not found
23 - Add the notice for sms
24 - perl misc/cronjobs/holds_reminder.pl -v -lettercode HOLDREMINDER -n -holidays -days 2 -mtt sms
25 - The notice should be sent
26 - Check patrons messaging tab to confirm
27 - prove -v t/db_dependent/Koha/Patrons.t

Sponsored by: The Hotchkiss School (http://www.hotchkiss.org/)

Signed-off-by: Kim Gnerre <kgnerre@hotchkiss.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-04-16 14:15:37 +02:00
da9006b20d Bug 21325: Prevent authentication when sending userid and password in querystring
This patch permits authentication via userid/password only when the
HTTP method is POST when using C4::Auth::checkauth().

The goal is to stop people from supplying userid and password in querystrings
in order to log into web pages.

Test plan:
0. Do not apply patch yet
1. Open a new browser (ie we don't want any existing CGISESSID cookies
available - opening a new tab/window isn't enough. It must be a
new instance or you can clear your cookies)
2. Go to http://localhost:8080/cgi-bin/koha/opac-reserve.pl?biblionumber=29&userid=koha&password=koha
3. Note the user has been logged in and is being asked to confirm hold.

4. Apply the patch

5. Go to http://localhost:8080/cgi-bin/koha/opac-reserve.pl?biblionumber=29&userid=koha&password=koha
6. Note the user is not logged in and the user is presented with a login screen

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-04-16 13:56:46 +02:00
1dfa520da5 Bug 28001: (bug 27050 follow-up) Fix delete_patrons if no category passed
GetBorrowersToExpunge must not crash if called with an empty patron
category list

Test plan:
Call delete_patrons.pl with and without patron categories. The script
must work as expected

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-04-16 12:28:17 +02:00
b795660540 Bug 21260: (follow-up) Consolidate preference
The bug added a local preference. This should now become a real one.
Renamed in the process to better reflect its use.

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-04-14 16:05:53 +02:00
Petro Vashchuk
a34e1f9f0d Bug 27746: prevent regexp undefined warning when checking subfield contents ($oclc)
Some of the books have subfield "a" of field "035" absent
and it's possible to have such
(it doesn't says "Require" in the interface),
which causes "Use of uninitialized value $oclc in pattern match (m//)"
error.

Solved by screening regex with check for empty variables before the
match.

To reproduce:
    1) Edit existing book or create a new one with empty marc
subfield "a" of field "035"
    2) Use the search feature that will find it along with some other
books.
    3) Check /var/log/koha/kohadev/plack-intranet-error.log to find
"Use of uninitialized value $oclc in pattern match (m//)" error.
    4) Apply the patch.
    5) Repeat the search and check the logs again to ensure
that error didn't appear again.

Signed-off-by: David Nind <david@davidnind.com>

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-04-14 15:25:26 +02:00
48ac552699 Bug 24695: Move to Koha::Report->is_sql_valid
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>
2021-04-12 15:27:50 +02:00
Pasi Kallinen
ba5be802e0 Bug 24695: Improve SQL report validation
The saved SQL report code validates the SQL in multiple places:
when saving, when updating, and when executing the query.
Move the validation code into Koha::Reports, and write tests for it.

Test plan:
1) Apply patch
2) Create a new valid SQL report, save it (success)
3) Create a new illegal SQL report, try to save (fails)
4) Update already saved SQL report by adding one of
   the forbidden words, eg. delete or drop (saving will fail)
5) Edit a save_sql in the database, changing it to eg.
   "drop borrowers", and try to execute it (fails)
6) Prove t/db_dependent/Koha/Reports.t

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Work as described, no qa errors.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

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

Bug 24695: (QA follow-up) Fix number of tests

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-04-12 15:27:50 +02:00
c32d46b854 Bug 28094: Deal with encoding for overriden sysprefs
The string we retrieved is UTF-8 encoded, to prevent encoding issue we
must decode it first.

Test plan:
Easily testable with LibraryName:
1. edit /etc/apache2/sites-available/kohadev.conf
2. Add
  RequestHeader add X-Koha-SetEnv "OVERRIDE_SYSPREF_LibraryName Library of ❤️"
3. restart_all
4. Hit the mainpage and look at the source (or title) and confirm that
the string is displayed correcty

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-04-09 15:45:07 +02:00
626d1f22a0 Bug 28013: (follow-up) Reset the patron loop
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>
2021-04-07 16:28:07 +02:00
ded05deb10 Bug 28013: (follow-up) Correct and update call to CanItemBeReserved
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>
2021-04-07 16:28:07 +02:00
Joonas Kylmälä
25d6024655 Bug 28013: (QA follow-up) Remove unused variable
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-04-07 16:28:07 +02:00
0b8f4ec614 Bug 28013: Performance improvements to CanBookBeRenewed
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>
2021-04-07 16:28:07 +02:00
a620c1bb11 Bug 28078: (follow-up) Add param check to 'itemAlreadyOnHold' test
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-04-07 16:28:06 +02:00
d3adcec676 Bug 28078: Add 'ignore_hold_counts' param to CanItemBeReserved
This patch adds an optional param 'ignore_hold_counts' to the routine, while still forbidding holds when 0 are allowed

To test:
1 - prove -v t/db_dependent/Holds.t

Signed-off-by: David Nind <david@davidnind.com>
JK: Commit message amended: Fixed title formatting
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-04-07 16:28:06 +02:00
1d9d05613b Bug 27069: Adapt uses of holdallowed
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>
2021-04-07 16:08:04 +02:00
Colin Campbell
cd40febe67 Bug 28052: Do not check SYSENV values only keys in loop
Loop is only interested in key values, because 'keys' is omitted
it is checking values as well. This can lead to misleading errors
being logged (e.g. key is undefined)
Extract keys only to loop array as in the other loops below this

Testing:
1 - tail /var/lib/koha/kohadev/sip-output.log
2 - in another tab use the sip cli tester:
   perl misc/sip_cli_emulator.pl -a localhost -p 6001 -su term1 -sp term1 -l CPL -m checkin --item CHESS1
3 - Note errors:
Use of uninitialized value $key in index at /kohadevbox/koha/C4/SIP/Sip/MsgType.pm line 337, <STDIN> line 1.
Use of uninitialized value $key in index at /kohadevbox/koha/C4/SIP/Sip/MsgType.pm line 337, <STDIN> line 2.
4 - Apply patch and restart all the things
5 - Repeat 2
6 - Errors mentioned are gone

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>
2021-04-01 18:51:37 +02:00
297dbf4a59 Bug 28053: Squash warning in C4::Members
This patch sets guarantess_non_issues_charges to '0' upon creation of
the variable and thus prevents 'Use of uninitialized value
$guarantees_non_issues_charges in numeric gt (>) at
/home/martin/kohaclone/C4/Members.pm line 183.' being thrown when the
patron has guaruntees without any charges.

Test plan
1. Run t/db_dependant/SIP/SIPServer.t and note the warning above is
   thrown at least once.
2. Apply the patch
3. Run the test again and note the warning has gone.

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>
2021-04-01 18:51:37 +02:00
Colin Campbell
dfe3753dee Bug 28054: Reinstate the shebang in SIPServer.pm
Bug 26673 renoved the shebang from SIPServer.pm. This was an error
SIPServer.pm is a program not a perl module, and needs the shebang
for the OS to assign an interpreter unless the command line
calls it as an argument to perl. Restore the shebang

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-04-01 18:51:37 +02:00
cb03909af6 Bug 27921: Log correct timestamp for HOLD MODIFY when set waiting
The HOLD MODIFY log at the end of ModReserveAffect is not using an
up-to-date $hold object.

$hold is modified at
1201         $hold->set_waiting($desk_id);
But not refreshed before logged (and so the timestamp is not logged
correctly).

Test plan:
Turn on HoldsLog
Place an item on hold
Check it in to mark it waiting

Confirm that the timestamp logged is the one from the check in, not when
you created the hold

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>
2021-04-01 18:51:37 +02:00
93a297a7a2 Bug 27928: FindDuplicate should use simple_search_compat
This patch switches from calling SimpleSearch directly to using the compat
method so that Elastic is used when it is the chosen search engine

To test:
 1 - Set SearchEngine syspref to 'Zebra'
 2 - Add a suggestion with an existing title, e.g.:
   'E street shuffle'
 3 - Confirm you get a duplication warning
 4 - Change SearchEngine to ES
 5 - Repeat get same warning
 6 - sudo koha-zebra --stop
    'ps aux | grep zebra'  to confirm they are stoped
    'sudo pkill -9 zebra' if they arent;
 7 - Repeat suggestion - no duplication warning (because zebra returns nothing)
 8 - Delete the suggestion
 9 - Apply patches
10 - Restart all the things, stop zebra
11 - Repeat suggestion
12 - Successful warning from ES
13 - prove -v t/db_dependent/Search.t

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>
2021-04-01 18:03:23 +02:00
07741c30b6 Bug 21260: Introduce local pref to affect status grouping
Introducing a local preference Available_NFL to control which not for
loan statuses are considered to be 'available for reference'.
Standard value is '1|2' which comes down to the former >0 when using
the initial Koha defaults.

Test plan:
[1] Pick a biblio with an available item A, an item B with notforloan 1,
    an item C with notforloan 2 and two damaged items. (Former patch.)
    Include it in a OPAC search. You should see:
        Available: A. Reference: B, C. Not-available: Damaged(2).
[2] Add local pref Available_NFL with value '1'. Repeat the search.
    You should see now:
        Available: A. Reference: B. Not-available: S.C.(1), Damaged(2).
    [where S.C. stands for Staff Collection]
[3] Run test t/db_dependent/XSLT.t

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-04-01 17:58:49 +02:00
Ere Maijala
016bb29b60 Bug 21958: Fix bibliographic record field comparison with authority
This makes the comparison between bibliographic field and authority field more robust and per subfield. This makes the comparison not consider the same e.g. the following fields:

$a Test User
$a Test $b User

The actual issue cannot be as easily reproduced with the patches for bug 21826 applied, but here's a test plan anyway:

1. Make sure tests pass (especially t/db_dependent/AuthoritiesMarc*)
2. Make sure authority linking still works properly
3. Make sure authority and biblio frameworks allow subfield i
4. Make sure that even if you add subfield i to 700 in biblio, authority link is kept the same
5. Make sure that even if you add subfield i to the authority record, the authority link is kept the same

Signed-off-by: Frank Hansen <frank.hansen@ub.lu.se>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-04-01 17:13:56 +02:00
c4c33e532d Bug 26517: Avoid deleting patrons with permission
The patron deletion script misc/cronjobs/delete_patrons.pl uses C4::Member::GetBorrowersToExpunge() to get patrons that may be deleted.
This method filters patrons from a staff category.
I propose to also filter patrons having permission, so a staff member.
Some small libraries do not define a "staff" category and give permissions to regular patrons.

Test plan :
1) Create a patron on adult type category with expiry date in the past and permission to access staff interface
2) Without patch
3) Run delete script : ./src/misc/cronjobs/delete_patrons.pl -v --expired_before='$(date -I)'
4) You see the patron will be deleted
5) Apply patch
6) Run delete script : ./src/misc/cronjobs/delete_patrons.pl -v --expired_before='$(date -I)'
7) You dont see the patron

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>
2021-04-01 17:06:34 +02:00
2bad23f1c1 Bug 27860: Add message_id param to SendQueuedMessages
We could use C4::Letter::GetMessage, but the query in
_get_unsent_messages join on borrowers and return the branchcode.
To prevent any regressions it's preferable to not modify
SendQueuedMessages.

Ideally we obviously need a Koha methods to have better and clean code..

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>
2021-03-16 12:04:05 +01:00
Aleisha Amohia
90491bb85a Bug 27268: Move GetMarcNotes to Koha namespace
This patch moves C4::Biblio::GetMarcNotes to
Koha::Biblio->get_marc_notes. This is so get_marc_notes can be
used in templates and notices.

To test:
1. Find a record that has a note (3xx for UNIMARC, 5xx for MARC21).
Confirm the notes still show as normal under the Descriptions tab.
2. Add the record to the cart.
3. View your cart and click 'more details'. Confirm notes show as
normal.
4. Log in to the OPAC. Find the record and add it to the cart
5. View the cart and click 'more details'. Confirm notes show as
normal.
6. View the record detail page and confirm the notes show as normal
under the Title Notes tab.
7. Confirm tests pass:
- t/Biblio.t
- t/db_dependent/Koha/Biblio.t

Sponsored-by: Bibliotheksservice-Zentrum Baden-Württemberg (BSZ)

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>
2021-03-09 17:20:47 +01:00
baa2260503 Bug 26199: Include LDR for match check/match point
This patch adds the ability to specify the LDR instead of a numeric tag
when defining match checks in record matching rules. This is particularly
useful for checking LDR6 "Type of record" when importing records.

Test plan:
0. prove t/Matcher.t

1. Go to http://localhost:8081/cgi-bin/koha/catalogue/detail.pl?biblionumber=1
2. Click "Save" and choose "MARCXML"
3. Copy bib-1.marcxml to bib-1-mod.marcxml
4. Change "01344cam a22003014i 4500" to "01344cmm a22003014i 4500"
5. Go to http://localhost:8081/cgi-bin/koha/admin/matching-rules.pl?op=edit_matching_rule&matcher_id=3
6. Click "Add match check"
7. Fill out the match checks:
Source:
Tag: LDR
Offset: 6
Length: 1
Target:
Tag: LDR
Offset: 6
Length: 1

8. Go to http://localhost:8081/cgi-bin/koha/tools/stage-marc-import.pl
9. Choose "bib-1-mod.marcxml" and click "Upload file"
10. Choose "KohaBiblio (999$c)" for "Record matching rule"
11. Click "Stage for import"
12. Click "Manage staged records" on resulting page
13. Note the "Match type" says "No match"

14. Go to http://localhost:8081/cgi-bin/koha/tools/stage-marc-import.pl
15. Choose "bib-1.marcxml" and click "Upload file"
16. Choose "KohaBiblio (999$c)" for "Record matching rule"
17. Click "Stage for import"
18. Click "Manage staged records" on resulting page
19. Note the Match type says "Match found"

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-03-09 17:20:47 +01:00
d776fa7b27 Bug 27545: Use NewItemsDefaultLocation from places where an item is created
The syspref NewItemsDefaultLocation is used to set a default value for item's location.

But it seems that there are some weirdness in the behaviour:
1. It's only used from additem. It seems that it should be used from acq and serial modules as well. And maybe for the items import too.
2. It set the location even if another one has been picked from the UI
=> We UI must preselect the syspref's value, but the controller must pick what has been selected on the UI

This patch is adding the default to the UI and extend the use of the
pref to other areas.

Test plan:
Set a value to NewItemsDefaultLocation
Catalogue a new item and confirm that the syspref's value is picked to
selected the default value on the add item form
Same behaviour should apply to the acquisition and serial modules
When importing items, the default location must be used if the imported
items did not have a location defined.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-03-09 17:20:47 +01:00
Agustin Moyano
445ca21ff0 Bug 24359: Remove hold from holds queue when captured
This patch removes a hold from holds queue when captured by check in.

To test:

1. check out an item from a book with multiple items for patron_1
2. place a biblio level hold for patron_2
3. perl misc/cronjobs/holds/build_holds_queue.pl
CHECK => holds queue shows the placed hold
4. check in the item from step 1 and confirm hold for patron_2
SUCCESS => hold for patron_2 is no longer in holds queue
5. prove t/db_dependent/HoldsQueue.t

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
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>
2021-03-09 10:31:46 +01:00
Joonas Kylmälä
9dcceb66fa Bug 27058: (follow-up) Clarify notforloan checks with a comment
In IsAvailableForItemLevelRequest the check is for holdability and in
ItemsAnyAvailableAndNotRestricted the check is for
checkoutability. These comments should make it more clear because the
notforloan value is used for these two different purposes and is a bit
confusing (we might want to add a new field "notforhold" in future to
make the code self documenting)

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-03-08 15:15:49 +01:00
Joonas Kylmälä
18128cc810 Bug 27058: Make checkout availability use all notforloan reasons
The ItemsAnyAvailableAndNotRestricted function is checking whether an
item can be checked out and it incorrectly only checks the positive
notforloan values when there can be also negative notforloan
values. If notforloan value is not 0 then it means the item cannot be
checked out. In the case of ordered items the value is negative (-1)
and thus before this change the checkout availability was reported
incorrectly.

To test:
 1) Run prove t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t
    Notice it fails
 2) Apply this patch
 3) Run prove t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t
    Notice it passes now.

To test via Koha sandbox (Alternatively):
 1) Create circ rule with If all unavailable
 2) Create new biblio
 3) Order a new item to the biblio via acquisitions and set the not
    for loan value to Ordered / -1
 4) Notice you cannot place a hold to the biblio
 5) Apply patch
 6) Notice you can now place a hold to the biblio

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

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-03-08 15:15:49 +01:00
Petro Vashchuk
ebf123bf13 Bug 27745: prevent warning for uninitialized value as a hash key
When you edit and save items with an empty "c - Shelving location"
field, it gets stored into the database as "NULL",
which is allowed in the database, and the field is not "required",
so this seems expected.

But when the search page is rendered, it gets value by location key
($shelflocations->{ $item->{location} }) which gives undef warning.

Solved by checking if $item->{location} is perl true.

To reproduce:
    1) Edit the book or create new one with an empty shelving location.
    2) Use the search feature to have that book in the searched list.
    3) Check plack-intranet-error.log to find "Use of uninitialized
value in hash element" error.
    4) Apply the patch.
    5) Repeat the search and check the logs again to ensure that error
didn't appear again.

Signed-off-by: Didier Gautheron <didier.gautheron@biblibre.com>

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-03-05 13:48:31 +01:00
592ab10fe5 Bug 22824: Remove C4::Boolean, true_p, boolean_preference, etc.
It seems that we don't really need all this overhead.
YesNo must be a boolean and contain 1 or 0.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.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: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-03-04 16:18:42 +01:00
d6d0116950 Bug 22824: Replace YAML::Syck with YAML::XS
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.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: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-03-04 16:18:42 +01:00
Joonas Kylmälä
17479dd3f9 Bug 25690: (follow-up) Supress warning about unitialized string
This fixes the warning:
Use of uninitialized value in string eq at /kohadevbox/koha/C4/Reserves.pm line 860.

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-03-04 16:18:42 +01:00
1b289f74cc Bug 25690: (QA follow-up) PROCESSED -> PROCESSING for consistency
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>
2021-03-04 16:18:42 +01:00
Joonas Kylmälä
fd0bc2204c Bug 25690: Make CanBookBeIssued return In Processing state as needing confirmation
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>
2021-03-04 16:18:42 +01:00
Joonas Kylmälä
338ba1d769 Bug 25690: Remove duplicated logic in SIP2 checkouts
Because AllowItemsOnHoldCheckoutSIP only affects the checkoutability
of non-attached, i.e. RESERVED holds in SIP2 we can therefore use the
common code from CanBookBeIssued and ignore only the RESERVED confirmation
message case in SIP2 checkout code.

This slightly changes the checkout error message given for "In
processing" holds that someone other than the holdee tries to
checkout. Otherwise there is no logic changes. The message that this
changes is "Item is on hold for another patron." vs. now "Item cannot
be issued: $confirmation". It is easier to create follow-up patch to
properly add INPROCESSING confirmation to CanBookBeIssued and then
show correct message based on the CanBookBeIssued return value.

To test:
 1) Apply all patches from bug 25690 to get latest Transaction.t version
 2) prove t/db_dependent/SIP/Transaction.t => passes

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>
2021-03-04 16:18:42 +01:00
Joonas Kylmälä
9b5913d308 Bug 25690: SIP2: Don't allow checking out attached hold that is being transferred
Items that are attached to a hold and being transferred (found = T) is
final decision and you cannot checkout those to other patrons.

To test (if you have applied all patches from bug 25690):
 1) prove t/db_dependent/SIP/Transaction.t => passes

To test (if you don't have all patches):
 1) Create hold and set pickup library to something else where the
    item is at
 2) Return the item and it should confirm the hold and start transfer
 3) Apply this patch
 4) Try to checkout this item now to some other patron via SIP2 and
    notice it doesn't allow it
 5) Revert this patch
 6) Notice you can now checkout somebody else's hold that is being transferred!

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>
2021-03-04 16:18:42 +01:00
Joonas Kylmälä
fbef547832 Bug 25690: Remove double usage of 'Reserved' return value
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>
2021-03-04 16:18:42 +01:00
15d4aa2b4d Bug 26618: Improve Exception Class Names
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-03-03 15:36:13 +01:00
138212f856 Bug 26618: (QA follow-up) Fix unit test for TranferCollection change
We update TransferCollection to use the settled upon standard for
passing error messages back from a method. This patch updates the
corresponding unit test.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-03-03 15:36:13 +01:00
7369fa5e50 Bug 26618: (QA follow-up) Update messages syntax
As highlighted, we were not using the standard form of message passing
here. This patch updates the template and controller to adopt the more
usual syntax.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-03-03 15:36:13 +01:00
87c3911bb8 Bug 26618: Remove use of transferbook in RotatingCollections
This patch replaces the use of C4::Circulation::transferbook in
C4::RotatingCollections with calls to Koha::Item->request_transfer and
adds handling for the various failure cases which that can throw.

We also introduce additional feedback for the end user where it did not
exist before. Now we notify the user if some of the collection could not
be transfers or if transfers were queued rather than set to request
immediately.

Test plan
1/ Set up a rotating collection
2/ Transfer the collection
3/ Confirm the action succeeds
4/ Set up some branch transfer limits that will affect items in your
collection
5/ Transfer the collection
6/ Note that the transfer succeeds but some items are returned as
failures

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>
2021-03-03 15:36:13 +01:00
e1b92345cd Bug 24446: (QA follow-up) Use 'receive' method in ModReserveAffect
ModeReserveAffect was setting all transfers in the queue to received by
looping through a resultset. This patch updates the logic to try and
catch the in_transit transfer and receive just that one instead.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-03-03 15:36:13 +01:00
4bf43fc066 Bug 24446: (QA follow-up) Set TransferTrigger to 'Reserve' for bug 24298
The SIP initiated transfer was set to use the 'TransferTrigger' passed
back from AddReturn; However, TransferTrigger is not set for reserves in
AddReturn (It's is set controller side by circ/returns.pl). We thus need
to also hard code the trigger type in the SIP transaction for reserves.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-03-03 15:36:13 +01:00
6ec94fb198 Bug 24446: Update ModItemTransfer for daterequested/datecancelled
C4::Items::ModItemTransfer is used throughout the codebase and currently
it will never set the daterequested or datecancelled fields.  With the
modifications to how circulation deals with transfers we need to update
this function to set those fields appropriately. Functionality has been
retained, ModItemTransfer will continue to add a transfer regardless of
limits or current transits existing.

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>
2021-03-03 15:36:13 +01:00
2675c4ef05 Bug 24446: Add datesent/datecancelled handling to GetTransfersFromTo
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>
2021-03-03 15:36:13 +01:00
5b3befe58b Bug 24446: Add datecancelled handling to GetTransfers
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>
2021-03-03 15:36:13 +01:00
7e919d12d8 Bug 24446: Catch daterequested in circ returns
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>
2021-03-03 15:36:13 +01:00
26b57ad5d2 Bug 27808: Refresh the item object when AddReturn is called
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>
2021-03-01 15:14:23 +01:00
44a54eb526 Bug 27808: Mark the onloan column as dirty in AddIssue
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>
2021-03-01 15:14:23 +01:00
Matthias Meusburger
8691809111 Bug 26591: SIP option prevcheckout_block_checkout to block checkout of previously checked-out documents
This patchs adds a new SIP option to block checkout of previously
checked-out documents: prevcheckout_block_checkout

See the CheckPrevCheckout system preference to enable previously
checked-out verification in Koha.

Test plan:
  1) Apply this patch
  2) Enable the CheckPrevCheckout syspref (on "Do" for instance)
  3) Enable prevcheckout_block_checkout in the SIP server config file
  4) Checkout and checkin an item for a user
  5) Checkout the item again with the SIP CLI tool
  6) Check that the SIP message is "This item was previously checked out by you"
     and that the item was not checked out
  7) Disable prevcheckout_block_checkout in the SIP server config file
  8) Checkout the item again with the SIP CLI tool
  9) Check that the SIP message is "This item was previously checked out by you"
     and that the item was checked out.
 10) Prove t/db_dependent/SIP/Message.t

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-03-01 15:14:22 +01:00
8650f8111b Bug 8976: Code cleaning
It was tricky to write the previous patch so I decided to clean the
ground (a bit) for later, taking advantage of having people who are
going to test the whole area.
We can move it to its own bug report if QA wants to.

Sponsored-by: Orex Digital

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-03-01 15:14:22 +01:00
d9ff0b5023 Bug 8976: Change for the item forms
Sponsored-by: Orex Digital

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-03-01 15:14:22 +01:00
ebe11f70e2 Bug 8976: Sort subfields for authority records
Same patch for the authority frameworks/records

Sponsored-by: Orex Digital

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-03-01 15:14:22 +01:00
b387502eaf Bug 8976: Add the ability to sort subfields for the MARC frameworks
This new enhancement adds the ability to change the default order the subfields.

Both bibliographic and authority MARC subfield structure are taken into
account. And so the item edition as well (in the different screens when
item can be added/edited).

This will answer the following needs that have been listed in the
comments of the bug report:
- $i in 7xx fields should be the first subfield in the sequence
- 300 fields are sorted number first when cataloguers enter the letter fields first
- 100 field, it's commonly $a, $q, $d.

Test plan:
1. Edit a MARC frameworks, field 300
2. Resort the subfield (drag and drop the tab of the subfield) as you
like
3. Save
=> Notice that the list of fields are displayed following the order you
chose
4. Edit it again
=> The order is correctly kept!
5. Create a new bibliographic record
6. Notice that the subfields are order in the same sequence
7. Fill different subfields, not all
8. Save, edit again
9. Note that the subfields that have been filled are listed first, then
the empty ones. But the sequence defined at the framework level is kept.
10. Do the same for an authority framework and create/edit an authority
record
11. Modify item (952) subfields order
12. Create an item and confirm that the order is correct
13. Modify the ACQ framework, 952, modify the order of the subfield
14. Create a new order and confirm that the item form has the subfield
ordered following the sequence defined at the framework level

QA: Note that this patch is about bibliographic records only, next
patches deal with authotiries and items.

Sponsored-by: Orex Digital

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-03-01 15:14:22 +01:00
099e2fe2b7 Bug 7806: Fix remaining occurrences of 0000-00-00
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>
2021-03-01 11:16:42 +01:00
3beeb020ef Bug 26457: (QA follow-up) Switch to PK index in UPDATE on issues
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>
2021-03-01 10:51:15 +01:00
e05ad0c1f8 Bug 26457: Unit test
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>
2021-03-01 10:51:15 +01:00
4062116352 Bug 26457: Throw exception if update of issues table fails
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>
2021-03-01 10:51:15 +01:00
c6606d4a65 Bug 27071: Code simplification
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>
2021-03-01 09:56:19 +01:00
197c1fedc6 Bug 27715: (QA follow-up) Remove conditional declaration
Resolves:
FAIL critic # Variables::ProhibitConditionalDeclarations: Got 1 violation(s).

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-02-24 00:12:59 +01:00
Joonas Kylmälä
b2b5570f08 Bug 27715: Use $dbh->quote_identifier to quote untrusted input
The sanitization using regex and \w class of characters might be
enough but given the vast number of unicode characters in \w and
possibility of in the future the database engines interpreting some of
those characters with special meaning it is better to wrap the column
identifier to quotes using $dbh->quote_identifier so it is only
interpreted as identifier and nothing else.

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>
2021-02-24 00:12:59 +01:00
9223d4d9be Bug 27715: Add a deprecation notice
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-02-24 00:12:59 +01:00
893e60acbc Bug 27715: Sanitize order by DT params
We are not on the safe side when we build the ORDER BY clause from the
DataTables parameters.

I've started to limit the columns by using Koha::Objects->columns, but
for instance for the patron search we need (at least) the columns from
the branches, categories and members tables.
It seems easier, and still safe, to use a regex.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-02-24 00:12:59 +01:00
3a8fcc28db Bug 27715: Remove unused dt_build_having
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>

Amended: removed warn.

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-02-24 00:12:58 +01:00