Signed-off-by: Phillip Berg <phillip.berg@mainlib.org>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Add UNIQUE KEY entries to table definition
The UNIQUE KEY are required as a hold group 'might have' a target hold, not 'has many' target holds.
This update and the other things this patch updates in atomicupdate + kohastructure should fix the error thrown by command:
perl /kohadevbox/misc4dev/run_tests.pl --instance=kohadev --db-password=password --run-db-compare-only --compare-with=HEAD
This command no longer produces any errors.
Also in this patch:
Renamed table from 'hold_group_target_holds' to 'hold_groups_target_holds' and move it alphabetically to be after 'hold_groups' in kohastructure.sql
This is cleaner, as it matches the 'hold_groups' related table, and allows for alphabetical sorting on kohastructure.sql that makes sense i.e. hold_groups is created first, then hold_group_target_holds is created second with an FK to the former.
All of this passes:
prove ./t/db_dependent/Koha/Hold.t
prove ./t/db_dependent/Reserves/HoldGroup.t
prove ./t/db_dependent/Circulation*
prove ./t/db_dependent/Hold*
prove ./t/db_dependent/Reserves*
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This further rebases on top of 40058 which removed RevertWaitingStatus.
It also fixes a warning where 'eq was being utilized on initialized' coming from sub remove_as_hold_group_target
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
If a hold that is a hold group target, i.e. it's 'waiting' or 'in transit', is reverted it needs to remove itself as its own hold group target
Tests added here as well:
prove t/db_dependent/Koha/Hold.t
Signed-off-by: Anneli Österman <anneli.osterman@koha-suomi.fi>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
The skipping of non target holds was skipping all holds that are part of a hold group which has a target, including the target hold itself. This was wrong. Target holds need to be included as they are considered to be a regular 'found' hold.
Tests that prove the bug and its fix added to:
prove ./t/db_dependent/Koha/Hold.t
Signed-off-by: Anneli Österman <anneli.osterman@koha-suomi.fi>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
When a hold that belongs to a hold group becomes 'In transit' or 'Waiting',
it now becomes that hold group's target hold.
Subsequent check-ins that would satisfy other holds in the same hold
group are skipped, as they are part of a group that already has a
target.
The behavior change introduced here that differs from the groundwork
from bug 15516 are:
1) Before, only 'Waiting' holds would consider an action based on hold
groups i.e. cancel the other holds in the group. Now 'In transit'
also does this.
2) This hold groups related action used to be to cancel other holds in
the group. This is no longer the case. Now, the respective hold is
set as the hold group's target. This causes the holds in the same group that
are not the target to be skipped when a check-in that would otherwise
satisfy one of those holds happens. This behavior change allows the following
scenario which wasn't possible before:
A hold is marked 'Waiting'. It becomes its group target. For any reason,
that hold is then cancelled. Any subsequent check-in of an item of a
hold in this group may now becomes that group's new target.
Similarly, the same use case above is also true for 'In transit' holds.
----
The 'skip non-target holds' behavior has been added to check-in logic
(Reserves::_Findgroupreserve), 'holds to pull' logic
(pendingreserves.pl) and holds queue (HoldsQueue.pm).
Signed-off-by: Anneli Österman <anneli.osterman@koha-suomi.fi>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Hold group target hold:
The hold that is set as the hold group's target. If a hold group has a
hold target, subsequent check-ins that would 'trap' items to the
remaining holds in the group are instead skipped.
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
To test:
1. prove t/db_dependent/Koha/Item.t
2. Observe success
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
For reusability and to limit files to the git index
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Test plan:
Read the patch.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Aleisha Amohia <aleisha@catalyst.net.nz>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This adds the UI elements required to group selected holds from the
patron pages as well as 2 new concepts:
Visual group ID:
The visual group ID is designed to be easy to understand from a UI/UX
perspective and not show the user an indefinitely incremented value.
When holds are cancelled, filled or moved to a different group, if the
old group is left empty or with a single hold, the hold group is deleted.
Signed-off-by: Anneli <anneli.osterman@koha-suomi.fi>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Anneli <anneli.osterman@koha-suomi.fi>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
list and add actions:
/patrons/{patron_id}/hold_groups
New test file:
prove t/db_dependent/api/v1/patrons_hold_groups.t
Signed-off-by: Anneli <anneli.osterman@koha-suomi.fi>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Refactor: Update controller logic to make use of patron->create_hold_group
This ensures consistency when creating a new hold group for a patron,
regardless if on the Staff UI or REST API
Signed-off-by: Anneli <anneli.osterman@koha-suomi.fi>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This patch makes the Public Biblio endpoint respect the OpacSuppression
system preference and the biblio.opac_suppressed field introduced in
Bug 38330.
When OpacSuppression is enabled and a biblio has opac_suppressed set to 1,
the endpoint now returns a 404 (Not Found) response instead of exposing
the bibliographic record.
Test plan:
1. Apply patch
2. Run: prove t/db_dependent/api/v1/biblios.t
3. Verify all tests pass, including the new OpacSuppression subtest
4. Test manually:
- Enable OpacSuppression syspref
- Set a biblio's opac_suppressed field to 1
- Access /api/v1/public/biblios/{biblio_id}
- Confirm it returns 404
- Set opac_suppressed to 0
- Confirm it returns 200 with the record
- Disable OpacSuppression syspref
- Confirm record is visible regardless of opac_suppressed value
Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
The size of the commit id can vary, depending on the git repo.
For instance Jenkins had stored results using length of 9 (e9a07fe6a)
because it's a shallow clone.
But 9 is not enough for our repo:
% git rev-list --all | sort | uniq -w 9 -D
29dfcf49f28bb8e75ab4b71a2d68f194928d3de9
29dfcf49f6
We need to force the length for both commands we use, so commits matched
during the comparison.
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Development wise it is much simpler to do this check in perl when
opening the editor than getting this logic into the template with
javascript. Furthermore, older data may be corrupted, but we trust
the current plugins to not insert such fields anymore.
Test plan:
Run t/db_dependent/Koha/Biblio/Metadata/Extractor/MARC/MARC21.t
Open an existing record in the basic editor. Shorten 008 without plugin.
Save and open the record again. Do you get the alert?
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
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>
This adds two new methods to Koha::Patron for managing two-factor authentication:
* has_2fa_enabled() - Returns 1 if patron has 2FA enabled, 0 otherwise
* reset_2fa() - Clears secret and sets auth_method to 'password'
The reset_2fa method includes audit logging when BorrowersLog is enabled
and returns the patron object for method chaining.
To test:
1. Apply patch
2. Run:
$ ktd --shell
k$ prove t/db_dependent/Koha/Patron.t
=> SUCCESS: Tests pass!
3. Sign off :-D
Sponsored-by: ByWater Solutions
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>
As the title says, we are missing such endpoints and this patch adds
them.
To test:
1. Apply this patches
2. Run:
$ ktd --shell
k$ yarn api:bundle
k$ prove t/db_dependent/api/v1/patrons_accounts.t
=> SUCCESS: The spec builds!
=> SUCCESS: The tests pass!
3. Verify all the codepaths are covered by tests
4. Sign off :-D
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
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>
Development wise it is much simpler to do this check in perl when
opening the editor than getting this logic into the template with
javascript. Furthermore, older data may be corrupted, but we trust
the current plugins to not insert such fields anymore.
Test plan:
Run t/db_dependent/Koha/Biblio/Metadata/Extractor/MARC/MARC21.t
Open an existing record in the basic editor. Shorten 008 without plugin.
Save and open the record again. Do you get the alert?
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
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>
The routine to create the message can return nothing if there is no content,
we should handle this case and not attempt to send.
To test:
1 - Test plan from last patch
2 - Apply this patch
3 - POST to patron API again
4 - Success!
Signed-off-by: Claire Hernandez <claire.hernandez@biblibre.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Currently we assume BadParameter has been thrown with a parameter that is a valid patron field.
It may only include a message, or it may have come from outside the patron object, we should
handle all cases.
To test:
1 - Wrap "WELCOME" notice content in conditional:
[% IF borrower.categorycode != 'SELFREG' %]
...
[% END %]
2 - Enable preference AutoEmailNewUser
3 - Attempt to self register a patron via API with body:
{"surname":"PATRON","firstname":"PROBLEM","library_id":"CPL","category_id":"SELFREG","email":"eample@example.org"}
4 - Response error: "Given does not exist"
5 - Apply this patch, restart all
6 - Repeat 3
7 - Response error: "Exception 'Koha::Exceptions::BadParameter' thrown 'Parameter message_id cannot be empty if passed.'\n"
8 - It's uglier, but actually helpful
Signed-off-by: Claire Hernandez <claire.hernandez@biblibre.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Fix several API inconsistencies in the FTP transport class:
1. Message format: Use standardized `message => $operation` format
instead of hardcoded strings like "Upload succeeded"
2. Operation tracking: Add $operation variable to all methods for
consistent error reporting and logging
3. Error handling: Update _abort_operation() to accept operation
parameter and provide more detailed error payloads
4. Success payloads: Replace empty `detail => ''` payloads with
meaningful operation-specific information (host, port, file paths,
directory info, file counts)
All transport classes now have consistent APIs for:
- Standardized message formats
- Consistent operation tracking
- Uniform error handling
- Informative success payloads
Sponsored-by: ByWater Solutions
Signed-off-by: Hannah Dunne-Howrie <hdunne-howrie@westminster.gov.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This patch completely refactors the EDI transport handling to use the
modern file_transports system instead of legacy direct connections.
Changes:
- Remove legacy FTP/SFTP connection code
- Update constructor to load file_transport from vendor_edi_accounts
- Update download_messages/upload_messages to use modern transport methods
- Use new rename_file() method for EDI file processing workflow
- Remove dependencies on Net::FTP and Net::SFTP::Foreign
- Maintain all EDI-specific functionality (file filtering, ingestion)
Cleanup:
- Remove obsolete methods: file_download, sftp_download, ftp_download,
sftp_upload, ftp_upload, file_upload, _abort_download
- Update POD documentation to reflect new architecture
- 184 lines of legacy code removed
Removed imports:
- use DateTime; - Not used, dt_from_string() used instead
- use English qw{ -no_match_vars }; - No English variables used
- use File::Copy qw( copy move ); - File operations now handled by
Koha::File::Transport internally
The transport system now provides:
- Unified connection handling across all transport types
- Enhanced error reporting and status tracking
- Encrypted credential storage
- Connection testing capabilities
- Support for local directories (testing/development)
Sponsored-by: ByWater Solutions
Signed-off-by: Hannah Dunne-Howrie <hdunne-howrie@westminster.gov.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Fix API inconsistency where different transport types returned different
formats from list_files():
- SFTP: returned array of hashrefs with detailed file information
- FTP: returned simple array of filenames
- Local: returned simple array of filenames
Now all transports return an array reference of hashrefs with consistent
structure containing filename, longname, and additional metadata.
This eliminates the need for workarounds like the ref($file) eq 'HASH'
check in EDI code and provides a more robust, consistent API.
Sponsored-by: ByWater Solutions
Signed-off-by: Hannah Dunne-Howrie <hdunne-howrie@westminster.gov.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This patch adds proper connection cleanup functionality to the file
transport system, addressing a gap identified during EDI migration.
Changes:
- Add disconnect() method to base Transport class interface
- Implement SFTP disconnect using connection->disconnect()
- Implement FTP disconnect using connection->quit()
- Add DESTROY methods to ensure connections are cleaned up on object destruction
- Local transport includes no-op disconnect for API consistency
This ensures that FTP/SFTP connections are properly closed, preventing
connection leaks and potential resource exhaustion issues.
Sponsored-by: ByWater Solutions
Signed-off-by: Hannah Dunne-Howrie <hdunne-howrie@westminster.gov.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This patch enhances the file transport system with file renaming
capabilities and adds a Local transport class for development/testing.
Changes:
- Add rename_file() method to base Transport class and SFTP/FTP subclasses
- Create Koha::File::Transport::Local for local directory operations
- Add 'local' transport type to polymorphic mapping
- Update transport instantiation to handle Local transport
- Add Local transport tests to verify polymorphic behavior
- Enhanced connect() method for Local transport validates directories
The rename_file functionality is needed for EDI file processing workflow
where files are renamed after download to mark them as processed.
Sponsored-by: ByWater Solutions
Signed-off-by: Hannah Dunne-Howrie <hdunne-howrie@westminster.gov.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
If we don't want the incremental run we should return early in ->new to
prevent the git clone.
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
If the file had a failure and still has, we should not list it twice.
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
git clone needs https://, but we need to remove it to build the push url
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Ignore deleted or renamed files
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
The idea of this patchset is to allow increment runs for some of our
tests.
The current (full) test suite runs in 2h20 (Koha_Main/3253 on Docker_15).
One of the main reason is the recent introduction of new xt tests that are run on all files (valid, tidy, pod, codespell, etc).
From a previous run (last week) I noted this top 5:
xt/vue_tidy.t 105446 ms
xt/perltidy.t 175970 ms
xt/js_tidy.t 239615 ms
xt/author/codespell.t 467041 ms
xt/pl_valid.t 2502354 ms
We are going to merge js_tidy.t and vue_tidy.t as it does no longer make
sense to have a difference between them (we had at the beginning of the
vue work, because of differences in the options, now everything is
handled in the .prettierrc config file)
Once this is pushed, we will be able to create a new project on gitlab
and pass some env variables from Jenkins.
Suggestion: project would be named https://gitlab.com/koha-community/koha-ci-results
When everything will be setup, jenkins will run the tests one the whole
test suite, then the next run will be "incremental": we are going to run
the tests on the files that were failing and the files modified by the
last push.
Example:
* "commit abc": all the files are tested, xt/perltidy.t is failing on Koha.pm
* koha-ci-results/perltidy/abc is created and contains ['Koha.pm']
* "commit def" has a modification on about.pl
* xt/perltidy.t is checking tidy on Koha.pm and about.pl, both are tidy
* koha-ci-results/perltidy/def is created and contains []
Test plan:
* Create an empty project on gitlab (keep the default readme)
eg. https://gitlab.com/me/koha-ci-results
* Generate an access token with the developer role
(https://gitlab.com/me/koha-ci-results/-/settings/access_tokens)
Copy the token (in the following noted as TOKEN_REPLACE_ME)
* Unprotect the "main" branch
* Run the following prove command:
```
KOHA_CI_INCREMENTAL_RUN_REPO_URL=https://gitlab.com/me/koha-ci-results.git \
KOHA_CI_INCREMENTAL_RUNS=1 \
KOHA_CI_INCREMENTAL_RUNS_REPORT=1 \
KOHA_CI_INCREMENTAL_RUNS_TOKEN=TOKEN_REPLACE_ME \
prove xt/perltidy.t
```
=> a new commit is created on your repo
* Add some mess in one of your perl file, commit using `--no-verify` to
bypass the git hook
* Run the prove command again
=> only the file you have modified is tested
=> a new commit is created and the file is reported as failing
* Add another commit, fix the failure, add other mess
=> confirm that what you are seeing makes sense :)
* Ideally redo with the different tests modified by this patchset
* Note that you can use the incremental run without the report (the report will
be only for Jenkins).
It will help RM as xt tests will run (way) faster.
Note that you can also export the following env vars to set git author
detail:
* GIT_COMMITTER_NAME
* GIT_COMMITTER_EMAIL
* GIT_AUTHOR_NAME
* GIT_AUTHOR_EMAIL
Question: What are we going to use on Jenkins?
Discussion for QA: How should we handle concurrency?
It's crucial to prevent multiple tests from pushing simultaneously.
To minimize the risk of the repository becoming outdated, we might need to perform
a pull right before each push. Is that enough? Keep in mind that only one job will
be run at the same time (Koha_Main).
Maybe later we will also use it for stable branches.
Rebased-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This patch makes it so that a successful SSO will return you to
the place where you initiated the SSO. This is especially useful
when doing searches, placing holds, etc.
Test plan:
0. Apply patch and koha-plack --restart kohadev
1. Set up OIDC SSO using
https://wiki.koha-community.org/wiki/Testing_SSO
2. Go to http://localhost:8081/cgi-bin/koha/circ/circulation-home.pl
3. Do a staff login using SSO
4. Note that you're redirected to
http://localhost:8081/cgi-bin/koha/circ/circulation-home.pl
after the successful login
5. Go to http://localhost:8080/cgi-bin/koha/opac-search.pl?idx=&q=test&weight_search=1
6. Do a OPAC SSO login
7. Note that you're returned to
http://localhost:8080/cgi-bin/koha/opac-search.pl?idx=&q=test&weight_search=1
after the successful login
8. Logout of the staff admin and OPAC
9. Note your URL ends in ?logout.x=1
10. Try logging back into them using SSO
11a. Note that you're logged in and not automatically logged out
11b. Note that if you're on the OPAC, you should be redirected
to opac-user.pl instead of opac-main.pl or the root path
12. If you change the domain for the SSO users to something
that your user does not match, try logging in again, and note
that the error message should display on mainpage.pl or opac-main.pl/opac-user.pl
for the OPAC. (This is because these are the pages that show SSO errors.)
Signed-off-by: Lukas Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
When a basket is generated via an EDI QUOTE message, the QUOTE message
includes a library's EAN code (buyer_ean). This patch makes the 'Create
EDIFACT order' button automatically use the EAN from the original QUOTE
message instead of requiring manual EAN selection.
This patch also fixes a bug in process_quote where all baskets in
multi-message QUOTE transports incorrectly used the first message's
buyer_ean, even when subsequent messages had different EANs.
Changes made:
1. Fixed Koha/EDI.pm process_quote auto-order creation to use the correct
buyer_ean for each message/basket pair in multi-message transports
2. Enhanced acqui/basket.pl to extract and provide the quote_ean from
QUOTE messages, handling both single and multi-message scenarios
3. Updated basket.tt template to automatically select the EAN from the
original QUOTE message when available and matching
Test plan:
1. Create a vendor with EDI account and multiple EANs configured
2. Process a QUOTE message that creates a basket
3. Go to the basket page and click "Create EDIFACT order"
4. Verify it automatically uses the EAN from the QUOTE message
(shows tooltip "Using EAN from original QUOTE message")
5. Test with multi-message QUOTE files to ensure each basket uses
the correct EAN from its corresponding message
6. Verify fallback behavior when no QUOTE EAN matches available EANs
Sponsored-by: OpenFifth <https://openfifth.co.uk/>
Signed-off-by: Hannah Dunne-Howrie <hdunne-howrie@westminster.gov.uk>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This patch implements proper override logging for holds following the
Alternative implementation pattern used for checkouts and renewals.
Instead of just removing the orphaned logging code from the original
Bug 9762 implementation, this patch properly implements override logging
by updating AddReserve to accept and log override information, similar
to how AddIssue handles confirmations and forced overrides.
Changes:
C4/Reserves.pm:
- Added JSON qw(to_json) import
- Updated AddReserve to accept confirmations and forced parameters
- Enhanced the existing logaction call to include override information
in JSON format when overrides are present
- Log format matches the Alternative implementation pattern:
* hold ID
* branchcode
* biblionumber
* itemnumber
* confirmations array
* forced array
Koha/REST/V1/Holds.pm:
- Added confirmations array building in the add() method
- Pass 'HOLD_POLICY_OVERRIDE' confirmation when override_all is true
- Pass confirmations parameter to AddReserve call
reserve/request.pl:
- Removed orphaned original implementation logging code
- Removed unused variables and imports (message, @message, user, etc.)
- Removed C4::Log import
koha-tmpl/intranet-tmpl/prog/en/includes/action-logs.inc:
- Added 'HOLD_POLICY_OVERRIDE' case to translate_log_override block
- Displays as "Hold policy override" in log viewer
koha-tmpl/intranet-tmpl/prog/en/modules/tools/viewlog.tt:
- Added JSON display handling for HOLDS module (similar to CIRCULATION)
- Displays hold ID, biblionumber, and optional itemnumber with links
- Shows forced and confirmation overrides using translate_log_override
This ensures hold overrides are logged consistently with the Alternative
implementation pattern, providing proper audit trails for policy overrides
that are properly displayed in the log viewer.
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
The bug 32950 fix introduced a regression that broke copy_field operations
when copying between different fields or when copying a subfield to itself
within the same field.
The original fix correctly handled moving subfields between different
subfields within the same field (e.g., 020$z to 020$a) by using
field_exists() to identify which fields have the source subfield.
However, this logic was applied too broadly:
1. Cross-field operations (e.g., 650$a to 651$a): The fix passed source
field_numbers to _update_subfield for the target field, causing it to
look for non-existent target field positions. This resulted in
insert_fields_ordered creating new fields out of order.
2. Same-field, same-subfield operations (e.g., 952$d to 952$d): The fix
prevented copying to fields that didn't have the source subfield, but
the original behavior was correct for duplicating a subfield to itself.
This patch restricts the bug 32950 logic to only apply when:
- Source and target fields are the same AND
- Source and target subfields are different
Additionally, it ensures field_numbers are not passed to _update_subfield
when copying between different fields.
Test plan:
1. Run prove t/SimpleMARC.t
2. All tests should pass, including:
- Cross-field copy operations (650$a to 651$a)
- Same-field, different-subfield operations (020$z to 020$a)
- Same-field, same-subfield operations (952$d to 952$d)
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Pedro Amorim
Lucas Gass
Jonathan Druart
Lari Taskula
Marcel de Rooy
Martin Renvoize
Tomás Cohen Arazi
Nick Clemens
David Cook