When renewing a book on hold with the RenewalOnHoldOverride sys pref set
to "Allow", if the due date is not specified then it becomes "now". By
checking the input is set rather than just defined the due date is set as
expected based on circulation rules.
Also change the description of the RenewalOnHoldOverride sys pref as the
due date would no longer be required to be entered with this change.
Test plan:
1. make sure RenewalOnHoldOverride is set to 'Allow'
2. check out a book, then place a title-level hold on it
3. renew the book, see the hold override dialog
4. leave the due date empty and hit the override button
5. observe the new due date is "now"
6. apply patch and restart plack
7. renew the book again, overriding the hold, and see the expected due date
as per circulation rules.
Signed-off-by: Marion Durand <marion.durand@biblibre.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit c84de245e3)
Signed-off-by: Jacob O'Mara <Jacob.omara@openfifth.co.uk>
The previous POD follow-up added headings for the private helper subs but
introduced several typos and used =head2, which puts them at the same level
as the public API in the generated docs. This patch:
- Fixes typos: matchpint, pionts, march matcher, mathcpoints, etc.
- Clarifies a couple of descriptions.
- Demotes each private helper from =head2 to =head3 so they render as
implementation notes rather than peers of the public methods, matching
the pattern used in Koha::Authority's "Internal methods" block.
Helper subs remain interleaved with the public API in source order to keep
the diff minimal; a wholesale move into a dedicated "Internal functions"
section would be a larger refactor outside the scope of this bug.
Test plan:
1 - perldoc C4/Matcher.pm
2 - Confirm no typos are rendered and the private helpers appear one level
deeper than the public methods.
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 9f1ae44cc7)
Signed-off-by: Jacob O'Mara <Jacob.omara@openfifth.co.uk>
The previous defensive block still dereferenced $target_auth unconditionally
when attempting to recover via record_strip_nonxml. If Koha::Authorities->find
returned undef (stale search index pointing at a deleted authid) the eval
around ->record would catch the first failure, but the recovery call on the
next line would die uncaught.
This patch:
- Skips silently when the authid returned by the search engine no longer
exists in the database, so get_matches cannot die on a stale index.
- Wraps record_strip_nonxml in eval so an exception there is also handled
and the record is simply skipped - matching the stated intent of the
"skip when not possible" path.
Test plan:
1 - Run t/db_dependent/Matcher.t - still passes.
2 - Simulate a stale index: mock search_auth_compat to return an authid
that is not present in auth_header. Confirm get_matches no longer dies
and returns without error.
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 9d2b2c2493)
Signed-off-by: Jacob O'Mara <Jacob.omara@openfifth.co.uk>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 94a9712745)
Signed-off-by: Jacob O'Mara <Jacob.omara@openfifth.co.uk>
When matching authorities we can die on a bad record, we should add protection against this
on top of bug 41843
To test:
1 - Apply index patch and rebuild elastic
perl misc/search_tools/rebuild_elasticsearch.pl -r -v
2 - Make a new record matching rule
threshold: 1000
type: authority
index: authid
score: 1000
tag: 001
3 - Choose an authority record and corrupt it:
sudo koha-mysql kohadev
UPDATE auth_header SET marcxml = UpdateXML(marcxml, '//datafield[@tag="040"]/subfield[@code="c"]',CONCAT('<subfield code',CHAR(27),'="c">OSt</subfield>')) WHERE auth_header.authid=92;
4 - View and save the record as marcxml
http://localhost:8081/cgi-bin/koha/authorities/detail.pl?authid=92
5 - Cataloging - stage marc records - authority/marcxml/no matching
6 - Confirm it stages
7 - Attempt to match using your new rule
8 - Get an error parsing the marc
9 - Apply patch, restart all
10 - Relaod the staged batch
11 - Attempt matching agaion
12 - Success!
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit c5adbe0818)
Signed-off-by: Jacob O'Mara <Jacob.omara@openfifth.co.uk>
The EAN-13 barcode query used max(abs(barcode)) which allowed non-EAN-13
barcodes (e.g. 14-digit KTD sample barcodes) to dominate the result.
The saved 13-digit EAN-13 barcodes were always smaller, so $nextnum
never advanced and the same barcode was generated on every page load.
Fix by extracting EAN-13 logic into C4::Barcodes::ValueBuilder::EAN13
(consistent with other barcode type packages) and scoping the query to
WHERE length(barcode) = 13 so only valid-length barcodes contribute to
the max. Both barcode.pl and Koha::REST::V1::Biblios now call the shared
function, eliminating the duplicated inline code.
Tests: adds EAN-13 test cases to t/db_dependent/Barcodes_ValueBuilder.t
Signed-off-by: Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit fc6e5839d4)
Signed-off-by: Jacob O'Mara <Jacob.omara@openfifth.co.uk>
Reports may consume too much resources of the db. To avoid that a new config param
'report_sql_max_statement_time_seconds' was created, which sets the maximum time a report statement may
take to execute in seconds. If it takes longer it will be aborted.
reflowed the code, so that is is executed after is_sql_valid, and put
the code in a subroutine
Test plan:
0. if you run this test in ktd you should check if you are running
mysql or mariadb, and then be able to switch database
1. create a new report by going to
More > Reports > New SQL Report
(/cgi-bin/koha/reports/guided_reports.pl?op=add_form_sql)
enter the following SQL (which only purpose is to run long)
select ExtractValue(bm1.metadata,'//controlfield[@tag="001"]')
from biblio_metadata bm1
join biblio_metadata bm2 on
ExtractValue(bm1.metadata,'//controlfield[@tag="001"]') =
ExtractValue(bm2.metadata,'//datafield[@tag="773"]/subfield[@code="w"]')
give the Report a name eg "extract_long"
2. run the report
3. report should be executed and return several 1000 lines on ktd
4. apply patch
5. add a the parameter to koha-conf.xml to the <config> </config> part
<report_sql_max_statement_time_seconds>0.001</report_sql_max_statement_time_seconds>
OR use the following statemtn
sed -i '/<biblioserver>/a <report_sql_max_statement_time_seconds>0.001<\/report_sql_max_statement_time_seconds>' /etc/koha/sites/kohadev/koha-conf.xml
6. restart koha
koha-plack --restart kohadev
7. go back to the report page and rerun report which now should lead to an error
The following error was encountered:
The database returned the following error:
Query execution was interrupted, maximum statement execution time exceeded
Please check the log for further details.
8. in koha-conf.xml set
<report_sql_max_statement_time_seconds>20</report_sql_max_statement_time_seconds>
9. restart koha
10. rerun query, which now should execute as before
11. stop ktd, and restart with the "other" database
DB_IMAGE=mysql:8.0 ktd up
DB_IMAGE=mariadb:10.5 ktd up
12. repeat from step 1 (omit applying the patch)
13. Sign off
14. Thx
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 07ee120d50)
Signed-off-by: Jacob O'Mara <Jacob.omara@openfifth.co.uk>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit c6ef3094ca)
Signed-off-by: Jacob O'Mara <Jacob.omara@openfifth.co.uk>
This silences warnings about undefinved values in the cache key
This seems to come from some calls in _build_languages_arrayref where we might not
have all variants of a language - the call could be prevented, but I am not sure
what the effects would be, so I simply convert undef (null value) to "undef" (string)
and use that as the cache key
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 365770c396)
Signed-off-by: Jacob O'Mara <Jacob.omara@openfifth.co.uk>
This should have no visible effect, just internal caching
To test:
1 - Do a search, check the language facets
2 - Apply patch, restart all
3 - Do a search, check the language facets
4 - No change
5 - Sign off
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 694765da2f)
Signed-off-by: Jacob O'Mara <Jacob.omara@openfifth.co.uk>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 6393701c2a)
Signed-off-by: Jacob O'Mara <Jacob.omara@openfifth.co.uk>
Koha's XSLT display path now calls a plugin hook so plugins can
extend the record processor filter list before MARC is transformed.
This enables prototype punctuation filters (and similar) to run
as plugins without core changes.
Test plan:
1) Enable plugins in koha-conf.xml.
2) Install the TestPunctuation plugin from https://github.com/HKS3/koha-test-punctuation
3) Enable the plugin in Koha admin.
4) View a biblio in staff or OPAC with XSLT display enabled and confirm
the filter runs (e.g., 245$a gets a trailing period if missing).
Signed-off-by: Alexander Wagner <alexander.wagner@desy.de>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit bb0d94b261)
Signed-off-by: Jacob O'Mara <Jacob.omara@openfifth.co.uk>
When merging (or updating) authorities, the biblios using the auths might also be updated. If this happens, Koha used to create one background job for each biblio. If an auth is used in a lot of biblios, this creates a lot of background jobs, each to index one single biblio.
This patch fixes that by collecting the affected biblios and putting all of them into one reindex background job.
Test plan:
* Apply the patch
* run the unit test t/db_dependent/Authority/Merge.t
* the test should now have 16 tests in total, with a new test #15 output like this:
1..3
ok 1 - 3 bibliographic record updated while merging two authorities
ok 2 - we have only one more background job
ok 3 - we have the correct biblio ids in the job to be indexed
ok 15 - merge with ModBiblio only creates one indexer background job for multiple biblios
ok 16 - no warnings
Manual test plan (rather cumbersome):
* Start KTD with ElasticSearch
* Create an PERSON authority (eg "100a Test")
* Create three biblios which use that authority in eg 100
* Access the DB to get the count of background jobs
* `ktd --dbshell`
* `select count(*) from background_jobs;`
* the count should be the same as the number of biblios you created (eg 3). But whatever it is, remeber it
* Edit the authority (change 100 to "Test new")
* check if the change is reflected in the linked biblios (100a = "Test new")
* Check the Background jobs in the DB again:
* `select count(*) from background_jobs;`
* should be three more than previously (eg 6)
* You can use this SQL to verify that the last three jobs each contain one biblionumber (which is what we want to fix):
* `select json_extract(data,'$.record_ids'),enqueued_on from background_jobs order by enqueued_on desc limit 3;`
+-----------------------------------+---------------------+
| json_extract(data,'$.record_ids') | enqueued_on |
+-----------------------------------+---------------------+
| [2299] | 2026-03-25 13:19:34 |
| [2300] | 2026-03-25 13:19:34 |
| [2301] | 2026-03-25 13:19:34 |
+-----------------------------------+---------------------+
* Now apply the patch and run restart_all
* Edit the authority again, setting 100a to "Test new patched".
* The biblios should change accordingly
* Check the Background jobs in the DB again:
* `select count(*) from background_jobs;`
* This should now be only 1 more then the last time (so eg 7)
* Check the details of the background jobs:
* `select json_extract(data,'$.record_ids'),enqueued_on from background_jobs order by enqueued_on desc limit 3;`
* The most recent entry should now contain three items in one row, and the following two should be the same as in the previous query (i.e. the old jobs from the edit before the patch)
+-----------------------------------+---------------------+
| json_extract(data,'$.record_ids') | enqueued_on |
+-----------------------------------+---------------------+
| [2299, 2300, 2301] | 2026-03-25 13:29:17 |
| [2300] | 2026-03-25 13:19:34 |
| [2299] | 2026-03-25 13:19:34 |
+-----------------------------------+---------------------+
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 1bb30a26cd)
Signed-off-by: Jacob O'Mara <Jacob.omara@openfifth.co.uk>
Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Amended-by: Jonathan Druart
Tidy C4/Search.pm
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit b3cddc9994)
Signed-off-by: Jacob O'Mara <Jacob.omara@openfifth.co.uk>
Testing:
- Follow plack intranet and/or opac error logs, tail -f /var/log/koha/kohadev/plack-*-error.log
- Login using web browser
- Do a POST request on either intranet or OPAC, for instance:
- Staff: Reports > create a new SQL report "select 'hello'"
- Public: search, select a book, add to a list
- 'uninitialized string' error appears in respective logs
- Apply patch
- restart_all
- Do another POST request
- No further errors should appear in the logs
Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit f7327f6e45)
Signed-off-by: Jacob O'Mara <Jacob.omara@openfifth.co.uk>
This patch adds predefined values to the record_sources table on installation of a new Koha instance. The values are
- batchmod
- intranet
- batchimport
- z3950
- bulkmarcimport
- import_lexile
and were formerly hardcoded in marc-overlay-rules.tt. This was introduced in 35380 which took care of updated instances via a db_revs script but forgot installation scripts.
Testplan
a) start up a fresh ktd
b) login and check go to Administration -> Record sources
c) check that no source exists
d) apply patch
e) go to mysql shell 'koha-mysql kohadev'
f) empty the database to trigger Kohas installer: 'drop database koha_kohadev;' followed by a 'create database koha_kohadev;'
g) restart_all
h) open Koha in browser again, the web installer should appear now. Credentials are 'koha_kohadev' with pasword 'password'.
i) run through the installation and create an admin user,
j) login with that user and repeat b) and c)
k) check that record sources are now set with the values from the descriptioni above.
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit d56976e9a6)
Signed-off-by: Jacob O'Mara <Jacob.omara@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 9539b21089)
Signed-off-by: Jacob O'Mara <Jacob.omara@openfifth.co.uk>
This patch adds a new rule for itemtypes to allow/prevent filling other holds on a bibliographic record
The rule will obey ReservesControlBranch for determining the rules
To test:
1 - Apply patches
2 - Setup/choose 2 item types in Administration->Item types
3 - In Administration -> Circulation and fine rules scroll to the bottom
4 - Set one item type to "Fill other holds on record" = 'Yes' and the other to 'No'
5 - Place a next available hold on the record
6 - Checkout the 'Yes' item type to the patron
7 - Their hold should fill
8 - Check in the items
9 - Place a next available hold on the record
10 - Checkout the 'No' itemtype to the patron
11 - Their hold should not be filled
12 - Check in the item and confirm the hold
13 - Checkout the itme to the patron, the hold fills
NOTE: While seemingly counter intuitive, the rule controls whether a hold NOT tied to this item will fill
Confirming the hold adds an itemnumber to the hold. In most cases, these items will not be holdable, so won't trigger at checkin
14 - Set a rule for the patrons branch to No and set system preference ReservesControlBranch to 'Patrons library'
15 - repeat 5&6
16 - The hold should not fill
17 - Change reserves control branch to items home library
18 - checkin /checkout the yes item type
19 - The hold should fill
Signed-off-by: Andrew Auld <andrew.auld@openfifth.co.uk>
Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 876869544f)
Signed-off-by: Jacob O'Mara <Jacob.omara@openfifth.co.uk>
When logging a hold create action, include all hold fields that may be
defined at hold creation.
Fields that are not defined until later (such as found status, waiting
date, etc) are not included to avoid clutter.
To test:
1. System preference setup
i. Enable HoldsLog system preference
ii. Set AllowHoldPolicyOverride to Allow
2. Place a hold in the staff interface
3. View the action logs entry for that hold creation
i. Go to Tools > Log viewer
ii. Select Modules: Holds and Action: Create
iii. Click Submit
--> Note that the "info" column only shows the hold id
4. Apply patch and restart_all
5. Repeat steps 2-3
--> The "info" column should now show hold details
Signed-off-by: Emmanuel Bétemps <e.betemps@gmail.com>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
(cherry picked from commit 83e2c3888d)
Signed-off-by: Jacob O'Mara <Jacob.omara@openfifth.co.uk>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
(cherry picked from commit 0d84b49ea7)
Signed-off-by: Jacob O'Mara <Jacob.omara@openfifth.co.uk>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
(cherry picked from commit 6fbcedd5db)
Signed-off-by: Jacob O'Mara <Jacob.omara@openfifth.co.uk>
FixPriority has a read-modify-write structure: it SELECTs all active
holds for a biblionumber, computes new priority order in Perl, then
issues N UPDATEs. Two concurrent callers (e.g. two patrons adjusting
holds on a popular title at the same moment) could both read the same
snapshot and then overwrite each other's renumbering, leaving duplicate
or incorrect priority values.
Fix by:
1. Wrapping the entire function body in txn_do so it runs atomically.
txn_do uses MySQL savepoints when called inside an existing
transaction, so callers that already hold a transaction are unaffected.
2. Adding FOR UPDATE to the initial SELECT so the first caller
acquires row-level locks on all holds for that bib. A second
concurrent caller will block at the SELECT until the first commits,
then reads the already-corrected priorities and produces a consistent
result.
Note: FOR UPDATE locks only rows that exist at read time; a concurrent
INSERT of a brand-new hold can still slip through, but priority
assignment for new holds goes via AddReserve → FixPriority, which will
also acquire the lock and renumber correctly.
(cherry picked from commit e0bdf3e859)
Signed-off-by: Jacob O'Mara <Jacob.omara@openfifth.co.uk>
The lowestPriority rank-clamping block fired a second SQL query to get
max(priority) WHERE lowestPriority=0 and used that value as both a
threshold and a splice index. This was fragile: if priorities had
become discontiguous (e.g. due to a race or a bug elsewhere), the DB
priority value would not match the 1-based position in @priority, and
the splice could land the hold in the wrong slot.
The @priority array already contains all the information needed: it is
sorted lowestPriority ASC so the count of rows where lowestPriority=0
is exactly the number of non-lowest holds, which equals the last valid
rank for a normal hold. Fetch lowestPriority in the existing SELECT and
use scalar grep { !$_->{lowestPriority} } @priority instead.
This eliminates the extra DB round-trip and makes the clamping operate
on the same data snapshot that will be written back.
(cherry picked from commit 4d61ac803f)
Signed-off-by: Jacob O'Mara <Jacob.omara@openfifth.co.uk>
When FixPriority is called with a reserve_id but no rank (rank defaults to ''),
the empty string evaluates to 0 numerically, satisfying `$rank <=
$highest_non_lowest_priority`. This caused the clamping block to unexpectedly
set rank to a positive value, which then triggered an unintended splice and
moved the hold.
Add `$rank > 0` to the guard condition so the block only fires when an explicit
positive rank was requested, matching the pre-existing semantics of the splice
block below it.
(cherry picked from commit 4f32ba4323)
Signed-off-by: Jacob O'Mara <Jacob.omara@openfifth.co.uk>
The recursive self-call loop that ignoreSetLowestRank guarded against was
removed by the main patch. Remove the now-unused parameter from both the
function body and the POD documentation, and fix a double-word typo in the
POD while here.
(cherry picked from commit 8e8a5f914e)
Signed-off-by: Jacob O'Mara <Jacob.omara@openfifth.co.uk>
This patch removes the recursive calls that set each 'lowestPriority' hold to the bottom of the priority list
and replaces it them by simply ordering the list of 'remaining' holds by lowestPriorty, then priority.
A check is also added to ensure that if a lowestPriority hold is being adjusted, it cannot be moved above holds
not marked as lowest priority.
To test:
1 - Place many holds on a record, 50 - 150
https://github.com/kidclamp/handy-koha-script/blob/main/randhold.pl
This will randomly make some of the holds lowest priority
2 - View the record in the staff interface:
http://localhost:8081/cgi-bin/koha/catalogue/detail.pl?biblionumber=312
3 - Click on the 'Holds' tab
4 - Adjust the priority of several holds using the dropdowns
5 - Note the timing
6 - Apply the patch
7 - Adjust the priority of several holds using the dropdowns
8 - Confirm the timing is improved
9 - Confirm that holds adjust as expected
10 - Confirm you cannot move a lowest priority hold above a hold not marked lowest priority
11 - Confirm holds move correctly on a record where all holds are lowestPriority
12 - Confirm holds move correctly on a record where no holds are lowestPriority
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
(cherry picked from commit 1f089c41c1)
Signed-off-by: Jacob O'Mara <Jacob.omara@openfifth.co.uk>
This patch adds tests for the item_level and item_id parameters in the holds API.
It also updates the C4/Reserves.pm module to handle these parameters correctly.
Test plan:
1. prove t/db_dependent/api/v1/holds.t
Sponsored-by: Koha-Suomi Oy
(cherry picked from commit ee4f4f3357)
Signed-off-by: Jacob O'Mara <Jacob.omara@openfifth.co.uk>
This patch updates C4::Output::output_error to take
an additional argument which contains an "interface"
parameter, so that opac/tracklinks.pl can generate
an OPAC error instead of an Intranet error.
Test plan:
0. Apply patch and koha-plack --restart kohadev
1. Go to http://localhost:8081/test.pl
2. Note normal Intranet 404 error
3. Go to http://localhost:8080/cgi-bin/koha/tracklinks.pl?uri=https%3A%2F%2Ftest.com&biblionumber=29
4. Note normal Opac 404 error
4b. For extra points, note that tracklinks.pl returns a 404 instead of a 302 to the 404.pl page
5. Change 'TrackClicks' syspref to "Track"
6. Go to http://localhost:8080/cgi-bin/koha/tracklinks.pl?uri=https%3A%2F%2Ftest.com&biblionumber=29
7. Note normal Opac 404 error
7b. For extra points, note that tracklinks.pl returns a 404 instead of a 302 to the 404.pl page
8. Rejoice!
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
(cherry picked from commit 532d10a800)
Signed-off-by: Jacob O'Mara <Jacob.omara@openfifth.co.uk>
This patch changes "op" to "login_op" when logging in, so that the original op can
flow through to whichever controller is processing the login and subsequent op.
Test plan:
0. apply the patch and koha-plack --restart kohadev
1. Without logging in, go to the following:
http://localhost:8081/cgi-bin/koha/reports/guided_reports.pl?id=1&op=run
2. Log in and note that you can see the report
3. Try logging into the OPAC
4. Try logging into the self-checkout
5. Create a new self-reg account and try logging in with the auto-filled details
6. Test anything else you can think to test here
Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
(cherry picked from commit 16c8d0d9a4)
Signed-off-by: Chloé Zermatten <chloe.zermatten@openfifth.co.uk>
Do not apply patches before
1 - Create a new subscription to a serial
2 - Go to 'Serial Collection' panel and click on edit serials button
3 - Try and edit the serial number to "{1}" -> error 500
4 - Apply only unit tests patch and run `prove t/Serials/ModSerialStatus.t` -> Tests will
not pass
5 - Apply patch and proceed again to 2&3 -> you will get no error
6 - run `prove t/Serials/ModSerialStatus.t` -> tests will pass
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit c48aef304f)
Signed-off-by: Jacob O'Mara <Jacob.omara@openfifth.co.uk>
It would be at least helpful to add a reference to get_transfer here,
since we are replacing it here.
Used the opportunity to update the older comments
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 7f8f5cbb5c)
Signed-off-by: Jacob O'Mara <Jacob.omara@openfifth.co.uk>
This patch removes a fetch of the item object followed but get_transfer to simply search the
transfers by itemnumber and status
To test:
1 - Add many items to your records
https://github.com/kidclamp/handy-koha-script/blob/main/randitems.pl
NOTE: this script makes a lot of items withdrawn or damaged, undo that for more dramatic difference
UPDATE items SET withdrawn=0,damaged=0
perl misc/search_tools/rebuild_elasticsearch -v
2 - Set Koha system preferences to display more results and check more items:
maxItemsInSearchResults 60
numSearchResults 50
3 - Do a search, several times, note the time it takes
4 - Apply patch, restart all
5 - Repeat search several times, note the time it takes
6 - Confirm it is better after the patch
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 13d2eb3d1b)
Signed-off-by: Jacob O'Mara <Jacob.omara@openfifth.co.uk>
To reproduce :
1) Install a plugin which uses the biblionumber of a created record in the "after_biblio_action".
2) Verify the biblionumber is missing
This is because the call to the hooks is done before the DB transaction is actually done (in AddBiblio function).
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit c3ce3b17c7)
Signed-off-by: Jacob O'Mara <Jacob.omara@openfifth.co.uk>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 6b61f010f4)
Signed-off-by: Jacob O'Mara <Jacob.omara@openfifth.co.uk>
This patch removes the hardcoded "404 Not Found" status
code and reason phrase from output_error and instead
uses just the code passed into the function.
Test Plan:
0. Apply patch and koha-plack --restart kohadev
1. Go to http://localhost:8080/cgi-bin/koha/tracklinks.pl?uri=https%3A%2F%2Ftest.com&biblionumber=29
2. Note normal OPAC 404 error
Bonus points:
1. Change 404 to 403 in the first output_error in opac/tracklinks.pl
2. koha-plack --restart kohadev
3. Go to http://localhost:8080/cgi-bin/koha/tracklinks.pl?uri=https%3A%2F%2Ftest.com&biblionumber=29
4. Note normal OPAC 403 error
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit beea9253e5)
Signed-off-by: Jacob O'Mara <Jacob.omara@openfifth.co.uk>
As Jan Kissig correctly mentioned on the BZ report, the various
summary info on charged items etc. are included.
This is done via the summary_info sub.
Note that the TODOs about
BZ FID_HOLD_ITEMS_LMT
CA FID_OVERDUE_ITEMS_LMT
CB FID_CHARGED_ITEMS_LMT
are still valid. This is not included yet.
Test plan.
Since we are just removing comments, read the code.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 0cc3190122)
Signed-off-by: Jacob O'Mara <Jacob.omara@openfifth.co.uk>
This bug adds Koha::Patron::Message::message_without_newlines, which has been added to C4::SIP::ILS::Patron in lieu of Koha::Patron::Message::message. The aim of this change is to prevent SIP2 units from crashing out when they encounter a patron message with newlines (\n or \r) in it, as units generally use a newline character to denote the end of a SIP2 transaction.
To test:
a) go to /cgi-bin/koha/sip2/institutions
*) create an institution, name CPL, timeout 300, the rest default
b) go to /cgi-bin/koha/sip2/accounts
*) create an account, userid koha, institution CPL, terminator crlf, the rest default
c) go to /cgi-bin/koha/members/moremember.pl?borrowernumber=51
*) click add message in toolbar
*) enter some multi-line message e.g. test[RETURN]1[RETURN]2[RETURN]3[RETURN][RETURN][RETURN]
d) run tail -f /var/log/koha/kohadev/sip-output.log, keep this window open
e) run ./misc/sip_cli_emulator.pl -a 127.0.0.1 -p 6001 -su koha -sp koha -l CPL --patron 42 -m patron_information
f) look at the tail log, notice how the MSG 64 line has a newline before the final pipe (|) character
== APPLY PATCH ==
g) restart_all
h) repeat steps e-f
i) notice how in the tail log, the MSG 64 line now has the final pipe character contiguously placed with the successive characters
== SIGN OFF ==
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit ab88c85f3b)
Signed-off-by: Jacob O'Mara <Jacob.omara@openfifth.co.uk>
This patch additionally fetches the reply to address for advance notices
To test:
1 - Have some issues that will be due soon for a patron(s)
Checkout with the due date set to tomorrow
2 - Verify the patron(s) have an email defined, verify the issuing branch has a reply to set
3 - Verify the patron(s) have messaging preference set to receive Advance notices by email 1 day in advance
4 - perl misc/cronjobs/advance_notices.pl -v -c
5 - Check the message queue
SELECT * FROM message_queue WHERE borrowernumber={##}
6 - Note no rpely to is set
7 - Apply patch
8 - perl misc/cronjobs/advance_notices.pl -v -c
9 - Check the message queue
10 - Confirm newer message has the reply to populated
11 - Ensure the issued item has a different homebranch with a reply to address se
12 - perl misc/cronjobs/advance_notices.pl -v -c --frombranch item-homebranch
13 - Confirm message has correct reply to
14 - Set patron to want digest in messaging preferences
15 - Generate notices
16 - Confirm digets has the right reply to
Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Tidied inline
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit d7028939da)
Signed-off-by: Jacob O'Mara <Jacob.omara@openfifth.co.uk>
If a message ends in a non-alphanumeric character, that character is stripped from the message.
For example:
6300120260210 182641 AOMPL|AA23529000035676(|
64 00120260210 182831000000000000000000000000AOCPL|AA23529000035676|AEHenry Acevedo|BLY|BHUSD|BV0|CC5|BD4345 Library Rd. Springfield, MA 44224|BF(212) 555-1212|PB19580530|PCS|PIY|AFGreetings from Koha. |
versus:
6300120260210 182641 AOMPL|AA23529000035676(|ACkoha|
64YYYY 00120260210 182835000000000000000000000000AOCPL|AA23529000035676(|AE|BLN|AFInvalid cardnumber|
The former should be returning the same "Invalid cardnumber" message the latter does.
Test Plan ( assumes you are using KTD ):
1) Install telnet: sudo apt install -y telnet
2) telnet 127.0.0.1 6001
3) Log in with: 9300CNkoha|COkoha|CPMPL|
4) Send the patron information request: 6300120260210 183346 AOMPL|AA23529000035676(|
5) Note you get a patron back
6) Apply this patch
7) Restart SIP!
8) Repeat steps 2-5
9) Noe you doe not get a patron back in the response!
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 791648b643)
Signed-off-by: Jacob O'Mara <Jacob.omara@openfifth.co.uk>
1 - Create suggestion
2 - Display a suggestion
3 - Amend a suggestion
To do with and without patch
Signed-off-by: Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk>
Amended-by: Jonathan Druart
tidy
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 1a5bb27e8a)
Signed-off-by: Jacob O'Mara <Jacob.omara@openfifth.co.uk>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 109e0d973d)
Signed-off-by: Jacob O'Mara <Jacob.omara@openfifth.co.uk>
Thinking about this today, we really just need the plugins initialized anytime we are using Koha::Plugins
Moving the loading into the BEGIN there seems to resolve the issue
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 84e0caae64)
Signed-off-by: Jacob O'Mara <Jacob.omara@openfifth.co.uk>
This patch addresses the circular dependency issues and code quality
concerns identified in QA by introducing a clean architectural solution.
Changes:
1. Created Koha::Plugins::Loader - A lightweight plugin loader with
minimal dependencies that can be safely called during early
initialization. Uses DBIx::Class when available for proper
transaction support, falls back to raw DBI for early init.
2. Added Koha::Database::table_exists() - Consolidated table checking
logic into a reusable method that can be called as both class and
instance method.
3. Simplified C4::Context BEGIN block - Reduced from ~120 lines to
~13 lines (91% reduction). Removed XML::Simple dependency and
manual database connection code.
4. Updated Koha::Plugins::get_enabled_plugins() - Now delegates to
the Loader module for consistency.
5. Updated C4::Installer::TableExists - Delegates to the new
Koha::Database::table_exists method.
This addresses all QA concerns:
- Eliminates XML::Simple usage (David Cook)
- Uses Koha::Database instead of manual DBI (David Cook)
- Reuses table checking logic (Nick Clemens)
- Eliminates circular dependencies (Paul Derscheid)
- Much cleaner and maintainable code
Test plan:
1. Run tests:
prove t/db_dependent/Koha/Database.t
prove t/db_dependent/Koha/Plugins/Loader.t
prove t/db_dependent/Koha/Plugins/Plugins.t
2. All tests should pass
3. Install and enable a plugin via the UI
4. Verify plugin hooks are called correctly
5. Verify no regressions in plugin functionality
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit e091b0d4bc)
Signed-off-by: Jacob O'Mara <Jacob.omara@openfifth.co.uk>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 6dd32161af)
Signed-off-by: Jacob O'Mara <Jacob.omara@openfifth.co.uk>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit f0e223bfd9)
Signed-off-by: Jacob O'Mara <Jacob.omara@openfifth.co.uk>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 654556247f)
Signed-off-by: Jacob O'Mara <Jacob.omara@openfifth.co.uk>
Michael Hafen
Martin Renvoize
Nick Clemens
Mark Hofstetter
Kyle M Hall
Thomas Klausner
Petro Vashchuk
Eric Phetteplace
Jan Kissig
Lucas Gass
Emily Lamancusa
Andreas Jonsson
Johanna Räisä
David Cook
Baptiste Wojtkowski
Marcel de Rooy
Arthur Suzuki
Jonathan Druart
Jake Deery
Paul Derscheid
Jacob O'Mara