Commit Graph

61890 Commits

Author SHA1 Message Date
Nick Clemens f94cfcfee4
Bug 43033: Test with a lot of holds
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-07-13 15:46:24 +00:00
Martin Renvoize 611328b5db
Bug 42430: (QA follow-up) Fix nested plugin package corruption on reload
InstallPlugins forces every discovered plugin class to be deleted and
re-required from disk on every call, so plugin_methods reflects the
.pm file as it currently exists rather than whatever was already
loaded into a long-lived Plack worker. That deletion used
Symbol::delete_package, which clears the target package's whole
symbol table -- including any already-loaded packages nested under it
(e.g. a REST API controller a plugin bundles alongside itself, such as
Koha/Plugin/Foo/Controller.pm). Those nested classes keep their own
%INC entry, so they are never re-required afterwards and are left
permanently blanked out for the rest of the worker's life.

This was caught when running
`prove -r t/db_dependent/Koha/Plugins/ t/db_dependent/Koha/REST/Plugin/`
on top of the original patch. Tests fail several files that have nothing
to do with the bug being fixed (Biblio_and_Items_plugin_hooks.t,
Holds_hooks.t, TemplateIncludePathHook.t, REST/Plugin/PluginRoutes.t),
because Koha::Plugin::Test::Controller (a bundled test-fixture
controller living in t/lib/plugins/Koha/Plugin/Test/Controller.pm)
gets its symbol table wiped as collateral damage every time
Koha::Plugin::Test is reloaded.

_forget_plugin_class() replaces the delete_package call with the same
technique used by the CPAN module Class::Unload (not added as a
dependency here, just the technique): only clear the target class's
own symbols and reset its @ISA, explicitly leaving any nested package
entries in its symbol table alone. Module::Pluggable discovers bundled
nested classes as their own entries in the plugin list regardless, so
they still get reloaded correctly on their own turn.

I also annotate the no strict 'refs' block added by the previous commit
with a '## no critic' justification, as flagged by koha-qa.pl.

Test plan:
1. Confirm the bug exists without this patch:
   a) git checkout <previous commit> (drop this commit, keeping the
      first 3 commits of the bug applied)
   b) Inside KTD:
      prove -r t/db_dependent/Koha/Plugins/ t/db_dependent/Koha/REST/Plugin/
   c) Observe failing files unrelated to this bug's own tests:
      Biblio_and_Items_plugin_hooks.t, Holds_hooks.t,
      TemplateIncludePathHook.t and REST/Plugin/PluginRoutes.t.
2. Confirm the fix:
   a) Re-apply this commit
   b) Re-run the same command from 1(b) -- all tests now pass
3. Confirm the original bug this branch set out to fix is still fixed,
   both via the manual UI steps from the first commit's test plan
   (install Batch Permission Modifier Plugin 1.3.1, upload 1.4.1 over
   it via Tools > Plugins, confirm the renamed method takes effect
   without restarting Plack) and via:
   prove t/db_dependent/Koha/Plugins/Plugins.t
   (see the 'InstallPlugins() reloads stale symbol tables tests'
   subtest)
4. Confirm QA tooling is clean:
   koha-qa.pl -c 4
   => PASS for Koha/Plugins.pm and all touched test files, no critic
      or tidiness failures.

Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-07-13 15:46:23 +00:00
Nick Clemens c1c733fc08
Bug 42430: (follow-up) Add missing POD
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-07-13 15:46:22 +00:00
Kyle M Hall fff6a7574c
Bug 42430: Add unit tests
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-07-13 15:46:21 +00:00
Kyle M Hall ef26c1acaa
Bug 42430: Fix issue with stale plugin methods after plugin upgrade
When a plugin is upgraded in place (by uploading a new version over
the old one), the list of methods Koha knows about for that plugin
can get out of date. For example, if a plugin used to have a "tool"
method and the new version renames it to "admin", the staff interface
can still show the old "Run tool" link, and clicking it produces an
error saying the plugin has no "tool" method.

This happens because the plugin upload runs inside a long-lived
Plack worker that has already loaded the old version of the plugin
in memory. When Koha scans the plugin to record its methods, Perl
has no reason to re-read the file from disk, so the scan sees the
old method list and stores that in the plugin_methods table. The
plugin code on disk is new, but the record of what methods it has
is stale.

This patch forces Koha to throw away the in-memory copy of each
plugin and reload it from disk before scanning, so the recorded
list of methods matches the plugin that is actually installed.

Test Plan:
1) Install the Batch Permission Modifier Plugin version 1.3.1
(this version exposes a "tool" method)
2) Go to Tools > Plugins and confirm the plugin row shows
"Run tool" in its actions menu
3) Upload Batch Permission Modifier Plugin version 1.4.1 over
the top of 1.3.1 (this version exposes "admin" instead of
"tool")
4) Reload the Plugins page
5) Note the plugin still shows "Run tool" in its actions menu
6) Click "Run tool"
7) Note the error: the plugin has no "tool" method!
8) Apply this patch
9) Restart all the things!
10) Uninstall the plugin
11) Repeat steps 1-4
12) Note that "Run tool" is gone and "Run admin tool" appears
in its place
13) Click "Run admin tool" and confirm it loads without error!

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-07-13 15:46:20 +00:00
Martin Renvoize 6563255743
Bug 42995: (QA follow-up) Harden nested +count FK-chain resolution
The FK-chain resolution added in the previous patch only handles ONE
level of nesting: it requires $parent_alias to be a direct relationship
of $main_source. This holds for every +count embed currently declared
in api/v1/swagger/ (all exactly one level deep, e.g.
biblio.items+count), but if that assumption is ever violated - a
deeper nested +count, a parent relation with a coderef (custom join)
condition, or no matching FK column name - the previous code silently
fell back to the pre-fix expression ($parent_alias.$fk_self), which can
reference a join alias that isn't available inside DBIC's limiting
subquery, reproducing the original 500 with no diagnostic.

This patch makes _build_count_subquery decline to build a subquery
(return undef) when the correlation can't be safely resolved, instead
of emitting SQL that may be broken. The caller already has an existing,
tested degradation path for "no DBIC relationship info" (prefetch the
association directly; to_api falls back to the Perl-level ->count call;
sorting on it is rejected with a clean 400) - unresolved nested
correlations now go through that same path rather than risking a 500.

Also:
- Extract the repeated "_resultset vs _result" result-source lookup
  into a shared _result_source() helper (was duplicated in
  dbic_merge_prefetch and _build_count_subquery).
- Document the one-level-deep limitation in _build_count_subquery's POD.
- Add a regression test pinning the 2-level-nested-+count case: it must
  fall back to a plain prefetch (no COUNT subquery) rather than error
  or emit unsafe SQL.

Test plan:
1. ktd --shell
2. prove t/Koha/REST/Plugin/Query.t t/db_dependent/Koha/Object.t \
       t/db_dependent/Koha/REST/Plugin/Objects.t \
       t/db_dependent/api/v1/acquisitions_orders.t \
       t/db_dependent/api/v1/acquisitions_vendors.t \
       t/db_dependent/api/v1/ill_requests.t \
       t/db_dependent/api/v1/tickets.t \
       t/db_dependent/api/v1/patrons.t
=> All pass, no regressions.

Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-07-13 15:46:18 +00:00
Tomás Cohen Arazi f431fe0da0
Bug 42995: Fix nested +count subqueries with pagination and has_many
When nested +count embeds (e.g. biblio.items+count) are used alongside a
has_many prefetch (e.g. biblio.suggestions or the implicitly added
biblio.biblioitem) and pagination, DBIC's limiting subquery optimization
places +select scalar refs in the inner subquery where the parent join
alias (e.g. 'biblio') is not yet available.

This patch fixes _build_count_subquery to resolve the correlation back to
the main table (me) via the FK chain. Instead of generating:

  (SELECT COUNT(*) FROM items
   WHERE items.biblionumber = biblio.biblionumber)

It now generates:

  (SELECT COUNT(*) FROM items
   WHERE items.biblionumber = me.biblionumber)

This works because me.biblionumber IS the FK to biblio, holding the same
value. The main table columns are always available regardless of DBIC's
query structure.

Changes:
- Add $main_source parameter to _build_count_subquery for FK resolution
- Thread main_source through _parse_prefetch and dbic_merge_prefetch
- When parent_alias != 'me', resolve through parent relationship condition
- Update Query.t assertion to match the new correlation expression

Test plan:
1. Apply the regression tests from the previous commit
2. Run:
   $ ktd --shell
   k$ prove t/db_dependent/api/v1/acquisitions_orders.t
=> FAIL: The Bug 42995 subtest fails with 500 errors
3. Apply this patch, repeat from step 2
   k$ prove t/Koha/REST/Plugin/Query.t \
       t/db_dependent/Koha/Object.t \
       t/db_dependent/Koha/REST/Plugin/Objects.t \
       t/db_dependent/api/v1/acquisitions_orders.t \
       t/db_dependent/api/v1/acquisitions_vendors.t \
       t/db_dependent/api/v1/ill_requests.t \
       t/db_dependent/api/v1/tickets.t \
       t/db_dependent/api/v1/patrons.t
=> SUCCESS: Tests pass!
4. Sign off :-D

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-07-13 15:46:17 +00:00
Tomás Cohen Arazi e560b85c6f
Bug 42995: (regression tests) Nested +count embeds with has_many prefetch
This patch adds regression tests for the 500 Internal Server Error
triggered when GET /api/v1/acquisitions/orders is called with nested
+count embeds (e.g. biblio.items+count) combined with a has_many
prefetch (e.g. biblio.suggestions.suggester) and pagination.

The root cause is that DBIC's limiting subquery optimization places
+select scalar refs in the inner subquery where the parent join alias
(biblio) is not yet available, resulting in:
  DBD::mysql: Unknown column 'biblio.biblionumber' in 'WHERE'

The regression was introduced in:
  commit e51334604f
  Bug 41950: Make +count embeds sortable via SQL COUNT subqueries

Test plan:
1. Apply this patch
2. Run:
   $ ktd --shell
   k$ prove t/db_dependent/api/v1/acquisitions_orders.t
=> FAIL: The new subtest fails with 500 errors instead of 200
3. Sign off :-D

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-07-13 15:46:13 +00:00
Owen Leonard b0fa3d7e6a
Bug 42618: Incorrect sidebar menu link to MARC order accounts
The administration sidebar menu has an incorrect link for MARC ordering
accounts. Clicking the link takes you to EDI accounts. This patch fixes
it.

To test, apply the patch and run 'yarn build'

- Go to Administration -> Libraries (for example)
- In the sidebar menu, click the MARC ordering accounts link.
- You should be taken to the MARC ordering accounts page.

Sponsored-by: Athens County Public Libraries
Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Signed-off-by: Laura_Escamilla <laura.escamilla@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-07-01 09:34:47 +00:00
Caroline Cyr La Rose 2b2b4b74ba
Bug 42885: FineNoRenewals description mentions fines rather than charges
This patch fixes the terminology in the FineNoRenewals system preference
description to use the term 'charges' instead of 'fines'.

To test:

1. Apply patch
2. Go to Administration > System preferences
3. Search for FineNoRenewals
4. Make sure the description is gramatically correct and doesn't have
   contain typos

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Laura_Escamilla <laura.escamilla@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-07-01 09:34:46 +00:00
Chris Mathevet 4003a4aa90
Bug 42579: Added "no-export" class to ratings div
"Title" column of Checkout history export contains too much
informations, making the export file hard to read and understand.

Test plan :
1 - Connect on OPAC
2 - Go to "Your account"
3 - Go to "Checkout history"
4 - Click on "CSV"
5 - Open file
6 - Notice that the "title" column is too long and contains ratings
    informations
7 - Go back on OPAC
8 - Click on "Copy"
9 - Copy to a .txt file
10 - Notice the same problem as the CSV export
11 - Apply patch
12 - Repeat previous instructions
13 - Notice that the "title" column contains only the title

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Laura_Escamilla <laura.escamilla@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-07-01 09:34:45 +00:00
Kyle M Hall 839f3492bc
Bug 42571: Sending EDI order results in variable not available warnings
The warnings look like:

[2026/05/08 15:02:42] [32918] [WARN] Variable "@messages" is not available at /usr/share/koha/intranet/cgi-bin/acqui/basket.pl line 698.
[2026/05/08 15:02:42] [32918] [WARN] Variable "$op" is not available at /usr/share/koha/intranet/cgi-bin/acqui/basket.pl line 704.

Note in the fix, we are switching to the "our" keyword. When we declare
things with "our", and we run this under plack, those variables and
values persist across multiple requests! Due to this behavior we always
need to reinitialize anything that uses "our" unless we specifically
want that behavior ( like caching ).

Test plan:
1) Send an EDI order, note the warnings in the logs
2) Apply this patch
3) Restart all the things!
4) Send another EDI order, note the warnings are gone!

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-07-01 09:34:44 +00:00
Jonathan Druart 2472e423cb
Bug 42551: Fix QA failures
[FAIL] t/db_dependent/Koha/Plugins/C3.t
   FAIL   file_permissions
                File must have the exec flag
   FAIL   test_no_warnings
                'use Test::NoWarnings' must be present in this test file.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-07-01 09:34:42 +00:00
Kyle M Hall c79b0b9ef8
Bug 42551: Skip preload when running perl with -c
Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-07-01 09:34:41 +00:00
Kyle M Hall e48c124ac3
Bug 42551: Add unit test
Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-07-01 09:34:40 +00:00
Lisette Scheer 9be8db9c8a
Bug 41889: Update API to allow for null patron_id to be returned when listing checkouts
If a patron is deleted and there's a checkout associated with them, an api call to {url}/api/v1/checkouts?checked_in=true
will result in an error. This patch updates the api to allow for a null value to be returned for patron_id

To test:
Before applying the patch:

1. In Administration->System preference enable the RESTBasicAuth system preference.
2. Checkout an item to a patron (I used item barcode: 39999000000252 and patron barcode 23529000121682)
3. Check the item back in
4. In your brower go to {sandbox/ktdurl}/api/v1/checkouts?checked_in=1
5. Observe there is a patron ID in the JSON response
6. Delete the patron
7. Repeat step 4
8. Observe an error because a "null" value was returned
9. Apply patch
   Sandbox:
       In sandbox management Actions->Apply patches
       Once complete Actions->Build CSS and JS
       Actions->Restart services
   KTD:
       ktd --shell
       git bz apply 41889
       yarn build
       restart_all

10. Repeat step 4
11. Observe the checkout loads, but now has a null patron id.
12. Sign off
On KTD can also run the script: qa
to run tests.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-07-01 09:34:39 +00:00
Jonathan Druart cb47e6fad7
Bug 42278: Fix JS error
Uncaught SyntaxError: redeclaration of non-configurable global property ClaimReturnedLostValue

Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-07-01 09:34:35 +00:00
Jonathan Druart dd918e0de1
Bug 42522: Add link to biblio from eholding show view
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-06-29 16:20:19 +00:00
Jonathan Druart 4844c93a23
Bug 42290: Tidy all script tags - suggestion (auto tidy)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Jan Kissig <bibliothek@th-wildau.de>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-06-29 16:20:17 +00:00
Jonathan Druart e4293f93d2
Bug 42290: Tidy all script tags - suggestion
In order to tidy all the script tags of the .tt files we isolate the
interpolation of Template::Toolkit variables in a separate script tag.

This patch focuses on the suggestion module.

Trivial changes, the main changes have been done already, we only move
them to a separate script tag.

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Jan Kissig <bibliothek@th-wildau.de>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-06-29 16:20:16 +00:00
Jan Kissig 36e00f5aff
Bug 42283: (QA follow-up) Change icons
Changes the icon on concerns resolved filter to suit other filters set via AV TICKET_STATUS.

Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-06-29 16:20:14 +00:00
Paul Derscheid a4ec38d5fa
Bug 42283: (follow-up) URI-encode z3950 search params in GetZ3950Terms
The original template rendered each Z39.50 search param with the
"| uri" TT filter, so the generated query string was safe for
values containing spaces, ampersands or other reserved characters.

The JS port concatenates the raw values, producing a broken query
string when any search field (title, author, subject, etc.) is not
already URL-safe.

Wrap name and value in encodeURIComponent to restore the original
behaviour.

To test:
- Import or create a biblio whose title or author includes a space
  and an ampersand (e.g. "Q & A: Practical tests").
- From the cataloging search results, trigger the Z39.50 search for
  that record.
- Confirm the request URL includes correctly encoded parameters
  (space as %20, & as %26) and that the Z39.50 results load.

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Jan Kissig <bibliothek@th-wildau.de>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-06-29 16:20:13 +00:00
Paul Derscheid c7422c5c78
Bug 42283: (follow-up) Fix checkbox selector in concerns status filter
Porting the per-status click handler out of the TT loop dropped the
"hide_" prefix on the checkbox selectors:

- the `is(":checked")` test looked at `#<av>` instead of
  `#hide_<av>`, so the branch that removes the filter never fired
- the toggle line read `!$("#hide${av}")` (no underscore) before
  writing `#hide_<av>`, so the checkbox state was always forced to
  true and further clicks stopped toggling

Restore `#hide_${av}` in both places so the filter toggles match the
original TT-rendered selectors (IDs `hide_<av>` on the hidden
checkboxes).

To test:
- Set up TICKET_STATUS authorised values and enable CatalogConcerns
  and OpacCatalogConcerns.
- Create concerns in multiple statuses, open
  /cgi-bin/koha/cataloguing/concerns.pl.
- Click a "Filter <status>" link: rows with that status disappear.
- Click the same link again: it flips back to "Include <status>"
  and rows reappear. Repeat across several statuses.

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Jan Kissig <bibliothek@th-wildau.de>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-06-29 16:20:12 +00:00
Jonathan Druart 2cf1f980d0
Bug 42283: Tidy all script tags - cataloguing (auto tidy)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Jan Kissig <bibliothek@th-wildau.de>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-06-29 16:20:11 +00:00
Jonathan Druart 0aeb26417e
Bug 42283: Tidy all script tags - cataloguing
In order to tidy all the script tags of the .tt files we isolate the
interpolation of Template::Toolkit variables in a separate script tag.

This patch focuses on the cataloguing module.

The 3 main changes are in the following files:
* addbooks.tt
* additem.tt
* concerns.tt

There are non-trivial changes in concerns.tt. You need to create several
authorised values for TICKET_STATUS, turn on CatalogConcerns and OpacCatalogConcerns, create several concerns and go to /cgi-bin/koha/cataloguing/concerns.pl
Confirm that the filters are working correctly

Note that cataloguing/addbiblio.tt is on a separate bug (bug 42284)

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Jan Kissig <bibliothek@th-wildau.de>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-06-29 16:20:10 +00:00
Jan Kissig 5988883898
Bug 42282: (QA follow-up) Split JS to TT block in results, auto tidy
Splits JS to TT and non TT block. Auto tidy

Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-06-29 16:20:09 +00:00
Jan Kissig c9bfc66026
Bug 42282: (QA follow-up) Split JS to TT block in advsearch, auto tidy
Splits JS to TT and non TT block. Auto tidy

Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-06-29 16:20:08 +00:00
Jan Kissig a0c2e80678
Bug 42282: (QA follow-up) restore searchid in stockrotation
This patch sets the TT param searchid to enable browsing though search results on stockrotation.

Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-06-29 16:20:07 +00:00
Jan Kissig 3bf91a73dc
Bug 42282: (QA follow-up): Fix call of HTML5MediaEnabled and HTML5MediaSets.size
This switches the assignment of Koha.prefs.HTML5MediaEnabled from a non existant template param HTML5MediaEnabled to Koha.Preference('HTML5MediaEnabled').
Also a straying template param HTML5MediaSets.size was left in a JS block.

Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-06-29 16:20:05 +00:00
Jonathan Druart 8147f968aa
Bug 42282: Avoid JS error
Fix conflict with "Bug 42177: bundlesEnabled fix"
SyntaxError: redeclaration of const bundlesEnabled

Signed-off-by: Jan Kissig <bibliothek@th-wildau.de>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-06-29 16:20:04 +00:00
Paul Derscheid 9db99a08e2
Bug 42282: (follow-up) Switch Coce timeout log to console.warn
qa script flags the stray console.log in the Coce wait-for-images
helper under forbidden_patterns. The line is a genuine soft-failure
diagnostic (Coce never resolved after 5s), matching the
console.warn pattern used for AJAX failures elsewhere in the staff
templates.

To test:
- qa script no longer flags forbidden_patterns on detail.tt.
- With IntranetCoce enabled but Coce unreachable, loading a biblio
  logs "Could not retrieve the images" as a warning after ~5s.

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Jan Kissig <bibliothek@th-wildau.de>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-06-29 16:20:03 +00:00
Paul Derscheid 774c8d1e94
Bug 42282: (follow-up) Fix broken mailto concatenation in Elasticsearch 404 modal
The port folded the admin email into a single string literal
    " <a href='mailto: + Koha.prefs.KohaAdminEmailAddress'>"
so the href became the literal text "mailto: + Koha.prefs..." instead
of mailto:<admin email>.

Close the string and concatenate Koha.prefs.KohaAdminEmailAddress.

To test:
- Set KohaAdminEmailAddress syspref.
- SearchEngine = Elasticsearch, open a biblio detail page.
- Trigger the Elasticsearch preview with a bad id so the 404 branch
  renders, and confirm the "Send email" link points at mailto:<email>.

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Jan Kissig <bibliothek@th-wildau.de>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-06-29 16:20:02 +00:00
Jonathan Druart f645702cd8
Bug 42282: Tidy all script tags - catalogue (auto tidy)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Jan Kissig <bibliothek@th-wildau.de>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-06-29 16:20:01 +00:00
Jonathan Druart ee299e4b86
Bug 42282: Tidy all script tags - catalogue
In order to tidy all the script tags of the .tt files we isolate the
interpolation of Template::Toolkit variables in a separate script tag.

This patch focuses on the catalogue module.

The 7 main changes are in the following files:
* ISBDdetail.tt
* MARCdetail.tt
* detail.tt
* imageviewer.tt
* labeledMARCdetail.tt
* moredetail.tt
* stockrotation.tt

Several identical changes (logged_in_user_borrowernumber, searchid,
biblio_id, browser, etc.)
detail.tt needs to be tested a bit more thoroughly but the changes are
still trivial.

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Jan Kissig <bibliothek@th-wildau.de>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-06-29 16:20:00 +00:00
Owen Leonard ea62f3b835
Bug 40610: Update OPAC templates to improve ease of translation
This patch updates the markup structure of some OPAC include files in
order to make the translation process easier.

To test, apply the patch and test that the correct strings are
translatable. In this example I'm testing fr-FR:

- Update a translation:

  > gulp po:update --lang fr-FR
  > perl misc/translator/translate update fr-FR

- Open the corresponding .po file for the strings pulled from
  the include file and locate the updated strings.

  - misc/translator/po/fr-FR-messages.po
    - opac-tmpl/bootstrap/en/includes/biblio-title.inc lines 4 & 5
    - opac-tmpl/bootstrap/en/includes/date-format.inc lines 4, 6, 8
    - opac-tmpl/bootstrap/en/includes/holds-table.inc lines 132 & 135
    - opac-tmpl/bootstrap/en/includes/masthead.inc line 498

  - misc/translator/po/fr-FR/fr-FR-opac-bootstrap.po
    - opac-tmpl/bootstrap/en/includes/biblio-title-head.inc
    - opac-tmpl/bootstrap/en/includes/holds-table.inc line 129 & 141

- Edit the "msgstr" string however you want (it's just for testing).
- Remove the "fuzzy, " text if it's there.
- Install the updated translation:

  > perl mis/translator/translate install fr-FR

- Switch to your translated language in the OPAC.
- Confirm that the strings you translated show up correctly on these
  pages:
  - The page title when viewing a bibliographic record detail page for
    a record without a title.
  - On a user summary page, under the hold tab:
    - A hold that is waiting
    - A hold that is in transit
- In Administration -> Identity providers define a new identity provider
  (it doesn't have to be functional).
  - In the OPAC, test that the "Log in with [identity provider]" link is
    correct in each of the three login forms:
    - The modal triggered from the link in the header
    - The form in the right-hand sidebar
    - The form you see if you navigate directly to a user-related page
      e.g. /cgi-bin/koha/opac-user.pl
- The changes to date-format.inc are only visible if JS is turned off
  because Flatpickr hides the format hint when it loads.

Sponsored-by: Athens County Public Libraries
Signed-off-by: Laura_Escamilla <laura.escamilla@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-06-29 16:19:59 +00:00
Jan Kissig f7941aee28
Bug 42278: (QA follow-up) Fix HIGHHOLDS to JSON serialisation
Using Koha::Template::Plugin::To to serialize HIGHHOLDS to JSON led to the following error:  Template process failed: undef error - encountered object '2026-05-31T23:59:00', but neither allow_blessed, convert_blessed nor allow_tags settings are enabled (or TO_JSON/FREEZE method missing) at /kohadevbox/koha/Koha/Template/Plugin/To.pm line 32.
This patch fixes the handling of HIGHHOLDS.

Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-06-29 16:19:57 +00:00
Jan Kissig e0aee23fc1
Bug 42278: (QA follow-up) Fix Uncaught TypeError in request-article
This fixes an Uncaught TypeError when searching for a patron and the search only returns a single patron.

To test:
 a) go to /cgi-bin/koha/circ/request-article.pl?biblionumber=262
 b) ienable dev tools console
 c) enter 'acev' and click Search
 d) The following error appears: Uncaught TypeError: can't access property "on", patrons_table is undefined
 e) apply patch and redo a) to c). The error is not thrown.

Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-06-29 16:19:56 +00:00
Jan Kissig c379c8301d
Bug 42278: (QA follow-up): Split JS to separate TT and tidy
This puts ArticleRequestsSupportedFormats in a separate script tag. Auto tidy

Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-06-29 16:19:55 +00:00
Paul Derscheid 0361fee0e4
Bug 42278: (follow-up) Guard bare-RHS TT interpolations
Several unquoted const assignments rendered to `const X = ;` when
their TT expression was empty (patron.borrowernumber without a
patron, CAN_user_circulate_manage_bookings absent, or the
AlwaysLoadCheckoutsTable syspref unset). The parser aborted the
whole script block, so has_patron, AlwaysLoadCheckoutsTable, etc.
were never defined and later scripts failed with ReferenceError.

Add TT fallbacks so the RHS is always a valid JS literal.

To test:
- /cgi-bin/koha/circ/circulation.pl with no patron → no console errors
- Search a patron, check out an item → still works

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Jan Kissig <bibliothek@th-wildau.de>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-06-29 16:19:54 +00:00
Paul Derscheid 72443454b2
Bug 42278: (follow-up) Restore findborrower autofocus when no patron loaded
The JS port inverted the original UNLESS condition, so the patron
search field was focused only after a patron was already selected.
Flip back to !borrowernumber.

To test:
- /cgi-bin/koha/circ/circulation.pl with no patron → cursor in findborrower
- Select patron, reload → no focus jump

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Jan Kissig <bibliothek@th-wildau.de>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-06-29 16:19:53 +00:00
Jonathan Druart 7cca8e81a2
Bug 42278: Tidy all script tags - circ (auto tidy)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Jan Kissig <bibliothek@th-wildau.de>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-06-29 16:19:52 +00:00
Jonathan Druart 650a8d8a64
Bug 42278: Tidy all script tags - circ
In order to tidy all the script tags of the .tt files we isolate the
interpolation of Template::Toolkit variables in a separate script tag.

This patch focuses on the circulation module.

The 4 main changes are in the following files:
* circulation.tt
As expected this one is messy to test, we need to look at the diff and
make sure the different features linked with the variables moved at
still working correctly (yes, it's vague!)
* circulation_batch_checkouts.tt
On the batch checkout table the due date hidden input should have the
value of the return date high demand items.
* overdue.tt
On the overdue report view, the "Return claims" column must be hidden if
ClaimReturnedLostValue is not set.
* transfers_to_send.tt
The transfers to send view should show all libraries

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Jan Kissig <bibliothek@th-wildau.de>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-06-29 16:19:51 +00:00
Pedro Amorim acfd02af12
Bug 41950: (QA follow-up): Add more tests
The tests were lacking a 'real' scenario proving the
sorting on a _counts works at the DB level

prove t/Koha/REST/Plugin/Query.t

Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-06-29 16:19:50 +00:00
Tomás Cohen Arazi 713c5c3343
Bug 41950: (follow-up) Add DBIC relationship aliases for +count embeds
Adds DBIC relationship aliases so that +count embeds advertised in
the API specs are backed by actual database relationships, making
them sortable via the SQL COUNT subquery infrastructure.

New aliases:
- Borrower: checkouts (issues)
- Aqbookseller: baskets (aqbaskets), invoices (aqinvoices)
- Ticket: updates (ticket_updates)
- ErmEholdingsPackage: resources (erm_eholdings_resources)
- Biblio: holds (reserves)

Koha methods are updated to use the new relationship names directly,
following the one-liner style: Koha::Foo->_new_from_dbic( scalar ... )

Note: checkouts on Borrower and subscriptions on Aqbookseller already
had matching DBIC relationships. comments on Illrequest, requests on
IllBatch, and items on Biblio also already existed.

Test plan:
1. Apply patch
2. Run:
   $ ktd --shell
   k$ prove t/Koha/REST/Plugin/Query.t \
       t/db_dependent/api/v1/patrons.t \
       t/db_dependent/api/v1/acquisitions_vendors.t \
       t/db_dependent/Koha/REST/Plugin/Objects.t \
       t/db_dependent/Koha/Biblio.t
=> SUCCESS: All tests pass
3. Verify sorting works:
   GET /api/v1/patrons
   x-koha-embed: checkouts+count
   _order_by: -me.checkouts_count
=> SUCCESS: Patrons sorted by checkout count
4. Sign off :-D

Assisted-by: Sonnet 4.6 (Anthropic)
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-06-29 16:19:49 +00:00
Tomás Cohen Arazi e51334604f
Bug 41950: Make +count embeds sortable via SQL COUNT subqueries
Currently, +count embeds (e.g. claims+count) are computed in Perl via
$object->$relation->count during to_api serialization. This means they
cannot be used in _order_by and each count triggers a separate SQL query
per row (N+1).

This patch makes dbic_merge_prefetch detect +count embeds and, when a
matching DBIC relationship exists, inject a correlated COUNT subquery
via +select/+as instead of adding a prefetch.

The order_by entries referencing count aliases are fixed up to strip the
me. prefix, since +as aliases are virtual columns not bound to a table.

In to_api, the is_count handler now checks for the pre-computed column
via get_column before falling back to the Perl-level ->count call. This
preserves backward compatibility for Koha-level methods that don't have
a corresponding DBIC relationship.

Test plan:
1. Apply patches
2. Run:
   $ ktd --shell
   k$ prove t/Koha/REST/Plugin/Query.t \
       t/db_dependent/Koha/Object.t \
       t/db_dependent/Koha/REST/Plugin/Objects.t
   => SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-06-29 16:19:48 +00:00
Tomás Cohen Arazi 9bab898922
Bug 41950: Unit tests for sortable +count embeds
Add tests to verify that +count embeds produce +select/+as attributes
instead of a prefetch, enabling SQL-level sorting by count columns.

Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-06-29 16:19:45 +00:00
Marcel de Rooy 1c9e5ed219
Bug 42321: Improve handling of empty Z3950/SRU search results
Skips the empty results; only reports the 'real failures'.
Includes adding a grand total of hits to the form.
Some preliminary work to remove the hardcoded 20 results per target
in the code.
This is not yet a complete solution, but an improvement. The code in
C4/Breeding needs a larger overhaul. (And there might be an issue on
the Folio server side at LOC.)

Test plan:
Without this patch:
Use the LOC SRU server (since if frequently generates errcode 61).
Try the same search several times in a row (with Go). You probably will
come across the "LOC SRU record x: 0" messages. (Random behavior.)
Apply this patch.
Repeat the above. Notice that you will sometimes see the error 61 (and the
tip to refresh) but not the code 0 messages.
Run t/db../Breeding.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-06-29 15:39:10 +00:00
Olivia Reynolds ea67056197
Bug 38621: Use GetPreparedLetter in EmailReport
As with guided reports through the admin interface, this patch switches
EmailReport to use the same function for processing templates. This
gives patron emails sent via cronjobs access to the same variables as
other report locations.

Test plan:
1. Under Tools > Notices and Slips, create a new notice in the Reports
   module. Choose a code and a name, and under Email assign a subject
   and the following body:

Date: [% today %]

2. Under Reports, create a new report from SQL with a name and the
   following query:

SELECT 1 AS borrowernumber, 'to@example.com' AS email

3. Run the following command from the base of Koha, replacing CODE with
   the notice code created earlier, and ID with the report ID created
   earlier:

misc/cronjobs/patron_emailer.pl -module report -from from@example.com -notice CODE -report ID

4. Verify that the report runs successfully, and the console output
   includes "Date:" on a line with nothing following it.
5. Apply patch.
6. Run the following command from the base of Koha, replacing CODE with
   the notice code created earlier, and ID with the report ID created
   earlier:

misc/cronjobs/patron_emailer.pl -module report -from from@example.com -notice CODE -report ID

7. Verify that the report runs successfully, and the console output
   includes "Date:" on a line with the current time following it.
8. Run `prove t/db_dependent/Reports/Guided.t`, and verify it passes.

Sponsored-by: Open Fifth
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-06-29 15:39:09 +00:00
Olivia Reynolds cbbfa8c8d2
Bug 38621: Use GetPreparedLetter for report templates
Previously, report templates were rendered using process_tt directly.
This made the available variables inconsistent with other notices and
slips. This patch switches to rendering report templates through the
same function as other notices. To match the arguments to this system,
this patch also makes the report system reference templates by their
code rather than ID.

Test plan:
1.  Under Tools > Notices and Slips, create a new notice in the Reports
    module. Choose a code and a name, and under Email assign a subject
    and the following body:

[% today %]
[% FOREACH d IN data %]
[% d.title %]
[% END %]

2.  Under Reports, create a new report from SQL with a name and the
    following query:

SELECT * FROM biblio LIMIT 1

3.  Create another report from SQL with a different name and the
    following query, specifying a runtime parameter:

SELECT * FROM biblio WHERE 'a'=<<Param>> LIMIT 1

4.  On the first report, select "Run with template" and choose the
    template created earlier.
5.  Verify that the report runs successfully, and the output does not
    include the current date.
6.  On the second report, select "Run with template" and choose the
    template created earlier. Choose "a" as the runtime parameter value
    when asked.
7.  Verify that the report runs successfully, and the output does not
    include the current date.
8.  Apply patch.
9.  On the first report, select "Run with template" and choose the
    template created earlier.
10. Verify that the report runs successfully, and the output now
    includes the current date along with the fetched data.
11. Under the Download dropdown, select "Rendered template (...)".
12. Verify the downloaded file matches the report output.
13. On the second report, select "Run with template" and choose the
    template created earlier. Choose "a" as the runtime parameter value
    when asked.
14. Verify that the report runs successfully, and the output now
    includes the current date along with the fetched data.
15. Under the Download dropdown, select "Rendered template (...)".
16. Verify the downloaded file matches the report output.

Sponsored-by: Open Fifth
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-06-29 15:39:08 +00:00
Jonathan Druart d77897c203
Bug 42453: Remove warning
[WARN] Use of uninitialized value $index_count in numeric ne (!=) at /kohadevbox/koha/about.pl line 899.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-06-29 15:39:07 +00:00