Reports may consume too much resources of the db. To avoid that a new config param
'report_sql_max_statement_time_seconds' was created, which sets the maximum time a report statement may
take to execute in seconds. If it takes longer it will be aborted.
reflowed the code, so that is is executed after is_sql_valid, and put
the code in a subroutine
Test plan:
0. if you run this test in ktd you should check if you are running
mysql or mariadb, and then be able to switch database
1. create a new report by going to
More > Reports > New SQL Report
(/cgi-bin/koha/reports/guided_reports.pl?op=add_form_sql)
enter the following SQL (which only purpose is to run long)
select ExtractValue(bm1.metadata,'//controlfield[@tag="001"]')
from biblio_metadata bm1
join biblio_metadata bm2 on
ExtractValue(bm1.metadata,'//controlfield[@tag="001"]') =
ExtractValue(bm2.metadata,'//datafield[@tag="773"]/subfield[@code="w"]')
give the Report a name eg "extract_long"
2. run the report
3. report should be executed and return several 1000 lines on ktd
4. apply patch
5. add a the parameter to koha-conf.xml to the <config> </config> part
<report_sql_max_statement_time_seconds>0.001</report_sql_max_statement_time_seconds>
OR use the following statemtn
sed -i '/<biblioserver>/a <report_sql_max_statement_time_seconds>0.001<\/report_sql_max_statement_time_seconds>' /etc/koha/sites/kohadev/koha-conf.xml
6. restart koha
koha-plack --restart kohadev
7. go back to the report page and rerun report which now should lead to an error
The following error was encountered:
The database returned the following error:
Query execution was interrupted, maximum statement execution time exceeded
Please check the log for further details.
8. in koha-conf.xml set
<report_sql_max_statement_time_seconds>20</report_sql_max_statement_time_seconds>
9. restart koha
10. rerun query, which now should execute as before
11. stop ktd, and restart with the "other" database
DB_IMAGE=mysql:8.0 ktd up
DB_IMAGE=mariadb:10.5 ktd up
12. repeat from step 1 (omit applying the patch)
13. Sign off
14. Thx
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz>
As Jan Kissig correctly mentioned on the BZ report, the various
summary info on charged items etc. are included.
This is done via the summary_info sub.
Note that the TODOs about
BZ FID_HOLD_ITEMS_LMT
CA FID_OVERDUE_ITEMS_LMT
CB FID_CHARGED_ITEMS_LMT
are still valid. This is not included yet.
Test plan.
Since we are just removing comments, read the code.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com>
This patch additionally fetches the reply to address for advance notices
To test:
1 - Have some issues that will be due soon for a patron(s)
Checkout with the due date set to tomorrow
2 - Verify the patron(s) have an email defined, verify the issuing branch has a reply to set
3 - Verify the patron(s) have messaging preference set to receive Advance notices by email 1 day in advance
4 - perl misc/cronjobs/advance_notices.pl -v -c
5 - Check the message queue
SELECT * FROM message_queue WHERE borrowernumber={##}
6 - Note no rpely to is set
7 - Apply patch
8 - perl misc/cronjobs/advance_notices.pl -v -c
9 - Check the message queue
10 - Confirm newer message has the reply to populated
11 - Ensure the issued item has a different homebranch with a reply to address se
12 - perl misc/cronjobs/advance_notices.pl -v -c --frombranch item-homebranch
13 - Confirm message has correct reply to
14 - Set patron to want digest in messaging preferences
15 - Generate notices
16 - Confirm digets has the right reply to
Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Tidied inline
Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com>
It would be at least helpful to add a reference to get_transfer here,
since we are replacing it here.
Used the opportunity to update the older comments
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com>
This patch removes a fetch of the item object followed but get_transfer to simply search the
transfers by itemnumber and status
To test:
1 - Add many items to your records
https://github.com/kidclamp/handy-koha-script/blob/main/randitems.pl
NOTE: this script makes a lot of items withdrawn or damaged, undo that for more dramatic difference
UPDATE items SET withdrawn=0,damaged=0
perl misc/search_tools/rebuild_elasticsearch -v
2 - Set Koha system preferences to display more results and check more items:
maxItemsInSearchResults 60
numSearchResults 50
3 - Do a search, several times, note the time it takes
4 - Apply patch, restart all
5 - Repeat search several times, note the time it takes
6 - Confirm it is better after the patch
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com>
If a message ends in a non-alphanumeric character, that character is stripped from the message.
For example:
6300120260210 182641 AOMPL|AA23529000035676(|
64 00120260210 182831000000000000000000000000AOCPL|AA23529000035676|AEHenry Acevedo|BLY|BHUSD|BV0|CC5|BD4345 Library Rd. Springfield, MA 44224|BF(212) 555-1212|PB19580530|PCS|PIY|AFGreetings from Koha. |
versus:
6300120260210 182641 AOMPL|AA23529000035676(|ACkoha|
64YYYY 00120260210 182835000000000000000000000000AOCPL|AA23529000035676(|AE|BLN|AFInvalid cardnumber|
The former should be returning the same "Invalid cardnumber" message the latter does.
Test Plan ( assumes you are using KTD ):
1) Install telnet: sudo apt install -y telnet
2) telnet 127.0.0.1 6001
3) Log in with: 9300CNkoha|COkoha|CPMPL|
4) Send the patron information request: 6300120260210 183346 AOMPL|AA23529000035676(|
5) Note you get a patron back
6) Apply this patch
7) Restart SIP!
8) Repeat steps 2-5
9) Noe you doe not get a patron back in the response!
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com>
This bug adds Koha::Patron::Message::message_without_newlines, which has been added to C4::SIP::ILS::Patron in lieu of Koha::Patron::Message::message. The aim of this change is to prevent SIP2 units from crashing out when they encounter a patron message with newlines (\n or \r) in it, as units generally use a newline character to denote the end of a SIP2 transaction.
To test:
a) go to /cgi-bin/koha/sip2/institutions
*) create an institution, name CPL, timeout 300, the rest default
b) go to /cgi-bin/koha/sip2/accounts
*) create an account, userid koha, institution CPL, terminator crlf, the rest default
c) go to /cgi-bin/koha/members/moremember.pl?borrowernumber=51
*) click add message in toolbar
*) enter some multi-line message e.g. test[RETURN]1[RETURN]2[RETURN]3[RETURN][RETURN][RETURN]
d) run tail -f /var/log/koha/kohadev/sip-output.log, keep this window open
e) run ./misc/sip_cli_emulator.pl -a 127.0.0.1 -p 6001 -su koha -sp koha -l CPL --patron 42 -m patron_information
f) look at the tail log, notice how the MSG 64 line has a newline before the final pipe (|) character
== APPLY PATCH ==
g) restart_all
h) repeat steps e-f
i) notice how in the tail log, the MSG 64 line now has the final pipe character contiguously placed with the successive characters
== SIGN OFF ==
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com>
The term 'If any unavailable' is not very clear but it is also
very hard to find something short and better.
A next available hold is possible when at least one item is
unavailable (checked out, on order, waiting/transit hold).
A item level hold is possible when the specific item is
unavailable.
Test plan:
Read the patch. The code did not change.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com>
Test plan (ktd):
1 - Have henry acevedo have no mail and set messaging preference "Hold filled to sms"
2 - Have edna acosta have not messaging preference
3 - Place an hold on two books for edna acosta and henry acevedo, check in both books and select the button to fill the hold
4 - Check in notices and see that both received a notice
5 - Apply tests and run tests -> they should fail
6 - Apply patch, run rest -> they should pass
7 - Unfill both holds and fill them back. Henry should keep receiving notices while Edna should not anymore.
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com>
To test:
1. Set 'EasyAnalyticalRecords' to 'Show'.
2. Find or create a parent record that has at least 2 items on it. Note the itemnumbers and the framework.
3. Edit that framework so you can add a 773$0, 773$9, and 773$t
4. Find or create a child record.
5. Make at least 2 773 entries on the child record. Add the biblionumber of the host record to the 773$0.
6. In each entry for the 773$9 add one of the itemnumbers from the host record.
7. For the 773$t and the biblio title of the host record.
8. Save.
9. Now attempt to put a hold on the child record.
10. 500 error:
11. APPLY PATCH and restart_all
12. Try placing the hold again, this time it succeeds.
Signed-off-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com>
When starting SIP, Koha passes an incorrect value to the
Net::Server "group" parameter, which can generate an
error in the logs":
Couldn't become gid "<uid>": Operation not permitted
This change ensures that a valid numeric GID is passed instead.
Test plan:
1. Enter KTD shell.
2. To reproduce the error: sudo groupmod -g 8001 kohadev-koha
3. Restart SIP: sudo koha-sip --restart kohadev
4. Check SIP logs:
/var/log/koha/kohadev/sip-output.log
5. Observe the error:
Couldn't become gid "<uid>": Operation not permitted
6. Apply this patch.
7. Repeat steps 2 to 4.
8. Verify that the "Couldn't become gid" error is no longer logged.
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com>
To test:
1. prove t/db_dependent/Auth.t
2. Observe success
Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit dbf812869d)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
This patch completes the deprecation cycle started in bug 36343 by
removing the backward compatibility code and deprecation warnings for
the old after_biblio_action hook signature.
Changes made:
1. Removed the duplicate biblio and biblio_id parameters from
_after_biblio_action_hooks in C4::Biblio that were marked for
deprecation in 24.11.00
2. Updated the test plugin to use only the new payload-based signature,
ensuring all parameters are accessed via $params->{payload}
The after_biblio_action hook now consistently uses the modern signature
with a payload parameter, matching the pattern used by other plugin hooks
like before_biblio_action, after_item_action, and after_hold_action.
Test plan:
1. Run: prove t/db_dependent/Koha/Plugins/Biblio_and_Items_plugin_hooks.t
2. Run: prove t/db_dependent/Koha/Plugins/
3. Run: prove t/db_dependent/Biblio.t
4. All tests should pass
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 1571fd489c)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
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>
If 'default_save_state' is not in the yaml file then the feature is
enabled by default.
Caught by the following failure:
t/db_dependent/selenium/patrons_search.t ..
# Failed test 'Search works when category of patrons is limited to a library we are not signed in at'
# at t/db_dependent/selenium/patrons_search.t line 471.
# got: 'Showing 1 to 5 of 26 entries (filtered from 82 total entries)'
# expected: 'Showing 1 to 15 of 26 entries (filtered from 82 total entries)'
# Looks like you failed 1 test of 1.
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit eba0060286)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
If a row exists in DB for a given table, default_sort_order will be NULL
in DB which will replace the flag from the YAML file: the feature won't
be available.
Test plan:
0. checkout main (we don't want bug 30633)
1. Go to http://localhost:8081/cgi-bin/koha/admin/columns_settings.pl#biblio-detail%7Choldingst
=> Notice that "Default sort order" is not available for 'holdingst'
(it's added by bug 30633)
2. Save
3. Verify what you have in DB:
select * from tables_settings where module="opac" and tablename="holdingst";
4. Apply 30633, restart_all
5. Go to http://localhost:8081/cgi-bin/koha/admin/columns_settings.pl#biblio-detail%7Choldingst
=> Notice that "Default sort order" is not available for 'holdingst' (not ok)
6. Save
7. Verify what you have in DB:
select * from tables_settings where module="opac" and tablename="holdingst";
8. Apply this patch set
9. Go to http://localhost:8081/cgi-bin/koha/admin/columns_settings.pl#biblio-detail%7Choldingst
=> Notice that "Default sort order" is available for 'holdingst'!
10. Save, verify what you have in DB.
Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit de27044ba2)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Writing these I found that transferbook was generating two queue rebuilds - fixed that
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 168f52f99b)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
This patch does two things:
1:
It prevents rebulding the real time holds queue during a checkin if a reserve was found.
The reserve will either:
a - be set to waiting if at correct location and no confirmation - this is fine because we will rebuild the queue when
filling the hold
b - trigger a popup whihc will rebuild the queue when confirming the transfer, or be ignored
2:
It adds a form to ignoring a hold so that we can rebuild the holds queue if we decided not to use the current item
to fill the found hold
To test:
0 - Enable RealTimeHoldsQueue
1 - Place several holds for different libraries on a title
2 - Checkout a copy to a patron at current library (ignore current holds)
3 - Clean up DB so we can see outcomes from next steps:
delete from branchtransfers; delete from background_jobs; update reserves SET found=NULL where found='T';
4 - Check in item, confirm hold
5 - Check background jobs, note two rebuilds of the holds queue
6 - Check item out to previous patron, repeat 3
7 - Check in item, ignore the hold
8 - Check background jobs, note only one rebuild
9 - Apply patch, restart all
10 - Checkout item, repeat 3
11 - Checkin item, confirm hold
12 - Check background jobs, onlt one rebuild of holds queue
13 - Checkout item, repeat 3
14 - Checkin item, ignore hold
15 - Check background jobs, only one rebuild of holds queue
Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit f9d096df27)
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>
This patch removes the duplicate logging code from the original
Bug 9762 implementation that was not properly removed when the
Alternative implementation was merged.
The original implementation added separate logaction() calls in
CanBookBeIssued and circulation.pl that created additional log
entries with a different JSON format. This caused double logging
when the Alternative implementation (which logs via AddIssue) was
merged on top.
Changes in this patch:
C4/Circulation.pm:
- Removed @message_log array and all pushes to it
- Removed the separate logaction() call at the end of CanBookBeIssued
- Removed 5th return value (@message_log) from CanBookBeIssued
- Removed all if ($issueconfirmed) blocks that were populating @message_log
circ/circulation.pl:
- Removed the problematic line that added OVERRIDDEN to every confirmed
checkout: $needsconfirmation->{'OVERRIDDEN'} = $issueconfirmed
This addresses the double logging issue where circulation overrides
were being logged twice with different formats.
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 2fea44ff25)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
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>
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>
This patch uses the PDF::Reuse::Barcode algorithm for calculating barcode
background box length relative to barcode length in order to determine
the actual barcode width and thus the X scale factor required to re-size
that barcode so that it fits on the label width (or rather the
"Barcode width" which is a percentage applied to the label width
to create a narrower usable surface for the barcode and barcode background
box).
To test:
0. Apply the patch
1. Go to http://localhost:8081/cgi-bin/koha/cataloguing/additem.pl?biblionumber=29
2. Add an item with barcode 123456, add an item with barcode 39999000001310399991,
and ensure an item with barcode 39999000001310 already exists
3. Go to http://localhost:8081/cgi-bin/koha/labels/label-manage.pl?label_element=batch
4. Create a patch that uses 3 items using 39999000001310, 2 items with 123456,
and 2 items with 39999000001310399991
4. Go to http://localhost:8081/cgi-bin/koha/labels/label-manage.pl?label_element=layout
5. Edit the layout "Label Test"
6. Change the "Barcode width" to 1, and enable "Draw guide boxes", and click "Save"
7. Export the batch using the template "Avery 5160 | 1 x 2-5/8" and the layout "Label Test" and open as PDF
8. Note that the whole barcode fits inside the red guidebox (there should be some
overlap of white on top of the red guidebox but that is normal since we're using 100% of the
barcode label width).
9. Edit the "Label Test" layout to try "Code39 + Modulo43" and "Code39 + Modulo10" and retry batch export
10. Edit the "Label Test" and try different "Barcode width" like the usual default .8
11. Celebrate that your barcodes fit on the label!
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 9da5f23f96)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Patron messages containing carriage returns, line feeds, or CRLF
combinations can break SIP protocol communication since CR is used
as the SIP message terminator. This causes message parsing failures
and protocol errors in self-check systems.
The previous regex only handled single carriage returns (\r),
missing Windows CRLF (\r\n) and Unix LF (\n) sequences.
This patch normalizes all line ending variations to spaces to
prevent SIP message corruption while preserving message content.
Test plan:
1. Go to a patron record and click "Add message"
2. Select "Add message for OPAC..." from dropdown
3. In a text editor (e.g., Word), create a message with line breaks:
"This is line one[Enter]This is line two[Enter]This is line three"
4. Copy and paste this multi-line text into the message box and save
5. Run the SIP emulator: misc/sip_cli_emulator.pl
6. Execute a patron information request for this patron
7. Before patch: Observe message breaks at line endings, corrupting
the SIP response
8. Apply patch and restart SIP server
9. Repeat steps 5-6
10. After patch: Verify the message appears as single line with
spaces replacing line breaks: "This is line one This is line two
This is line three"
11. Test with different line ending types (Windows, Mac, Unix) to
ensure all variations are handled correctly
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
[EDIT] Adjusted the comment in Sip.pm
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit affa3e62fd)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
When implemented, the message here was believed to either be about Duplicate IDs, or guarantor issues.
We are now seeing a wider variety of issues possible from SQL errors, lock timeout or missings rows as examples
This patch adds an 'ErrorMessage' message to the return from AddReturn and displays it to the user.
To test:
1 - Issue an item to a patron, note the itemnumber
2 - On the command line, lock the row:
sudo koha-mysql kohadev
START TRANSACTION;
SELECT * FROM issues WHERE itemnumber=975 FOR UPDATE;
leave mysql open at this point
3 - On the interface, check in the item, wait a bit
4 - After a bit you get the error to check the config page, but nothing is there
5 - close mysql, apply patch, restart all
6 - Repeat 2
7 - Check in the item, wait a bit
8 - Note you now get the error from the DB:
Error: (DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st execute failed: Lock wait timeout exceeded; try restarting transaction at /kohadevbox/koha/Koha/Object.pm line 174 )
9 - Sign off!
Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 3f61f6088f)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
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 47f0aab19f)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
CancelExpiredReserves had incorrect holiday logic that checked if **today** is a holiday instead of checking if the **hold's expiration date** was a holiday.
This caused:
* Holds that expired on non-holidays to remain active if cleanup script ran on holidays
* Holds that expired on holidays to be cancelled if cleanup script ran on non-holidays
The simple fix exposed additional edge cases around server downtime that could leave holds active indefinitely.
Changes:
* Implements comprehensive holiday logic when `ExpireReservesOnHolidays = 0`
* Same-day protection: Don't cancel holds that expire TODAY if today is a holiday
* Historical cleanup: Cancel holds that expired in the past when running on business days
* Retrospective cancellation: Handle server downtime by checking for missed business days
* Comprehensive test suite covering all scenarios including long-term expired holds
To test:
1. Apply the tests patch
2. Run:
$ ktd --shell
k$ prove t/db_dependent/Reserves/CancelExpiredReserves.t
=> FAIL: Tests fail! The logic in the method is wrong!
3. Apply this patch
4. Repeat 2
=> SUCCESS: All tests pass, demonstrating correct behavior in all scenarios
5. 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 31dced9c8b)
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 f2adf0ad82)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
To test:
1. In circ rules set a Default waiting hold cancellation policy to allow hold cancellation requests.
2. Add a hold for a patron and create a hold cancellation request.
3. Check the item in via SIP, I used the SIP emulator:
kohadevbox/koha/misc/sip_cli_emulator.pl -a localhost -p 6001 -l MPL -su term1 -sp term1 -m checkin --patron 23529000035676 --item 39999000001310
4. The hold remains.
5. APPLY PATCH, restart_all
6. Try 2 - 4 again, the hold should be cancelled
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 2246bfbb4a)
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>
This commit adds two key improvements to the Bug 9762 circulation override logging:
1. **Consistent JSON Logging**: Updated AddRenewal to always log in JSON format,
matching AddIssue behavior. Previously AddRenewal only logged JSON when
confirmations/forced were present, otherwise logged just the item number.
2. **Integration Tests**: Added comprehensive tests for both AddIssue and AddRenewal
JSON logging functionality:
- Test normal operations (empty confirmations/forced arrays)
- Test override operations (populated confirmations/forced arrays)
- Verify JSON structure and content accuracy
The AddRenewal function now consistently logs:
```json
{
"issue": 123,
"itemnumber": 456,
"confirmations": [...],
"forced": [...]
}
```
This ensures uniform action log format across all circulation operations and
provides better integration test coverage for the override logging functionality.
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit b31f8c566d)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 489780f3f9)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 255c0897f2)
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>
We never actually pass `$message` back to anywhere so there's no reason
to create it in the first place.
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 86f4eb93aa)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit b4e26e9c48)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit ee74749058)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 9183fca07d)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
This patch allows you to save commands that have been overridden by someone.
The patch has been updated but is not yet completed. Tests 10, 13, 14, and 17 are still not functional. Tests 15 and 16 need to be fixed as they do not display the barcode. All other tests should be functional.
To test:
--After each step, a message will be displayed in the action_logs table and in Tools > Log viewer --
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 (In circulation rules put "On shelf holds allowed" to "If any unavailable")
Sponsored-by: Solutions inLibro inc. <https://inlibro.com/>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 89811768b6)
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>
This patch updates all existing calls to the renamed method throughout
the codebase to maintain compatibility.
Changes:
* Updated C4::Circulation calls to use revert_found()
* Updated C4::Reserves calls to use revert_found()
* Updated test files to use revert_found()
* Maintained all existing functionality and behavior
All existing functionality is preserved - this is purely a method
name change to reflect the expanded capability.
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 4ad2c94690)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
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 a137c7a183)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
This change removes a line in the developer doc which is no
longer relevant.
There's not really much to test here.
Signed-off-by: Lin Wei Li <lin-wei.li@inLibro.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit a6713d8849)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
When adding a document that does not use 010d as price document, but 071d, the price is not imported from koha since it only checks 010d and 345d.
To reproduce:
1 - Create a Koha item, make sure there is neither 010d or 345d
2 - Add a 071d and fill a price
3 - Export the item as marcxml
4 - If you do not have one: in acquisition, create a vendor, a basket associated to this vendor, and a line associated to this vendor
5 - Add a line to any open basket, select: import "from a new file" and
select the exported marcxml -> The price in 071 is not imported
6 - Apply patch
7 - Repeat 5 -> the price is properly imported
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 1cb2f97e8c)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit d6883d9e3c)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Mark Hofstetter
Marcel de Rooy
Nick Clemens
Kyle M Hall
Jake Deery
David Cook
Baptiste
Lucas Gass
Slava Shishkin
Tomas Cohen Arazi
Lari Taskula
Martin Renvoize
Jonathan Druart
Bernard Scaife
Hammat Wele
Matthias Le Gac
emilyrose