add_field/maybe_add need $server to apply the hide_fields and
allow_fields account options; without it the AJ field is always sent.
- Pass $server at the FID_TITLE_ID call sites in the checkout, checkin,
item information, item status update and renew responses
Assisted-by: Fable 5 (Anthropic)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
This patch adds a custom aj_field_template to the sip accounts
This allows libraries to specify a template for the title (AJ) field in sip
messages
The template allows using item and biblio fields (item provides access to biblio. but we pass
both in order to makes the templates simpler
To test:
1 - Install patches
2 - updatedatabase, yarn build, restart_all
3 - Setup a sip account, KTD already has a sip patron, term1, you can use that
don't add the aj_field_template yet
4 - Find or create a biblio with 245a and 245b like 245$aTitle : $bSubtitle and add item with TESTBARCODE
5 - From the command line:
perl misc/sip_cli_emulator.pl -su term1 -sp term1 -l CPL -t CR -a localhost -p 6001 -m item_information --item TESTBARCODE
6 - Confirm AJ field shows "Title :" as expected
7 - Edit sip account, set aj_field_template to:
[% biblio.title %] [% biblio.subtitle %] [% item.barcode %]
8 - Repeat the sip test
9 - Confirm you get the response like:
Title : Subtitle TESTBARCODE
10 - Sign off!
Signed-off-by: Bob Bennhoff <bbennhoff@clicweb.org>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Test plan:
Turn on MARCOverlayRules
Go to /cgi-bin/koha/admin/marc-overlay-rules.pl
Confirm the "Filter" column is correctly adjusted when the "source"
dropdown is modified
Signed-off-by: Jan Kissig <bibliothek@th-wildau.de>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
GetIssuingCharges overwrites its initial $charge = 0 with the value of
itemtypes.rentalcharge, which is NULL when no rental charge is configured.
This causes 'uninitialized value' warnings in every caller that compares
$charge > 0 (automatic renewals, SIP checkout/renew, OPAC).
The fix normalizes NULL to 0 at the source. All callers already treat
the return value as numeric, so this is safe. Also removes a now-redundant
$charge && guard in AddIssue that was working around this.
Test plan:
1. Apply patch
2. Run:
$ ktd --shell
k$ prove t/db_dependent/Circulation.t
=> SUCCESS: Tests pass!
3. Sign off :-D
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Chris Mathevet <chris.mathevet@inLibro.com>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
The staff catalogue detail page called $biblio->normalized_upc,
normalized_ean, and normalized_oclc unconditionally. These methods
go through metadata_extractor which calls $biblio->metadata->record
internally, re-fetching the (corrupt) metadata from the DB and
ignoring the already-sanitized $marc_record.
When a record has invalid MARCXML, this causes an unhandled exception
instead of displaying the 'biberror' warning.
Fix: use the Extractor directly with the already-recovered
$marc_record (which was sanitized via StripNonXmlChars), and guard
the block with a check that $marc_record is defined.
Test plan:
1. Apply patch
2. Run:
$ ktd --proxy --selenium up -d
$ ktd --wait-ready 120
$ ktd --shell
k$ prove t/db_dependent/selenium/regressions.t
=> SUCCESS: Tests pass (including 'detail page must not explode')
3. Sign off :-D
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
To test:
1. Create or locate a bibliographic record with a MARC 505 field.
2. Leave the first indicator blank.
3. Add text to the 505 field.
4. View the record detail page.
5. Notice that the 505 contents display without a label.
6. Apply the patch
7. Refresh the record detail page. Notice that the 505 content displays a label now.
8. Sign off and have a lovely day! :D
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
prove t/db_dependent/Koha/ILL/Request.t
prove t/db_dependent/Koha/Plugins/ILL/Backends.t
NOTE: These must be run without having any ILL backend plugin installed
Signed-off-by: Hannah Prince <helm.consortium.nhs.net>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
The plugin cache used a single slot, so the first backend looked up
poisoned it for all subsequent calls. This caused 500 errors when
viewing ILL requests that had been migrated between backends.
Test plan:
1) Download and install 2 additional plugin backends that implement
their own templates:
https://github.com/openfifth/koha-ill-libkey-lending-tool/releases/download/v2.6.1/koha-ill-libkey-lending-tool-v2.6.1.kpzhttps://github.com/openfifth/koha-ill-reprintsdesk/releases/download/v4.2.0/koha-ill-reprintsdesk-v4.2.0.kpz
2) Enable ILLModule and run the following SQL:
INSERT INTO illrequests (borrowernumber, branchcode, status, backend, placed)
SELECT b.borrowernumber, br.branchcode, 'COMP', 'ReprintsDesk', NOW()
FROM borrowers b, branches br
LIMIT 1;
INSERT INTO action_logs (timestamp, user, module, action, object, info, interface)
VALUES
(NOW() - INTERVAL 2 HOUR, 1, 'ILL', 'INCDOCS_REQUEST_PLACED', 1,
'{"log_origin":"IncDocs","response":1}', 'intranet'),
(NOW() - INTERVAL 1 HOUR, 1, 'ILL', 'REPRINTS_DESK_REQUEST_ORDER_UPDATED', 1,
'{"log_origin":"ReprintsDesk","response":null}', 'commandline');
3) Attempt to open the 'Manage request' page for this newly created
request:
http://localhost:8081/cgi-bin/koha/ill/ill-requests.pl?op=illview&illrequest_id=1
4) Notice you get a 500 error:
Template process failed: file error - : not found
5) Apply all patches, restart plack, refresh. Notice the 'Manage
request' page now renders correctly.
6) Click the 'ILL request log' toolbar button. Confirm that both the
IncDocs and ReprintsDesk entries are shown.
Signed-off-by: Hannah Prince <helm.consortium.nhs.net>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Koha::Edifact::Transport reported every transport failure (no transport
configured, connect failure, directory-change failure, file-listing
failure and per-file download/upload failure) with carp. Under
edi_cron.pl these go to STDERR, so on a normal cron run the messages are
lost and EDI simply goes quiet with nothing in editrace.log to explain
why.
Route these failures through Koha::Logger on the 'edi' interface (the
same interface edi_cron.pl already uses) so they are recorded in the EDI
log. The two ingest()-level data warnings (duplicate filename, unreadable
file) are intentionally left as carp: one is asserted by the existing
unit test and neither is a transport failure.
No change to the transfer logic.
Test plan:
1) prove t/db_dependent/Koha/Edifact/Transport.t
2) Point an EDI account at an unreachable transport and run edi_cron.pl;
confirm the failure is now written to the edi log rather than only to
STDERR.
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
In "Edit patron card batch" the pageLength is coming from the syspref,
the value is a string.
But DataTable's needs an integer.
Let's fix this in datatables.js, in the kohaTable's constructor
Test plan:
Got to cgi-bin/koha/patroncards/edit-batch.pl
Enter more than 20 borrowernumbers and "Add patron(s)"
Click "next" and notice that with this patch the pagination works.
Without this patch the JS error is
"Uncaught TypeError: can't access property "nTr", p is undefined"
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Bug 41267 defined the new rule, but didn't set the default.
For consistencies sake we should create these in the DB
To test:
1 - Create 'Default holds and bookings policies by item type' for two itemtypes
Set 'Fill other record level holds on record at checkout' to yes for one and no for the other
2 - Apply this patch
3 - updatedatabase
4 - Verify the rules are not altered
5 - Chekc the DB:
SELECT * FROM circulation_rules WHERE branchcode IS NULL and categorycode IS NULL;
6 - Note the number of rules/ids
7 - Run the DB update and confirm nothing changes
8 - DELETE from circulation_rules WHERE rule_name='fill_other_biblio_holds_policy';
9 - Run the DB update
10 - Confirm rules are replaced, both as yes
11 - Delete a single one of the rules now:
DELETE from circulation_rules WHERE rule_name='fill_other_biblio_holds_policy' AND id=#;
12 - Run the DB update
13 - Confirm only that rule is replaced
14 - DELETE from circulation_rules WHERE branchcode IS NULL and categorycode IS NULL;
15 - Run the DB update
16 - No new rules are created
follo# Koha/.Patrons.pm.swp
Signed-off-by: Lawrence O'Regan-Lloyd <loreganlloyd@clamsnet.org>
Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
To test
1 - have three test biblios. Bib A has just one item, a Book. Bib B has two items, a Book and a Computer File. Bib C has one item, a Computer File
2 - have KTD default circ rules (holds allowed for all itemtypes in the circ rules table, no values defined in "Default checkout, hold and return policy" or "Default holds and bookings policies by item type
")
3 - set "Default holds and bookings policies by item type" for Computer Files to Hold Policy = "No holds allowed."
4 - observe that "Fill other record level holds on record at checkout" defaults to "Yes" when a value is set to Hold Policy
5 - confirm you can place a holds on Bib A and Bib B but not Bib C
6 - confirm your hold on Bib B is filled when checking out the Computer File item from Bib B
7 - update "Default holds and bookings policies by item type" so Computer Files has Hold Policy = "No holds allowed" and "Fill other record level holds on record at checkout" = No
8 - confirm you're still able to place a hold on Bib B
9 - confirm your hold on Bib B is now *not* filled by checking out the Computer Files item
10 - open the commandline SQL interface (sudo koha-mysql kohadev on KTD)
11 - run query "select * from circulation_rules where rule_name = 'fill_other_biblio_holds_policy'"; confirm an entry for Computer Files with rule_value=0
12 - run command "delete from circulation_rules where rule_name = 'fill_other_biblio_holds_policy'"
13 - reload your circ rules page, confirm the interface still shows "Fill other record level holds on record at checkout" = No for comoputer files
14 - confirm your hold on Bib B can again be filled by checking out the Computer File item
APPLY PATCH, restart_all
15 - reload circ rule page, confirm it now shows "Fill other record level holds on record at checkout" = Yes for Computer Files
16 - run query "select * from circulation_rules where rule_name = 'fill_other_biblio_holds_policy'"; confirm there is still no entry for Computer Files
17 - place a new hold on Bib B, confirm it fills by checking out the Computer File (not having any rule defined is enforced the same as a "Yes" and displays a "Yes")
18 - in circ rules, set "Fill other record level holds on record at checkout" = Yes for computer files
19 - run query "select * from circulation_rules where rule_name = 'fill_other_biblio_holds_policy'"; confirm there is an entry for Computer Files with rule_value=1
20 - place a new hold on Bib B, confirm it fills by checking out the Computer File (having an explicit Yes rule defined is enforced correctly and displays correctly)
21 - in circ rules, set "Fill other record level holds on record at checkout" = No for computer files
22 - run query "select * from circulation_rules where rule_name = 'fill_other_biblio_holds_policy'"; confirm there is an entry for Computer Files with rule_value=0
23 - place a new hold on Bib B, confirm it does not fill by checking out the Computer File (having an explicit No rule defined is enforced correctly and displays correctly)
Signed-off-by: Lawrence O'Regan-Lloyd <loreganlloyd@clamsnet.org>
Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
This patch changes the default MaxAge from 8 hours to
168 hours or 7 days due to usability issues encountered from
using a short-lived token.
See OWASP for more information on why timestamps with expiry
are not necessary for CSRF tokens. The MaxAge is a requirement
of the WWW::CSRF token used by Koha, so let's just use a longer
expiry in lieu of no expiry.
Test plan:
0. apply patch
1. prove t/Token.t
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
When every result on a page is hidden (OpacHiddenItemsHidesRecord), rather
than leaving the patron on a dead-end "not available" page, look ahead a
bounded number of pages (3) for one with visible results and redirect
there directly. The lookahead is capped so a library with many
consecutive hidden pages doesn't turn a single search into a long chain
of extra search-engine queries; RSS/atom/opensearchdescription requests
are left alone since redirecting a feed reader makes no sense.
Whenever a hidden page was skipped over (whether we found a visible page
to land on, or exhausted the lookahead and fell back to the "not
available" message), show a caveat next to the results count so the
"Your search returned N results" figure doesn't read as contradicting
what the patron can actually see - this was raised as a sign-off concern
on the original patch (comment 4).
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
The read-only star rating widget in search results was changed from
<span> to <a href="#"> elements, unrelated to this bug's fix. That
activated a pre-existing but previously dead click handler
($(".br-readonly a") in this same template) and turned every star icon
into a focusable, non-functional link, adding up to 5 dead tab stops
per search result. Restore the <span> markup used everywhere else this
read-only widget appears.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Zeroing $hits alongside the no_page_results flag also zeroed $total
(since $total accumulates $hits, and the final template 'total' param
is set from $hits/$total). Because the top "No results found!" banner
and the entire search-results section are both gated on 'total' being
true, this meant the new no_page_results branch could never actually
render: whenever it was true, 'total' was false, so the classic
"No results found!" page won instead - recreating the exact bug this
patch is meant to fix.
Verified against the bug's own test plan (OPACnumSearchResults=5,
OpacHiddenItemsHidesRecord, OpacHiddenItems: withdrawn:[1], 6 bibs with
the first 5 withdrawn/hidden): before this change, page 1 still showed
"No results found!" with no pagination; after this change, it shows
"Your search returned 6 results" / "Search results on this page are
not available", with a working pagination link to the page containing
the visible record.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
This issue occurs when the records on the first page of results are not displayed. The message "No results found" is shown, and it is not possible to access records on other pages.
To test
1. Set the following system preferences:
OPACnumSearchResults to 5
OpacHiddenItemsHidesRecord to Hide
OpacHiddenItems to withdrawn: [1]
2. Create 6 bibliographic records with the following titles:
"notice test 1"
"notice test 2"
"notice test 3"
"notice test 4"
"notice test 5"
"notice test 6"
3. For the first 5 records, add an item with Withdrawn status.
4. For "notice test 6", add an item with Available status.
5. In the OPAC interface, search for "notice test".
6. Apply the patch
7. Repeat step 4
---> The results table is now shown with the message: "Search results on this page are not available."
---> You can now access the second page and see "notice test 6"
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: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Bug 33544 combined the ILL request detail fields into one column. The
combined column kept an empty data property without the related-object
search metadata. A global table search therefore generated a query against
"me.", which the REST endpoint rejected with a 500 error.
Restore the extended attribute search mapping for the fields displayed in
the combined Request details column. Add Cypress coverage for both the REST
query and the matching result.
Test plan:
Before applying the patch:
1. Make sure the ILLModule system preference is enabled.
2. Open a patron record.
3. Click "ILL requests history" in the menu on the left.
4. Type any text in the Search box above the table.
5. Confirm that Koha shows a 500 Internal Server Error message.
Apply the patch.
After applying the patch:
1. Repeat steps 2 to 4 above.
2. Confirm that the table refreshes without an error. It may show no
matching requests.
3. If the patron has an ILL request, search for part of a title or author
shown under Request details.
4. Confirm that the matching request remains in the table.
Co-Authored-By: Loontik <loontik@nugged.team>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Caused by
commit e9b8004d19
Bug 38255: Replace dataTable constructor with kohaTable - Staff
grouped and ungrouped should be DataTable objects.
Test plan:
Create a basket, close it
Create a basket group and test the "Add to group" and "Remove" button on
the basket group edit view
Without this patch there was a JS error:
"Uncaught TypeError: grouped.row is not a function"
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
If HidePatronName is turned on and a hold is being placed on a hold for
a patron with a different library, there is an alert the duplicate the
library's name:
Pickup library is different. Patron: A patron from library Springfield Patron's home library: (Springfield / SPL)
With this patch:
Pickup library is different. Patron's home library: Springfield / SPL
Pickup library is different. Patron: Keith Hendrix Patron's home library: Springfield / SPL
Test plan:
Go to /cgi-bin/koha/reserve/request.pl?biblionumber=117&borrowernumber=26
And confirm that the changes make sense
Signed-off-by: Anneli Österman <anneli.osterman@koha-suomi.fi>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
The main goal of the previous patch was to prevent the randomness,
the languages were displayed by code.
The languages in the footer are the languages enabled for the staff interface.
This patch suggests to use the order of the syspref, and append the
languages not enabled at the end of the list.
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
The list of languages are displayed in a random order in:
* language picker at the bottom of the page
* language tab when editing a notice template or an additional content
This patch fixes it to display them in a determinist order
Test plan:
Have several languages installed, and some of them enabled, change the
order of the languages in the syspref
Reload the mainpage several times and notice that the languages are
always displayed in the same order.
Same when editing a notice template or an additional content.
Note that I have not investigated yet why on the notice template and
additinal content edit views they are not listed in the same order.
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
This patch updates circulation.tt in order to remove event attributes in
favor of defining event handlers in the relevant JS file.
To test, apply the patch and clear your cache if necessary.
- If necessary, enable the SpecifyDueDate system preference.
- Check out to a patron.
- Click the checkout settings icon inside the barcode field.
- Click the "Specify due date" field's "Remember for session" checkbox
- The focus should move to the barcode field.
- Enable recalls if necessary
- Locate a checked out item which can be recalled.
- Log in to the OPAC and recall the item.
- In the staff interface, try to check that item out to another patron.
- You should get a "Please confirm checkout" message.
- Click the "Don't check out and print slip" button.
- A popup should open for printing a recall slip.
- The checkout page should reload without checking out the item.
- If necessary, place a hold on an item for delivery to your logged-in
library, check in the item, and confirm the hold.
- Try to check out the item to another patron.
- You should get a "Please confirm checkout" message.
- Click "Don't check out, confirm hold, and print slip"
- A popup should open for printing a hold slip.
- The checkout page should reload without checking out the item.
- If necessary, place a hold on an item but do not check it in.
- Try to check out the item to a patron.
- You should get a "Please confirm checkout" message.
- Click "Cancel checkout and place hold for..."
- You should be redirected to the holds page, placing a hold for that
patron and that item.
- The item should not have gotten checked out in the process.
Sponsored-by: Athens County Public Libraries
Signed-off-by: Olivia Reynolds <olivia.reynolds@openfifth.co.uk>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
The shelving location cell is always built with data: an empty HTML tag
is added even if there is no location.
So the "Shelving location" column is always displayed, it should only be
displayed if there are items with data.
Test plan:
Have a biblio with all items without shelving location (or without
several pages and at least one page without data).
If all rows have empty shelving loc, the column should be hidden
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
If bc_1 is checked out and I try to add bc_2 I get:
"Failure: Item 'bc_2' belongs to another bundle"
It should be:
"Failure: Bundle is currently checked out"
Test plan (using KTD):
1. Pick a record with items, for example: The definitive guide to Catalyst (146)
2. Edit the record and change 000 (the leader):
- Click the tag editor
- Change "7- Bibliographic level" to "C- Collection"
3. In the holdings table there is now a "Manage bundle (0|0)" option
in the far right column for each item.
4. Check an item out for the record out, for example
I checked out 39999000006117 to Mary Burton.
5. In the holdings table for the record, click on "Manage bundle" for the item
you checked out (39999000006117).
6. In the expanded text area below the item row, click "+ Add to bundle".
7. In pop-up window, enter 39999000006131 "Item barcode" field (this is one
of the other items for the record that is not checked out).
8. Click Submit.
9. You get an error:
Failure: Item '39999000006131' belongs to another bundle
10. Apply the patch.
11. Restart everything: restart all
12. Hard refresh the page for the record.
13. Repeat steps 5 to 8.
14. You now get the correct error:
"Failure: Bundle is currently checked out"
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
The + character in RFC3339 timezone offsets (e.g. +00:00) is decoded
as a space when interpolated directly into a URL query string. Use
Test::Mojo form => { q => ... } which properly URL-encodes the
parameter value.
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
_parse_dbic_query maps field names but did not convert values. When
datetime or boolean filters are nested inside DBIC operators (-and,
-or) in the q= parameter, attributes_from_api (which normally handles
the conversion via _recursive_fixup) cannot reach them because it only
iterates top-level keys.
This patch fixes the issue in two places:
1. _parse_dbic_query (Query.pm): When a key maps to a known column
(on the primary table or a related object), call _recursive_fixup
to convert RFC3339 timestamps to MySQL format and booleans to 0/1.
Related object instances are cached on the result_set to avoid
repeated prefetch_whitelist lookups during recursive parsing.
2. Objects.pm: Move attributes_from_api to process named query params
before merging with q= params. This avoids double-processing since
q= values are already fixed up by _parse_dbic_query.
Test plan:
1. Apply the regression tests from the previous commit
2. Run:
$ prove t/Koha/REST/Plugin/Query.t
$ prove t/db_dependent/Koha/REST/Plugin/Objects.t
=> FAIL: Tests fail!
3. Apply this patch
4. Repeat step 2
=> SUCCESS: Tests pass!
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: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
This patch adds regression tests proving that RFC3339 datetime values
and boolean values nested inside DBIC operators (-and, -or, -between)
in q= queries are not converted to their DB-native format.
Unit tests (Query.t):
- Datetime: >=, <=, range, -or, -between, nested -and/-or, me. prefix
- Boolean: true/false inside -and, -or, nested -and within -or
- Non-datetime values pass through unchanged
Integration tests (Objects.t):
- Full API stack tests using action_logs with controlled data
- Same operator/nesting coverage as unit tests
Test plan:
1. Apply patch
2. Run:
$ prove t/Koha/REST/Plugin/Query.t
$ prove t/db_dependent/Koha/REST/Plugin/Objects.t
=> FAIL: Tests fail! Values inside -and are not converted
3. Sign off :-D
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
The web-based self-checkout (opac/sco/sco-main.pl) builds the
response JWT cookie with -expires => $jwt ? '+1d' : ''. On logout
$jwt is undef, so the cookie is sent with an empty value and no
Expires attribute - i.e. a session cookie with an empty value
rather than a deletion. Some browsers/proxies retain the previous
valid JWT in this scenario, which presents as the Finish/close
button "doing nothing" or working only intermittently: the patron
appears to remain logged in despite clicking Finish.
Use a past Expires ('-1d') when no JWT is present so the browser
deletes the cookie entirely. The subsequent ?op=logout render
cannot then rediscover the patron from a stale cookie.
Test plan:
1. Enable web-based self-checkout (SelfCheckoutByLogin or
SelfCheckAllowByIPRanges).
2. Log in as a patron at /cgi-bin/koha/sco/sco-main.pl.
3. Open DevTools -> Application -> Cookies and note the JWT cookie.
4. Click the green "Finish" button.
5. Without the patch: Set-Cookie on the ?op=logout response shows
JWT= with no Expires; the cookie persists as an empty session
cookie.
6. With the patch: Set-Cookie shows JWT=; expires=<past date>; the
browser removes the cookie entirely.
7. Confirm the kiosk reliably returns to the login screen and
subsequent page loads do not re-authenticate from a stale JWT.
Sponsored-by: OpenFifth
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Store scalar preference values when saving the acquisition preferences
and clean up the invoice explicitly.
Exercise the rendered receiving behavior for a basket that inherits
AcqCreateItem, one that explicitly overrides it with "ordering", and a
basket that creates items on receiving. Keep the explicit "ordering"
case as a control for behavior that already works without the production
fix.
This changes tests only.
Automated tests:
yarn cypress run --spec \
t/cypress/integration/Acquisitions/OrderReceivePreferences_spec.ts
Co-Authored-By: Loontik <loontik@nugged.team>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Bug 41563 converted AcqCreateItem and UniqueItemFields to booleans when
adding them to Koha.prefs. Both preferences are strings: AcqCreateItem
controls where items are created, while UniqueItemFields contains a
pipe-separated list.
When a basket inherits AcqCreateItem=ordering, the boolean value matches
none of the receiving page branches and existing items are hidden. When
items are created on receiving, the numeric UniqueItemFields value also
fails when the page calls split().
Preserve both string values in Koha.prefs. Add a Cypress regression test
which renders orderreceive.pl and verifies the values exposed to
JavaScript.
Test plan:
Before applying the patch:
1. Set AcqCreateItem to "placing an order" and UniqueItemFields to
"barcode".
2. Create a basket that inherits the system preference. Add an order with one
item, close the basket, and start receiving it.
3. Confirm that the Items table has no existing item row and no Receive
checkbox.
4. Set AcqCreateItem to "receiving an order" and open the same order for
receiving.
5. Confirm that the Items section is empty: no item-entry fields and no Add
item button are shown.
Apply the patch.
After applying the patch:
1. Set AcqCreateItem back to "placing an order" and open the order for
receiving again.
2. Confirm that its existing item is listed and has a Receive checkbox.
3. Set AcqCreateItem to "receiving an order" and open the same order again.
4. Confirm that the Items section contains item-entry fields and an Add item
button.
Co-Authored-By: Loontik <loontik@nugged.team>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
The daemon runs as a long-lived process and never clears its L1 cache.
System preference changes (e.g. IncludeSeeFromInSearches) are not
picked up until the daemon is manually restarted.
Flush L1 caches at the start of each commit() call so that preference
changes take effect on the next indexing batch without requiring a
daemon restart.
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
The servicing_instruction column was not mapped in
Koha::Acquisition::Order->to_api_mapping, so it was exposed verbatim in
the REST API order representation. Since api/v1/swagger/definitions/order.yaml
does not declare this property and sets additionalProperties: false, any
request touching the acquisitions/orders endpoint (e.g. the "Receive
shipments" page) failed with a 500 error: "Properties not allowed:
servicing_instruction".
Exclude it from the API representation, consistent with the other
EDIFACT-only columns (line_item_id, suppliers_reference_number, etc.)
that are already excluded the same way.
Test plan:
1) Go to a vendor page, create a basket, add an order line, and close
the basket.
2) From the vendor page, click "Receive shipments".
3) Without this patch, the orders table fails to load with a 500 error
mentioning "Properties not allowed: servicing_instruction".
4) With this patch applied, the orders table loads correctly.
5) prove t/db_dependent/api/v1/acquisitions_orders.t
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
The servicing instructions widget added new user-facing strings (alerts,
button labels, hints) built entirely in JS via string concatenation, none
of which were wrapped for translation - unlike the pre-existing JS in the
same file, which already uses _() throughout. Wrap them the same way,
using _("...").format(...) for the strings that interpolate a value,
matching the existing pattern used elsewhere in this file.
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
The RM's review found two stored XSS vectors in the servicing
instruction free-text (LVT) handling:
1. neworderempty.tt embedded the servicing instruction JSON (and the
EDIFACT_SI authorised values list) directly into an inline <script>
block via JSON.parse('[% ... | $raw %]'). TT's html filter does not
escape single quotes, so a free-text value like
test' + alert(1) + ' broke out of the JS string literal and
executed on the next page load of the edit form.
2. basket.pl built the "Servicing instruction" column display by
concatenating raw LVC/LVT values with '<br>' and rendering the
result with | $raw in basket.tt, so HTML/script content in a
free-text value executed directly on the basket page.
Fix the first by moving the JSON into existing/new hidden input
value="" attributes (escaped with the html filter, which is safe for
double-quoted HTML attributes) and reading them back via jQuery's
.val() before JSON.parse(), instead of interpolating untrusted data
into a JS string literal.
Fix the second by passing the display parts to the template as a list
and escaping each with the html filter at render time, instead of
building a pre-escaped raw HTML string in Perl.
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Tests were failing on upgrade route because they tried to create
EDIFACT_SI authorised values that already existed from the atomicupdate
script, causing unique constraint violations.
This patch:
- Removes test code that created EDIFACT_SI values (tests should assume
database defaults exist)
- Adds EDIFACT_SI category to auth_val_cat.sql for fresh installs
- Adds all 32 EDItEUR List 3B servicing instruction codes to
auth_values.yml for fresh installs
Now the authorised values exist in both installation paths:
- Upgrade route: Created by atomicupdate script
- Fresh install: Created by installer SQL/YAML files
Test plan:
1. prove t/db_dependent/Koha/EDI.t
2. prove t/db_dependent/Koha/Edifact/Order.t
3. Verify both tests pass without warnings
Sponsored-by: Martin Renvoize <martin.renvoize@gmail.com>
Signed-off-by: Hannah Dunne-Howrie <hdunne-howrie@westminster.gov.uk>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
This commit adds comprehensive tests for servicing instruction
processing in both incoming quotes and outgoing orders:
1. Integration tests for incoming quote processing (Koha/EDI.pm)
- Tests LVC (coded) and LVT (freetext) extraction
- Tests multiple GIR occurrences with different servicing instructions
- Tests all combinations: LVC only, LVT only, and LVC+LVT together
2. Test EDIFACT quote file (QUOTES_SERVICING.CEQ)
- Contains 3 line items with 4 total orders
- Demonstrates different servicing instruction patterns
3. Fix for multi-occurrence servicing instruction extraction
- Modified quote_item() to extract servicing instructions
per GIR occurrence instead of across all occurrences
- Ensures each order gets only its own servicing instructions
- Aligns with existing multi-item processing for funds and stock rotation
Test coverage:
- t/db_dependent/Koha/EDI.t: 29 tests for incoming quote processing
- t/db_dependent/Koha/Edifact/Order.t: 18 tests for outgoing order generation
(previously added)
All tests pass successfully.
Sponsored-by: Martin Renvoize <martin.renvoize@gmail.com>
Signed-off-by: Hannah Dunne-Howrie <hdunne-howrie@westminster.gov.uk>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
This patch adds comprehensive unit tests for the servicing instruction
functionality in t/db_dependent/Koha/Edifact/Order.t.
Changes:
- Updated test count from 6 to 7
- Added servicing_instruction => undef to test orders to prevent
TestBuilder from generating random invalid JSON
- Added servicing instruction subtest with 18 tests covering:
* Single LVC instruction
* Single LVT instruction
* Multiple LVC in one group (tests GIR segment overflow)
* Mixed LVC/LVT in one group
* Multiple groups
* Empty servicing instruction
* Invalid JSON handling (logged to EDI log)
- Tests correctly handle GIR segment 5-element limit and overflow
to additional segments
Test plan:
1. Run: prove t/db_dependent/Koha/Edifact/Order.t
2. Verify all tests pass
Sponsored-by: Martin Renvoize <martin.renvoize@gmail.com>
Signed-off-by: Hannah Dunne-Howrie <hdunne-howrie@westminster.gov.uk>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
This patch adds the user interface for managing and displaying
EDIFACT servicing instructions in the acquisitions module.
Changes to acqui/neworderempty.pl:
- Parses servicing_instruction JSON for editing existing orders
- Fetches EDIFACT_SI authorized values for dropdown
- Passes data to template in both array and JSON formats
Changes to acqui/addorder.pl:
- Accepts servicing_instruction_groups_json from form submission
- Validates and stores JSON in database
Changes to acqui/basket.pl:
- Fetches EDIFACT_SI authorized values
- Formats servicing instructions for display in get_order_infos()
- Creates human-readable display strings with:
* LVC codes replaced with descriptions
* Groups separated by <br> tags
* Format: "Description" or "Description: free text"
Changes to neworderempty.tt:
- Dynamic UI for adding/removing servicing instruction groups
- Each group can contain:
* One LVC (coded) instruction only, OR
* One LVT (free-text) instruction only, OR
* One LVC + one LVT combination
- Validation prevents:
* More than 2 instructions per group
* Duplicate LVC or LVT in same group
- LVC instructions use dropdown populated from EDIFACT_SI authorized values
- LVT instructions use free-text input
- Groups serialized to JSON hidden field for form submission
- Smart button labels show what can be added to each group
Changes to basket.tt:
- Displays servicing instructions in basket table
- Pre-formatted in Perl (no JavaScript parsing needed)
- Shows LVC descriptions instead of codes
- Multiple groups displayed on separate lines
Changes to columns_settings.yml:
- Adds servicing_instruction column to basket table configuration
Display format examples:
- LVC only: "Barcode labelling"
- LVT only: "Handle with care"
- Combined: "Binding: Apply to spine"
- Multiple groups (on separate lines):
Barcode labelling
Binding: Special instructions
Test plan:
1. Create/edit order
2. Add servicing instruction groups
3. Try to add more than 2 instructions to a group (should prevent)
4. Try to add duplicate LVC or LVT to group (should prevent)
5. Save order
6. View basket and verify instructions display correctly
7. Edit order and verify instructions load into form
Sponsored-by: Martin Renvoize <martin.renvoize@gmail.com>
Signed-off-by: Hannah Dunne-Howrie <hdunne-howrie@westminster.gov.uk>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Paul Derscheid
Nick Clemens
Jonathan Druart
Tomás Cohen Arazi
Pedro Amorim
Laura_Escamilla
Martin Renvoize
Emily Lamancusa
Andrew Fuerste Henry
David Cook
Chris Mathevet
Hammat Wele
Andrii Nugged
Owen Leonard