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>
(cherry picked from commit c1fa513be9)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
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>
(cherry picked from commit ae14a11937)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Test plan:
Read POD.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit f0991861e2)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
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>
(cherry picked from commit 47546f0849)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
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>
(cherry picked from commit 1884f68c5b)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
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>
(cherry picked from commit 83126898de)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Moving one subfield to another was failing for repeatable fields where
the field to be moved doesn't exist in the first instance. When moving
subfields within the same field type (e.g. 020$z to 020$a), existing
subfield values in fields that didn't contain the source subfield were
being overwritten with values from other fields.
This patch fixes the issue by determining which fields actually contain
the source subfield and only updating those corresponding target fields,
preserving existing values in fields that don't have the source subfield.
The fix adds logic to _copy_move_subfield to:
1. Use field_exists() to identify which fields have the source subfield
2. Pass these field numbers to _update_subfield for targeted updates
3. Preserve all existing functionality while fixing the data loss bug
Test plan:
1. Create a MARC modification template to move 020$z to 020$a
2. Create a record with multiple 020 fields:
- Some with existing $a values
- Some with $z values to be moved
3. Apply the template
4. Verify existing $a values are preserved
5. Verify $z values are moved to $a only in appropriate fields
6. Verify $z subfields are removed after the move
Signed-off-by: Bernard Scaife <bernard.scaife@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 346dd2073a)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
This patch adjusts the logic to consider all permissions and not just the top level flags
Test plan:
1) Create or edit a user to have all acquisitions permissions except one (not relevant which one, any will do)
2) Log into the staff interface and go to acquisitions > vendors and click the vendor in the table
3) The left menu will be missing everything except invoices
4) Apply patch and restart_all
5) Refresh the page, the menu should now be populated
Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Signed-off-by: Laura_Escamilla <laura.escamilla@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 23fe21df61)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 1a2a7e4619)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
We should not call discard_changes after accessing ->id but before
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 9e8b9f607a)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
To reproduce
1- Enable the ERMModule system preference
1- Go to Administration > System Preferences
2- Search for ERMModule
3- Change the value to "Enable"
4- Click on "Save all e-resource management preferences"
5- Return to the main screen
1- Create an agreement
1- Go to e-resource management
2- Click on "Agreements"
3- Click on "New agreement"
4- Fill out the form (mandatory fields)
- Agreement name: Natural Medicines
- Status: Active
5- Click on "Submit"
2- Edit the agreement
1- Click on "Agreements"
2- Click on "Edit" to the right of the Natural Medicines agreement
3- Click on "Submit"
--> Something went wrong: SyntaxError: JSON.parse: unexpected character at line 1 column 1 of the JSON data
We got the same error when editing a license, a package or a title
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit ddbd11cbad)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Same as bug 34303 and bug 33743, our test suite must parse/process files
that are part of the index, not all the other files.
It's currently failing on kohadev-db-request.txt that is owned by root.
But it's not part of the Koha codebase, this file should not be tested.
There were also node_modules and po files that were processed.
Test plan:
prove t/00-merge-conflict-markers.t
must now return gren
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit cc72eabc5f)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
All was not returning all the files present in the git index, but all
Koha core files. Let's rename it with 'core' to remove ambiguity.
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 36bd4bd7b4)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
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>
(cherry picked from commit f7897d7d81)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 4df64c79f0)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Bug 38010 broke the vendors pages when Koha is running as a Mojolicious
application
This adds new routes to Koha::App::Intranet to mimic the Apache rewrite
rules added in bug 38010.
To be able to do that, the CGIBinKoha plugin's code had to be moved to a
controller. The logic remains the same.
Test plan:
1. Run `bin/intranet daemon -l http://*:3000/`
2. Point your browser to http://localhost:3000 (replace localhost by the
hostname/IP of koha's container, if needed)
3. Go to Acquisitions and search for a vendor. You should see the list
of vendors.
4. Click on "New vendor" button. Notice the URL changed to
.../acquisition/vendors/add.
5. Reload the page. You should still see the form to create a new vendor
6. Verify that you can create a vendor and that you are then redirected
to the list of vendors
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>
(cherry picked from commit b92fda21ce)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
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>
(cherry picked from commit 9482590909)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
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>
(cherry picked from commit dcf28e5b8a)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Eric Phetteplace <phette23@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit ff82cd37f5)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
This moves the original SUSHI COUNTER logic to a new
Koha::ERM::EUsage::COUNTER::5 class that extends
Koha::ERM::EUsage::SushiCounter.
Adds a new Koha::ERM::EUsage::COUNTER::5_1 class with COUNTER 5.1
specific logic.
Keeps common logic in Koha::ERM::EUsage::SushiCounter.
This ensures we support both COUNTER 5.0 and COUNTER 5.1, but should
hopefully help us better maintain COUNTER 5.1 going forward, without having
to worry about COUNTER 5.0, as the code is seperated.
This should also better set us up for adding support for possible future COUNTER releases.
Test plan, k-t-d:
1) Enable ERMModule
2) Add a new usage data provider at:
<staff_url>/cgi-bin/koha/erm/eusage/usage_data_providers/add
3) Add the required credential parameters e.g. Customer ID and Requestor ID
4) Update the 'Report release' to '5.1', no single quote.
(If you add anything other than '5', it'll default to '5.1')
5) Visit the list of data providers and click the 'Run now' button for your data provider:
<staff_url>/cgi-bin/koha/erm/eusage/usage_data_providers
6) Click the background job link at the top. Verify the background job runs as expected.
7) After the successful harvest, run a custom report at, ensure the data is all correct:
<staff_url>/cgi-bin/koha/erm/eusage/reports
8) Visit the data provider at:
<staff_url>/cgi-bin/koha/erm/eusage/usage_data_providers/<provider_id>
9) Click the 'Import logs' tab, download the COUNTER 5.1 counter file.
Open the file and verify the data is correct.
Signed-off-by: Eric Phetteplace <phette23@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit f58e2110ca)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
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>
(cherry picked from commit 2bfb86afb5)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Test plan:
1) Have a record with 8 items.
2) Place a booking on 6 of the items (6 bookings total) for a full month
(e.g. August)
3) Check-out all 6 of those bookings
4) Attempt to place a new booking for one of the 2 free items for end of month, e.g. (Aug 26 -> Aug 28).
5) It will fail with error "Booking conflicts". This is because the code
is adding 6 checkouts with 6 bookings = 12 which is higher than 8
bookable items.
6) With this patch, check-outs that refer to an existing booking are
excluded, so the check becomes if 8 - 6 > 0 (which passes the check) rather than 8 - 12 > 0 (which fails the check)
Run existing tests:
prove t/db_dependent/Koha/Item.t
prove t/db_dependent/Koha/Biblio.t
prove t/db_dependent/Koha/Booking.t
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 6a729b4442)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
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>
(cherry picked from commit cd4e1da3ec)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
In our codebase, there's a pattern we tend to use for searching patrons
by userid and falling back to find them by cardnumber:
* C4::Auth
* C4::SIP::ILS::Patron
* (slightly different) Koha::REST::V1::Auth::Password
* (slightly different) Koha::REST::V1::Auth
* C4::Circulation
* C4::Auth_with_cas
It generally uses the following pattern:
```perl
my $patron = Koha::Patrons->find( { userid => $identifier } );
$patron //= Koha::Patrons->find( { cardnumber => $identifier } );
```
The problem with this is that `find` actually produces a DBIX::Class
warn because `find` is being called without primary key parameters. I
haven't seen it in the wild, until the latest change in
`checkpw_internal` which made it throw warnings in SIP. My
interpretation is that SIP's special approach with the Trapper.pm class
made it show where other places just get it hidden.
That said, I implemented this using `search()` to overcome this.
To test:
1. Apply the patches
2. Run:
$ ktd --shell
k$ prove t/db_dependent/Koha/Patrons.t
=> SUCCESS: Tests pass!
3. Sign off :-D
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>
(cherry picked from commit 3f19d4bf85)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
An alternative approach to the original submission for bug 9762. Instead
of recording a distinct action log line for a set of override messages
in addition to the standard issue record, this patch updates the
existing issue log to include any confirmation or impossible responses
from the preceeding CanBookBeIssued call.
We continue to record these as a JSON structure as the prior patch did,
but instead of human messages we use the predefined codes as returned
from the existing CanBookBeIssued response.
To test:
After each step, a single action log message should be displayed under
`Tools > Log viewer`. The message itself will now consist of a JSON
string with an array of 'confirmations' and 'forced' strings
representing the overriden feedback from CanBookBeIssued.
Circulation -- Checking out
1) Check out an item that is age-restricted (AgeRestrictionOverride syspref)
2) Check out an item that has a “not for loan” status (AllowNotForLoanOverride
syspref)
3) Check out an item that has a “lost” status (IssueLostItem syspref)
4) Check out an item to a patron who has reached the checkout limit
(AllowTooManyOverride syspref)
5) Check out an item to a patron who is not allowed to borrow this item type
6) Check out an item to a patron who has unpaid fines (AllFinesNeedOverride
and/or AllowFineOverride + noissuescharge sysprefs)
7) Check out an item on hold for someone else
8) Check out an item on hold ready for pickup by someone else
9) Check out an item already checked out to someone else
(AutoReturnCheckedOutItems syspref)
10) Check out to a patron who has restrictions
Circulation -- Checking in
11) Ignore a hold upon check in
12) Ignore a hold and transfer upon check in
Circulation – Renewing
13) Renew an item on hold for someone else
(AllowRenewalOnHoldOverride syspref)
14) Renew an item that has reached the maximum number of renewals
(AllowRenewalLimitOverride syspref)
Holds
15) Place a hold for a patron who is not allowed to place this item
type on hold (AllowHoldPolicyOverride syspref)
16) Place a hold for a patron who has reached the maximum number of
holds
17) Place an item-level hold when rules dictate that this is not
allowed
Sponsored-by: OpenFifth <https://openfifth.co.uk/>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 5350a9276e)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Currently request_transfer checks for the existence of a current transfer on the item, and a new one can either be
enqueued or replace this transfer. This patch adds a check to see if the requested transfer matches an existing transfer
on the item. In the case that it does we simply return that transfer rather than create a new transfer.
This has the effect of eliminating 'WrongTransfer' generation if an item is checked in twice at the same branch, which feels correct.
To test:
1 - place a hold on an item that is currently at branch A, for pickup at branch B
2 - check your item in at Branch A, click to confirm the hold and transfer (print slip if you want to)
3 - sudo koha-mysql kohadev: SELECT * FROM branchtransfers -- See one transfer
4 - check your item in at Branch A again, get a message that the item is supposed to go to Branch B. Don't click on the modal yet
5 - repeat 3 -- See two transfers, one cancelled as WrongTransfer, one pending
6 - Click 'Confirm and transfer' on the modal
7 - repeat 3 -- See three tranfers - the two from before and a new active transfer
8 - DELETE FROM branchtransfers
9 - Apply patch, restart all
10 - repeat 3 - no transfers
11 - Check item in at branch A again and confirm
12 - repeat 3, one transfer
13 - Check item in, don't confirm the modal
14 - repeat 3, one transfer
15 - Confirm the modal
16 - repeat 3, one transfer
17 - Check the item in at branch C
18 - repeat 3, two transfers - one cancelled WrongTransfer, one active
19 - Confirm modal
20 - repeat 3, two transfers
21 - Check item in at branch B
22 - repeat 3, two transfers, one cancelled, one completed
Signed-off-by: Christine Lee <chlee@pascolibraries.org>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit acdc512cea)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
This patch adds desk_id reset functionality to the revert_found method,
but only for waiting holds where desk_id is actually relevant.
Changes:
* Modified revert_found() to conditionally reset desk_id
* Only resets desk_id for waiting holds (found=W)
* Leaves desk_id unchanged for in transit (T) and in processing (P) holds
* Added comprehensive test coverage for all scenarios
* Updated POD documentation to explain desk_id behavior
The logic is conservative and only clears desk_id when it makes sense:
waiting holds have their desk assignment cleared since they are no longer
waiting at a specific desk, while transit and processing holds remain
unchanged as they do not use desk_id.
To test:
1. Apply patch
2. Run tests:
$ ktd --shell
k$ cd /kohadevbox/koha
k$ prove t/db_dependent/Koha/Hold.t
=> SUCCESS: All tests pass including new desk_id handling tests
3. Verify all hold test suites still pass:
k$ prove t/db_dependent/Koha/Hold*.t
=> SUCCESS: All hold-related tests pass
4. Sign off :-D
Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 2da5c04c2d)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
This patch added the option to update borrowers.lastseen when a patron username
and password is verified via the REST API. This can be enabled or disabled
selectively using the TrackLastPatronActivityTriggers system preference
using the [ ] Patron authentication via API option..
To test:
1. Create a new user, ensuring to set the Username and password fields.
*Import to do this first*
2. Edit the TrackLastPatronActivityTriggers syspref and check all options.
3. Ensure the RESTBasicAuth syspref is set to Enable.
4. Check the lastseen date of your new user using a SQL report
SELECT borrowernumber, userid, lastseen FROM borrowers WHERE userid = 'USERNAME'
(Replace USERNAME with your chosen username for the new patron)
NOTE: **Ensure the cache expiry of the SQL report is set to 0**
5. The lastseen for your new user should be null
6. 'restart_all' Koha services to clear plack cache.
7. Run the attached api_verify_lastseen.pl test script
**Change P_USERNAME and P_PASSWORD to the username and password of your new user.
(script contains expected output for successful verification)
8. Re-check the lastseen date of your new user again via SQL (step 4)
The lastseen should still be null.
9. Apply Patch
10. 'restart_all' Koha services
11. Edit the TrackLastPatronActivityTriggers syspref and check the '[ ] Patron authentication via API' option
12. Rerun the api_verify_lastseen.pl
13. Re-check the lastseen date of your new user again via SQL (step 4)
The lastseen should now be updated to the time of the last script run.
For good measure:
14. Edit the TrackLastPatronActivityTriggers syspref and **UNcheck** the '[ ] Patron authentication via API' option
15. 'restart_all' Koha services to clear plack cache.
16. Rerun the api_verify_lastseen.pl
17. Re-check the lastseen date of your new user again via SQL (step 4)
The lastseen should have remained the same as previous.
Sponsored-by: Westlake Porter Public Library <https://westlakelibrary.org>
Signed-off-by: Bernard Scaife <bernard.scaife@openfifth.co.uk>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit ba43483f4d)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Test plan, k-t-d, don't apply patch yet:
1) Install a plugin that sends the cookie to output_html e.g.:
https://github.com/openfifth/koha-ill-libkey-lending-tool/releases/tag/v2.3.8
2) Log out. Access this plugin's tool link directly:
<staff_url>/cgi-bin/koha/plugins/run.pl?class=Koha%3A%3APlugin%3A%3ACom%3A%3APTFSEurope%3A%3AIncDocs&method=tool
3) Notice you're presented the login screen. Login using koha:koha
4) Notice you're shown the plugin's respective tool page. It looks like you're authenticated but you're not. Click on any of the top links e.g. 'Circulation' or 'Patrons'.
5) Notice you're shown the login page again.
6) Apply patch. Repeat. Step 5 should no longer show the login page but instead the page you clicked on.
Signed-off-by: Tomas Jiglind <tomas.jiglind@mariestad.se>
Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit f16187e605)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
We need a skip_future_holds flag in item->current_holds to make renewals
act as they did before. There is no need to add it to biblio->current_holds.
Test plan:
Run t/db_dependent/Holds.t
Interface test: Issue a book, place hold on tomorrow, set ConfirmFutureHolds
to 2 days. Verify that renew is not blocked by this hold.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit c11dce94c1)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
We were missing POD here.
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit d2f20b00f1)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Test plan:
Easy way is to run the unit tests in the next patch.
One call of biblio->current_holds is hidden in acqui/parcel.pl. The
table column "Item holds / Total holds" should now make a difference
for an order referring to an item having future holds.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit eedd4bcf2e)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Test plan:
Add future item level hold for another branch.
Check in. Confirm and transfer.
Note that without this patch, there is no patron info for the column
On hold for on transferstoreceive.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 503a81efe1)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
This patch refactors the revert_waiting method to handle all found statuses
instead of just waiting holds, making it more versatile and consistent.
Changes:
* Renamed method from revert_waiting() to revert_found()
* Expanded functionality to handle all found statuses (W, T, P)
* Updated exception logic to check is_found instead of is_waiting
* Changed exception message from hold_not_waiting to hold_not_found
* Added comprehensive tests for all found statuses
* Updated POD documentation to reflect new behavior
The method now reverts any hold with a found status back to a regular
hold with priority 1, regardless of whether it was waiting, in transit,
or in processing.
To test:
1. Apply the patches
2. Run tests:
$ ktd --shell
k$ cd /kohadevbox/koha
k$ prove t/db_dependent/Koha/Hold.t
=> SUCCESS: All tests pass including new subtests for T and P statuses
3. Verify all hold test suites pass:
k$ prove t/db_dependent/Koha/Hold*.t
=> SUCCESS: All hold-related tests pass
4. Sign off :-D
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit bd9fa395e5)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit ad66f7d7d3)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
This patch adds a new method to the `Koha::Hold` class. The method
follows the original implementation in C4::Reserves.
Tests are added to cover all the code paths.
To test:
1. Apply this patch
2. Run:
$ ktd --shell
k$ prove t/db_dependent/Koha/Hold.t
=> SUCCESS: Tests pass :-D
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: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit a258f253b7)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
This patch simplifies the code a bit, removing duplicated pieces of code
and making it slightly more readable.
The main change is that it now just filters the `@plugin_classes` list
as required, and leaves the original codebase for dealing with loading
the plugins untouched.
A BIG change it implements is that it now requires full class names. I
don't see the use case for partial names, and I generally think we
should be stricter in this area. If there's the need for partial class
names, this should be handled with specific parameters, etc.
It also makes it explicit that the `include` and `exclude` parameters
are mutually exclusive.
A minor idiomatic issue is fixed as well.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Hammat Wele <hammat.wele@inlibro.com>
Signed-off-by: Amaury GAU <amaurygau@gmail.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit a9dbb26290)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
I have added the options --include and --exclude to the install_plugins.pl script. When running the script, you can now specify which plugins you want to install (--include) and which ones you do not want to install (--exclude).
--include installs only the plugins from the specified classes.
--exclude installs all plugins except those from the specified classes.
It's possible to specify the package to install a specific plugin.
--include and --exclude cannot be used simultaneously.
Usage example:
./misc/devel/install_plugins --include <package::pluginClass> --include <anotherPluginClass>"
TEST PLAN
1) Apply the patch
2) Add at least three plugins to the plugin directory ([Koha]/plugins/Koha/Plugin/)
3) Empty the "plugin_data" and "plugin_methods" tables (SQL command: "delete from plugin_data; delete from plugin_methods;")
4) In a terminal, execute this command "./misc/devel/install_plugins.pl --include <package::pluginClass> --include <anotherPluginClass>" (Replace <package::pluginClass> and <anotherPluginClass> with the class and package of your plugins)
5) In your Koha intranet, go to "Koha administration > Manage plugins"
6) Verify that only the specified plugins are installed
7) Repeat step 3
8) In a terminal, execute this command from your Koha project"./misc/devel/install_plugins.pl --exclude <package::pluginClass> --exclude <anotherPluginClass>"
9) Verify that all plugins except the specified ones are installed
10) Run the tests from the Plugins.t file ([Koha]/t/db_dependent/Koha/Plugins/Plugins.t)
11) Ensure that all tests are successful
Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Hammat Wele <hammat.wele@inlibro.com>
Signed-off-by: Amaury GAU <amaurygau@gmail.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit dd60490292)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Also add tests to searching for specific language code
prove t/db_dependent/api/v1/additional_contents.t
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 7c6ad4bbe3)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
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>
(cherry picked from commit 38b19d72cb)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
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>
(cherry picked from commit 316745f3e2)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
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>
(cherry picked from commit 2da24a2002)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Ensure no regressions are introduced, run:
prove t/db_dependent/Koha/AdditionalContents.t
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>
(cherry picked from commit ef0bde190a)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
1. Turn on EmailPurchaseSuggestions and add an email address to EmailAddressForSuggestions.
2. Create a new suggestion and set the "
Created by:" field to a patron with an email address
3. Check the message queue ( select * from message_queue where letter_code = "NEW_SUGGESTION" )
4. There is a NEW_SUGGESTION queued, the to_address should be the email you entered in EmailAddressForSuggestions.
5. The borrowernumber is the borrower who made the suggestion.
6. APPLY PATCH, restart_all
7. Repeat steps 2 - 4.
8. This time a NEW_SUGGESTION should be queued, but the borrowernumber will be undef
9. Check in the patron's notice tab, there should be nothing queued.
Signed-off-by: Lin Wei Li <lin-wei.li@inLibro.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 6b6d9f275e)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
When NotifyPasswordChange is enabled and GetPreparedLetter fails to find
a PASSWORD_CHANGE letter template, the set_password method was returning
early with undef, preventing the password from being changed.
This fixes the issue by:
1. Removing the 'or return;' statement after GetPreparedLetter
2. Wrapping the letter processing in an if block to handle missing templates
3. Ensuring password change continues regardless of notification success
The password change should not fail just because the notification cannot
be sent. The notification is a nice-to-have feature, but the core
functionality (changing the password) should always work.
Test plan:
1. Apply the tests patch
2. Run:
$ ktd --shell
k$ prove t/db_dependent/Koha/Patron.t
=> FAIL: Tests fail!
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!
5. Verify that when the letter template exists, notifications are still sent
6. Verify that when NotifyPasswordChange is disabled, password changes work
7. Sign off :-D
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Flater <flaterdavid@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit f748205ad0)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Nick Clemens
Marcel de Rooy
Martin Renvoize
Matt Blenkinsop
Jonathan Druart
Hammat Wele
Julian Maurice
Pedro Amorim
Tomas Cohen Arazi
CJ Lynce
Martin Renvoize
Pedro Amorim
Emily-Rose Francoeur
Lucas Gass