To test:
1. prove t/db_dependent/Koha/ImportBatch.t
2. Observe success
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 f54cce344c)
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>
suggestion_ref is quite messy, it needs to be reworked in order to use
objects in suggestion.pl.
As far as I studied, it is used as
- the whole input from forms,
- the default content to input in a form
- the content that will be amended in order to propose another hash to
modSuggestion. However, ModSuggestion will be removed so we don't need
such a hash
- every suggestion that need to be updated using the "change selected
suggestion" tool
- create the object suggestion_only, whose purpose is still unclear to
me.
As a result it is created by :
> 25 my $suggestion_ref = { $input->Vars };
And by removing a number of keys depending on the use of suggestion_ref
in the script.
In this patch, we remove every code made to use suggestion_ref as the
source object for form and use another variable, $stored_suggestion which is an object.
In next patch we aim at fully removing $suggestion_ref, which is still
used by ModSuggestion
Signed-off-by: Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit ba0b968a5a)
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 cc8b13a11c)
Signed-off-by: Jacob O'Mara <Jacob.omara@openfifth.co.uk>
remaning ocurrences in test files seem okay, this covers the others
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 7cdb17cc2f)
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 654556247f)
Signed-off-by: Jacob O'Mara <Jacob.omara@openfifth.co.uk>
Previously this could have caused issues if enable_plugins was disabled
in koha-conf as we were not checking enabled status before instantiating
an instance of the plugins object.
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 05423a313b)
Signed-off-by: Jacob O'Mara <Jacob.omara@openfifth.co.uk>
This patch adds hooks to allow 'Valuebuilder' plugins to be installable
using the standard plugin architecture for Koha. This enables cataloguing
plugins to be dynamically installed, enabled, and disabled through the
plugin system rather than being hardcoded framework components.
Test plan:
1. Install the example valuebuilder plugin from GitHub
(https://github.com/Jacobomara901/koha-plugin-value-builder)
2. Go to Administration > Plugins
3. Verify the valuebuilder plugin appears in the plugins list
4. Enable/disable the plugin and confirm the custom valuebuilder
appears/disappears in the framework configuration
5. Assign the custom valuebuilder to a MARC subfield in the
bibliographic framework
6. Create/edit a bibliographic record and verify the valuebuilder
button appears next to the configured subfield
7. Click the valuebuilder button and confirm it functions correctly
8. Disable the plugin and verify the valuebuilder button disappears
9. Re-enable the plugin and confirm functionality is restored
10. Uninstall the plugin and verify it's completely removed
Sponsored-by: Open Fifth <https://openfifth.co.uk>
Co-authored-by: Mark Hofstetter <koha@trust-box.at>
Signed-off-by: Catherine Small <c.small@rcs.ac.uk>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 8e6b687f51)
Signed-off-by: Jacob O'Mara <Jacob.omara@openfifth.co.uk>
- Fix potential crash when C4::Context->userenv returns undef:
use the established ternary guard pattern used throughout other
TT plugins (AuthorisedValues, Branches, Desks, etc.)
- Remove dead second argument passed to GetLettersAvailableForALibrary,
which only accepts one argument; the group_by/columns/order_by
hashref was silently dropped
- Fix typo "librries" -> "libraries" in GetByModuleForLibrary POD
- Update POD example to use 'add_message' module matching actual usage
- Add test coverage for GetByModuleForLibrary, verifying that a user
logged in to a branch sees branch-specific and all-libraries notices,
while users at other branches see only all-libraries notices
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
(cherry picked from commit b18fbab806)
Signed-off-by: Jacob O'Mara <jacob.omara@openfifth.co.uk>
To test
The following should work with either email or SMS notices sent from the Add Message button in a patron record
1 - create a notice in module Patrons (custom message) for all libraries
2 - create a notice in module Patrons (custom message) for Centerville
3 - while logged in at Centerville, confirm you can see and enqueue your all libraries message
4 - while logged in at Centerville, confirm you can see but cannot successfully enqueue your Centerville message
5 - while logged in at Fairfield, confirm you can see but cannot successfully enqueue your Centerville message
6 - apply patch, restart_all
7 - while logged in at Fairfield, confirm you can see and enqueue your all libraries message
8 - while logged in at Fairfield, confirm you cannot see your Centerville message
9 - while logged in at Centervielle, confirm you can see and enqueue your Centerville message
Signed-off-by: Juliet Heltibridle <jheltibridle@rcplib.org>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
(cherry picked from commit 3a1bd59197)
Signed-off-by: Jacob O'Mara <jacob.omara@openfifth.co.uk>
This patch prevents deletion of items that have item level holds on
them.
Test plan:
1. Create an item level hold.
2. Delete the item using alternative A or B:
A. Open the biblios Detail-view -> From the toolbar, open
"Edit"-dropdown menu -> Manage items -> Delete the item that has
an item level hold attached to it from "Actions"-dropdown menu
B. Open the biblios Detail-view -> Select the item that has an item
level hold attached to it -> Click "Delete selected items" ->
Confirm deletion
3. Notice that the item is deleted without any warning and the item
level hold has disappeared.
4. Apply this patch.
5. Repeat steps 1-3.
6. Notice that the item cannot be deleted and a warning is shown.
7. prove t/db_dependent/Koha/Item.t
8. Run the REST API tests: prove t/db_dependent/api/v1/items.t
Sponsored-by: Koha-Suomi Oy
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 9445ffc9aa)
Signed-off-by: Chloé Zermatten <chloe.zermatten@openfifth.co.uk>
This patch implements an algorithm for selecting the item that would
leave the largest gap after the proposed booking to maximize the
opportunity for future bookings.
Key Changes:
Koha::Booking:
- Added set_itemtype_filter() method for transient itemtype parameter
- Added _select_optimal_item() method using single aggregated query
* Groups bookings by item_id with MIN(start_date)
* Selects items with no future bookings (optimal)
* Among items with bookings, selects one with latest next booking
* O(1) complexity - single database query with GROUP BY
- Updated _assign_item_for_booking() to use optimal selection
* Filters by itemtype when itemtype_id is provided
* Replaces previous random selection (RAND())
API Changes:
- Added itemtype_id parameter support to add() endpoint
- Added itemtype_id parameter support to update() endpoint
- Validates exactly one of item_id or itemtype_id is provided
- itemtype_id is transient - not persisted to database
- Extracted and applied via set_itemtype_filter() before store()
Architecture:
- itemtype_id is NOT a database column (transient only)
- Used as selection criterion, then discarded
Test plan:
1. Apply patch and restart services
2. Create "any item" booking via staff interface
3. Verify item with longest future availability is selected
4. Test via API with itemtype_id parameter
5. Verify itemtype_id is not persisted to database
6. Run t/db_dependent/Koha/Booking.t
7. Run t/db_dependent/api/v1/bookings.t
Signed-off-by: Kristi Krueger <kkrueger@cuyahogalibrary.org>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit c7fa646929)
Signed-off-by: Chloé Zermatten <chloe.zermatten@openfifth.co.uk>
This change impacts the display of patron fields listed in system preferences:
BorrowerMandatoryField
BorrowerUnwantedField
PatronSelfModificationBorrowerUnwantedField
PatronSelfRegistrationBorrowerMandatoryField
PatronSelfRegistrationBorrowerUnwantedField
To test:
1 - confirm that the sysprefs listed above include "Alternate contact: First name (contactfirstname)" and "Alternate contact: Surname (contactname)"
2 - confirm that toggling the display of these fields hides/shows the Guarantor first name and Guarantor surname fields in a patron record
3 - apply patch, restart_all
4 - confirm that sysprefs now show "Guarantor first name (contactfirstname)" and "Guarnator surname (contactname)"
5 - confirm toggling the display of these fields still hides/shows the correct fields in a patron record in the staff client, in self-ref, and in OPAC self-modification
Signed-off-by: Anneli Österman <anneli.osterman@koha-suomi.fi>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit cc68988bce)
Signed-off-by: Jacob O'Mara <Jacob.omara@openfifth.co.uk>
The ES query builder correctly handles this parameters (passed from
search.pl and opac-search.pl) when it builds the ES query. But the
returned 'query_cgi' that is used later for keeping the search results
query context for building things like facets, just doesn't pass this
values around, leading to a loss of them on the generated links.
This patch makes sure they are passed around.
To test:
1. Apply the regression tests
2. Run:
$ ktd --shell
k$ prove t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t
=> FAIL: Tests fail! These params are not passed around!
3. Set ElasticsearchMARCFormat="Searchable array"
4. Reindex all the things:
k$ koha-elasticsearch --rebuild -d -v kohadev
5. Go to advanced search, using the 'More options' option.
6. Check 'Search entire MARC record' and 'Apply weights to search'
7. Perform a search on 'FULL TEXT'
=> SUCCESS: The URL contains `whole_record=on` and `weighted_fields=on`
8. Hover on any facet and look at the link
=> FAIL: It doesn't contain them!
9. Apply this patch
10. Restart all
k$ restart_all
11. Refresh the search results
12. Repeat 6
=> SUCCESS: Links have the needed parameters!
13. Repeat 2
=> SUCCESS: Tests pass! the parameters are passed around!
14. Sign off :-D
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 402a251392)
Signed-off-by: Jacob O'Mara <Jacob.omara@openfifth.co.uk>
start_server from Net::Z3950::SimpleServer is going to fork and the DB
server is aborting the connection
db-1 | 2025-10-03 12:02:12 272 [Warning] Aborted connection 272 to db: 'koha_kohadev' user: 'koha_kohadev' host: '172.19.0.6' (Got an error reading communication packets)
Test plan:
prove t/db_dependent/Koha/Z3950Responder/ZebraSession.t
=> Without this patch you will see a warning in the db container log
=> With this patch apply the warning no longer appears
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 d4ee7bd5f0)
Signed-off-by: Jacob O'Mara <Jacob.omara@openfifth.co.uk>
Signed-off-by: Richard Bridgen <richard.bridgen@nhs.net>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 7277f04342)
Signed-off-by: Jacob O'Mara <Jacob.omara@openfifth.co.uk>
Generated using
`perl misc/devel/tidy.pl --perl`
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 849f67988f)
Signed-off-by: Jacob O'Mara <Jacob.omara@openfifth.co.uk>
This patch restores the previous perl sorting to restore speed. For English language libraries
this will be correct in most instances, and is much more performant.
To test:
1 - Apply patch, update database
2 - Perform some searches, note ordering of facets
3 - Update system preference 'FacetOrder' to 'simple alphabetical'
4 - Peform some searches, note ordering of facets, should be correct unless you have unicode characters
5 - Sign off
NOTE: Benchmarks attached as a comment, you can also enable NYTProf and view results for yourself:
https://wiki.koha-community.org/wiki/Profiling_with_Devel::NYTProf#NYTProf_with_Plack
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 66825efa06)
Signed-off-by: Jacob O'Mara <Jacob.omara@openfifth.co.uk>
1) Install the following dummy backend plugin:
https://github.com/openfifth/koha-ill-backend-plugin/releases/tag/v2.0.9
2) Sys prefs preparation: Enable ILLModule, ILLOpacUnauthenticatedRequest and check PluginBackend on AutoILLBackendPriority
3) Logout, create a new Unauthenticated ILL request on the OPAC, visit:
<opac_url>/cgi-bin/koha/opac-illrequests.pl?op=create&backend=Standard
4) Fill in the mandatory top fields (first name, last name, email address and library)
5) Pick 'Book' for type and enter 'green' in title. This will cause the dummy plugin to be available in the AutoILL logic.
6) At the bottom of the form, on the "Verification" captcha, purposely input a wrong captcha e.g. '123'
7) Click 'Create'. Notice an odd looking form is presented, this is the dummy backend form, but it should present the 'Standard' form when using AutoILLBackendPriority as that was also the form we were presented with originally.
This happens because currently it's the backend that delegates back to Koha what template to render. When using AutoILLBackendPriority, however, Koha should handle this template delegation instead and present the Standard form.
8) Apply patches. Repeat test plan. Notice that now on step 7 you get the correct form, and error message (coming from Standard, as expected when using AutoILLBackendPriority).
9) Input the correct captcha this time
10) Confirm the created request landed in the PluginBackend backend.
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 6473a06916)
Signed-off-by: Jacob O'Mara <Jacob.omara@openfifth.co.uk>
This change explicitly turns off RELATIVE in Koha/TemplateUtils.pm
Test plan (Koha/TemplateUtils.pm):
0. Apply the patch and koha-plack --restart kohadev
1. Go to http://localhost:8081/cgi-bin/koha/tools/letter.pl
2. Click "New notice" and choose "reports"
3. Add the following to the report:
[% IF ( x == 1 ) %]
One
[% ELSE %]
Two
[% END %]
4. Go to http://localhost:8081/cgi-bin/koha/reports/guided_reports.pl?phase=Use%20saved
5. Create a report like the following:
SELECT biblionumber FROM biblio;
6. Click "Run with template" and choose the Reports template you made
7. Note that the output shows "Two"
8. Edit your Reports notice and include the following:
[% INSERT ../etc/passwd %]
9. Re-run the report using "Run with template"
10. Note that the output shows the regular SQL results (ie template transform not successful)
Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit b86095ffe7)
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 356903a96a)
Signed-off-by: Jacob O'Mara <jacob.omara@openfifth.co.uk>
This patch adapts the Koha::Auth::Identity::Provider class to
make use of this, and also to prove the behavior is correct (expected
results, exceptions, encoding, etc).
To test:
1. Run:
$ ktd --shell
k$ prove t/db_dependent/Koha/Auth/Identity/Provider.t \
t/db_dependent/api/v1/idp.t
=> SUCCESS: Identity provider related tests pass!
2. Apply this patches
3. Repeat 1
=> SUCCESS: Tests still pass!
4. Sign off :-D
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 71fce974e3)
Signed-off-by: Jacob O'Mara <jacob.omara@openfifth.co.uk>
This patch introduces a new class for extending Koha::Object-based
classes so they have reusable ways to deal with JSON encoded fields.
As documented in the POD, classes can now
```perl
use base qw(Koha::Object Koha::Object::JSONFields);
```
and will have the following methods available:
* decode_json_field
* set_encoded_json_field
For convenience, the Koha::Auth::Identity::Provider class is tweaked to
make use of this, and also to prove the behavior is correct (expected
results, exceptions, encoding, etc).
To test:
1. Apply this patch
2. Run:
$ ktd --shell
k$ prove t/db_dependent/Koha/Object/JSONField.t
=> SUCCESS: Tests pass!
3. Sign off :-D
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 6d4965f2d3)
Signed-off-by: Jacob O'Mara <jacob.omara@openfifth.co.uk>
Signed-off-by: Richard Bridgen <Richard.Bridgen@nhs.net>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit d8989fe57f)
Signed-off-by: Jacob O'Mara <jacob.omara@openfifth.co.uk>
The after_hold_action plugin hook is currently called from within the
hold cancellation transaction in Koha::Hold->cancel(). This creates
a fragile architecture where any plugin performing database queries
can cause lock timeouts and transaction failures.
The issue manifests when plugins query tables with foreign key
relationships to biblio records. Even lightweight queries on seemingly
unrelated tables (like illrequests) can trigger lock conflicts due to
foreign key constraints that require shared locks on referenced rows.
Since the plugin hook executes while the transaction holds locks on
reserves/biblio tables, any plugin database access creates potential
for circular lock dependencies and timeouts.
This patch moves the plugin hook call outside the transaction boundary,
ensuring that:
- Core hold operations complete without plugin interference
- Plugins receive the same data but execute after transaction commit
- Database lock conflicts are eliminated
- Plugin failures cannot corrupt core hold cancellation logic
The hook receives identical data through a stored reference to the
cancelled hold, maintaining full backward compatibility while
eliminating the transaction fragility.
Test plan:
1. Install any plugin that implements after_hold_action hook with
database queries (e.g., ILL plugins checking hold relationships)
2. Create a hold and attempt to cancel it
3. Without patch: Operation may timeout with "Lock wait timeout exceeded"
4. With patch: Operation completes immediately
5. Verify plugin hook still receives correct hold data
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 8f7a1b861f)
Signed-off-by: Jacob O'Mara <jacob.omara@openfifth.co.uk>
The added patches make some changes:
* $c->req->json is used to access the request body, as explained by the
Mojolicious OpenAPI plugin author it should be done.
* Instead of calling `$basket->to_api` we use the `$c->objects->to_api`
helper which deals with calling context (embeds, etc). Otherwise it
would require doing it manually if embeds are added.
* Tests are grouped together the same way the rest of them are.
To test:
1. Apply this patches
2. Run:
$ ktd --shell
k$ prove t/db_dependent/api/v1/acquisitions_baskets.t
=> SUCCESS: Tests pass!
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Laura_Escamilla <laura.escamilla@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit ac9944f7b0)
Signed-off-by: Jacob O'Mara <jacob.omara@openfifth.co.uk>
Example usage:
POST /api/v1/acquisitions/baskets
{
"vendor_id": 1,
"name": "Basket #1",
}
Test plan:
1. Try requesting this endpoint with your favorite API tool
(I recommend https://github.com/frigus02/RESTer)
2. Run `prove t/db_dependent/api/v1/acquisitions_baskets/post.t`
Edit (tcohen): Re-did the commit with some fixes in the spec
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Laura_Escamilla <laura.escamilla@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 5e0a72f8a8)
Signed-off-by: Jacob O'Mara <jacob.omara@openfifth.co.uk>
Instead of re-fetching the patron object.
This also adjusts tests:
prove t/db_dependent/Koha/ILL/Requests.t
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 63d1023317)
Signed-off-by: Jacob O'Mara <jacob.omara@openfifth.co.uk>
Signed-off-by: Stephen Graham <s.graham4@herts.ac.uk>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit ee348649a2)
Signed-off-by: Jacob O'Mara <jacob.omara@openfifth.co.uk>
Currently, a job is marked failed if there are no records added or updated.
We should take ignored records into account.
Additionally, I mark the job failed when the number of records processed
doesn't equal the number of records in the batch.
To test:
1) Export some records from your Koha, include some with items
2) Stage and import those records into Koha, matching on biblionumber,
ignoring matcheds, updating items
3) View the import job, it is marked failed
4) View the batch, all records imported
5) Apply patch, restart all
6) Stage/import as before
7) Job is now marked finished
8) Records imported as before
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 aef17e613d)
Signed-off-by: Jacob O'Mara <jacob.omara@openfifth.co.uk>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit e549d21441)
Signed-off-by: Jacob O'Mara <jacob.omara@openfifth.co.uk>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 85b8096284)
Signed-off-by: Jacob O'Mara <jacob.omara@openfifth.co.uk>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 4551a2d1ba)
Signed-off-by: Jacob O'Mara <jacob.omara@openfifth.co.uk>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 368e8124c5)
Signed-off-by: Jacob O'Mara <jacob.omara@openfifth.co.uk>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit a4c0746370)
Signed-off-by: Jacob O'Mara <jacob.omara@openfifth.co.uk>
test plan:
1. Export a record with an item from the staff interface as MARCXML (Save > MARCXML).
2. Edit the file and remove the 952$a (home library) and 952$b (current library).
3. Delete the item and record in the staff interface.
4. Import the exported record and item from the MARCXML file:
- Cataloging > Import > Stage for import
. Settings > Format: MARCXML
- Stage for import
- View batch link on the job details page (assuming successfully staged)
. New record framework: change to Books, Booklets, Workbooks
5. Turn on the browser developer tools and view the console.
6. Access the details page for the new record.
7. Note that you get:
- A "Processing" dialog box on screen
- A browser pop-up window/dialog box with:
Something went wrong when loading the table.
500: Internal server error
- Click OK
- The console shows:
. Failed to load resource: the server responded with a status of 500 (Internal Server Error)
. Got 500 (Internal Server Error)
. DataTables warning: table id=holdings_table - Ajax error. For more information about this error, please see https://datatables.net/tn/7
8. Run misc/maintenance/search_for_data_inconsistencies.pl, you get these result (the itemnumber may be different):
== Not defined items.homebranch and/or items.holdingbranch ==
* Item with itemnumber=973 does not have home and holding library defined
=> Edit these items and set valid homebranch and/or holdingbranch
(↑↑ don't do it, it's part of the message content)
9. Apply the patch.
10. Restart everything.
11. Refresh the details screen for the imported record.
12. You now get the same as step 7, but with added "Have a look at the "Audit" button in the toolbar" text in the browser pop-up dialog box:
- A "Processing" dialog box on screen
- A browser pop-up window/dialog box with:
Something went wrong when loading the table.
500: Internal server error
Have a look at the "Audit" button in the toolbar
- Click OK
- The console shows:
. GET http[URL for the record....] 500 (Internal Server Error)
. Got 500 (Internal Server Error)
. DataTables warning: table id=holdings_table - Ajax error. For more information about this error, please see https://datatables.net/tn/7
13. Click on the Audit button.
14. You get a yellow box at the top of the record with:
Errors found
Item with itemnumber=973 does not have home and holding library defined
(Note that there is still the "Processing" dialog box, you can edit the item by select the "Items" option in the sidebar menu and then edit the item details - the home and current library files will be auto-populated with the current library, Centerville with KTD)
15. Repeat step 8, you get the same results.
16. The tests pass: prove t/db_dependent/Koha/Database/DataInconsistency.t
17. Check the details page for another record that doesn't have any data issues - note the 'Audit' toolbar item, and if clicked should show a blue box at the top of the record with "No data inconsistency found."
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit b88dd24139)
Signed-off-by: Jacob O'Mara <jacob.omara@openfifth.co.uk>
This patch adds a new parameter to ModReserve: skip_fix_priority
The page for altering priority ensures each hold has a unique rank, so doesn't allow for setting two
holds to the same rank, so we don't need to reorder everything until the end (in case of some cancellations etc)
This patch also makes FixPriority exported and updates the name as it is already called in a few places outside of
C4:Reserve
To test:
1 - Place many holds on a record, 50 - 150
https://github.com/kidclamp/handy-koha-script/blob/main/randhold.pl
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 - Open the network tab in the browser console (F12) to view the timing
6 - Click 'Update hold(s)' and note the time
7 - Apply patch, restart all
8 - Repeat
9 - Confirm timing is better and numbering updates correctly
10 - Sign off!
Note: The script adds holds that may be marked as 'lowestPriority' these will always all group at the bottom of the record
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Jan Kissig <bibliothek@th-wildau.de>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit f21255ca54)
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: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 2c5efb40b1)
Signed-off-by: Jacob O'Mara <jacob.omara@openfifth.co.uk>
The Koha Manual has been translated into Greek:
https://koha-community.org/manual/latest/el/html/index.html
However, 'Greek' is missing from the list of languages
available in the 'KohaManualLanguage' System Preference.
This patch fixes that.
Test plan:
1) Without the patch, visit Administration > System preferences >
Enhanced content, and under the 'Manual' section notice that
'Greek' is missing from the options of the 'KohaManualLanguage'
syspref dropdown.
2) Apply the patch, and run updatedatabase.pl
The 'KohaManualLanguage' dropdown should now list 'Greek'.
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Laura_Escamilla <laura.escamilla@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 231a70ee45)
Signed-off-by: Chloé Zermatten <chloe.zermatten@openfifth.co.uk>
Bug 35451 added a record_table field to the additional_field_values
database table for performance optimization (better joins). However,
this field was not added to the OpenAPI schema, and with
additionalProperties: false, the API validator rejects responses
containing it.
Rather than adding this internal implementation detail to the public
API schema, this patch adds a custom to_api method to
Koha::AdditionalFieldValue that filters out the record_table field.
This approach:
1. Keeps the database optimization from Bug 35451
2. Prevents internal implementation details from cluttering the API
3. Maintains context that API consumers already have from parent objects
4. Fixes OpenAPI validation errors for ERM agreements, licenses, and
packages using extended_attributes
Test plan:
1. Create an ERM agreement with additional fields/extended attributes
2. Access the agreement via the API with extended_attributes embedded
3. Verify the response includes extended_attributes but not record_table
4. Confirm no OpenAPI validation warnings in plack-api-error.log
5. prove t/db_dependent/api/v1/erm_agreements.t
6. Verify all tests pass
Signed-off-by: Jan Kissig <bibliothek@th-wildau.de>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 60d84f91e6)
Signed-off-by: Chloé Zermatten <chloe.zermatten@openfifth.co.uk>
This changes "Koha administration" to "Administration" on the
staff interface home page and the administration module home page.
Everywhere else in the staff interface, such as browser page titles
and breadcrumbs, administration is used.
This improves consistency, and is how it is commonly referred to.
It also change some other occurences in the code base, such
as code comments.
Test plan:
1. Note the inconsistencies in how we name and refer to the
administration module:
1.1 Staff interface home page: Koha administration
1.2 Administration module home page: Koha administration, but note
that the breadcrumb and browser page title is "Administration".
1.3 Breadcrumbs and browser page titles for any of the administration
areas are just "Administration".
1.4 More > Administration.
2. Apply the patch.
3. Note that on the staff interface home page and the administration
home page are now labelled "Administration".
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit ef2f7524c0)
Signed-off-by: Chloé Zermatten <chloe.zermatten@openfifth.co.uk>
To test:
1. prove t/db_dependent/Koha/Patron.t
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 d214d441a8)
Signed-off-by: Jacob O'Mara <jacob.omara@openfifth.co.uk>
Adding a new guarantee and storing it fails with "Creating a patron"
selected in TrackLastPatronActivityTriggers when guarantors are set
to be mandatory. The problem comes from Koha::Patron->store called
in update_lastseen. Since we validate guarantor in Koha::Patron->store
but don't store them in there, new call to store fails because it
doesn't receive guarantor info as parameter.
As a solution, we should validate guarantors in separated method.
Validation can't be moved e.g. to Koha::Patron::Relationship->store
since this could lead to situations where guarantee is stored without
guarantor. This patch adds new method validate_guarantor and adjusts
old guarantor requirements tests to match this new method.
To test:
1. Select option "Creating a patron" in "TrackLastPatronActivityTriggers" syspref.
2. Enable syspref "ChildNeedsGuarantor" and make sure you have options in
"borrowerRelationship" syspref.
3. Create a guarantee patron, fill all required fields and add guarantor for patron
(either add Patron guarantor or fill Non-patron guarantor inputs).
4. Attempt to save patron.
=> Error box is displayed with text "The following fields are wrong. Please fix them.".
=> Logs show error "Patron creation failed! - [Child patron needs a guarantor]".
5. Apply this patch.
6. Create a guarantee patron again and save.
=> Patron should be saved correctly.
7. Create a new guarantee patron but this time don't add patron guarantor or fill
non-patron guarantor inputs.
8. Attempt to save patron.
=> Save should fail with error "A child patron needs a guarantor."
9. Add patron guarantor who can also be a guarantee and save.
=> Save should fail with error "A guarantor cannot be a guarantee."
Also prove t/db_dependent/Koha/Patron.t
Sponsored-by: Koha-Suomi Oy
Signed-off-by: Magnus Enger <magnus@libriotech.no>
Works as advertised.
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 382ba17a8c)
Signed-off-by: Jacob O'Mara <jacob.omara@openfifth.co.uk>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit f47ab213fd)
Signed-off-by: Jacob O'Mara <jacob.omara@openfifth.co.uk>