The comment above the day-boundary calculation still described the
previous dayjs.utc() approach, which was replaced by dayjs.tz() in the
parent commit. Update it to describe the current behaviour so it
doesn't mislead future maintainers into re-introducing the timezone
shift bug this patch fixed.
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
To test:
1 - Edit koha-conf.xml
2 - Set timezone:
<timezone>America/New_York</timezone>
3 - In Koha, enable bookings syspref
4 - Find a record, click the items tab, make an item bookable
5 - Place new booking, choose a date range
6 - View the bookings tab - note the booking extends into the date before
7- Check the DB, the start_date and end_date are wrong
8. APPLY PATCH, restart_all, clear browsewr cache
9. Place a new booking.
10. MariaDB [koha_kohadev]> select * from bookings WHERE booking_id={new booking}
11. Notice the start time should be {start_date} 00:00:00 and the end time should be {end_date} 23:59:59
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>
On the holdings table the filter by status=recalls is not working correctly.
Test plan:
Add a recall on an item of a bibliographic record that has several
items.
Filter the items table by status=recall
=> Without this patch all items are displayed
=> With this patch applied only the item with a recall is displayed
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Aleisha Amohia <aleisha@catalyst.net.nz>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
If a session has expired, the login form will get an empty CSRF token
and the login form will return 403
To recreate:
hit /cgi-bin/koha/sco/sco-main.pl
then hit /cgi-bin/koha/opac-main.pl
login
=> 403
Apply this patch, try again
=> Success
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This patch adds two new parameters to control the deletion of background jobs that did not complete.
Some libraries may want to keep a failed job longer than a completed one for debugging reasons.
To test:
0 - Apply patch
1 - Make some old failed jobs:
INSERT INTO background_jobs (status,type,enqueued_on) VALUES
('started','update_elastic_index','2020-01-01'),
('failed','update_elastic_index','2026-01-01'),
('started','other_type','2021-01-01'),
('failed','other_type','2026-01-01');
2 - Confirm defaults of 365 and update_elastic_index for new params:
perl misc/cronjobs/cleanup_database.pl -v --jobs-unfinished-days
3 - Should indicate 1 job older than 365 of type update_elastic_index would have been deleted
4 - Confirm days param works
perl misc/cronjobs/cleanup_database.pl -v --jobs-unfinished-days 30
5 - Shoudl find 2 elastic jobs
6 - Confirm type param
perl misc/cronjobs/cleanup_database.pl -v --jobs-unfinished-days --jobs-unfinished-type=other_type
7 - Should find 1
8 - Confirm type and days
perl misc/cronjobs/cleanup_database.pl -v --jobs-unfinished-days 30 --jobs-unfinished-type=other_type
9 - Should find 2
10 - Confirm repeated type
perl misc/cronjobs/cleanup_database.pl -v --jobs-unfinished-days=30 --jobs-unfinished-type=other_type --jobs-unfinished-type=update_elastic_index
11 - Finds 4
12 - Confirm deletion
perl misc/cronjobs/cleanup_database.pl -v --jobs-unfinished-days=30 --jobs-unfinished-type=other_type --jobs-unfinished-type=update_elastic_index --confirm
13 - All your made up jobs are removed
Signed-off-by: Trevor Diamond <trevor.diamond@mainlib.org>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Filters are broken for many reasons. Previous patch contains some
(little) tidying.
There are 3 parts in this patch:
1 - The column name is incorrectly named in the dropdown for "branch",
it is added in this patch
2 - There is a definition for 'Day' and 'Month', but not for 'Week', it
is also added in this patch
3 - Dbh variables are passed in the string definition AND in the call. I
removed them from the string definition since it violates the coding
guidelines.
Test plan:
1. On an fresh ktd db, run the script provided here (https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8127#c3) to fill the database with checkins/checkouts:
Visit http://your_koha:8081/cgi-bin/koha/reports/cat_issues_top.pl
2. Run a baseline test with following params:
Checkout date from 01/01/2020
Checkout date to: tomorrow
Library: Any library
Fill nothing else
Notice it is works
3. Test with some limits (Library, Week, and any other), notice you encounter errors on Library and Week, and that any other is empty, without error.
4. Apply patch
5. Rerun 2-3 and notice you now have results. Check with each possible limit, limiting to 5. Test once with 10 as a limit.
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Removes unnecessary colons when adding currencies and edi accounts.
1. Enable EDIFACT syspref
2. Navigate to Admin > Currencies.. > New currency
3. Note unnecessary colons after checkbox labels
4. Navigate to Admin > EDI accounts > New account
5. Note unnecessary colons after checkbox labels
6. Apply patch
7. Repeat steps 2 and 4, note colons have been removed
Signed-off-by: Kris Becker <kbecker@jcls.org>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This patch introduces a check if the variable change is defined, before its property innerHTML is accessed.
When calling "Write off selected" an Uncaught TypeError is thrown on form submission when accessing change.innerHTML as the variable change is undefined.
This regression was introduced by bug 42466 in a new submit handler which did not check if 'change' was defined at all.
Test plan:
1. In the staff interface, go to a patron's account
2. Go to Accounting > Create manual invoice
3. Fill out the manual invoice form with an amount
4. Click Save
5. Go to Make a payment
6. Click "Write off selected" (not write off on a single row)
7. IMPORTANT : Open the browser console
8. Click Confirm
9. apply patch and redo steps 1 - 8.
Signed-off-by: Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
$Test::Strict::TEST_STRICT = 0 is only set for all_perl_files_ok
It's no longer used since bug 42126
Test plan:
prove t/db_dependent/00-strict.t
should still return green
Remove a use Modern::Perl for mainpage.pl
=> Should still return green
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
The patch for bug 21453 replaced an Advanced editor-specific (in the "[% IF rancor %]" block) "[%- END -%]‡9[% authid | html %]";" which trims whitespace with several lines of "[% ... %]" which do not trim whitespace. The result is that when you look up an authority from the Advanced editor, you get extra whitespace between the last subfield from the authority record and the $9 field with the authority id.
Test plan:
1. Without the patch, set the preference EnableAdvancedCatalogingEditor to Enable
2. Cataloging - Advanced editor
3. Click in any line of the editor and press Return to get a blank line
4. Type 655 and press Tab three times
5. Type Ctrl+Shift+L to open the authority search popup
6. Click Search (no need for search terms) and click Choose for any record
7. Note that the editor shows twenty blank spaces between the end of the subfield a
contents and the ‡9
8. Apply patch
9. Repeat steps 3-6, note that now there is no extra space between the end of the
content and the ‡9
Sponsored-by: Chetco Community Public Library
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Note that t/Makefile.t and xt/check_makefile.t were doing the same test.
Merging them.
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
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>
The ILL requests page loads css/ill-requests.css from the staff template. The stylesheet is generated by build-resources.PL and ignored by git, but Makefile.PL does not list it in the generated asset map.
As a result package installs do not copy the generated stylesheet into blib, so Asset.css cannot find it and no link tag is emitted.
Add ill-requests.css to the generated staff CSS file map so it is installed with the other generated staff stylesheets.
Test plan:
1. perl -c Makefile.PL
2. prove -v t/Makefile.t
3. Confirm that /cgi-bin/koha/ill/ill-requests.pl includes a link tag for css/ill-requests.css on a package/source-style install.
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>
Two follow-up fixes from QA review of the previous commit:
- Detect Search::Elasticsearch::Error::NoNodes via ref($_) rather than
regex-matching the stringified exception, matching the existing
pattern already used in about.pl.
- Only reset jobs to 'new' for retry when NoNodes is the sole problem.
If a genuine indexing error also occurred in the same batch, record
the batch as 'failed' instead of silently discarding that error via
the NoNodes retry path.
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
When update_index() throws, the daemon now:
- On NoNodes: aborts the batch, recreates the indexer object to reset
the dead-node connection pool, and resets jobs to 'new' for retry.
- On other errors: marks jobs as 'failed' instead of 'finished'.
This prevents silent data loss where records disappear from search
results because their indexing jobs were incorrectly marked as done.
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
ownPagination set recordsFiltered on json.data instead of json, causing
DataTables to display "NaN total entries". Remove the artificial 5000
mechanism, propagate real counts from plugins that provide them, and
fall back to actual result counts for those that don't. Also removes a
dead drawCallback overwritten before it could fire.
Test plan:
1) Install this test version of the unpaywall plugin:
https://github.com/openfifth/koha-plugin-ill-avail-unpaywall/releases/tag/test
2) Enable ILLModule and ILLCheckAvailability
3) Configure the plugin and add any emaill address
4) Create a new ILL request:
<staff_url>/cgi-bin/koha/ill/ill-requests.pl?method=create&backend=Standard
5) Enter '42' for cardnumber, any library, and any DOI e.g. '123'.
6) Verify that pagination works as expected
Before patch, pagination controls dont show and the following label is shown:
No entries to show (filtered from NaN total entries)
Signed-off-by: Edith Speller <Edith.Speller@ukhsa.gov.uk>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This patch adds a maxlength attribute to a few form fields in the MARC
order accounts entry form. Text inputs should now allow the user the
enter more characters than the db will accept.
To test, apply the patch and enable the MarcOrderingAutomation system
preference if necessary.
- Go to Administration -> MARC order accounts -> New account.
- Test these inputs to confirm that you can't enter more characters than
their maxlengths:
- Description (250)
- Match field (10)
- Match value (50)
- Basket name field (10)
Sponsored-by: Athens County Public Libraries
Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
The default message in the "Self-renewal failure message" field
(Administration > Patrons and circulation > Patron categories >
Account expiry and self-renewal) uses incorrect terminology.
Test plan:
1. Go to Administration > Patrons and circulation > Patron categories.
2. Click on a patron category.
3. Scroll down to the "Self-renewal failure message" field (in the
Account expiry and self-renewal section).
4. Note the text says "...your local branch...".
5. Apply the patch.
6. Reload the page.
7. Note that the text now says "...your library...".
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Bug 41604 made every page do a bit more work when it loads. This
Selenium test didn't wait for that work to finish before typing into
the search box, so sometimes it searched too early and got the wrong
results.
This adds a short wait after each page load so the test only searches
once the page is actually ready.
Introduced by: Bug 41604: Save user decision per column when visibility changes
Test plan:
prove t/db_dependent/selenium/patrons_search.t
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
If we define visibility_condition we must have implemented
bKohaColumnsUseNames correctly.
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Example: supplier_report on the basket page
Signed-off-by: Jason Robb <jrobb@sekls.org>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
- Modify column-visibility.dt event handler to store the visibility
preference of the user
- Improve _dt_force_visibility() to correctly pick the correctly visibility value depending
on visibility_condition, is_hidden, and the user preference
- Add _dt_get_expected_column_visibility() to guess what's the expected visibility
- Force boolean true/false (vs 1/0) using !!
- Rely on a global variable kohaTableUserChoices to store the user
choice. It will be part of the state and store in localStorage.
Signed-off-by: Jason Robb <jrobb@sekls.org>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Jason Robb <jrobb@sekls.org>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Jason Robb <jrobb@sekls.org>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Jason Robb <jrobb@sekls.org>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
If a column must be displayed but was hidden before
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Jason Robb <jrobb@sekls.org>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Jason Robb <jrobb@sekls.org>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Jason Robb <jrobb@sekls.org>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Jason Robb <jrobb@sekls.org>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Jason Robb <jrobb@sekls.org>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
There are 3 other places where we force the visibility of columns. They
must als use this new visibility_condition flag, so they are all using
it.
Test plan:
The 3 tables are:
1. basket with the columns edifact, modify, cancel
2. hold history with the column itemtype
3. batch edit patron with the columns checkbox, password expiration,
protected
Signed-off-by: Jason Robb <jrobb@sekls.org>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Jason Robb <jrobb@sekls.org>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
We want to hide column if:
* the feature is not accessible (pref disabled or insufficient
permissions)
or
* the column is hidden in the settings
or
* the column has been hidden by the user
Signed-off-by: Jason Robb <jrobb@sekls.org>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Jason Robb <jrobb@sekls.org>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Bug 41314 caused a regression, making it impossible to hide the Checkin column in the issues table in the patron's circulation page.
To recreate:
1. Hide the checkin column by default in circulation issues-table
1.1. Go to Administration > Table settings > Circulation tab > Jump to page : circulation > Table id: issues-table
1.2. Check 'Is hidden by default' for 'checkin'
1.3. Click 'Save'
2. logout and login
3. Checkout an item to a patron
3.1. In the search bar, set to Checkout, search for Henry
3.2. In the checkout field, enter the barcode 39999000010114 and press enter
--> In the table of issues, Checkin column is visible, it should be hidden by default
4. Apply the patch
5. Repeat step 2, 3
--> In the table of issues, Checkin column is not visible
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Jason Robb <jrobb@sekls.org>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Jason Robb <jrobb@sekls.org>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
To test:
1 - In Koha testing docker record 369 has invalid xml, otherwise corrupt a record like:
sudo koha-mysql kohadev
UPDATE biblio_metadata SET metadata = UpdateXML(metadata, '//datafield[@tag="040"]/subfield[@code="c"]',CONCAT('<subfield code',CHAR(27),'="c">OSt</subfield>')) WHERE
biblionumber=438;
2 - View the record in the staff client - note bad metadata message
http://localhost:8081/cgi-bin/koha/catalogue/detail.pl?biblionumber=438
3 - Click 'Edit'
4 - Error 500, in logs:
Invalid data, cannot decode metadata object
5 - Apply patch, restart all
6 - Reload record and click edit
7 - Success! Message about bad metadata, but can edit/save
Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
The outer SELECT hardcoded unitprice_tax_included while the inner
subquery selects the field returned by FieldsForCalculatingFundValues.
With CalculateFundValuesIncludingTax disabled the derived table only
exposes unitprice_tax_excluded, so spent.pl dies with:
Unknown column 'unitprice_tax_included' in 'field list'
To test:
1. Set CalculateFundValuesIncludingTax to 'Exclude'
2. View spent.pl for a fund with received orders
3. Observe error 500
4. Apply patch, restart_all
5. Reload, page renders correctly
6. Set CalculateFundValuesIncludingTax back to 'Include'
7. Reload, page still renders correctly
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
To test:
1 - Have some received orders and some invoice adjustments
2 - View spent.pl for a fund
3 - Apply patch, restart_all
4 - Reload
5 - Confirm the display has not changed
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
The previous patch hides the 'Account credit' option in the refund modal
when the accountline has no borrowernumber, but that is a UI convenience
only. A crafted POST (or a stale page) could still submit
refund_type=AC against an anonymous accountline, which would create a
REFUND credit with no patron to attach it to and leave the register
unbalanced because the payout branch is skipped.
Add a server-side guard in pos/register.pl that refuses the operation
and surfaces a new error_refund_anonymous_ac flag, rendered in
pos/register.tt in the same style as the other refund errors.
Test plan:
1. Apply the patch
2. Log in as a user with cash_management/anonymous_refund permission
3. Record an anonymous sale against a register
4. Using the browser devtools (or curl), POST to pos/register.pl with
op=cud-refund, refund_type=AC and the accountline id of the sale
5. => Confirm the request is rejected and the
"Account credit refunds are not available for anonymous transactions"
message is shown
6. => Confirm no REFUND accountline was created and the register
balance is unchanged
7. Repeat the POST with refund_type=CASH (or any non-AC type) and
confirm the refund + payout are created normally
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
When processing a refund through the POS register with refund type 'AC'
(Account Credit), the system was incorrectly creating both a REFUND credit
and a PAYOUT debit with payment_type = 'AC'.
When 'AC' is selected, the refund should remain as a credit on the patron's
account without creating a payout. This behavior was already correct in the
member account refund workflow (members/boraccount.pl) but was missing
from the POS refund workflow.
This patch:
1. Adds the same check to pos/register.pl that exists in members/boraccount.pl
to skip payout creation when refund_type is 'AC', making both workflows
consistent.
2. Fixes the JavaScript in register.tt to properly hide the 'AC' option for
anonymous transactions. The original check only tested for empty string,
but borrowernumber can be undefined/null, which needs to be handled.
Test plan:
Part 1: Test patron transaction refund with Account Credit (AC)
1. Find or create a test patron
2. On the patron's Accounting tab, manually add a charge (e.g., Manual fee)
3. Pay the fee off from the patron's Accounting tab
4. Navigate to Point of Sale > Transaction History
5. Find the payment transaction you just created
6. Click "Issue refund" on the payment line item
7. Verify that "Account credit" appears as an option in the "Transaction type"
dropdown
8. Select "Account credit" as the transaction type
9. Enter the refund amount and click "Confirm"
10. Navigate back to the patron's Accounting tab
11. Verify that a REFUND credit appears on their account
12. Verify that NO PAYOUT debit was created
13. Verify the credit remains on the patron's account (positive balance)
Part 2: Test patron transaction refund with Cash payout
16. Using the same patron, create another manual fee and payment
17. Navigate to Point of Sale > Transaction History
18. Click "Issue refund" on the new payment
19. Select "Cash" (or another payment type) as the transaction type
20. Enter the refund amount and click "Confirm"
21. Navigate back to the patron's Accounting tab
22. Verify both a REFUND credit and a PAYOUT debit appear
23. Verify the PAYOUT has the correct payment_type (e.g., CASH)
Part 3: Test anonymous transaction (no Account Credit option)
24. Navigate to Point of Sale
25. Add an item to sell and complete the payment
27. Navigate to Point of Sale > Transaction History
28. Find the anonymous payment you just created
29. Click "Issue refund" on the payment line item
30. Verify that "Account credit" does NOT appear in the "Transaction type"
dropdown (only payment types like Cash, Check, etc. should appear)
31. This confirms anonymous transactions cannot have account credits since
there's no patron account to credit
Expected results:
- Patron transactions show "Account credit" option in refund dropdown
- Anonymous transactions do NOT show "Account credit" option
- AC refunds create only REFUND credit (no PAYOUT debit)
- Non-AC refunds create both REFUND credit and PAYOUT debit
- Run: prove t/db_dependent/Koha/Cash/Register/Cashup.t
Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Bug 40866 introduced JSON logging for HOLDS CREATE but stored null for
'confirmations' and 'forced' when no overrides were present. This patch
mirrors the earlier change to CIRCULATION ISSUE by defaulting them to
empty arrays, keeping the two log streams structurally identical and
simplifying reporting queries.
Changes:
C4/Reserves.pm:
- Default confirmations/forced to [] in the AddReserve log payload.
t/db_dependent/Reserves.t:
- Update the Bug 40866 subtest to assert the empty-array defaults on
normal holds, and fix a 'my $log_data' redeclaration.
tools/viewlog.pl:
- Extend the CSV export JSON-decode block to also cover HOLDS logs,
guarding the item lookup on itemnumber being present (HOLDS payloads
may lack one) and filling the CSV item columns from the decoded
payload. The legacy bare-itemnumber fallback is restricted to
CIRCULATION so a legacy HOLDS hold_id is not mistaken for an
itemnumber.
koha-tmpl/intranet-tmpl/prog/js/viewlog.js:
- Extend the info column override rendering (renderCircInfo) to HOLDS
entries so forced/confirmations arrays display translated, exactly as
they do for CIRCULATION. The log viewer moved to an ajax datatable in
bug 29016, so this replaces the earlier json_found change to
viewlog.pl/viewlog.tt.
misc/maintenance/migrate_action_logs_to_json.pl:
- Extended to handle legacy pre-Bug 40866 HOLDS CREATE logs where info
is a bare hold_id, recovering branchcode / biblionumber / itemnumber
from old_reserves / reserves.
- Uses the same keyset-pagination pattern as the circulation loop so
rows are not skipped under --commit.
Sponsored-by: OpenFifth <https://openfifth.co.uk/>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This patch updates offline circulation and ILL checkouts to properly
record their nature in circulation logs as forced transactions.
Offline circulation (process_koc.pl):
- Offline transactions have already occurred without access to normal
circulation checks
- Now marks these as forced with 'OFFLINE_CIRCULATION' override code
- This helps identify historical transactions that bypassed checks
ILL checkouts (Koha/ILL/Request.pm):
- ILL checkouts are system-initiated and may bypass certain restrictions
by design
- Now marks these as forced with 'ILL_CHECKOUT' override code
- This helps track and report on ILL circulation activity separately
Display updates (action-logs.inc):
- Added translation strings for both new override codes:
- OFFLINE_CIRCULATION: "Offline circulation checkout"
- ILL_CHECKOUT: "ILL system checkout"
- These will now display properly in Tools > Log viewer
This ensures these special checkout types are properly identified in
reporting and analysis, making it easier to understand the context
of historical transactions.
Sponsored-by: OpenFifth <https://openfifth.co.uk/>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This patch adds an out-of-band migration script to convert legacy
CIRCULATION ISSUE action logs from the old itemnumber-only format
to the new consistent JSON format.
The script:
- Finds all CIRCULATION ISSUE logs where info is just an itemnumber
- Converts them to JSON format with the standard structure
- Attempts to recover issue_id and branchcode from old_issues/issues
- Runs in dry-run mode by default (--commit flag to actually update)
- Processes in batches to handle large databases safely
- Includes verbose mode for detailed output
Usage:
perl misc/maintenance/migrate_action_logs_to_json.pl --help
perl misc/maintenance/migrate_action_logs_to_json.pl --verbose
perl misc/maintenance/migrate_action_logs_to_json.pl --commit
This is designed as an out-of-band maintenance script rather than
an atomic update because:
1. May be expensive on large databases with many log entries
2. Migration is optional - display code handles both formats
3. Can be run during off-peak hours at admin's discretion
Sponsored-by: OpenFifth <https://openfifth.co.uk/>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Previously, CIRCULATION ISSUE logs would store just the itemnumber when
there were no confirmations or forced overrides, but would store a full
JSON structure when overrides were present. This inconsistency makes
reporting difficult as queries need to handle both formats.
This patch changes AddIssue to always store logs in JSON format,
matching the pattern already used in AddRenewal. The JSON structure
now consistently includes:
- issue: issue_id
- branchcode: branchcode
- itemnumber: itemnumber
- confirmations: array of confirmation codes (empty array if none)
- forced: array of forced override codes (empty array if none)
Benefits:
- Consistent data format simplifies reporting and analysis
- Matches existing RENEWAL logging pattern
- Display code (tools/viewlog.pl) already handles both formats,
so backward compatibility is maintained
Test plan:
1. prove t/db_dependent/Circulation.t
2. Perform a normal checkout - verify action log contains JSON
3. Perform a checkout with overrides - verify confirmations/forced
are populated in the JSON
4. Check Tools > Log viewer - both types should display correctly
Sponsored-by: OpenFifth <https://openfifth.co.uk/>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Note: this requires a command line access to the test machine
TEST PLAN:
1 - Anywhere on the computer running koha, run following command:
mkdir tmp
echo '<!DOCTYPE html><html><head><meta charset="UTF-8"></head><body><p>Тетрис — это классика!</p></body></html>' > tmp/tmp.html
echo '* { color: red };' > tmp.css
./misc/cronjobs/printoverdues.sh tmp tmp.css
2 - Open the pdf in tmp/tmp.pdf, notice it is broken
3 - Apply patch
4 - Rerun ./misc/cronjobs/printoverdues.sh tmp tmp.css
5 - Open the pdf in tmp/tmp.pdf, notice it is now displaying properly, notice the css property still applies
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
The previous two commits on this bug fixed two ordering assumptions in
the RETRY loop of _allocateWithTransportCostMatrix(): resetting $inf at
the start of each attempt, and splicing unallocated task indices out of
@requests in descending order (removing a lower index first would shift
the position of any higher index still waiting to be removed).
This is a third instance of the same underlying problem, a few lines
earlier in the loop that compacts the working cost matrix $m before
that splice runs. @unallocated is built by iterating over agents (item
rows), pushing the column index Munkres assigned to each row that
turned out infeasible - and Munkres' row-to-column assignment is a
permutation, not a monotonic function of the row index, so
@unallocated is no more reliably sorted here than it was for the
splice.
That compaction loop scans matrix columns in ascending order and
advances a single pointer into @unallocated, expecting to meet each
entry in that same ascending order. When @unallocated arrives out of
order, the pointer never catches up: columns that should have been
dropped are left in place, and the columns after them are never
shifted left to fill the gap. From that point on, $m no longer lines
up with @requests, which *is* correctly reduced by the splice - so on
the next retry pass a hold's transport cost gets evaluated using a
different hold's matrix row. Depending on the exact misalignment, this
can silently drop a fillable hold from the queue while a genuinely
available item sits unused, or fill a hold via the wrong-cost item.
It didn't show up in the "many holds" regression test added earlier on
this bug, because that test's transport cost matrix is uniform (every
disabled pair costs the same); this defect only surfaces once the
columns competing for a scarce item have distinct costs.
Sort @unallocated once, ascending, and use that copy for the
compaction pass; reverse it for the splice, which needs descending
order.
Test plan:
1. Run the new t/db_dependent/HoldsQueue/RetryAllocationOrdering.t -
it calls _allocateWithTransportCostMatrix() directly with a small
hand-built scarce transport cost matrix and a pinned srand() (the
specific item-to-branch permutation needed to trigger the bug
depends on the shuffle() inside the routine). Reverting just this
patch makes 3 of its 6 subtests fail: the L2 pickup hold is dropped
even though an available item could have filled it.
2. Run t/db_dependent/HoldsQueue/TransportCostOptimizations.t to
confirm no regression in the existing retry-loop coverage.
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Martin Renvoize
Lucas Gass
Jonathan Druart
Nick Clemens
Baptiste Wojtkowski
Jason Robb
Jan Kissig
Phil Ringnalda
David Cook
Andrii Nugged
Tomás Cohen Arazi
Pedro Amorim
Owen Leonard
Samuel Sowanick
Hammat Wele
Paul Derscheid