Commit Graph

6037 Commits

Author SHA1 Message Date
Jonathan Druart e9eb20249e
Bug 40541: Add missing new line at the end of files when missing
We have insert_final_newline = true to ensure files will contain a new
line at the end. However we have seen commits with "No new line" added
in the git log.

We should have a xt test to prevent more occurrences.

We should also have a test at the QA script level:
https://gitlab.com/koha-community/qa-test-tools/-/issues/96

Test plan:
  prove xt/find-missing-new-lines-at-the-end-of-file.t
should return green.

Remove the new line at the end of a file, eg.
vim -b mainpage.pl
:set noeol
:wq

Now
  prove xt/find-missing-new-lines-at-the-end-of-file.t
is failing on mainpage.pl.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2025-07-31 12:42:09 -06:00
Tomás Cohen Arazi 5c95c02476
Bug 39632: Make plugin install and upgrade errors more useful
This patch makes the install and upgrade process for plugins to add the
newly added extra parameters to the thrown exceptions.

This is covered by the tests

To test:
1. Apply this patch
2. Run:
   $ ktd --shell
  k$ prove -r t/Koha/Exceptions.t \
           t/db_dependent/Koha/Plugins
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Laura_Escamilla <laura.escamilla@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2025-07-28 16:26:42 -06:00
Tomás Cohen Arazi afd530ab0d
Bug 39632: Extend plugin exceptions with more useful parameters
This patch tweaks the `Koha::Exceptions::Plugin` class by adding an
extra parameter to the plugin install and upgrade related exceptions so
they carry the actuall error (if any).

* Koha::Exceptions::Plugin::InstallDied: install_error
* Koha::Exceptions::Plugin::UpgradeDied: upgrade_error

I didn't pick plain 'error' because it is a reserved word for
`Exception::Class` which makes it stringify using its value only.

To test:
1. Apply this patch
2. Run:
   $ ktd --shell
  k$ prove t/Koha/Exceptions.t
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Laura_Escamilla <laura.escamilla@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2025-07-28 16:26:42 -06:00
Marcel de Rooy 64b78fe3c4
Bug 40132: Remove POD for get_opac_news_by_id
We are removing a wrong method name at the same time ;)

Test plan:
Read the patch. Just removing a few outdated POD lines.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2025-07-28 16:26:36 -06:00
Pedro Amorim 6649b406c1
Bug 39091: Add cash registers list REST API endpoint
Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2025-07-23 16:36:40 -06:00
Matt Blenkinsop f30b6f668c
Bug 40417: Remove 'validation' method
We shouldn't use the ->validation method anymore as it isn't stable. We need to pass the params into the method accounting for any param changes made

Test plan:
1) Apply the regression test patch and run the test
2) Test should fail
3) Apply this patch
4) Run test again
5) Test should pass

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2025-07-23 16:36:39 -06:00
Tomas Cohen Arazi 17b851bff0
Bug 40424: Make query-in-body rely on `$c->req->json`
This patch removes the reliance on `$c->validation->output` for the
query-in-body feature. This will allow bug 40417 to move forward.

It also fixes some wrong design decisions we made 5-6 years ago. This
was discussed with the Mojolicious::Plugin::OpenAPI maintainer and the
agreement was `$c->validation->output` was not stable nor intended to be
accessed like we do.

To test:
1. Run:
   $ ktd --shell
  k$ prove t/Koha/REST/Plugin/Query.t \
           t/db_dependent/api/v1/*.t \
           t/db_dependent/Koha/REST/Plugin/*
=> SUCCESS: Tests pass!
2. Apply this patchset
3. Repeat 1
=> SUCCESS: Tests pass!
4. Sign off :-D

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2025-07-23 16:36:38 -06:00
Tomas Cohen Arazi 710bf4bb1b
Bug 40424: Simplify existing code - preparation
I wanted to isolate the changes for the query-in-body handling and it
made sense to refactor this dupicated code at the same time. So I add
this patch for tidying the area in preparation for the other part.

Duplicated code gets simplified.

To test:
1. Apply the unit tests patch
2. Run:
   $ ktd --shell
  k$ prove t/Koha/REST/Plugin/Query.t \
           t/db_dependent/api/v1/*.t \
           t/db_dependent/Koha/REST/Plugin/
=> SUCCESS: Tests pass!
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!
5. Sign off :-D

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2025-07-23 16:36:38 -06:00
Tomas Cohen Arazi 5e94e44ee2
Bug 40423: Add helper to stash x-koha-request-id header value
This patch aligns handling of the header with how other headers are
managed.

* A helper is added which reads the header and stashes the value
* The helper is called in the same place similar helpers are called
  (authenticate_api_request)
* The `objects.search_rs` helper is adjusted to use the stashed value
  instead of picking it from the reserved params.

To test:
1. Apply this patches
2. Run:
   $ ktd --shell
  k$ prove t/Koha/REST/Plugin/Query.t \
           t/db_dependent/api/v1/*.t \
           t/db_dependent/Koha/REST/Plugin/*
=> SUCCESS: Tests pass! No behavior change!
3. Sign off :-D

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2025-07-23 16:36:37 -06:00
Martin Renvoize ac3eced5d3
Bug 40387: Add Test::NoWarnings and replace carp with Koha::Logger
This patch adds Test::NoWarnings to our EDI unit tests. In doing so, it
reminded me that we still had some old carp style error handling in the
process_quote section and that we shoudl replace those with calls to
Koha::Logger instead.

Changes made:
1. Added Test::NoWarnings import to catch any warnings
2. Replace 'carp "Message"' appropriately with
   '$logger->warn("Message")'

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>
2025-07-23 16:36:34 -06:00
Martin Renvoize 68aa216317
Bug 40387: Fix undefined tax rate warnings in EDI invoice processing
When processing EDI invoices without TAX segments, warnings were
generated due to undefined tax rates in multiplication operations.

The tax_rate() method in Koha::Edifact::Line correctly returns undef
when no TAX segments are present in the EDI message. However, the
invoice processing code in Koha::EDI didn't handle this case properly.

This patch fixes the issue by:
- Adding checks for undefined tax rates before using them in calculations
- Defaulting to 0 tax rate when no tax information is present
- Ensuring tax_value calculations use 0 instead of undefined values

This eliminates "Use of uninitialized value in multiplication" warnings
while maintaining correct processing of invoices without tax information.

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>
2025-07-23 16:36:33 -06:00
Martin Renvoize ac266cad8b
Bug 40386: Remove warnings from EDI when running tests
This patch fixes several warnings that occur when running Edifact tests:

1. Changed `my $self = ()` to `my $self = {}` in Koha::Edifact constructor
   to properly initialize as hashref instead of empty list
2. Added null/empty checks in Koha::Edifact::Segment::_parse_seg() to prevent
   warnings when processing undefined or empty segments
3. Added null/empty checks in Koha::Edifact::Segment::_get_elements() to
   handle undefined segments gracefully
4. Added Test::NoWarnings to t/Edifact.t to catch any remaining warnings

These changes eliminate "Use of uninitialized value" warnings and ensure
clean test runs without affecting EDI functionality.

Test plan:
1. Run prove t/Edifact.t and verify no warnings are displayed
2. Test EDI message processing to ensure no regression

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2025-07-23 16:36:33 -06:00
Lucas Gass 8b494cbad2
Bug 40405: DBIC Schema changes
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2025-07-23 16:36:32 -06:00
Jonathan Druart 15a8f0444d
Bug 40371: Remove warnings from Budgets.t output
t/db_dependent/Budgets.t .. 153/156 Use of uninitialized value in multiplication (*) at /kohadevbox/koha/Koha/Acquisition/Order.pm line 543.
Use of uninitialized value in numeric eq (==) at /kohadevbox/koha/Koha/Acquisition/Order.pm line 554.
t/db_dependent/Budgets.t .. 154/156 Use of uninitialized value in multiplication (*) at /kohadevbox/koha/Koha/Acquisition/Order.pm line 543.
Use of uninitialized value in numeric eq (==) at /kohadevbox/koha/Koha/Acquisition/Order.pm line 554.
t/db_dependent/Budgets.t .. 155/156 Use of uninitialized value in addition (+) at /kohadevbox/koha/Koha/Acquisition/Order.pm line 510.

This change seems correct, but I am not 100% sure it won't introduce a
regression. What about 0 vs undef?

Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2025-07-23 16:36:30 -06:00
Baptiste Wojtkowski ff2fda90d4
Bug 40458: add pod coverage to Discharge.pm
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
[EDIT] Replaced borrowers in the ->request POD by borrower
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2025-07-21 14:01:34 -06:00
Martin Renvoize 097ffd0af9
Bug 40401: (follow-up) Fix grammar and rollback in tests
- Fixed grammar: "Returns true if the patron is the anonymous patron"
- Fixed test cleanup: use txn_rollback instead of txn_begin
- Fixed test subroutine closure syntax

Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2025-07-21 14:01:31 -06:00
Jonathan Druart e49602a87e
Bug 40401: Implement Koha::Patron->is_anonymous
The first motivation of this patch is to remove the following warnings
from t/db_dependent/Auth.t:
Use of uninitialized value $anonymous_patron in string eq at /kohadevbox/koha/C4/Auth.pm line 1497

Actually a correct fix seems to add a new `is_anonymous` method to
Koha::Patron in order to remove the warnings from everywhere else.

Test plan:
Confirm that you cannot log in using the anonymous patron and that the 2
tests are passing
  prove t/db_dependent/Auth.t t/db_dependent/Koha/Patron.t

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>
2025-07-21 14:01:31 -06:00
Jonathan Druart 45493d0f06
Bug 40410: Remove warning from Letters.
t/db_dependent/Letters.t .. 95/104 Use of uninitialized value in string eq at /kohadevbox/koha/Koha/Patron.pm line 389.

contactname can be NULL in DB.

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>
2025-07-18 11:09:59 -06:00
Jonathan Druart 1974dd9c91
Bug 40437: Remove warning from Koha/Installer.t
t/db_dependent/Koha/Installer.t .. 1/4
Use of uninitialized value $db_version in numeric eq (==) at
/kohadevbox/koha/Koha/Installer.pm line 32.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2025-07-18 11:09:57 -06:00
Doris Tam f72a235de0
Bug 38966: Fix wrong POD in Koha/CoverImages.pm and Koha/Acquisition/Order/Claims.pm
Test plan:
1. perldoc Koha/Acquisition/Order/Claims.pm
2. Check that it mentions Koha::Acquisition::Order::Claims
3. perldoc Koha/CoverImages.pm
4. Check that it mentions Koha::CoverImages.pm
4. These files no longer mention 'Koha::Cities'

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>
2025-07-18 11:09:55 -06:00
Tomas Cohen Arazi 6f1bd24e87
Bug 40419: Fix wrong license
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2025-07-18 11:09:54 -06:00
Jonathan Druart 3aa81fa654
Bug 40380: Remove warnings from Patrons/Import.t - Argument "" isn't numeric in numeric eq (==)
Using perl -MCarp::Always:
Argument "" isn't numeric in numeric eq (==) at /usr/share/perl5/DBIx/Class/Row.pm line 1017, <$handle_1> line 2.
DBIx::Class::Row::_eq_column_values(Koha::Schema::Result::Borrower=HASH(0x57c409b9ec08), "autorenew_checkouts", "", 1) called at /usr/share/perl5/DBIx/Class/Row.pm line 945

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>
2025-07-18 11:09:49 -06:00
Jonathan Druart e21d6c04a9
Bug 40380: Remove warnings from Patrons/Import.t - Use of uninitialized value $csvkeycol{"dateexpiry"} in array element
Use of uninitialized value $csvkeycol{"dateexpiry"} in array element at /kohadevbox/koha/Koha/Patrons/Import.pm line 310, <$handle_2> line 2.
Use of uninitialized value $csvkeycol{"dateexpiry"} in array element at /kohadevbox/koha/Koha/Patrons/Import.pm line 310, <$handle_3> line 2.
Use of uninitialized value $csvkeycol{"dateexpiry"} in array element at /kohadevbox/koha/Koha/Patrons/Import.pm line 310, <$handle_4> line 2.
Use of uninitialized value $csvkeycol{"dateexpiry"} in array element at /kohadevbox/koha/Koha/Patrons/Import.pm line 310, <$fh> line 2.

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>
2025-07-18 11:09:49 -06:00
Pedro Amorim 4bd5b58cd9
Bug 40171: Return fail if no transports for patron
Test plan, k-t-d, don't apply patches yet:
1) Enable ILLModule
2) Create a new ill request at:
<staff_url>/cgi-bin/koha/ill/ill-requests.pl?method=create&backend=Standard
3) Pick any type and any library, pick '42' for patron. This is the 'koha' user.
4) Create the request, click 'Send notice to patron'. Pick any template. Notice nothing is shown on the UI.
5) Apply patches. Repeat. Notice you get the message:
  "The requested notice was NOT queued for delivery by email, SMS".
6) Edit the 'koha' patron and tick the following boxes on 'Patron messaging preferences':
   - Interlibrary loan ready
   - Interlibrary loan unavailable
7) Go back to the request and click 'Send notice to patron' and pick any option again. Notice now you get a success message.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2025-07-16 14:31:41 -06:00
Jonathan Druart d6cbe0a25e
Bug 40351: Remove warning from oha/SearchEngine/Elasticsearch/Search.t
Use of uninitialized value in string comparison (cmp) at /kohadevbox/koha/Koha/SearchEngine/Elasticsearch/Search.pm line 536.

I think this is the correct fix. We generate a random av category that
is used in the data, so $label get undef:
521                 $label = $authorised_values{$t};

Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2025-07-15 16:32:56 -06:00
Jonathan Druart 12e4e92e53
Bug 40348: Remove warnings from api/v1/two_factor_auth.t
02:28:36 koha_1       | WARNING: The key derivation method "opensslv1" is deprecated. Using -pbkdf=>'pbkdf2' would be better.
02:28:36 koha_1       | Pass -nodeprecate=>1 to inhibit this message.
02:28:36 koha_1       |  at /kohadevbox/koha/Koha/Patron.pm line 3216.

We should deal with that on a separate bug report. For now we can remove
the warnings from the log/output.

Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2025-07-15 16:32:55 -06:00
Janusz Kaczmarek d0ce815600
Bug 40128: StripWhitespaceChars can create empty subfields
When StripWhitespaceChars is enabled and the user by chance enters
some white spaces in the input field, Koha generates empty subfield
(or even entirely empty field).

Test plan:
==========
1. Enable StripWhitespaceChars system preference.
2. Edit an existing record, putting just some spaces into an empty
   subfield. Save the record.
3. Go to bibliographic record Normal view and in modal "MARC preview"
   notice that the subfield you entered spaces in exists as an empty
   subfield. If it was the only subfield in a field, the field exists
   with no data at all. This is wrong.
4. Apply the patch ; restart_all.
5. Repeat 2. and 3. You should not see any empty subfield now. If the
   empty subbfield was the only subfield in a field, the field should
   have been removed from the record.

Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2025-07-15 16:32:53 -06:00
Martin Renvoize 5acbf16948
Bug 36947: (QA follow-up) Add missing POD documentation to Koha::I18N
The Koha::I18N module was missing POD documentation. This adds complete
documentation for all exported functions including usage examples and
descriptions.

Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2025-07-11 15:26:56 -06:00
Martin Renvoize 146aca25f5
Bug 36947: Add system preference for configurable locale-based facet sorting
This follow-up adds two major enhancements to the locale-based facet sorting
implementation:

1. **System Preference Integration**
   - Adds FacetSortingLocale system preference for configurable locale selection
   - Dynamically detects available system locales using Koha::I18N
   - Provides user-friendly dropdown with locale descriptions
   - Supports both Elasticsearch and Zebra search engines

2. **Zebra Search Engine Support**
   - Extends locale-based sorting to Zebra (in addition to existing Elasticsearch)
   - Implements _sort_facets_zebra() function in C4::Search
   - Maintains consistent Unicode-aware sorting behavior across search engines

Key Features:
- Dynamic locale detection via Koha::I18N::available_locales()
- Proper module architecture with comprehensive unit tests
- Clean fallback chain: preference → system LC_COLLATE → default
- Admin UI integration in System Preferences → I18N/L10N
- Universal facet sorting for international library systems

Test plan:
1. Apply patch and restart services
2. Install/run database update
3. Navigate to Administration → System Preferences → I18N/L10N
4. Verify "Sort facet names using" dropdown shows available system locales
5. Test facet sorting with different locales and both search engines
6. Run: prove t/Koha/I18N.t t/Koha/SearchEngine/Elasticsearch/Search.t t/db_dependent/Search_FacetSorting.t

Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2025-07-11 15:26:56 -06:00
David Cook b76df3a168
Bug 36947: Do a locale-based sort for ES facet names
This change uses a configurable locale-based collator to sort
the ES facet names.

Test plan:
0. Apply the patch
1. vi /etc/locale.gen
2. Uncomment the locale you want to generate (e.g. fi_FI.UTF-8 UTF-8)
3. locale-gen
4. vi "/etc/default/koha-common"
5. Add the following to the bottom of the file:
export LC_ALL=fi_FI.UTF-8
6. koha-plack --restart koha-common
7. Setup some test records with authors with accented and unaccented names,
    and different cases for the lead letter
e.g. Aa author, Åa author2, aa author, étienne
8. Switch to using Elasticsearch and reindex
koha-elasticsearch -b -v --rebuild kohadev
9. Do a test search
    e.g. http://localhost:8081/cgi-bin/koha/catalogue/search.pl?q=test
10. Confirm the facet names are sorted in ascending order following
Finnish collation rules
    e.g.
aa author
Aa author
étienne
Farley, David
Humble, Jez
Martin, Robert C.
Åa author

NOTE: Any collation and language can be used. Finnish is just
an example of a Latin-based script which has a different
alphabetical ordering than just A-Z

Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2025-07-11 15:26:55 -06:00
Martin Renvoize 5f025aee75
Bug 39545: (QA follow-up) Restore 240 field processing for subfield s
This patch restores the generation of subfield s from field 240 (Uniform Title)
in the MARC21 773 field generation, addressing cataloger feedback about the
critical importance of this field for:

- Bibles and books of the Bible
- Laws and statutes
- Translations
- Academic and specialized library materials
- Legal materials and treaties

The MARC21 standard defines subfield s as "Uniform title (NR)" and this data
is essential for proper bibliographic relationships.

This patch also fixes a warning when 245 indicator 2 contains non-numeric
values by adding proper validation.

Test plan:
1. Create a record with a 240 field containing uniform title data
2. Generate a child record from it
3. Verify the 773 field now contains subfield s with the uniform title
4. Run prove t/db_dependent/Koha/Biblio.t to ensure all tests pass

Sponsored-by: Open Fifth <https://openfifth.co.uk>
Signed-off-by: Heather Hernandez <flyingendpaper@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2025-07-11 15:26:54 -06:00
Martin Renvoize 610ad29bf8
Bug 39545: (follow-up) Apply same logic to Koha::Biblio
This patch updates the Koha::Biblio->generate_marc_host_field method to
more closely match the updated logic in C4::Biblio::prep_marc_host.

We change a number of things including:
 1) Constructinging the 773 subfield generation from a hash to an array
    to preserve field order.
 2) Updating main entry handling for 100/110/111 fields to clone the
    field and then remove relevant subfields as apposed to fetching only
    a limited set of subfields.
 3) Add handling to generate subfield 7 content depending on LDR and
    Main Entry information
 4) Improve handling for title subfield construction from 245 to respect
    non-filing characters and remove trailing punctuation
 5) Remove trailing punctuation from subfield b when generated from 250
 6) Remove generation of subfield s from the 240 field entirely
 7) Remove trailing punctuation from subfield d when generated from 260
 8) Add generation of subfield k from 800-830 fields

Rebased-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
We dropped the upstream aforementioned C4::Biblio method, but this patch
faithfully replicates it's functionality within
Koha::Biblio->generate_marc_host_field

Sponsored-by: Open Fifth <https://openfifth.co.uk>
Signed-off-by: Heather Hernandez <flyingendpaper@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2025-07-11 15:26:53 -06:00
Jonathan Druart 5b797b2560
Bug 40337: Do not set checkprevcheckout to an empty string when importing patrons
We must remove it so that it will be set as the default value defined
at the DBMS level

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2025-07-10 17:34:04 -06:00
Jonathan Druart b4a561c133
Bug 40337: Handle enum in Koha::Patron->anonymize
We need to reset to the default value. We might have a problem (later)
if it's NOT NULL and no default is set, which is not handled by this
patch. But the situation does not seem to exist yet anyway.

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2025-07-10 17:33:56 -06:00
Jonathan Druart 51473dfd36
Bug 40337: DBIC schema changes
Signed-off-by: Eric Garcia <cubingguy714@gmail.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2025-07-10 09:43:27 -06:00
Tomas Cohen Arazi 85b65afd1d
Bug 39876: Add build-git-snapshot type resolution
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2025-07-08 15:42:01 -06:00
Jonathan Druart 48263039af
Bug 39876: Apply module to other tests
We are ready to reuse Koha::Devel::Files in other places where
`git ls-files` is used.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2025-07-08 15:41:59 -06:00
Jonathan Druart e7e42c5e88
Bug 39876: Add context for tidy
With this patch we adjust what's needed to make misc/devel/tidy.pl run
identically as before

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2025-07-08 15:41:59 -06:00
Jonathan Druart 1d4071298c
Bug 39876: Move `git ls-files` to Koha::Devel::Files
This patch introduces a new Koha::Devel namespace. It clearly shows
that its purpose is for development and will not contain any business
logic.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2025-07-08 15:41:58 -06:00
Martin Renvoize 32860c2778
Bug 37305: (follow-up) Only add control number subfield when defined
The previous commit introduced a bug where the control number subfield 'w'
was being added to the MARC field even when undefined, causing test failures.

This patch adds a condition to only push the control number subfield when
the value is actually defined, matching the pattern used for other subfields
in the method.

Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2025-07-07 10:20:24 -06:00
Martin Renvoize 3d9778b2cd
Bug 37305: Subfield order matters
This patch updates the use of a hash to store and then pass subfields to
the MARC::Field new method to an array with key/value pairs using fat comma's.

This allows us to maintain the integrity check that we're passing string
as the first arguament in each pair whilst also allowing us to ensure
fields are passed in the order they are found.

Sponsored-by: Open Fifth <https://openfifth.co.uk>
Signed-off-by: Heather Hernandez <flyingendpaper@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2025-07-07 10:20:23 -06:00
Tomas Cohen Arazi afd00523a3
Bug 40254: Fix error in overrides check allowing too much
This patch fixes a subtle (yet naive) error making the controller not
care about blocking conditions if they were more than one and the first
one was overridden.

To test:
1. Apply the regression tests
2. Run:
   $ ktd --shell
  k$ prove t/db_dependent/api/v1/holds.t
=> FAIL: Tests fail! Some tests expect a 409 (denied for a conflict) but
return a 201!
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!
5. Sign off :-D

Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2025-06-30 08:30:45 -06:00
Tomas Cohen Arazi f245ad1605
Bug 39657: Add more checks and overrides to hold creation endpoint
This patch adds new behaviors to the `POST /holds` endpoint.

It uses the framework we created in the past for overriding policy
rules, and add new options:

* expired
* debt_limit
* bad_address
* card_lost
* restricted
* hold_limit

Some status codes are changed from 403 to 409. This should be revisited
accross the codebase, as I think we made a wrong choice. Happy to review
in this bug.

The feature makes use of the newly introduced
`$patron->can_place_holds()` method, which accepts to be passed through
the overrides.

To test:
1. Apply this patches
2. Run:
   $ ktd --shell
  k$ yarn api:bundle
  k$ koha-plack --restart kohadev
  k$ prove t/db_dependent/api/v1/holds.t
=> SUCCESS: Tests pass!
3. Test the endpoint with the various scenarios using your favourite
   REST tool (Postman!)
4. Sign off :-D

Signed-off-by: Kristi Krueger <kkrueger@cuyahogalibrary.org>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2025-06-26 12:47:52 -06:00
Tomas Cohen Arazi 61c59ac1b1
Bug 40101: Add `Koha::Patron->can_place_holds()`
This patch adds a new method to the `Koha::Patron` class.

As described in the POD, this method makes checks on the patron's
current situation and returns a boolean telling if it should be allowed
to place holds. This doesn't check specific item/biblio holdability. It
only covers patron specific conditions that could prevent holds to be
placed.

Both in `opac-reserve.pl` and `reserve/request.pl` this checks are all
run, and all blocking situations are presented to the UI user. Feedback
in community (so far) expressed this is a desirable situation, so that
behavior can be retained with the use of the `no_short_circuit`
parameter.

If `no_short_circuit` is not passed, then the routine will return on the
first blocking condition it finds.

An `overrides` parameter is added, allowing to be passed a list of
strings that match the possible error messages.

This way, this method can be asked not to check for patron expiration,
for example, and things like that. This is of course designed with the
API overrides use case in mind.

To test:
1. Apply this patches
2. Run:
   $ ktd --shell
  k$ prove t/db_dependent/Koha/Patron.t
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kristi Krueger <kkrueger@cuyahogalibrary.org>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2025-06-26 11:10:02 -06:00
Nick Clemens 3664281d37
Bug 39912: Update RealTimeHoldsQueue when hold pickup location is updated
When a hold's pickup location is changed we should rebuild the holds queue.

To test:
1 - Enable RealTimeHoldsQueue
2 - Enable LocalHoldsPriority where pickup library matches home library
3 - Place two biblio level holds on a bib with one item for two different patrons for delivery to a different library then the home library
4 - Check the queue - firstpatron should have priority
5 - Change the pickup location for the second hold to be picked up at the home library
6 - Check the queue, unchanged
7 - Change the pickup location back
8 - Apply patch, restart all
9 - Change the pickup location to the items home library again
10 - Check the queue
11 - Second patron has now been given priority

Signed-off-by: Michelle Spinney <mspinney@clamsnet.org>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2025-06-26 11:09:57 -06:00
Marcel de Rooy 0ed2b7cfa8
Bug 40242: Remove double operator in Quotes.pm
Test plan:
Read patch.
Run t/db_dependent/Koha/Quotes.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2025-06-26 11:09:56 -06:00
Marcel de Rooy 221126f3dd
Bug 39206: Add an allowlist to Koha::CookieManager
This patch adds a bit more control to what CookieManager does by
adding a hardcoded allowlist of cookie names that are cleared at
logout. Allowing at the same time to add entries to that list by
using koha-conf <remove_cookie> lines or removing entries from the
hardcoded list by using <do_not_remove_cookie> lines.

The patch fixes the expiration of cookies that should be removed
by passing max-age 0.

Also it adds a theoretical path correction for always_show_holds but
since we do not clear that cookie, it is currently unused. This seems
to be the only Koha cookie where we use a longer path.

Test plan:
Run t/CookieManager.t

Go to OPAC, login, select a few OPAC search results and send them
to cart. This would create cookie bib_list. (Check dev tools.)

Logout from OPAC and check cookie in your browser dev tools. What
you see, depends on the browser. But the cookie should be either
gone or empty and expired (FF: Session).

Now add a <do_not_remove_cookie> line for bib_list in koha-conf.
Restart all. Repeat search, add to cart. Logout. Check again in dev
tools that bib_list is not empty, not expired.

Check out an item. And click on 'Always show checkouts...' on
the patron checkout form. This should create the cookie with
value DO. Logout from intranet. Check that cookie was not affected.
Now add a <remove_cookie> line for the following cookie:
issues-table-load-immediately-circulation. Restart all.
Login and logout from staff again. Check that cookie is empty
and expired, or just gone.

Bonus for devs: Create some custom cookie, and test keeping or
removing it similar as above.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2025-06-26 11:09:54 -06:00
Lucas Gass ccf4bc0626
Bug 23010: Report on count of items not modified via batchMod
Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2025-06-24 16:10:20 -06:00
Lucas Gass c4acafe29b
Bug 23010: Update pref name to PreventWithdrawingItemsStatus
Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2025-06-24 16:10:20 -06:00
Lucas Gass b74dbf3d59
Bug 23010: Add ability to block withdrawning of items with certain statuses
Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2025-06-24 16:10:18 -06:00