This was causing extra warnings when the permanent_location field was
unmapped.
We only need to check if there is a field defined as you can define a mapping
without both a field and subfield, so the existence of one implies the other
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(cherry picked from commit 0e00dc78e4)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit b56b902f73)
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
By always passing permenent location from the items editor, we can know
if permanent location was defined in the framework. This means we can
know if we should update permanent location independently of location
even if permenent location has an empty value.
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(cherry picked from commit f5bd16410a)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit 040861392c)
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
If permanent location is mapped, it will exist in the MARC.
However, when we are transforming our item hash to MARC,
we are losing our permanent location mapping, at least in unit tests.
By checking the original MARC for the permenent location, we remove the
risk of losing it the "empty" permenent location through the
transformation process. We only need to know that it existed in the
original item marc to know we need to update it in the database.
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(cherry picked from commit db382ebaff)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit 4aaf0b3077)
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(cherry picked from commit aa1d86920b)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit bdbd2c7225)
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Signed-off-by: Rhonda Kuiper <rkuiper@roundrocktexas.gov>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(cherry picked from commit 97647da9a0)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit ed682c0f0a)
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
There were 2 occurrences!
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(cherry picked from commit ecb0ff8bbe)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit 69f55189d1)
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
C4/Auth_with_shibboleth.pm has:
if ( any { /(^psgi\.|^plack\.)/i } keys %ENV )
When others have:
if ( any { /(^psgi\.|^plack\.)/i } keys %ENV )
This has been highlighted by https://gitlab.com/koha-community/koha-testing-docker/-/issues/249 that introduces a PLACK_WORKERS env vars, and t/Auth_with_shibboleth.t was failing with
# Failed test 'checkpw_shib tests'
# at t/Auth_with_shibboleth.t line 335.
Invalid parameter passed, categorycode= does not exist# Looks like your test exited with 255 just after 5.
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(cherry picked from commit 21d8298c77)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit 794e3b3505)
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
This patch adds a lookup of the sip user during an SC status, confirming that
our DB ocnnection is working, and that our user is still valid
Additionally, it adds support for SC status to the sip_cli_emulator and adds basic
test coverage for the SC status message
To test:
1 - Apply patch
2 - Restart SP server
3 - perl misc/sip_cli_emulator.pl -a localhost -p 6001 -l CPL -m sc_status_request -su term1 -sp term1
4 - prove -v 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>
(cherry picked from commit d85ae826c6)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit 06a99ac9bb)
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
We have a test in C4/XSLT/transformMARCXML4HTML which is meant to exclude the 942n, however,
it doesn't work because it checks the $subfield parameter against n
$subfield is an arrayref with the code and value as members
we need to check $subfield->[0]
To test:
1 - Apply only unit tests
2 - prove -v t/db_dependent/XSLT.t
3 - It fails
4 - Confirm 952$n in default framework is tied ot authorised value 'YES_NO' or do so
5 - Set OpacSuppression to 'Hide'
6 - Edit a record in the default template, changing 942$n to 'Yes
7 - Confirm the record is suppressed in opac
8 - Note there is no suppression notice in the record in staff client
9 - Apply second patch
10 - Reload and confirm staff side shows suppression notice
11 - Confirm record still suppressed in OPAC
12 - Edit record, set 942$n to 'no'
13 - Confirm there is no suppression notice in staff client
14 - Confirm you can view the record in OPAC
15 - prove -v t/db_dependent/XSLT.t
16 - Tests pass!
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>
(cherry picked from commit 91fb5d6a15)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit 3f7d334455)
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
There is code in populate_order_with_prices that is intended to use ecost over unitprice,
it just doens't seem to be working.
Making it more explicit seems to take care of the issue.
To test:
1 - Create a basket
2 - Add an order line, don't set 'Actual cost: ', but use vendor price
and a discount.
3 - Save
4 - Display all columns, 'Actual cost tax inc.' are GST columns aren't set.
5 - Apply batch
6 - Redo 1,2, 3
7 - Display all columns, GST is calculated correctly, unitprice remains 0.
Signed-off-by: Marjorie <marjorie.barry-vila@collecto.ca>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(cherry picked from commit 66d26fa301)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit a103cdbf1f)
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
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>
(cherry picked from commit b3baca4137)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit d384e46018)
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
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>
(cherry picked from commit 7e6f30e60e)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit 9f16f42953)
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
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>
(cherry picked from commit bf13a36c8a)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit f76e840970)
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
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>
(cherry picked from commit 01f0e04172)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit 5b4dd72d3a)
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
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>
(cherry picked from commit 83941d2ee1)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit abbdf03233)
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
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>
(cherry picked from commit 7c3c066714)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit 9b3538678e)
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
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>
(cherry picked from commit 91307707ed)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit 26e7262963)
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
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>
(cherry picked from commit bda9b9e122)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit fa0af2da9f)
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
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>
(cherry picked from commit 626d1f22a0)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit 60a44d0da1)
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
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>
(cherry picked from commit ded05deb10)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit e26eb6df85)
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
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>
(cherry picked from commit 0b8f4ec614)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit 81254e0db0)
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
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>
(cherry picked from commit d3adcec676)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit da72406fa5)
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
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>
(cherry picked from commit f35571410b)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit 56bc8c53d1)
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
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>
(cherry picked from commit d8f34430d1)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit cefb8bcf0c)
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
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>
(cherry picked from commit a34e1f9f0d)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit 4f618fb33a)
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
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>
(cherry picked from commit c32d46b854)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit b0a677be7f)
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
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>
(cherry picked from commit cd40febe67)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit 0a7cf00d20)
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
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>
(cherry picked from commit 297dbf4a59)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit d448f7a8b9)
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
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>
(cherry picked from commit dfe3753dee)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit 9906197508)
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
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>
(cherry picked from commit cb03909af6)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit 66a698fc72)
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
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>
(cherry picked from commit 93a297a7a2)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit 5a5566ca21)
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
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>
(cherry picked from commit c4c33e532d)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit 550577ed3a)
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
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>
(cherry picked from commit ebf123bf13)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit 4b6e421f1f)
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
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>
(cherry picked from commit 26b57ad5d2)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit 52babece97)
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
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>
(cherry picked from commit 44a54eb526)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit d4f047c672)
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
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: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
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>
Bug 26457: Unit test
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
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: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
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>
(cherry picked from commit c6606d4a65)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit 9104c191f3)
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
A few lines of code were added to CheckReserves containing the wrong
use of two perl functions: grep and split on bug 25232.
A test was added even making these things pass.
Test plan:
Run t/db../Holds.t
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
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>
(cherry picked from commit c4c3d1e93c)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit a299624404)
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Caused by bug 23403 - when performing a checkin we lookup the SIP patron
using the borrowernumber, however, SIP only knows how to find a patron
via cardnumber or userid
The change on 23403 was to avoid using an 'id' that didn't always exist
(as some users don't have a userid or cardnumber
When checking in, however, we are not passed a user cardnumber or
borrowernumber, so we don't have those on hand to get the patron.
Test Plan:
1) Check in an item via SIP, note patron is not found
2) Apply this patch
3) Restart all the things!
4) Check in an item via SIP, patron should be found!
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(cherry picked from commit 05e16389ef)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit ea74c99683)
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
The local hold priority exclusions feature is only present in 20.11, so
the use case handling shouldn't be included in the backport. This code
removes the couple lines in which this is handled (trivial) and tested
(trivial, as it is disabled by default on the tests, we just remove the
places in which the original patch set the non-existent attributes).
To test:
1. Run:
$ kshell
k$ prove t/db_dependent/HoldsQueue.t
=> FAIL: Tests fail!
2. Apply this patch
3. Repeat 1
=> SUCCESS: Tests pass!
4. Sign off :-D
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
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: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit 9a03861c8f)
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
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: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit 0d1e5ea69b)
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Amended: POD line for $import_record_id.
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(cherry picked from commit 03a9bdc851)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit aba575daed)
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Nick Clemens
Kyle M Hall
Jonathan Druart
Tomas Cohen Arazi
Matthias Meusburger
Fridolin Somers
Joonas Kylmälä
Didier Gautheron
Petro Vashchuk
Colin Campbell
Martin Renvoize
Marcel de Rooy