We had a local method for getting opac_suppressed in Koha::Biblio which
relied on extracting from the MARC record. Now that we have an
opac_suppressed field in the Biblio table, we no longer need this
wrapper method.
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Jenkins failed with:
21:50:07 koha_1 | # got : '2025-06-26T19:50:06Z'
21:50:07 koha_1 | # expect : '2025-06-26T19:50:07Z'
A guess is that we change second between the 2 dt_from_string calls, and
the comparison fails.
One thing however: it seems that we want to deal with timezone correctly
here, but if we change the timezone in koha-conf.xml, the tests fail...
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Change looks good, but does not work for another timezone.
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This patch adds comprehensive unit tests for the edititem() method in the
Standard ILL backend to establish baseline behavior before refactoring.
The tests cover:
- Form stage (initial load) - displays current attributes for editing
- Commit stage (form submission) - processes updates with validation
- Core field updates (title, author, year, etc.)
- Custom field handling (null-delimited key/value pairs)
- Attribute persistence after updates
These tests demonstrate the current raw SQL implementation behavior
and will ensure the upcoming refactoring maintains compatibility.
Current behavior tested:
- edititem() uses multi-stage workflow (init -> form -> commit)
- Validation requires 'type' and 'branchcode' fields
- Custom fields use null-delimited strings for multiple values
- Method returns 'create' (not 'edititem') in commit stage
- Raw SQL delete-all-and-recreate pattern for attribute updates
Test plan:
1. Apply patch
2. Run:
$ ktd --shell
k$ prove t/db_dependent/Koha/ILL/Backend/Standard.t
=> SUCCESS: Tests pass! Current edititem behavior documented
3. Tests establish baseline for upcoming refactoring
4. Sign off :-D
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Currently, ILL backends and plugins must implement custom logic to update
existing extended attributes on ILL requests, as the core extended_attributes()
method only supports adding new attributes and throws duplicate key errors when
attempting to update existing ones. This has led to code duplication across
multiple ILL plugins (Rapido, INNReach, SLNP) that all implement identical
add_or_update_attributes() methods, and forces the Standard backend to use
error-prone patterns when managing request metadata.
This patch adds add_or_update_attributes() method to Koha::ILL::Request that
provides a safe, efficient way to add new attributes or update existing ones
in a single operation, eliminating duplicate key errors and reducing code
duplication.
The method:
- Accepts a hashref of attribute type => value pairs
- Updates existing attributes only when values actually change
- Creates new attributes when they don't exist
- Skips undefined or empty values to avoid constraint errors
- Uses transactions for atomicity
- Returns the request object for method chaining
Test plan:
1. Apply patch
2. Run:
$ ktd --shell
k$ prove t/db_dependent/Koha/ILL/Request.t
=> SUCCESS: Tests pass! New method works correctly
3. Test adding new attributes works
4. Test updating existing attributes works
5. Test mixed add/update operations work
6. Test empty/undefined values are skipped
7. Sign off :-D
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This patch refactors the copyright clearance confirmation functionality
to follow proper separation of concerns by moving business logic from
the controller to the model.
Changes:
- Add set_copyright_clearance_confirmed() method to Koha::ILL::Request
- Add get_copyright_clearance_confirmed() method to Koha::ILL::Request
- Update controller to use dedicated method instead of direct attribute manipulation
- Update staff template to use the new dedicated accessor method
- Add comprehensive test coverage for both methods
Benefits:
- Improved separation of concerns (business logic in model)
- Consistency with existing patterns (get_type, get_type_disclaimer_value)
- Better reusability and testability
- Centralized attribute name management
Test plan:
1. Apply patch
2. Run:
$ ktd --shell
k$ prove t/db_dependent/Koha/ILL/Request.t
=> SUCCESS: Tests pass! New methods work correctly
3. Test ILL copyright clearance workflow still works:
- Enable ILLModule
- Create HTML customization for ILLModuleCopyrightClearance
- Create OPAC ILL request with copyright clearance
- Verify confirmation is saved and displayed in staff interface
4. Sign off :-D
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
PA amended: Test file name in commit message
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This patch:
* Removes unused logger object instantiation
* Adds missing Test::NoWarnings to the API tests
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This patch renames the entire API structure from SFTP-specific to generic
file transport terminology to reflect the polymorphic nature of the system.
API Changes:
- Endpoints: /config/sftp_servers -> /config/file_transports
- Parameters: sftp_server_id -> file_transport_id
- Definitions: sftp_server -> file_transport
- Controller: Config::SFTP::Servers -> Config::File::Transports
File Changes:
- api/v1/swagger/definitions/sftp_server.yaml -> file_transport.yaml
- api/v1/swagger/paths/config_sftp_servers.yaml -> config_file_transports.yaml
- Koha/REST/V1/Config/SFTP/Servers.pm -> Koha/REST/V1/Config/File/Transports.pm
- t/db_dependent/api/v1/sftp_servers.t -> file_transports.t
Updated References:
- swagger.yaml: paths, definitions, parameters, tags
- Template JavaScript: API URLs and variable names
- Controller: package name, variable names, error messages
- Tests: endpoints, parameters, descriptions
- API mapping: to_api_mapping field name correction
To test:
1. Apply this patch
2. Run: yarn api:bundle (to regenerate swagger bundle)
3. Run: prove t/db_dependent/api/v1/file_transports.t
4. All API tests should pass
5. Verify admin interface uses new API endpoints
6. Test CRUD operations work correctly
The API now consistently uses 'file_transports' terminology matching
the polymorphic file transport system that supports FTP, SFTP, and
future transport types.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This patch adds comprehensive tests to verify that Koha::File::Transports
correctly returns polymorphic objects based on the transport field value.
The tests cover:
- build_object() polymorphic creation
- search() with polymorphic resultset iteration
- find() with polymorphic object return
To test:
1. Apply this patch
2. Run:
$ ktd --shell
k$ prove t/db_dependent/Koha/File/Transports.t
=> FAIL: Tests fail - objects return base Koha::File::Transport
instead of polymorphic Koha::File::Transport::SFTP/FTP classes
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
We add triggers to store in the base class and then use them in the SFTP
transport class to write out the key file to the filesystem for later
use in connect.
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This patch adds polymorphic transport classes to Koha for FTP and SFTP
file transport configurations.
We introduce a base class for interacting with the base configuration
storage and subclasses to impliment the protocol specific transport
handling.
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
We update the testbuilder tests here to look for signs of a polymorphic
class map and use it to test the different class options for this base
class
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This patch adds a central FTP/SFTP management UI, with an associated
database object and class, plus the beginnings of an API. We can use
this code as a building block for adding FTP/SFTP connections to
other parts of the system, as well as to improve the usability of the
EDI transports. It is also now possible to test these connections
from within the Koha UI.
To test:-
You will need access to an FTP or SFTP server
Contact me if you need a disposable FTP/SFTP server
a) Go to Koha Administration
b) Notice no FTP/SFTP referenced on this page
c) APPLY PATCH, reset_all
d) Create new user, let's say sftpuser
e) Set permissions for new user to include:
1) catalogue
2) parameters -> manage_sftp_servers
f) Log in with this new user, go back to Koha Administration
1) Notice the FTP/SFTP servers option
g) Undo step e2) for the sftpuser via the koha admin user
1) Refresh the Koha Administration page as the sftpuser
2) Notice how FTP/SFTP servers has disappeared
h) Back to koha admin, go to Koha Administration -> FTP/SFTP Servers -> New
i) Complete form depending on the FTP or SFTP server you have access to, Submit
1) Notice you are prompted to confirm before saving
j) Select Edit, observe your details are the same, and Submit again
k) Repeat step J one additional time
l) Select Test, observe all tests passing
m) Select Delete, confirm deletion, and observe server disappear
n) Optionally, run prove on new tests
1) t/Koha/Auth/Permissions.t
2) t/db_dependent/Koha/SFTP/Server.t
3) t/db_dependent/api/v1/sftp_servers.t
o) SIGN-OFF
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
# 'Details for opac holds test place hold button › Koha online catalog'
# doesn't match '(?^u:Details for: opac holds test place hold button)'
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
We currently have a sysprefs named "Elasticsearch*" and some name only "ES*". This patch unifies the names to always start with "Elasticsearch".
Test plan:
* Start koha-testing-docker, open the staff interface and go to Administration.
* Enter "Elasticsearch" into the System Preference search box and click "search"
* You will find a bunch of sysprefs
* Now search for "ESPreventAutoTruncate", which should find one match
* Apply the patch
* run (in ktd) the script `updatedatabase`
* You should see output like
DEV atomic update /kohadevbox/koha/installer/data/mysql/atomicupdate/bug_39526_unify_syspref_variable_names_for_elasticsearch.pl [09:35:03]: Bug 39526 - Unify system preference variable names for Elasticsearch
Renamed system preference 'ES..' to 'Elasticsearch..'
* Go again to to sysprefs and search again for "Elasticsearch"
* you should now find "ElasticsearchPreventAutoTruncate"
* Search for "ESPreventAutoTruncate", which should show "No system preferences matched your search for .." (and the list of ALL sysprefs)
Please note that the qa script explains about installer/data/mysql/mandatory/sysprefs.sql, but the last line is not touched by this patch, so not sure why this happens.
Sponsored-by: HKS3
Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Test plan:
1) prove t/db_dependent/Koha/BackgroundJob/ImportKBARTFile.t
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
prove t/db_dependent/Koha/BackgroundJob/ImportKBARTFile.t
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Test:
1. Have a record (A) with 3 items.
2. Create an item group with 2 of these items.
3. Have a second record (B) with 4 items
4. Create an item group with 2 of these items.
5. Merge the two records together keeping record A
6. Results will show that all items are moved over to record A, but the item group from record B, is not there.
7. Apply patch
8. Create a new record (C) with 3 items.
9. Create an item group with 2 of these items
10. Merge this into record A
11. The item groups are moved!
Signed-off-by: Miranda Nero <mnero@oslri.net>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
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>
I have no idea why this is fixing the test!
Without this patch they fail:
t/db_dependent/Koha/EDI.t .. 1/5
# Failed test '14 log lines recorded for passed invoice file'
# at t/db_dependent/Koha/EDI.t line 725.
# got: '12'
# expected: '14'
# Failed test 'Warn recorded for transferring items'
# at t/db_dependent/Koha/EDI.t line 728.
# 'Unmatched item at branch:WID'
# doesn't match '(?^u:transferring.*)'
# Failed test 'Warn recorded for transferring items'
# at t/db_dependent/Koha/EDI.t line 728.
# ''
# doesn't match '(?^u:transferring.*)'
# Failed test 'Warn recorded for unmatched item'
# at t/db_dependent/Koha/EDI.t line 728.
# ''
# doesn't match '(?^u:Unmatched item at branch:.*)'
# Looks like you failed 4 tests of 32.
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Jenkins was failing on Koha_Master/3350
With this patch we won't rely on the order (timestamp are identical and
may not be sorted as expected)
23:56:28 koha-1 | # Failed test 'GetLogs returns results in the log viewer for the merge of 526'
23:56:28 koha-1 | # at t/db_dependent/Koha/Patron/Log.t line 67.
23:56:28 koha-1 | # got: 'G7X8jre jVr_oz8M (eQXWiW9b4jX16424uC3Chn) has been merged into Mwqx7a zkSkFX7 (y_L7flupzFShb8JWYVCwYGSuv)'
23:56:28 koha-1 | # expected: 'exgnQtw lLmxQL9eWY (nHc_VZdFHIXr90p6WrIijDPgFrz8JM38) has been merged into Mwqx7a zkSkFX7 (y_L7flupzFShb8JWYVCwYGSuv)'
23:56:28 koha-1 |
23:56:28 koha-1 | # Failed test 'GetLogs returns results in the log viewer for the merge of 527'
23:56:28 koha-1 | # at t/db_dependent/Koha/Patron/Log.t line 82.
23:56:28 koha-1 | # got: 'exgnQtw lLmxQL9eWY (nHc_VZdFHIXr90p6WrIijDPgFrz8JM38) has been merged into Mwqx7a zkSkFX7 (y_L7flupzFShb8JWYVCwYGSuv)'
23:56:28 koha-1 | # expected: 'G7X8jre jVr_oz8M (eQXWiW9b4jX16424uC3Chn) has been merged into Mwqx7a zkSkFX7 (y_L7flupzFShb8JWYVCwYGSuv)'
23:56:28 koha-1 | # Looks like you failed 2 tests of 4.
23:56:28 koha-1 |
23:56:28 koha-1 | # Failed test 'Test Koha::Patrons::merge'
23:56:28 koha-1 | # at t/db_dependent/Koha/Patron/Log.t line 113.
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
To test this on the UI, you can add the following line of code at the beginning on 'new' in SushiCounter.pm:
->{response}->{Report_Header}->{Release} = 6;
This will force the release to be '6'. Then you can perform a harvest using the UI. The background job will fail with the error message saying that counter release 6 is not supported
prove t/db_dependent/Koha/BackgroundJobs/ErmSushiHarvester.t
Signed-off-by: Eric Phetteplace <phette23@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
QA script still fails but I believe these are false positives caused by the tests files being moved
Signed-off-by: Eric Phetteplace <phette23@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Update original tests to consider COUNTER 5 specifically.
Add new test for COUNTER 5.1.
Signed-off-by: Eric Phetteplace <phette23@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
We are now considering more than one COUNTER release. Move test files accordingly.
prove t/db_dependent/Koha/ERM/EUsage/*
Signed-off-by: Eric Phetteplace <phette23@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This patch adds regression tests that demonstrate the bug in
CancelExpiredReserves where the holiday logic checks if today is a
holiday instead of checking if the hold's expiration date was a holiday.
The bug causes:
1. Holds that expired on non-holidays to remain active if the cleanup
script runs on a holiday
2. Holds that expired on holidays to be cancelled if the cleanup script
runs on a non-holiday
Test scenarios added:
- Hold expired on Monday (non-holiday), script runs on Tuesday (holiday)
- Hold expired on Wednesday (holiday), script runs on Thursday (non-holiday)
- Hold expired on Friday (non-holiday), script runs on Sunday (holiday)
- Hold expired on Saturday (holiday), script runs on Sunday (holiday)
These tests currently fail, demonstrating the bug exists. When the bug
is fixed, all tests should pass.
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This patch adds a new method to Koha::Calendar that returns a boolean indicating
whether there are any business days between two given dates (exclusive).
Implementation details:
* Returns 1 if any business days exist between start and end dates
* Returns 0 for consecutive days, same dates, or only holidays between
* Uses exclusive range (does not include start or end dates)
* Handles date comparison and timezone normalization
* Efficient early return when first business day is found
This is designed specifically for use cases like detecting missed
business days due to server downtime or scheduling issues.
To test:
1. Apply patch
2. Run:
$ ktd --shell
k$ prove t/db_dependent/Koha/Calendar.t
=> SUCCESS: All tests pass!
3. Sign off :-D
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
If you apply this patch before the other one:
prove t/db_dependent/Koha/Biblio.t
Will fail. Apply both patches. The above now passes.
This highlighted something. Following the test plan I can't reproduce
this anymore. In order to reproduce, the check-outs need to happen
before the bookings, otherwise when the check-outs happen on an existing
booking it'll 'complete' or 'cancel' the booking thus not causing the
error.
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This patch adds unit tests.. but it also adds some FIXME and QUESTION
statements to those tests
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Without the patch:
t/db_dependent/Circulation/NoIssuesChargeGuarantees.t fails
t/db_dependent/Circulation/dateexpiry.t fails
With the patch:
t/db_dependent/Circulation/NoIssuesChargeGuarantees.t passes
t/db_dependent/Circulation/dateexpiry.t passes
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Without this patch:
t/db_dependent/Circulation/CanBookBeIssued.t failes
With this patch:
t/db_dependent/Circulation/CanBookBeIssued.t passes
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Without this patch:
prove -v /kohadevbox/koha/xt/use-test-no-warnings.t fails
With this patch:
prove -v /kohadevbox/koha/xt/use-test-no-warnings.t passes
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This commit adds two key improvements to the Bug 9762 circulation override logging:
1. **Consistent JSON Logging**: Updated AddRenewal to always log in JSON format,
matching AddIssue behavior. Previously AddRenewal only logged JSON when
confirmations/forced were present, otherwise logged just the item number.
2. **Integration Tests**: Added comprehensive tests for both AddIssue and AddRenewal
JSON logging functionality:
- Test normal operations (empty confirmations/forced arrays)
- Test override operations (populated confirmations/forced arrays)
- Verify JSON structure and content accuracy
The AddRenewal function now consistently logs:
```json
{
"issue": 123,
"itemnumber": 456,
"confirmations": [...],
"forced": [...]
}
```
This ensures uniform action log format across all circulation operations and
provides better integration test coverage for the override logging functionality.
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Martin Renvoize
Jonathan Druart
Tomás Cohen Arazi
Martin Renvoize
Jake Deery
Thomas Klausner
Matt Blenkinsop
Nick Clemens
Julian Maurice
Lucas Gass
Paul Derscheid
Pedro Amorim