Commit Graph

61918 Commits

Author SHA1 Message Date
Lucas Gass d74dea01ac
Bug 41604: (QA follow-up) Add a semicolon
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-07-16 10:54:14 +00:00
Jonathan Druart 1abfc554f0
Bug 41604: Correctly restore alert when bKohaColumnsUseNames is off
If we define visibility_condition we must have implemented
bKohaColumnsUseNames correctly.

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-07-16 10:54:13 +00:00
Jonathan Druart 26d0124c6b
Bug 41604: Hide column if the feature is disabled
Example: supplier_report on the basket page
Signed-off-by: Jason Robb <jrobb@sekls.org>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-07-16 10:54:12 +00:00
Jonathan Druart 2a34413601
Bug 41604: Save user decision per column when visibility changes
- Modify column-visibility.dt event handler to store the visibility
  preference of the user
- Improve _dt_force_visibility() to correctly pick the correctly visibility value depending
  on visibility_condition, is_hidden, and the user preference
- Add _dt_get_expected_column_visibility() to guess what's the expected visibility
- Force boolean true/false (vs 1/0) using !!
- Rely on a global variable kohaTableUserChoices to store the user
  choice. It will be part of the state and store in localStorage.

Signed-off-by: Jason Robb <jrobb@sekls.org>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-07-16 10:54:10 +00:00
Jonathan Druart 85ccb3d4d7
Bug 41604: Add one more Cypress test
Signed-off-by: Jason Robb <jrobb@sekls.org>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-07-16 10:54:09 +00:00
Jonathan Druart 4dab120d32
Bug 41604: Add Cypress tests
Signed-off-by: Jason Robb <jrobb@sekls.org>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-07-16 10:54:08 +00:00
Lucas Gass d20f7bc7a7
Bug 41604: (follow-up) Fix the visibility when visibility_condition is 0 or null, instead of false
Signed-off-by: Jason Robb <jrobb@sekls.org>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-07-16 10:54:07 +00:00
Jonathan Druart f1160e7a70
Bug 41604: Fix force display of a column
If a column must be displayed but was hidden before

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Jason Robb <jrobb@sekls.org>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-07-16 10:54:06 +00:00
Jonathan Druart 6e5ee1fdf7
Bug 41604: Fix typo
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Jason Robb <jrobb@sekls.org>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-07-16 10:54:05 +00:00
Jonathan Druart 060c8918d2
Bug 41604: QA fixes
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Jason Robb <jrobb@sekls.org>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-07-16 10:54:04 +00:00
Jonathan Druart 3efad3f74a
Bug 41604: take visibility_condition when save state is disabled
Signed-off-by: Jason Robb <jrobb@sekls.org>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Jason Robb <jrobb@sekls.org>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-07-16 10:54:03 +00:00
Jonathan Druart 4d2f963f1c
Bug 41604: Use visibility_condition in other places
There are 3 other places where we force the visibility of columns. They
must als use this new visibility_condition flag, so they are all using
it.

Test plan:
The 3 tables are:
1. basket with the columns edifact, modify, cancel
2. hold history with the column itemtype
3. batch edit patron with the columns checkbox, password expiration,
   protected

Signed-off-by: Jason Robb <jrobb@sekls.org>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Jason Robb <jrobb@sekls.org>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-07-16 10:54:02 +00:00
Jonathan Druart 7c688774f1
Bug 41604: Centralize visibility condition in datatables.js
We want to hide column if:
 * the feature is not accessible (pref disabled or insufficient
   permissions)
or
 * the column is hidden in the settings
or
 * the column has been hidden by the user

Signed-off-by: Jason Robb <jrobb@sekls.org>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Jason Robb <jrobb@sekls.org>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-07-16 10:54:01 +00:00
Hammat Wele 306ebc18ea
Bug 41604: Impossible to hide Checkin column in issues-table in circ/circulation.pl
Bug 41314 caused a regression, making it impossible to hide the Checkin column in the issues table in the patron's circulation page.

To recreate:
1. Hide the checkin column by default in circulation issues-table
   1.1. Go to Administration > Table settings > Circulation tab > Jump to page : circulation > Table id: issues-table
   1.2. Check 'Is hidden by default' for 'checkin'
   1.3. Click 'Save'
2. logout and login
3. Checkout an item to a patron
   3.1. In the search bar, set to Checkout, search for Henry
   3.2. In the checkout field, enter the barcode 39999000010114 and press enter
   --> In the table of issues, Checkin column is visible, it should be hidden by default
4. Apply the patch
5. Repeat step 2, 3
   --> In the table of issues, Checkin column is not visible

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Jason Robb <jrobb@sekls.org>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Jason Robb <jrobb@sekls.org>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-07-16 10:53:57 +00:00
Lari Taskula fe312b32d1
Bug 41796: "Forgot your password" link is not visible if OpacResetPassword is enabled but OpacPasswordChange is disabled
Before applying this patch, to reproduce the issue:
1. Set system preference OpacResetPassword to 'not allowed'
2. Set system preference OpacPasswordChange to 'Don't allow'
3. Set preference opacuserlogin to 'Allow'
4. Navigate to OPAC
5. Observe no "Forgot your password?" link on the OPAC main page
6. Set system preference OpacResetPassword to 'allowed'
7. Observe no "Forgot your password?" link on the OPAC main page
8. Set system preference OpacPasswordChange 'Allow'
9. Observe "Forgot your password?" link is present on the OPAC main page.

Instead, it should be already present when OpacResetPassword is allowed.

To test this patch:
1. Apply this patch
2. Set system preference OpacResetPassword to 'not allowed'
3. Set system preference OpacPasswordChange to 'Don't allow'
4. Set preference opacuserlogin to 'Allow'
5. Navigate to OPAC
6. Observe no "Forgot your password?" link on the OPAC main page
7. Set system preference OpacResetPassword to 'allowed'
8. Observe "Forgot your password?" link is present

Signed-off-by: Bo Gustavsson <bosse@gustavsson.one>
Signed-off-by: Laura_Escamilla <laura.escamilla@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-07-13 15:46:41 +00:00
Laura_Escamilla 787eb80e9d
Bug 42339: Make Record display customization cancel return to record display list
1. Go to the staff interface.
2. Go to Tools > Additional contents.
3. Select the Record display customizations tab/category.
4. Add a new Record display customization, or edit an existing one.
5. Click Cancel.
6. Confirm you are returned to:
   /cgi-bin/koha/tools/additional-contents.pl?category=record_display
7. Confirm you are not returned to the HTML customizations category.
8. Repeat with another Additional content category, such as Pages, and
   confirm its Cancel behavior is unchanged.
9. Sign off and have a great day! :D

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-07-13 15:46:40 +00:00
Jonathan Druart 1ed63c079d
Bug 42735: Fix SQL injection
This change fixes an SQL injection in the circulation stats.

Test plan:
0. Apply the patch
1. sudo koha-plack --restart kohadev
2. Go to /cgi-bin/koha/reports/issues_stats.pl
3. Try out a bunch of options
4. Try to break it
5. Note that you just get output and no errors

Signed-off-by: Sanjar Tulkinov Anvar o'g'li <sanjarbiy11@gmail.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:38 +00:00
Jonathan Druart 9e39e68a98
Bug 42369: Use C4::Context::csv_delimiter for delimiter validation
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-07-13 15:46:37 +00:00
David Cook 310201b520
Bug 42369: Fix SQL injection
This change fixes SQL injection in bor issues top

Test plan:
0. Apply patch
1. sudo koha-plack --restart kohadev
2. Go to /cgi-bin/koha/reports/bor_issues_top.pl
3. Try out all the different options
4. Try to break it
5. Note that you get output and no errors

Signed-off-by: Sanjar Tulkinov Anvar o'g'li <sanjarbiy11@gmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-07-13 15:46:36 +00:00
Jonathan Druart 44c7c10a19
Bug 42368: Use C4::Context::csv_delimiter for delimiter validation
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:35 +00:00
David Cook 0af6fbb7e7
Bug 42368: Fix SQL injection in issue avg stats
This change fixes SQL injection in issues avg stats

Test plan:
0. Apply the patch
1. sudo koha-plack --restart kohadev
2. Go to /cgi-bin/koha/reports/issues_avg_stats.pl
3. Try out all the different options
4. Try to break the report
5. Note that you always get output instead of errors

Signed-off-by: Sanjar Tulkinov Anvar o'g'li <sanjarbiy11@gmail.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:34 +00:00
Jonathan Druart 70f6545a53
Bug 42363: Use C4::Context::csv_delimiter for delimiter validation
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-07-13 15:46:33 +00:00
David Cook e7f36c5b87
Bug 42363: Fix SQL injection
This change cixes an SQL injection in cat stats

Test plan:
0. Apply patch
1. sudo koha-plack --restart kohadev
2. Go to /cgi-bin/koha/reports/catalogue_stats.pl
3. Try out different values
4. Try to break the report
5. Note that you just get output and not errors

Signed-off-by: Sanjar Tulkinov Anvar o'g'li <sanjarbiy11@gmail.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-07-13 15:46:32 +00:00
Jonathan Druart 5827f2f774
Bug 42360: Move separator validation to C4::Context::csv_delimiter
So we protect other calls

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-07-13 15:46:31 +00:00
David Cook 1d9c8fd390
Bug 42360: Also validate sep param
Signed-off-by: Sanjar Tulkinov Anvar o'g'li <sanjarbiy11@gmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-07-13 15:46:30 +00:00
David Cook 033acbe94e
Bug 42360: Fix SQL injection
This change fixes an SQL injection in the acq stats.

Test plan:
0. Apply the patch
1. sudo koha-plack --restart kohadev
2. Go to /cgi-bin/koha/reports/acquisitions_stats.pl
3. Try out a bunch of options
4. Try to break it
5. Note that you just get output and no errors
NOTE: In a fresh KTD you likely don't have any actual statistics

Signed-off-by: Sanjar Tulkinov Anvar o'g'li <sanjarbiy11@gmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-07-13 15:46:28 +00:00
Martin Renvoize 7a10a59e3d
Bug 43033: Sort unallocated task indices before compacting the retry matrix
The previous two commits on this bug fixed two ordering assumptions in
the RETRY loop of _allocateWithTransportCostMatrix(): resetting $inf at
the start of each attempt, and splicing unallocated task indices out of
@requests in descending order (removing a lower index first would shift
the position of any higher index still waiting to be removed).

This is a third instance of the same underlying problem, a few lines
earlier in the loop that compacts the working cost matrix $m before
that splice runs. @unallocated is built by iterating over agents (item
rows), pushing the column index Munkres assigned to each row that
turned out infeasible - and Munkres' row-to-column assignment is a
permutation, not a monotonic function of the row index, so
@unallocated is no more reliably sorted here than it was for the
splice.

That compaction loop scans matrix columns in ascending order and
advances a single pointer into @unallocated, expecting to meet each
entry in that same ascending order. When @unallocated arrives out of
order, the pointer never catches up: columns that should have been
dropped are left in place, and the columns after them are never
shifted left to fill the gap. From that point on, $m no longer lines
up with @requests, which *is* correctly reduced by the splice - so on
the next retry pass a hold's transport cost gets evaluated using a
different hold's matrix row. Depending on the exact misalignment, this
can silently drop a fillable hold from the queue while a genuinely
available item sits unused, or fill a hold via the wrong-cost item.

It didn't show up in the "many holds" regression test added earlier on
this bug, because that test's transport cost matrix is uniform (every
disabled pair costs the same); this defect only surfaces once the
columns competing for a scarce item have distinct costs.

Sort @unallocated once, ascending, and use that copy for the
compaction pass; reverse it for the splice, which needs descending
order.

Test plan:

1. Run the new t/db_dependent/HoldsQueue/RetryAllocationOrdering.t -
   it calls _allocateWithTransportCostMatrix() directly with a small
   hand-built scarce transport cost matrix and a pinned srand() (the
   specific item-to-branch permutation needed to trigger the bug
   depends on the shuffle() inside the routine). Reverting just this
   patch makes 3 of its 6 subtests fail: the L2 pickup hold is dropped
   even though an available item could have filled it.
2. Run t/db_dependent/HoldsQueue/TransportCostOptimizations.t to
   confirm no regression in the existing retry-loop coverage.

Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-07-13 15:46:27 +00:00
Andreas Jonsson f68c481391
Bug 43033: retry allocation with valid items
* The representation of the value "infinity" must be reset to -1 at the
  start of each allocation attempt.
* Unallocated items must be spliced out from the candidate arrays in
  reverse index order.

Test plan:

1. Run the unit tests in
   t/db_dependent/HoldsQueue/TransportCostOptimizations.t

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:26 +00:00
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