**Problem:**
When using SIP checkout messages with the "no block" flag, the specified due date
was being ignored. Instead of honoring the provided due date, the system would:
1. Use the due date as the transaction timestamp (incorrect behavior)
2. Calculate a new due date based on circulation rules
3. Return an empty due date field (AH) in the SIP response
This affected SIP2 integrations where external systems needed to specify exact
due dates for checkouts, particularly for offline circulation scenarios.
**Root Cause:**
The `ProcessOfflineIssue` function in C4::Circulation was only returning a
success/failure message, but the SIP checkout transaction needed access to the
resulting checkout object to properly set the due date. The SIP Transaction::Checkout
module was calling `ProcessOfflineIssue` but couldn't retrieve the checkout details
to populate the transaction's due date field.
**Solution:**
1. Modified `ProcessOfflineIssue` to return both the status message AND the checkout
object: `return ( "Success.", $checkout );`
2. Updated `ProcessOfflineOperation` to handle the new return format by capturing
only the message: `( $report ) = ProcessOfflineIssue( $operation );`
3. Modified SIP Transaction::Checkout to capture both return values and use the
checkout object to set the proper due date via `duedatefromissue`
4. Fixed the timestamp parameter to use `dt_from_string` instead of the due date
**Test Plan:**
1. **Unit Tests:**
- Run `prove t/db_dependent/Circulation/OfflineCirculation.t` to verify
ProcessOfflineIssue returns checkout object and respects due_date parameter
- Run `prove t/db_dependent/SIP/Transaction.t` to verify SIP checkout honors
no_block_due_date parameter
2. **Manual SIP Testing:**
- Connect to SIP server via telnet on port 6001
- Send checkout message with no block flag and specific due date:
`111YN20250115 18000120241201 180001AP|AO|AC|AD|AB<barcode>|AA<cardnumber>|`
- Verify response contains correct due date in AH field
- Verify checkout record in database has the specified due date
3. **Regression Testing:**
- Verify normal SIP checkouts (without no block) still work correctly
- Verify offline circulation operations continue to function
- Verify ProcessOfflineOperation still processes queued operations correctly
Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
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 4d069e5006)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit d1f3ba0767)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
The regexp for the value builder 'hbyymmincr' doesn't allow for numerics in the branchcode,
so the nextnum found is always 0 and multiple Add always generate the same barcode.
This simply allow for numerics within the regex.
Test
- It's a bit complicated. You need
- all your branchcodes to be numerics (01,02,71...)
- all barcodes to already be a bunch of numbers
- Modify the syspref 'autobarcode' to hbyymmincr, the home branch + date one.
- Find an item in the pro-search,
- New -> New Item
- Click on the 'p' to have the value builder create the next barcode.
- Without the patch, the query won't have found anything, so it'll suggest xxxxxx0001
- Click Add. It reprensent the form to enter another one.
- Click on the 'p' edit box, it'll offer you again xxxxxx0001
With the patch, it'll offer valid ones everytime.
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit db067d48ee)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
When using the OPAC and you are not logged in, there are warnings
in the logs:
[WARN] Use of uninitialized value $branch_limit in concatenation (.) or string
It comes from C4::Koha::GetAuthorisedValues() :
my $branch_limit = C4::Context->userenv ? C4::Context->userenv->{"branch"} : "";
my $cache_key = "AuthorisedValues-$category-$opac-$branch_limit";
C4::Context->userenv->{"branch"} can be undef
The patch fixes using C4::Context::mybranch().
Test plan:
1. Set the OpacAdvancedSearchTypes system preference to something
other than "itemtypes", for example "loc".
2. Go to OPAC without logging in, and perform a search.
3. Check the /var/log/koha/kohadev/plack-opac-error.log and note
the warnings in the log:
[WARN] Use of uninitialized value $branch_limit in concatenation (.) or ...
(Alternative, tail the logs: tail -f /var/log/koha/kohadev/*.log
4. Apply the patch.
5. Repeat step 2 and check that there are no longer any warnings.
6. Sign off.
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 d60deb348c)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit 8854037fbd)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
This patch updates the only use of prepare_host_field to use the well
tested Koha::Biblio->generate_marc_host_field method instead and then
removes the aforementioned prepare_host_field method from C4::Biblio.
Test plan
1) Ensure the "enhanced workflow" for creating analytics still functions
as expected.
Sponsored-by: Open Fifth <https://openfifth.co.uk>
Signed-off-by: Heather Hernandez <flyingendpaper@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 3e627d7a77)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit a5949e7575)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Test that input values don't change when send to the
sub restore_form_values.
To test prove t/Form_MessagingPreferences.t
Sponsored-by: Koha-Suomi Oy
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 2c4a89801e)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit a57bd07127)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
If error occurs while creating a new patron messaging
preferences are either lost or reseted as default
fro patron category. This patch adds new method
restore_form_values to restore already added messaging
preferences.
To test:
1. Enable syspref EnhancedMessagingPreferences.
2. Create a new patron but cause an error to prevent it from
saving (e.g. add wrong age).
3. Add messaging preferences for patron (if there are default
preferences, add also new ones or remove defaults).
4. Attempt to save.
=> Note that messaging preferences were lost and you need to
readd them.
5. Apply this patch.
6. Repeat steps from 2 to 4.
=> Note that messaging preferences which you added are kept
even if error occurs.
Sponsored-by: Koha-Suomi Oy
Signed-off-by: Jesse Maseto <jesse@bywatersolutions.com>
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 8ed31509ae)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit 8d496f1bed)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
This patch modernizes the Scrubber.t unit test with comprehensive
coverage following Koha best practices:
- Uses subtest structure for logical grouping
- Tests all scrubber types (default, comment, note)
- Includes security testing for XSS prevention
- Covers edge cases and error handling
- Tests new list elements added in previous commits
Also fixes C4::Scrubber to handle undef parameters cleanly by
treating them as 'default' type instead of generating warnings.
Test plan:
1. Run prove t/Scrubber.t
2. Verify all tests pass without warnings
3. Confirm comprehensive coverage of module functionality
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 21f3340442)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit 1ab7ccde18)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Adds support for HTML description list elements (dl, dt, dd) to the
'note' profile in the HTML scrubber configuration. This allows
librarians to use description lists in note fields for better
structured content presentation.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 9ef8f098d8)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit 421968355e)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
This patch add the unordered list, ordered list and list item html tags
to the list of allowed tags in C4::Scrubber.
Test plan:
1) Add a course reserve and include the following in the public note
field:
<ul>
<li>First item</li>
<li>Second item</li>
</ul>
2) View the course details page and note "First item Second item" all
appears on the same line (the ul and li tags have been stripped)
3) View the course reserves page and note the same issue in the public
notes field of the table display
4) Apply this patch
5) Repeat 2 and 3 and note you now have a bullet pointed list displayed
in both places
6) The display of the note on the details page may look mis-aligned..
that will be dealt with on it's own bug
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 ddad4e81a2)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit fb2899f311)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
The reference to this bug report may be helpful to understand
later why we did so.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 01526ef545)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit 4efb86aec8)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Currently, when removing 040 $f from authority record with 008/11 =
'z' (MARC 21), merge function removes the content of $2 subfield in
linked 6XX fields, leaving the 6XX $2 subfield empty. This is not
correct. We should not have empty subfields in a MARC record.
Instead, the old content of the $2 subfield should be retained (if
auth 008/11 is still = 'z').
Test plan:
==========
1. Have an authority record with 008/11 = 'z' and 040 $f = 'whatever'.
2. Link the record to a bibliographic 6XX.
3. Edit the authority record, removing 040 $f.
4. Go to bibliographic record and in modal "MARC preview" from Normal
view notice that 6XX $2 exists but is empty. This is wrong.
5. Apply the patch ; restart all.
6. Repeat p. 1.-4. Note that the content of 6XX $2 has not been
removed.
Sponsored-by: Ignatianum University in Cracow
Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit bd43ebdc5e)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit ba31b8cc35)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Following the original test plan you notice, `THIS IS A TEST` is printed
twice:
* In the logs.
* In STDERR for the SIP server.
The latter is redundant, and also problematic for
`xt/author/pod_coverage.t` which fails the `no warnings` tests because
of that.
To test:
1. Run:
$ ktd --shell
k$ prove xt/author/pod_coverage.t
=> FAIL: Scary errors, lots of them
2. Apply this patch
3. Repeat 1
=> SUCCESS: Tests pass!
4. Follow the original bug test plan
=> SUCCESS: There's `THIS IS A TEST` in the logs, which was the author's
goal in the first place.
5. Sign off :-D
6. Visit bug 40144 :-P
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 9a4fd4facb)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit 036d940d54)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
At some point our SIP server stopped logging fatal errors to either logs or standard error. We should restore this vital functionality.
Test Plan:
1) Apply the first patch which will cause the patron information request
to fail
2) Stop the SIP server
3) Start it again with: perl /kohadevbox/koha/C4/SIP/SIPServer.pm /etc/koha/sites/kohadev/SIPconfig.xml
4) Trigger the failure with the following command:
./misc/sip_cli_emulator.pl -a 127.0.0.1 -p 6001 -su koha -sp koha -l MPL --patron koha -m patron_information
5) Note "THIS IS A TEST" is not found in logs or stderr
6) Apply the second patch
7) Repeat steps 2-4
8) Note the error message is now visible from stderr at the command line
and also in the SIP log
Signed-off-by: Magnus Enger <magnus@libriotech.no>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit f214db14dd)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit 7c06904612)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
This patch removes unused "tag" and "staff" scrubber profiles.
Test plan:
0. Apply the patch
1. prove t/Koha/Plugins/HtmlScrubber.t
2. prove t/Scrubber.t
3. Grep around the code for C4::Scrubber and make sure "tag"
and "staff" aren't used as arguments for C4::Scrubber
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 e846abeeb7)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit f72d9f2962)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
This patch makes the code in `CheckReserves` survive the non-existence
of the referenced item type in the `items` table for the passed item.
To test:
1. Apply the regression tests
2. Run:
$ ktd --shell
k$ prove t/db_dependent/Reserves.t
=> FAIL: Tests explode like this:
```
Can't call method "notforloan" on an undefined value at /kohadevbox/koha/C4/Reserves.pm line 870.
```
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!
5. Sign off :-D
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 854783c836)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit 3284f58e95)
Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com>
This patch fixes a regression introduced by Bug 38505.
To test:
1. Enabled OpacTrustedCheckout system preference
2. Search for an item and get the barcode
3. Go to the OPAC
4. Click the Self-checkout button at the top
5. Enter the barcode and submit
6. Notice you are given an error UNKNOWN_BARCODE and the item is not checked out
7. Apply the patch and restart services
8. Repeat steps 4-6 and confirm you are able to successfully check out the item
9. Test submitting the modal with no barcode entered - confirm you are shown an appropriate error
10. Test with a fake barcode that does not exist - confirm you are shown an appropriate error
11. Run tests to confirm no regressions
prove t/db_dependent/Circulation*
prove t/db_dependent/DecreaseLoanHighHolds.t
prove t/db_dependent/rollingloans.t
prove t/db_dependent/api/v1/checkouts.t
prove t/db_dependent/Patron/Borrower_PrevCheckout.t
prove t/db_dependent/Koha/ILL/Requests.t
Sponsored-by: Reserve Bank of New Zealand
Signed-off-by: Magnus Enger <magnus@libriotech.no>
Works as advertised, including the tests.
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit 2b770e5e77)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
This change checks that the string width is larger than 0 before
attempting to perform line wrapping on the line.
Test plan:
0. Apply the patch and koha-plack --restart kohadev
1. Follow instructions at end of this message[1]
2. Note that the infinite loop does not happen and that a blank
card is created
3. Export the batch using the test layout and template that come
with koha-testing-docker
4. Note that the patron card is correctly created
[1]
To reproduce:
0. Change Koha test user's firstnameto "Firstname" and surname to "Lastname"
1. Create a template:
Units: PostScript Points
Page height: 60pt
Page width: 60pt
Card height: 60pt
Card with: 60pt
Top page margin: 10pt
Left page margin: 10pt
Number of columns: 1
Number of rows: 1
Gap between columns: 10pt
Gap between rows: 10pt
2. Create a layout:
Units: US inches
Field 1:
Text: Library Name
Font: Times-Roman
Font size: 12 pt
Text alignment: Left
Lower left X coordinate: 100in
Lower left Y coordinate: 100in
Field 2:
Text: <firstname> <surname>
Font: Times-Roman
Font size: 10pt
Text alignment: Left
Lower left X coordinate: 5in
Lower left Y coordinate: 5in
3. Create a batch for borrowernumber 51 (if using "koha" test user in ktd)
4. Export using the above template and layout
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit e223b17d68)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Set these sysprefs:
- FinesMode: Calculate and Charge
- WhenLostForgiveFine: Forgive
- WhenLostChargeReplacementFee: Charge
In circ rules:
- Have a rule that charges fines
- Set Refund lost item replacement fee to "Refund lost item charge and charge new overdue fine"
1 - Have/create item with replacement price
2 - Check item out to patron 1, due date in past
3 - Run fines.pl
4 - confirm fines on patron 1
5 - mark item lost
6 - confirm replacement cost generated and fine forgiven on patron 1
7 - check item in
8 - confirm replacement cost forgiven and new fine generated on patron 1
9 - check item out to patron 2, due date in future
10 - check item in
11 - mark item lost (no replacement fee generated because item is not checked out)
12 - in database, update date_due for checkout to patron 2, set to a date in the past
13 - check item in
14 - confirm patron 2 now has a fine
15 - Apply patches, restart all
16 - Repeat test plan with another item and confirm no fine is added for patron 2
Note: At step 12 we're pushing the date_due of the old issue into the past to make date_due<curdate(). In production, this comes up when there are days between steps 10 and 13.
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 67c95addeb)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit 9c81bdf529)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
This change turns off ABSOLUTE and EVAL_PERL for user-entered templates,
as these options can allow for arbitrary code execution and information
disclosure.
Test plan:
0. Apply the patch
1. koha-plack --restart kohadev
2. Create a Notice for Reports with a report like:
[% IF ( x == 1 ) %]
One
[% ELSE %]
Two
[% END %]
3. Run a report using this template
4. The report should show "Two"
5. Turn on CHECKOUT emails for a test patron
6. Perform a checkout
7. Look at the CHECKOUT email and note that the email body
is correctly generated
Signed-off-by: Magnus Enger <magnus@libriotech.no>
Would be good to have this fixed as the default.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
The test plan should include PERL blocks or absolute paths! Tested those
instead. Testing x==1 seems irrelevant.
I tried:[% PERL %]print 1;[% END %] and [% INCLUDE /app/Dockerfile_1 %]
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Test plan:
0 - Apply other patches - reindex your catalog
perl misc/search_tools/rebuild_elasticsearch.pl -v -d
1 - Set the prefs AutoLinkBiblios and CatalogModuleRelink to Do
2 - Create three GEOGR_NAME authorities:
151$aDismal Swamp
151$aDismal Swamp$gSouth
151$aPit of Despair
3 - In a new bib record, add 651 fields
651$aPit of Despair
651$aDismal Swamp
651$aDismal Swamp$gSouth
4 - Click Link authorities automatically
5 - 'Pit of despair' links correct - other records both link to 'Dismal swamp'
6 - Apply this patch, restart_all
7 - Click Link authorities automatically
8 - Dismal Swamp and Dismal Swamp South are correctly linked
Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
Signed-off-by: Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit e93057f4fc)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
This patch adds the subfield g to the hardcoded list of controlled fields
so that it is no longer ignored
To test:
1 - Edit a GEOGR_NAME authority to add a g subfield to field 151
2 - Search for this authority in Authorities
3 - Don't see g
4 - Apply patch, restart all
5 - Search again, you see g!
6 - Edit a record and link to the authority
7 - g is copied over!
Signed-off-by: Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com>
Signed-off-by: Thomas Klausner <domm@plix.at>
Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
Signed-off-by: Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit 38d552f4cb)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
We change the conditions for filling holds to require that either:
- the hold is title level
- the hold is on the exact item
To test:
1 - Place a hold for patron A on item #1 of a record
2 - Place a hold for patron B on item #2 of a record
3 - Issue item #2 to patron A - their hold on item #1 is filled incorrectly, check the item in
4 - Apply patch, restart all
5 - Place hold for patron A on item #1
6 - Issue item #2 to patron A - their hold on item #1 is not filled, check the item in
7 - Cancel that hold
8 - Place a title level hold for the record for patron A
9 - Check item #1 in at a different branch and confirm transfer
9 - Issue item #2 to patron A
10 - The hold is filled as title level, even though item #1 was selected for the hold, check the item in
11 - Place an item level hold for item #2 for patron A
12 - Issue item #2 to patron A, hold is filled
Signed-off-by: Stephanie Petruso <spetruso@aacpl.net>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit 1df229719c)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
This patch adds the borrowernumber as a parameter to the overwrite_calc_fine
plugin hook. E.g., the borrower is needed for calculating graduated fines.
Sponsored-by: Karlsruhe Institute of Technology (KIT)
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit 936b5650c5)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
This patch avoids the introduction of a `foreach` loop and a `return`
call inside of it, by using grep on the array assignment, and then just
returning the first element on it, if any.
No behavior change is expected.
The code becomes easier to read and spot what is doing.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit 9345f6d784)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
This new hook allows to overwrite the koha standard calculation of fines.
This can be useful if your library has a fine policy with graduated fines
per overdue letter.
To test:
1) Apply patch.
2) Run prove -v t/db_dependent/Circulation/CalcFine.t
3) Run prove -v t/db_dependent/Koha/Plugins/Overdues.t
4) Sign off.
Sponsored-by: Karlsruhe Institute of Technology (KIT)
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit fa329e4a09)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
If the cronjob is configured to run every month we don't need to add
another test on the delay in the script itself.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit c26a989c64)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Duplicate barcodes are generated when placing an Order in Acquisitions when AcqCreateItem = "placing an order" and Autobarcode is turned on.
The problem is that you can accidentally attach 3 items to an order, but only 1 will be saved to the database. When you go to receive your order, you can only receive 1 item as the other two were never made, since the barcode wasn't unique.
In 3.8.0, a software error comes up which prevent any item creation, I believe, but master (3.9.x) doesn't throw any warnings or errors.
Yes.. This tries and solve a bug from 3.8.0 /o/
Test plan:
1 - set syspref "autoBarcode" to generated in the form <branchcode> yymm001
2 - set the barcode field to "barcode.pl" in marc structure
3 - create a new basket
4 - add one item to this basket
5 - click on the barcode field -> it should have a barcode
6 - click on add item and click on the new barcode fiels, it should have
the same value
APPLY PATCH:
7 - click on the barcode field -> it should have a barcode
8 - click on add item and click on the new barcode fields, it should have
an higher barcode.
Note : The barcode is incremented each time the barcode with an empty
value is clicked on. Therefore, if the
librarian removes the value from barcode and click again, they could get
another barcode. I do not know if it is an issue.
Note: This patch is not splitted in two parts anymore
Signed-off-by: Sam Sowanick <sam.sowanick@corvallisoregon.gov>
Signed-off-by: Magnus Enger <magnus@libriotech.no>
Followed the test plan, works as advertised.
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit b7ed38413a)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Before applying this patch, apply the CanBookBeIssued preparation: tests patch and run that,
Verify it fails, apply this patch. Run that same tests file again.
----- About this change -----
CanBookBeIssued wants to find one item given a barcode.
If a falsy (undef, empty) barcode is supplied, UNKNOWN_BARCODE should be returned.
Ensure this change does not introduce any regressions:
prove t/db_dependent/Circulation*
prove t/db_dependent/DecreaseLoanHighHolds.t
prove t/db_dependent/rollingloans.t
prove t/db_dependent/api/v1/checkouts.t
prove t/db_dependent/Patron/Borrower_PrevCheckout.t
Signed-off-by: Ray Delahunty <r.delahunty@arts.ac.uk>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit 89b7f160a7)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
This patch simply intercepts a call to sort by callnumber and changes it to cn_sort
To test:
1 - Add three items:
barcode TEST1 callnumnber D1015.B4 1965
barcode TEST2 callnumnber D102.D3 1930
barcode TEST3 callnumnber D101.2.A8 1951
2 - Go to item search
3 - barcode is 'TEST%'
4 - Sort the results by callnumber
5 - order is TEST3,TEST1,TEST2 (or reversed depending on sort direction)
6 - Apply patch, restart_all
7 - sort again
8 - order is TEST3,TEST2,TEST1 (or reversed depending on sort direction)
Signed-off-by: Gretchen Maxeiner <maxeinerGL@gcc.edu>
Signed-off-by: Heather Hernandez <flyingendpaper@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit eecb123bd5)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
We add an `ORDER BY` clause to the SQL statement to ensure we
consistently order accross databases by the iso code.
This fixes the failing test on MariaDB as it's default sort order
appears to differ from MySQL for this.
This also ensure we are consistently ordering which will lead to more
consistent and clear results for end users.
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit 7da684a507)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
We can use a plain AND in the WHERE clause here rather than requireing a
HAVING.
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit 24379efb10)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
My profiling with NYTProf has found that the call to C4::Languages::getLanguages is unreasonably slow, taking over 100 ms (and that's just on a single call of it apparently!!!). Indeed, the way it's coded, it'd run over 540 SQL queries for no good reason, slowing the already slow catalog search page loading further down.
This patch is gonna replace the inefficient and complicated function logic with proper SQL statement that joins the relevant functions. It also fixes the bug where user's native language would be displayed with native language in the parenthesis as well, such as "Polski (Polski)" instead of just "Polski".
The whole query is pretty fast and it gets cached by MySQL server, meaning its runtime is measured in microseconds instead of hundreds of miliseconds, making it easily a 1000x+ improvement.
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit 26a6e476f1)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Account level syspref overrides in SIP will "leak" between SIP requests.
Basically an account level system preference override will may be set by a given request, but those overrides will never be cleared for the life of that SIP process thus "contaminating" that process until it reaches end of life".
This is because the code
# Clear overrides from previous message handling first
foreach my $key ( keys %ENV ) {
delete $ENV{$key} if index($key, 'OVERRIDE_SYSPREF_') > 0;
}
is checking if the env key contains the substring OVERRIDE_SYSPREF_ at an index greater than 0. The problem is that for all overrides the subtring *starts at 0* since it's always the first part of the string. If the substring is not part of the string index will return -1. TLDR we have an "off by one" error. We need to check that the return value is zero instead of any positive value.
1) Set your SIP server params so that only one SIP process should run,
<server-params
min_servers='1'
min_spare_servers='0'
max_spare_servers='0'
max_servers='0'
custom_tcp_keepalive='0'
custom_tcp_keepalive_time='7200'
custom_tcp_keepalive_intvl='75'
/>
2) Set noissuescharge to $5
3) Create two SIP accounts
4) Set an account level override for noissuescharge to $500
5) Create a patron that owes $10 in fines
6) Run a patron information request on the "normal" SIP account for the
patron, the patron flags should show the patron being not able to
check out items
7) Run a patron information request of the "overridden" SIp account for
the patron, the patron flags should show the patron being allowed to
check out items
8) Run a second patron information request on the "normal" SIP account
for the patron, the patron flags should show the patron being able to
check out even though they should not be able to check out
9) Apply this patch
10) Restart all the things!
11) Repeast steps 6-8
12) The flags should be correct!
Signed-off-by: Jake Deery <jake.deery@ptfs-europe.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit de83983b98)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
When sending a notice of a hold ready for pickup (not digest) we use the library where the hold is at.
We should also use this when setting the reply to.
To test:
1 - Set the email/replyto for a patron's library to be:
patron@example.com
2 - Set the email/replyto for another branch to be:
holdbranch@example.com
3 - Place a hold for the patron for pickup at the other branch
4 - Check in and set hold waiting
5 - SELECT * FROM message_queue
6 - Note the message is from the hold branch, but has a null replyto
This means we will use patron branch when sending
7 - Apply patch
8 - Revert hold waiting status
9 - Checkin and confirm hold again
10 -Check message queue
11 - Hold now has reply address of branch
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit 723ed39c2a)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit 006485a036)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit 3407dc28a3)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
This change makes $sessionID consistently return using the session
ID and not sometimes the session object.
Test plan:
0. Apply the patch
1. koha-plack --restart kohadev
2. curl -v 'http://localhost:8081/cgi-bin/koha/svc/authentication' --cookie-jar /tmp/test.cookies
3. Note the Csrf-Token header value
4. Replace the <CSRF-TOKEN>, <USERID>, and <PASSWORD> tokens in the next step
using the appropriate values (eg the CSRF-TOKEN is from the previous step)
5. curl -v -H "Content-Type: application/x-www-form-urlencoded" \
-H "Csrf-Token: <CSRF-TOKEN>" -XPOST \
'http://localhost:8081/cgi-bin/koha/svc/authentication' \
-d "login_userid=<USERID>&login_password=<PASSWORD>" \
--cookie /tmp/test.cookies --cookie-jar /tmp/test.cookies
6. curl -XGET -v 'http://localhost:8081/cgi-bin/koha/svc/bib/29' \
--cookie /tmp/test.cookies > rec.marcxml
7. curl -v 'http://localhost:8081/cgi-bin/koha/svc/authentication' \
--cookie /tmp/test.cookies --cookie-jar /tmp/test.cookies
8. Note the Csrf-Token header value
9. curl -H "Content-Type: text/xml" -H "Csrf-Token: <CSRF-TOKEN>" \
-XPOST -v 'http://localhost:8081/cgi-bin/koha/svc/bib/29' \
--cookie /tmp/test.cookies --data @rec.marcxml
Signed-off-by: Jan Kissig <bibliothek@th-wildau.de>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit 04dbbcc2f2)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
C4::Heading::preferred_authorities seems to be not used for ages (maybe
never used?)
Test plan:
$ git grep preferred_authorities
--> should return nothing
Sponsored-by: Ignatianum University in Cracow
Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit 7c70b09dfc)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
This patch prevents forbidden columns from being selected using the reports dictionary.
To test:
1. Go to Reports -> View dictionary
2. Choose 'new dictionary'
3. After providing a name and description, select the 'patrons' table
4. Select the 'password' column and go Next
5. Notice it is possible to see a list of all the password hashes in the 'choose' selection box when asked to specify a value
6. Apply patch and restart services
7. Repeat steps 1-3
8. Notice the 'password' column and other forbidden columns are not available to be selected
9. Open the Inspector / developer tools in your browser and find the code for the columns selection
10. Change the value of one of the options in the code to "borrowers.password". So it may end up looking like this:
<option value="borrowers.password">Card number</option>
11. Choose that option from the dropdown and click Next.
12. Confirm you are redirected back to this page and warned about the column being used.
13. Ensure tests pass t/db_dependent/Reports/Guided.t
Sponsored-by: Reserve Bank of New Zealand
Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
reports/dictionary.tt and reports/guided_reports_start.tt display a list
of the columns available using optgroup to group the columns per table.
The template code is quite ugly because of the perl structure choose in
C4::Reports::Guided->get_columns
This patch removes $cgi and the 'first' variable that were not used.
Now we have an usual structure with the table names as keys and column
info in an arrayref.
Test plan:
Create a report and a new dictionary definition.
At step 3 you should see the list of the columns.
Note that they are now identical. The list when you create a dictionary
now show the description of the columns.
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit 10a41979e6)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
C4::Context->multivalue_preference is not used so far and split on |
However the values of "multiple" sysprefs are separated by... comma!
Let support both here.
This patch also removes silly JS code in the template.
Signed-off-by: Magnus Enger <magnus@libriotech.no>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Combining three regexes into one. Typo replcing.
Adding a test with pipe in pwd.
Test plan:
Run t/SIP/Sip.t
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Koha's SIP2 server does a lot of logging, including all incoming
requests, in full. This means that passwords are logged, both for
the user the SIP2 client uses for logging into Koha, as well as
for the end users who provide a password to e.g. check something
out. This patch replaces passwords with three asterisks in
log strings, before they are written to the log.
To test, in ktd:
- Run the new tests:
$ prove t/db_dependent/SIP/Sip.t
- Tail the SIP2 logs:
$ sudo tail -f /var/log/koha/kohadev/sip*.log
- Telnet into the SIP2 server:
$ telnet localhost 6001
- Try logging in by pasting this into the telnet session:
"9300CNterm1|COmypassword|CPCPL|"
- Verify that "mypassword" is replaced by "***" in the logs
- Try different values for the password, including the correct password
which is "term1" in ktd
- Try other SIP2 messages that include password fields (AC, AD, CO)
Update 2024-12-03: Fix issues pointed out by QA.
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
This change invokes Template::Filters->use_rfc3986 in the C4::Template
module.
Test plan:
0. Apply the patch
1. Note that "uri" and "url" filters now escape single quotes
Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Magnus Enger <magnus@libriotech.no>
The new test fails without this patch, and passes
when it is applied.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
To test:
1 - place, capture, and revert a hold
2 - view action logs for your hold via SQL so you can see the diff column
3 - confirm reversion of waiting hold generates a MODIFY line in logs with no diff
4 - apply patch, restart_all
5 - capture and revert your hold again
6 - confirm logs include diff showing change to found and priority
Signed-off-by: Sam Sowanick <sam.sowanick@corvallisoregon.gov>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit d86c662451)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
The purpose of no block checkouts in SIP is to indicate that the SIP machine has made an offline ( "store and forward" ) transaction.
The patron already has the item. As such, the item must be checked out to the patron or the library risks losing the item do to lack of tracking.
As such, no block checkouts should not be blocked under any circumstances.
The sub C4::SIP::ILS::Transaction::Checkout::do_checkout already honors this, but it is wrapped by
C4::SIP::ILS::checkout which has additional checks that do not.
Test Plan:
1) Apply the unit test patch
2) prove t/db_dependent/SIP/Transaction.t will fail
3) Apply the second patch
4) prove t/db_dependent/SIP/Transaction.t will pass!
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit 4d40fd7ab9)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Test plan:
Try things like [% borrower.dateexpiry.strftime('%d-%m-%y') %] in
a notice. Also test datetime fields like borrower.lastseen.
Add locale support with strftime('%d %B', 'nl_NL') etc.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit 9a2042c3e9cb975273e6c17e111b4baeb5278006)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Lari Strand
William Lavoie
Fridolin Somers
Martin Renvoize
Emmi Takkinen
Martin Renvoize
Marcel de Rooy
Janusz Kaczmarek
Tomas Cohen Arazi
Kyle M Hall
David Cook
Aleisha Amohia
Nick Clemens
Raphael Straub
Jonathan Druart
Baptiste Wojtkowski
Pedro Amorim
Michał Kula
Paul Derscheid
Magnus Enger
Andrew Fuerste Henry