The cashup functionality on both /pos/register.pl and /pos/registers.pl
pages was using GET requests via anchor tags, which could cause
duplicate cashup records when users refreshed the page after performing
a cashup.
This patch implements proper POST/REDIRECT/GET pattern with CSRF
protection:
1. Convert anchor tag buttons to proper HTML forms with submit buttons
2. Add CSRF token protection to all cashup operations
3. Change operation from 'cashup' to 'cud-cashup' for CSRF compliance
4. Implement redirects after successful cashup operations to prevent
resubmission
5. Update JavaScript to populate form fields instead of href attributes
This ensures cashup operations follow HTTP semantics properly (POST for
state changes) and prevents duplicate submissions while maintaining the
existing user experience.
Test plan:
1. Apply patch and restart services
2. Go to Point of Sale > Cash registers
3. Click "Record cashup" on any register - should open modal
4. Click "Confirm" - should perform cashup and reload page
5. Refresh the page - should NOT create duplicate cashup record
6. Go to Point of Sale > Register for individual register
7. Click "Record cashup" - should open modal
8. Click "Confirm" - should perform cashup and reload page
9. Refresh the page - should NOT create duplicate cashup record
10. On registers page, try "Cashup all" button - should work correctly
11. Verify all existing functionality (modals, summaries) still works
12. Check browser developer tools - all requests should be POST for
cashup operations
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Laura_Escamilla <laura.escamilla@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 26f1f91a13)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit d4d9a849b5)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
If no shipment_budget is set then undef is set as key of the hashref
which generates a warning:
Use of uninitialized value $row[0] in hash element at /usr/lib/x86_64-linux-gnu/perl5/5.40/DBI.pm line 2122.
Later we only access the hashref by budget_id, so this change seems
safe.
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit f073262263)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit d4fd8fd299)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
This patch alters the code to append instead of prepend, removing the need to reverse the list
To test:
1 - Write a report to get some patron
SELECT cardnumber,surname FROM borrowers WHERE cardnumber <>'' ORDER BY rand() LIMit 6
2 - Enabled 'showLastPatron' and set count to 3
http://localhost:8081/cgi-bin/koha/admin/preferences.pl?op=search&searchfield=showLastPatron
3 - Copy the barcodes in order from the report and search for them in patron search, you'll want two tabs so you can keep report open
4 - Notice at 4th patron that list order has changed
5 - When 5th patron is loaded, the 4th is removed from the list
6 - Apply patch
7 - Clear the list
8 - Repeat the test plan, but note that list order does not change
9 - Confirm the patrons are removed from the list in expected order when adding new
10 - Change the count to 1 and search for several patrons, ensuring the last patron link is always correct
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit f689133aaf)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit 0ca35af93a)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Signed-off-by: Trevor Diamond <trevor.diamond@mainlib.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 412879a418)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit fb29ac4205)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
When the authorised value of a REPORT_GROUP is numeric the filtering
does not work.
Test plan:
Create a new REPORT_GROUP av "123" (the description can be alphanumeric)
Create a new report and assign it to this new group
Confirm that with this patch the filtering works when the "123" tab is
selected
Signed-off-by: George <george@nekls.org>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit bed343d5ed)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit 82a10f632d)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
We should remove the regex metacharacters from the string or JS code
will raise an error and the query won't be generated, leading to the DT
neverending loading problem.
Test plan:
Go to http://localhost:8081/cgi-bin/koha/catalogue/detail.pl?biblionumber=4
In the global "Search" of the items table, search using a regex
metacharacters in order to form a broken regex (eg. '[')
=> Without this patch there is a JS error in the console "Uncaught
SyntaxError: unterminated character class" and you get the "Processing"
that never ends
=> With this patch applied the search succeeds.
Signed-off-by: Mathieu <mathsabypro@gmail.com>
Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 0f6f5ab252)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit c338abe8c3)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
To test:
1 - save and run a report of "SELECT borrowernumber FROM borrowers"
2 - observe your results page does not include the Show Data Menu button
3 - save and run a report of "SELECT cardnumber FROM borrowers"
4 - observe your results page does include the button
5 - save and run a report of "SELECT borrowernumber, cardnumber FROM borrowers"
6 - observe you are once again buttonless on the results page
7 - apply patch
8 - run your report from step 1, observe you now have the button
9 - run your report from step 3, observe you still have the button
10 - run your report from step 5, observe you now have the button
11 - confirm that the batch actions dropdown only includes the patron actions once
Signed-off-by: Magnus Enger <magnus@libriotech.no>
Works as advertised.
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 5e5ffeea82)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit 8c8128a7ba)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
This patch fixes malformed entries in .mailmap that were causing
display issues and incorrect author mapping.
Issues fixed:
- Marjorie Barry-Vila: Missing closing '>' in email address
- Mathieu Saby: Incorrect alias format instead of proper mailmap format
(was using excessive padding with alias, now uses proper
'Name <email> OldName <email>' format)
These structural issues were breaking terminal display in tools like
tig and preventing proper author consolidation.
Test plan:
1. Apply patch
2. Run: git log --oneline | head -20
=> SUCCESS: Authors display correctly
3. Check in tig or other git tools
=> SUCCESS: No excessive column width issues
4. Verify mailmap functionality:
git shortlog -sn | grep -E 'Mathieu|Marjorie'
=> SUCCESS: Authors properly consolidated
5. Sign off :-D
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
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 6c3153a481)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit 52b59aa736)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
This patch fixes the display logic for the LOC classification field
(050).
The current issue is that the inner components (each subfield) gets
separated with the pipe symbol (|) instead of grouping the components
together and using the separator for the outer 050 occurences.
To test:
1. Open a record to edit it
2. Add 050 like this:
```
050 4 $aE337.5 $b.O54 2025
050 4 $aE415.7 $b.A44 2025
```
3. Save
=> FAIL On the staff interface it displays:
```
LOC classification: E337.5 | .O54 2025 E415.7 | .A44 2025
```
4. Apply this patch
5. Run:
$ ktd --shell
k$ restart_all
6. Reload the detail page
=> SUCCESS: Things display correctly!
7. Sign off :-D
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit d2d85c9f4a)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit 1ad044b981)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
The default sort order for Koha's patron search includes many columns, some of which are not indexed:
[% CASE 'name-address' %] {
"data":"me.surname:me.preferred_name:me.firstname:me.middle_name:me.othernames:me.street_number:me.address:me.address2:me.city:me.state:me.postal_code:me.country",
Depending on the number of patrons in the database, and the specific configuration of the server, this can end up generating a query that creates a temp table in the hundreds of gigabytes of data or more. Not only does this risk crashing a database server with insufficient disk space, it's also incredibly slow.
We've found a compound index prevents the issue and also makes the search much faster. InndoDB indexes have a fixed maximum length which is too small ( 3072 bytes ) to encompass the full data for all the fields we need to sort by.
The following works well as a comprise to keep the index length within the maximum:
CREATE INDEX idx_borrowers_sort_order ON borrowers (
surname(100), -- 400
preferred_name(80), -- 320
firstname(80), -- 320
middle_name(50), -- 200
othernames(50), -- 200
streetnumber(20), -- 80
address(100), -- 400
address2(75), -- 300
city(75), -- 300
state(40), -- 160
zipcode(20), -- 80
country(40) -- 160
);
Test Plan:
1) Apply this patch
2) Run updatedatabase.pl
3) Perform a patron search
4) No change in behvior should be noted!
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 63c44cbd0e)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit b8fa49bb73)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.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>
(cherry picked from commit c1fa513be9)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit 91da4b4b38)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.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>
(cherry picked from commit ae14a11937)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit 7036902f57)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
For some reasons t/db_dependent/ILSDI_Services.t fails if run after
t/db_dependent/api/v1/biblios.t
When we modify the MARC subfield structure from the tests we must flush
the cache.
Actually I don't understand why the tests are even passing.
I have not tracked down why it only fails when run after t/db_dependent/api/v1/biblios.t
Test plan:
0. Do not apply this patch
1. prove t/db_dependent/api/v1/biblios.t t/db_dependent/ILSDI_Services.t
It fails
# Failed test 'Title hidden in output as expected'
# at t/db_dependent/ILSDI_Services.t line 874.
# got: '1'
# expected: '0'
2. Apply this patch, repeat 1.
=> it's green!
Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit b6aab57065)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit adbc9548fd)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Cronjob cleanup_database.pl usage is wrong :
Usage: $0 [-h|--help] [--confirm] (...) [--bg-days DAYS [--bg-type TYPE] ]
Those args has been renamed jobs-days and jobs-type.
Also some args are missing.
There are too many args, I propose to remove them from there, one need to read the full help.
Test plan:
Run command line `misc/cronjobs/cleanup_database.pl -h`
You see first line:
Usage: misc/cronjobs/cleanup_database.pl
-h --help prints this help message, and exits, ignoring all
...
Signed-off-by: Anke Bruns <anke.bruns@gwdg.de>
Signed-off-by: Thomas Klausner <domm@plix.at>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 3aefbaeaf6)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit d46e9d25a9)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
If the holds reminder cronjob would send a reminder on a holiday,when holidays are taken into account, it also will send it on the subsequent holidays and the first open day.
This bugfix adds a check to the cron when --holidays is set to skip sending the notices for that library if today is a holiday.
To Test:
Before applying:
1. Create a hold for a patron (I used patron 23529000035676 and 39999000000252)
2. Ensure your patron has an email address set and the hold reminder email selected.
3. Set the hold to waiting by checking it and confirming the hold.
4. In Tools->Calendar add a holiday to yesterday and a seperate holiday to today.
5. On the command line (the date should be set to 5 days ago for my commands to work and any dates adjusted accordingly)
koha-mysql kohadev
update reserves set waitingdate="2025-09-25"
exit
perl misc/cronjobs/holds/holds_reminder.pl -days 3 -t -v -holidays --date 2025-09-29
6. Observe that 1 notice would have sent to your patron
7. On the command line
perl misc/cronjobs/holds/holds_reminder.pl -days 3 -t -v -holidays
8. Observe that 1 notice would have sent to your patron
9. On the command line
perl misc/cronjobs/holds/holds_reminder.pl -days 3 -t -v -holidays --date 2025-10-01
10. Observe that 1 notice would have sent to your patron
11. Apply patches
12. Repeat the last command from step 5 and step 7
13. Observe that no notice should be sent for your patron
14. Repeat step 7
15. Observe that a notice should be sent for your patron.
16. Run
perl misc/cronjobs/holds/holds_reminder.pl -days 5 -t -v
17. Confirm the notice should be sent for your patron
18. Sign off
Signed-off-by: Katie Bliss <KEBliss@dmpl.org>
Bug 35700: (QA follow-up) fix POD and $today
This patch replaces the inncorrectly removed POD note and replaces $today with the more correct $date_to_run for checking if we should be skipping sending the message on the day.
Amended-by: Jonathan Druart
Squashed and adjusted commit title
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit b895eb9b7d)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit 00ee0cc163)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
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>
(cherry picked from commit ae9b83ae96)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Should be reverted as soon as Bug 40163 is backported.
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit 43c06a34ce)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
This patch adds debug logging to C4::SIP::Sip::MsgType::_initialize() to help
identify when critical fields (patron_id, item_id) are parsed as empty values.
Empty critical fields in SIP messages can later cause DBIx::Class constraint
errors when passed to Koha::Objects->find() operations, generating confusing
'Odd number of elements in anonymous hash' errors in SIP server logs.
The enhanced logging helps administrators identify the root cause by logging
when empty patron_id or item_id fields are detected during message parsing.
To test:
1. Apply regression tests patch first
2. Run:
$ ktd --shell
k$ prove t/db_dependent/SIP/MsgType.t
=> FAIL: Debug logging tests fail (no enhanced logging present)
3. Apply this patch
4. Run:
$ ktd --shell
k$ prove t/db_dependent/SIP/MsgType.t
=> SUCCESS: All tests pass, debug logging works correctly
5. Sign off :-D
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 220fdce180)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit c7330715b0)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
These tests verify that C4::SIP::Sip::MsgType correctly parses SIP messages without
creating problematic arrayref field values, and validates enhanced debug logging for
empty critical fields.
Test Coverage:
• Message parsing with correct protocol versions (PATRON_INFO/HOLD/RENEW use v2, CHECKOUT
uses v1)
• Empty patron_id and item_id field detection with debug logging
• Malformed delimiter handling (double delimiters, trailing empty fields)
• Multiple consecutive empty fields in single message
• Confusing delimiter patterns that could break parsing
• Verification that parsed fields are strings, not arrayrefs
• Working siplog mock infrastructure in C4::SIP::Sip::MsgType
Arrayref Prevention Tests:
Tests verify message parsing DOESN'T create empty arrayrefs that would later cause "Odd
number of elements in anonymous hash" errors when passed to Koha::Objects->find()
operations.
Enhanced Debug Logging Tests:
Tests validate that empty critical fields (patron_id, item_id) are detected during
parsing and generate appropriate debug messages to help troubleshoot downstream
constraint errors.
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 499b23d63c)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit 116d3d9f06)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
This change makes it so that the ILS-DI GetRecords output for
"marcxml" will contain the OPAC version of the record.
Test plan:
0. Apply the patches
1. prove t/db_dependent/ILSDI_Services.t
2. Go to http://localhost:8081/cgi-bin/koha/cataloguing/addbiblio.pl?biblionumber=29
3. Click "Settings" to change the framework to Default, and click "Save"
4. Go to http://localhost:8081/cgi-bin/koha/admin/marc_subfields_structure.pl
?op=add_form&tagfield=245&tagsubfield=a&frameworkcode=#subafield
5. Untick the "OPAC" box and click "Save changes"
6. Go to system preferences
7. Change "ILS-DI" to "Enable"
8. Change "ILS-DI:AuthorizedIPs" to 0.0.0.0/0
9. Go to http://localhost:8080/cgi-bin/koha/ilsdi.pl?service=GetRecords&id=29
10. Note that you cannot see the title in the MARCXML.
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 02aba8c0b2)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit 0ec1ea7fc8)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
This change adds the missing interface parameter which
Bug 38233 initially added.
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 e5a302d968)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit 1e35ae3e20)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Automatically redirect users who try to submit a PR at GitHub to instructions on how to submit patches and close the PR. Better experience for potential new developers (since they get immediate feedback, while still in the flow for submitting the patch).
The sparse checkout of readme is probably required, so that the gh command knows on what repo to work with.
Based on: https://github.com/mozilla-firefox/firefox/blob/main/.github/workflows/close-pr.yml
Signed-off-by: Imani Thomas <imani.thomas@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit d6b95024bc)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit f3538049b5)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
To test:
1. Place a hold on many items
2. Change the "Pickup at:" dropdown" to something else
3. Notice the "Pickup location:" dropdowns do not change with it
4. APPLY PATCH
5. Try steps 1 & 2 again. This time the dropdpwns should adjust accordingly
Signed-off-by: Chip Halvorsen <chip.halvorsen@westlakelibrary.org>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 3ffeb14e4b)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit 2f273e4655)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.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>
(cherry picked from commit 83126898de)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit b30e54a86c)
Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com>
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>
(cherry picked from commit 8166a95f2a)
Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com>
This test verifies that when moving subfields between repeatable fields,
existing values in fields that don't contain the source subfield are
preserved.
The test creates multiple 020 fields:
- Some with existing $a values
- Some with $z values to be moved to $a
After applying the move template, it verifies:
- Existing $a values are preserved
- $z values are moved to $a only in fields that had $z
- $z subfields are properly 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 daa661f86c)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit 84839808bb)
Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com>
Executing marc_ordering_process.pl will fail to run with the following errors:
/usr/share/koha/bin/cronjobs/marc_ordering_process.pl: line 21: =head1: command not found
/usr/share/koha/bin/cronjobs/marc_ordering_process.pl: line 23: marc_ordering_process.pl: command not found
/usr/share/koha/bin/cronjobs/marc_ordering_process.pl: line 25: =head1: command not found
/usr/share/koha/bin/cronjobs/marc_ordering_process.pl: line 27: --confirm]: command not found
/usr/share/koha/bin/cronjobs/marc_ordering_process.pl: line 27: --verbose]: command not found
/usr/share/koha/bin/cronjobs/marc_ordering_process.pl: line 27: ./marc_ordering_process.pl: No such file or directory
/usr/share/koha/bin/cronjobs/marc_ordering_process.pl: line 29: or,: command not found
/usr/share/koha/bin/cronjobs/marc_ordering_process.pl: line 31: 0: command not found
/usr/share/koha/bin/cronjobs/marc_ordering_process.pl: line 33: =head1: command not found
/usr/share/koha/bin/cronjobs/marc_ordering_process.pl: line 35: This: command not found
/usr/share/koha/bin/cronjobs/marc_ordering_process.pl: line 36: If: command not found
/usr/share/koha/bin/cronjobs/marc_ordering_process.pl: line 38: =head1: command not found
/usr/share/koha/bin/cronjobs/marc_ordering_process.pl: line 40: =over: command not found
/usr/share/koha/bin/cronjobs/marc_ordering_process.pl: line 42: syntax error near unexpected token `newline'
/usr/share/koha/bin/cronjobs/marc_ordering_process.pl: line 42: `=item B<-v|--verbose>'
This is due to an inadvertent newline before the shebang of the script.
Test Plan:
1) Run ./misc/cronjobs/marc_ordering_process.pl
2) Note the error
3) Apply this patch
4) Run ./misc/cronjobs/marc_ordering_process.pl
5) No error!
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit a067423f3f)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit edec2009dc)
Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com>
To test:
1. Select an existing record and manage items (or create a new record) so that you are on the additem.pl form.
2. Notice that the fields are uneven lengths.
3. Apply the patch. Do a yarn build and restart_all. Clear your browser cache if necessary.
4. Refresh the additem.pl page and notice that the fields are all the same length now.
5. Sign off and have a great day!
Signed-off-by: CJ Lynce <cj.lynce@westlakelibrary.org>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 932c1c07dd)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit 9b4891ea2f)
Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com>
Test plan:
Login at OPAC.
Pass an unexisting reserve_id in a opac-modrequest.pl URL.
Verify that you get a 403 redirect.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 013a469b45)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit 79c93d7a3d)
Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com>
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>
(cherry picked from commit 4bd5b58cd9)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit 82c23565be)
Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com>
Its tran_fail, not tran_error.
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit dbbdfaa7f2)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit 6e64b1b432)
Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com>
it only tests for the presence of "switch"
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit d996664353)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit 316c59c3e3)
Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com>
Test plan:
Enable PrivacyPolicyConsent; you can set it to Permissive.
Check if the OPAC user page (tab Consents) shows it correctly now.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 2a7d8cddc2)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit eb8b5e60cb)
Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com>
The es_indexer_daemon.pl was missing handling for the case where
Koha::BackgroundJob->connect() returns undef without throwing
an exception (which happens when connection fails silently).
This adds the missing warning for that case, matching the behavior
in background_jobs_worker.pl.
Test plan:
1. Set JobsNotificationMethod to 'STOMP'
2. Configure invalid STOMP settings that cause connect() to return undef
3. Run es_indexer_daemon.pl - should show connection warning
4. Verify jobs are still processed via database polling
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 9ca0e183b5)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit f39c370980)
Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com>
The background job workers are showing STOMP/RabbitMQ connection error
warnings even when the JobsNotificationMethod system preference was set
to 'polling', which doesn't require a message broker connection.
This patch adds checks for the JobsNotificationMethod preference before
displaying connection warnings, so warnings only appear when STOMP is
actually expected to be available.
Changes:
- misc/workers/background_jobs_worker.pl: Check syspref before warning
- misc/workers/es_indexer_daemon.pl: Check syspref before warning
Test plan:
1. Set JobsNotificationMethod to 'polling'
2. Run background_jobs_worker.pl - should not show STOMP warnings
3. Run es_indexer_daemon.pl - should not show STOMP warnings
4. Set JobsNotificationMethod to 'STOMP' without RabbitMQ running
5. Run workers - should show appropriate STOMP connection warnings
6. Sign off :-D
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 4ff1f4ca9c)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit c9f4566aa6)
Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com>
- The combobox dropdown in modals scrolled with the modal body instead of overlaying, due to Bootstrap's overflow: hidden on
modals clipping extending content.
- Added context parameter to combobox config; setting context: 'modal' uses Popper's fixed positioning strategy for proper
overlay.
- For reference: https://popper.js.org/docs/v2/constructors/#strategy
To test:
1. Make an item bookable
1.1. Go to a record (e.g. http://localhost:8081/cgi-bin/koha/catalogue/detail.pl?biblionumber=285)
1.2. Go to the Items tab on the left side of the screen
1.3. Under Bookable, choose Yes and click 'Update'
2. Place a booking
2.1. Click 'Place booking' at the top of the detailed record screen
2.2. Fill out the form
2.3. Click 'Submit'
3. Try to cancel the booking
3.1. Go to the 'Bookings' tab on the left side of the screen
3.2. Click 'Cancel' next to the booking
--> A modal appears with a text field for the cancellation reason
3.3. Click 'No, do not cancel'
4. Add one authorised value in BOOKING_CANCELLATION
4.1. Go to Administration > Authorized values
4.2. Search for BOOKING_CANCELLATION
4.3. Click 'Add'
4.4. Fill out the form
4.5. Click 'Save'
5. Redo step 3
--> The option appears as a dropdown below the text field -- OK
6. Add one or two more authorised values
7. Redo step 3
--> The options appear as a dropdown below the text field -- OK
Signed-off-by: Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit f6df86c170)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit 4b734ebaca)
Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.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>
(cherry picked from commit 73719c417f)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit a712b68635)
Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com>
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>
(cherry picked from commit 428ea2baf6)
Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com>
Martin Renvoize
Jonathan Druart
Nick Clemens
Pedro Amorim
Lucas Gass
Andrew Fuerste Henry
Tomás Cohen Arazi
Paul Derscheid
Kyle M Hall
Fridolin Somers
Lisette Scheer
Marcel de Rooy
David Cook
Michał Kula
Laura_Escamilla