Commit Graph

60571 Commits

Author SHA1 Message Date
Jonathan Druart 7fdf10b9ad
Bug 41558: Fix and standardize links to tab on opac-user
We have different implemented solution to select a tab on opac-user:
* a "tab" variable (from bug 37972), it's never used
* several variables passed from the controller to the template, depending on the tab to select (it works and is used)
* use of anchors (it does not work and is used)

We should fix and standardize it.

I've linked to several reports that seem to conflict with each others.

This patch suggests to have a "tab" variable

Test plan:
Confirm that the different links to tab to opac-user are not using a
query parameter to pass the tab to select, e.g. from the opac-main page.

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
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>
2026-03-05 08:33:55 -07:00
Nick Clemens 036d92e654
Bug 41316: Use patrons homelibrary for selecting overdues that should be sent
Currently the patron homelibrary option for OverdueNoticeFrom will select overdues for a branches triggers
based on the issuing branch

The other two options select the trigger/overdues based on the selection

This patch alters the selection of overdues to match only for the patrons branch

To test:
1 - Apply 41315 and follow the plan there
2 - Apply this bug and run the overdues script, assuming CPL is branch A:
    perl misc/cronjobs/overdue_notices.pl --triggered --library CPL --test --nomail -v -v
3 - No overdues should be triggered
4 - Run the script for one of the patrons home libraries, in my case MPL
    perl misc/cronjobs/overdue_notices.pl --triggered --library MPL --test --nomail -v -v
5 - Note only overdues for patrons from that branch are triggered

Signed-off-by: Ben Daeuber <bdaeuber@fargolibrary.org>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2026-03-05 08:33:54 -07:00
Owen Leonard 0b7c7eab86
Bug 40568: Various corrections to recalls templates
This patch makes various minor changes to recalls templates.

- Correct sorting of dates in tables
- Wrap recalls table in "page-section" div.
- Hide "Show old recalls" if there are no recalls.

To test, apply the patch and enable Recalls if necessary.

- If necessary, place some recalls.
- Check the sorting of date columns on the "Recalls queue" page. It
  should be correct.
- View a patron who has never placed a recall. Go to their recalls
  history and confirm that you see a "Patron has no recalls" message but
  no "Show old recalls" checkbox.

Sponsored-by: Athens County Public Libraries
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>
2026-03-05 08:33:54 -07:00
Baptiste Wojtkowski 4996ea8691
Bug 40711: Fix valuebuilder for UNIMARC 181c and 2
Test plan (UNIMARC ONLY):
1 - Make sure you have valuebuilder enabled for 181$c and 181$2 (see hereafter)
2 - Edit any record. Use valuebuilder to set 181$c to "cri - cartographic image", notice there is no change
3 - Use valuebuilder to set 181$c to "crm - cartographic moving image", notice it is set to "a"
4 - Open the valuebuilder for 181$2 and inspect the body ID. See it is cat_unimarc_field_182-2.
5 - Apply patch
6 - Repeat 2&3 and notice the codes are now cri respectively cri and crm
7 - Open the valuebuilder for 181$2 and inspect the body ID. See it is cat_unimarc_field_181-2.

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>
2026-03-05 08:33:53 -07:00
Marcel de Rooy 7f8f5cbb5c
Bug 41444: (QA follow-up) Rearrange comments
It would be at least helpful to add a reference to get_transfer here,
since we are replacing it here.
Used the opportunity to update the older comments

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2026-03-05 08:33:53 -07:00
Nick Clemens 13d2eb3d1b
Bug 41444: Search for transfers directly
This patch removes a fetch of the item object followed but get_transfer to simply search the
transfers by itemnumber and status

To test:
1 - Add many items to your records
    https://github.com/kidclamp/handy-koha-script/blob/main/randitems.pl
    NOTE: this script makes a lot of items withdrawn or damaged, undo that for more dramatic difference
    UPDATE items SET withdrawn=0,damaged=0
    perl misc/search_tools/rebuild_elasticsearch -v
2 - Set Koha system preferences to display more results and check more items:
    maxItemsInSearchResults 60
    numSearchResults 50
3 - Do a search, several times, note the time it takes
4 - Apply patch, restart all
5 - Repeat search several times, note the time it takes
6 - Confirm it is better after the patch

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2026-03-05 08:33:52 -07:00
Kyle M Hall 5779eec49b
Bug 39749: RestrictPatronsWithFailedNotices should not trigger for DUPLICATE_MESSAGE failures
Koha is built to prevent sending duplicate notices. Notices with static language ( e.g. "You have one or more holds ready to pick up" ) will easily trigger this filter.

RestrictPatronsWithFailedNotices should ignore these notices because they do not indicate any problem with contacting the patron.

Test Plan:
1) Enable RestrictPatronsWithFailedNotices
2) Enqueue to identical notices for a patron
3) Run process message queue to send one and fail the other as a
   duplicate ( alternately, just modify them in the database )
4) Run restrict_patrons_with_failed_notices.pl
5) Note the restriction
6) Remove the restriction
7) Apply this patch
8) Run restrict_patrons_with_failed_notices.pl
9) Note that no restriction was created!

Signed-off-by: Marion Durand <marion.durand@biblibre.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2026-03-05 08:33:52 -07:00
Lucas Gass 3d99867f60
Bug 23260: DBIC
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2026-03-05 08:33:51 -07:00
Lucas Gass c88a86d0af
Bug 23260: DBRev 25.12.00.028
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2026-03-05 08:33:51 -07:00
Martin Renvoize 599286cf40
Bug 23260: (QA follow-up) Validate AnonymousPatron preference value
Add validation to ensure AnonymousPatron preference contains a valid
borrowernumber before attempting to use it in the anonymization process.
This prevents foreign key constraint errors and orphaned records if the
preference contains an invalid value (non-existent borrowernumber,
string instead of number, etc.).

Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2026-03-05 08:33:50 -07:00
Martin Renvoize c074c66d51
Bug 23260: (QA follow-up) Optimize anonymize_last_borrowers to avoid N+1 queries
This commit refactors Koha::Patrons->anonymize_last_borrowers() to use
a single bulk UPDATE instead of looping through patrons individually.

Performance Problem:
-------------------
The original implementation had a severe N+1 query problem:
1. Query all patrons with items_last_borrower records
2. Loop through each patron (First N+1)
3. For each patron, search their items_last_borrowers (Second N+1)
4. Update each result set

For a library with 10,000 patrons and 100,000 items, this would
execute 20,000+ database queries when run as a cronjob.

The Fix:
--------
Query the items_last_borrower table directly with all conditions
in a single search, then perform one bulk UPDATE:

  $schema->resultset('ItemsLastBorrower')->search({...})->update({...})

This reduces execution from potentially thousands of queries to a
single SQL UPDATE statement, regardless of data volume.

Why This Pattern:
-----------------
While Koha::Objects->update() exists, it loops through records
individually if the object class has update()/store() methods,
unless {no_triggers => 1} is passed. Working directly with the
resultset guarantees bulk update behavior.

This pattern is used throughout Koha for bulk operations (see
Koha/Item.pm, Koha/Hold.pm, admin/koha2marclinks.pl, etc).

Testing:
--------
prove t/db_dependent/Koha/Patrons.t

All existing tests pass, confirming identical behavior with
dramatically improved performance.

Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2026-03-05 08:33:49 -07:00
Lucas Gass 2d75cac7d1
Bug 23260: Fix database update
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2026-03-05 08:33:49 -07:00
Lucas Gass a37d887df6
Bug 23260: Fix kohastructure.sql
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2026-03-05 08:33:48 -07:00
Lucas Gass fde40177fa
Bug 23260: Update cron to exit if AnonymousPatron is not set
Signed-off-by: Peter Moore <peter@mrcpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2026-03-05 08:33:48 -07:00
Lucas Gass 9165c0fa4b
Bug 23260: Fix DB update to NOT allow for NULL borrowernumber
Signed-off-by: Peter Moore <peter@mrcpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2026-03-05 08:33:47 -07:00
Andrew Fuerste Henry f0d4240ee2
Bug 23260: (QA follow-up) Prevent use of cronjob when AnonymousPatron not set
To test:
1 - configure StoreLastBorrower but not AnonymousPatron
2 - perl misc/cronjobs/anonymize_last_borrowers.pl -c
3 - the cron tells 'Preference 'AnonymousPatron' must be enabled to anonymize item's last borrower'
4 - put a value in AnonymousPatron
5 - repeat 2
6 - cron runs good

Signed-off-by: Peter Moore <peter@mrcpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2026-03-05 08:33:47 -07:00
Lucas Gass 7eda8d23fa
Bug 23260: Add a confirm flag
To test the confirm flag:
1. Run perl misc/cronjobs/batch_anonymise.pl --days 1
2. Notice the warning about needing a confirm flag
3. Run perl misc/cronjobs/batch_anonymise.pl --days 1 --confirm
4. The script runs

Signed-off-by: Peter Moore <peter@mrcpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2026-03-05 08:33:46 -07:00
Nick Clemens ecfd85ebb8
Bug 23260: Add cronjob anonymize_last_borrowers.pl
This patch adds the feature to anonymize patrons from items_last_borrower table. This code is run from batch_anonymize script, which is triggered from a new cron job anonymize_last_borrowers.pl

To test:
 1) apply this patch and the previous ones
 2) perl installer/data/mysql/updatedatabase.pl
 3) set StoreLastBorrower preference to Allow.
 4) set AnonymousPatron preference to a valid patron id
 5) Create a Check out followed by a Check in.
    CHECK => a row should appear in items_last_borrower table with the borrower and the item number.
 6) In mysql, update created_on of items_last_borrower and returndate of old_issues to two days earlier.
 7) perl misc/cronjobs/anonymize_last_borrowers.pl
    CHECK => borrower number in items_last_borrower is not anonymized yet.
 8) Repeat step 5 and 6 but instead of two days earlier, set the new entry in items_last_borrower to three days earlier.
 9) set AnonymizeLastBorrower to 'Anonymize' and AnonymizeLastBorrowerDays to 2 days.
10) perl misc/cronjobs/anonymize_last_borrowers.pl
    SUCCESS => borrower number in items_last_borrower changed to AnonymousPatron id
11) repeat step 8
12) set AnonymousPatron preference to 0
13) perl misc/cronjobs/anonymize_last_borrowers.pl
SUCCESS => borrower number in items_last_borrower changed to null
14) prove t/db_dependent/Koha/Patrons.t

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Peter Moore <peter@mrcpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2026-03-05 08:33:46 -07:00
Nick Clemens 39ce85bf84
Bug 23260: Unit tests
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Peter Moore <peter@mrcpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2026-03-05 08:33:45 -07:00
Nick Clemens 974007c0db
Bug 23260: Add anonymize_last_borrowers method to Patrons.pm
This patch adds a new routine to allow anonymizing the items_last_borrower table

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Peter Moore <peter@mrcpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2026-03-05 08:33:45 -07:00
Nick Clemens ab5efb97a4
Bug 23260: Database updates and new system preferences AnonymizeLastBorrower / AnonymizeLastBorrowerDays
This patch alters the items_last_borrowers table to allow for NULL borrowers

It also adds two new system preferences to allow controlling the anonymization of items_last_borrower data

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Peter Moore <peter@mrcpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2026-03-05 08:33:44 -07:00
Martin Renvoize e47511f889
Bug 41319: Automate sponsor tracking from git trailers
This patch replaces the hardcoded "Contributing companies and
institutions" list on the about page with an automated system that
extracts sponsor information from git commit trailers.

Changes:

1. about.pl: Load sponsor data from docs/sponsors.yaml using YAML::XS
2. about.tt: Replace hardcoded list with dynamic rendering from
   sponsors.yaml, displaying sponsors with their locations
3. docs/sponsors.yaml: Generated sponsor data file containing 212
   unique sponsors with contribution counts, locations, and URLs

The sponsor data is generated at release time using the
koha-update-sponsors script from release-tools (see release-tools
commit a30b8a3). The script:

- Extracts Sponsored-by: trailers from git history
- Applies alias resolution to consolidate duplicate names
- Preserves historical sponsors without git trailers
- Merges location and URL data from sponsor_map.yaml

This replaces the manual process of updating the sponsor list with
a maintainable approach that automatically captures all Sponsored-by:
entries from the git history while preventing duplicate entries through
alias resolution.

The about page now displays sponsors in the format:
"Organization Name, Country" instead of showing contribution counts.

To update the sponsor list at release time:
1. Run koha-update-sponsors from the Koha source directory
2. Review and commit the updated docs/sponsors.yaml

See release-tools README.md for detailed documentation.

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2026-03-05 08:33:44 -07:00
Owen Leonard 0ddcaa5e8a
Bug 41134: Add table settings to transfers
This patch updates the transfers page in order to add table settings to
the table of transfered items.

The patch also makes another minor correction: The transfer submit
button now has the standard class "btn-primary."

To test, apply the patch and restart services.

- Go to Circulation -> Transfer.
- Submit a barcode from a library other than your own so that the
  transfer will complete successfully.
  - Note that the "Transfer" button has the standard primary yellow
    color.
- In the table of transferred items you should see the columns setting
  menu, export option, etc. Confirm that these controls work correctly.
- Go to Administration -> Table settings -> Circulation -> Transfers
- Make some changes to the column visibility settings and return
  to the transfers page to confirm that your settings are now
  applied when you transfer more items.
  - Because of DataTables state saving feature you might have to clear
    your browser's local storage in order to see the new column
    configuration.

Sponsored-by: Athens County Public Libraries
Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2026-03-05 08:33:43 -07:00
Lucas Gass d7aae3632e
Bug 39698: DBRev 25.12.00.027
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2026-03-05 08:33:43 -07:00
Jake Deery 5b59301de2
Bug 39698: Add option to expand responsive datatable
This patch adds the 'OPACTableColExpandedByDefault' system preference,
disabled by default, which controls whether responsive datatable rows
are expanded or not. This work has been performed after recommendations
from a recent accessibility audit.

TO TEST:
a)  set 'OpacLocationBranchToDisplay' to both
b)  search the OPAC for history, select first result
c)  zoom your browser in to maxiumum, scroll down to holdings table
    -   notice how the rows are collapsed by default, with '+' visible
**apply patch**
e)  refresh details page, notice how the behaviour is currently unchanged
f)  change new 'OPACTableColExpandedByDefault' syspref to 'Expand'
g)  return to OPAC details page, refresh, zoom in
    -  notice how the rows are now expanded by default, with '-' visible
**sign off**

Sponsored-by: British Museum
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2026-03-05 08:33:42 -07:00
Shi Yao Wang 73c320e751
Bug 39580: Make Elasticsearch process_error error string translatable
Test plan:
1- Change to a language other than english
2- Search in the catalog with "!"
3- Notice the string "Unable to understand your search query, please
 rephrase and try again." is not translated
4- Apply the patch
5- Add translation in .po file of used language for "Unable to
understand your search query, please rephrase and try again.\n"
6- Update translation
7- Redo the search and notice the string is now translated

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2026-03-05 08:33:42 -07:00
Kyle M Hall 3487b56e20
Bug 36542: Update unit test
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2026-03-05 08:33:41 -07:00
Arthur Suzuki c3ce3b17c7
Bug 36542: after_biblio_action plugin hook is now called after the dbx transaction is finished
To reproduce :
1) Install a plugin which uses the biblionumber of a created record in the "after_biblio_action".
2) Verify the biblionumber is missing

This is because the call to the hooks is done before the DB transaction is actually done (in AddBiblio function).

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2026-03-05 08:33:41 -07:00
Pedro Amorim 0af6ad8cd5
Bug 40105: (QA follow-up): Have a 'additional information' section
This will encompass the 'Notes' field plus custom fields.

Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2026-03-05 08:33:40 -07:00
phette23 5940fb39d5
Bug 40105: Patrons cannot add notes when creating an ILL
Testing plan:
- Enable the ILLModule system preference
- Sign into the OPAC and make an ILL request at /cgi-bin/koha/opac-illrequests.pl
- Note there's no option to specify notes
- Apply the patch and restart services
- Make another ILL request
- Use the new notes field
- View the request and note that the notes value was saved

Sponsored-By: California College of the Arts
Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2026-03-05 08:33:32 -07:00
Jake Deery c3cf41c275
Bug 41619: (Follow-up) Fix for t/Koha/CSV.t
This patch fixes an omission that caused t/Koha/CSV.t to fail. To test, prove t/Koha/CSV.t and notice how it now passes.

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2026-02-27 13:12:37 -07:00
Martin Renvoize 300f121b66
Bug 35612: (QA follow-up) Add test for undefined LostChargesControl preference
Adds test coverage for the case where LostChargesControl system
preference is not set (returns undef). This ensures the code
handles this gracefully and defaults to ItemHomeLibrary behavior.

The test verifies that:
- No warnings are generated when the preference is undefined
- The default behavior (ItemHomeLibrary) is applied correctly
- The correct branch (item homebranch) is recorded in accountlines

This improves test coverage and prevents regression of the fix
for uninitialized value warnings.

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2026-02-27 11:38:54 -07:00
Martin Renvoize 30510e1d0a
Bug 35612: (QA follow-up) Document undef handling in branch_for_fee_context
Improves POD documentation for branch_for_fee_context to:
- Document all parameters (patron, item, issue objects)
- Explain the resolution logic for each control preference
- Clarify that returning undef is acceptable behavior
- Note how undef is handled by calling code

Adds inline comment explaining that undef return value is
gracefully handled by Koha::Account->add_debit.

This addresses concerns about unclear undef handling and
improves maintainability.

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2026-02-27 11:38:53 -07:00
Martin Renvoize 73ae8a15db
Bug 35612: (QA follow-up) Use interface parameter and remove self-referential use
This patch fixes two issues:

1. C4::Accounts::chargelostitem now properly uses the $interface
   variable that was set from $opts->{interface} instead of calling
   C4::Context->interface directly. This ensures the interface
   parameter is honored, particularly important for cronjob context.

2. Removes the self-referential 'use Koha::Checkout;' statement in
   Koha/Checkout.pm which is logically incorrect (a module should
   not use itself).

Test plan:
1. Run prove t/db_dependent/Accounts.t
2. Run prove t/db_dependent/Koha/Checkout.t
3. Verify all tests pass

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2026-02-27 11:38:53 -07:00
Martin Renvoize b6695f75bd
Bug 35612: (QA follow-up) Fix uninitialized value warnings
When LostChargesControl system preference is not set (undefined),
the code was generating 'Use of uninitialized value' warnings
when comparing $lost_control_pref in string equality checks.

This adds defined checks before the comparisons to handle the
case where the preference is undefined, treating it the same as
'ItemHomeLibrary' (the default).

Test plan:
1. Run prove t/db_dependent/Circulation.t
2. Verify all tests pass without warnings

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2026-02-27 11:38:52 -07:00
Laura_Escamilla 3308b7f61c
Bug 35612: Add tests for chargelostitem branch context
Adds test coverage for C4::Accounts::chargelostitem to verify that
LOST and PROCESSING fee debits correctly record branchcode based on
LostChargesControl and HomeOrHoldingBranch system preferences.

Tests verify all scenarios:

1. LostChargesControl behavior:
   - PatronLibrary: Uses patron's home branch
   - ItemHomeLibrary + homebranch: Uses item's home library
   - ItemHomeLibrary + holdingbranch: Uses item's holding library
2. Explicit library_id override:
   - When library_id is passed in options, it takes precedence
3. Deduplication logic:
   - Prevents duplicate LOST charges for same itemnumber+issue_id
   - Regression test for item_id column name bug
4. Issue_id handling:
   - Correctly looks up issue_id when not provided
   - Handles bundled items (looks up host item's checkout)
5. Processing fees:
   - Honors the same branch logic as LOST fees
   - Correctly applies lost_item_processing_fee circulation rule
     using the appropriate branch context

This test subtest was added to the existing "C4::Accounts::chargelostitem tests"
section and increases the test plan from 4 to 5 subtests.

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>
2026-02-27 11:38:52 -07:00
Laura_Escamilla e45e06dcf5
Bug 35612: Fix incorrect primary key reference
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>
2026-02-27 11:38:51 -07:00
Laura_Escamilla 0832933a96
Bug 35612: Add tests for branch context recording in LOST fees via LostItem
Adds comprehensive test coverage for C4::Circulation::LostItem to verify
that LOST and PROCESSING fee debits correctly record branchcode based on
LostChargesControl and HomeOrHoldingBranch system preferences.

Tests verify:
- LostChargesControl = PatronLibrary uses patron's home branch
- LostChargesControl = PickupLibrary uses checkout/issue branch
- LostChargesControl = ItemHomeLibrary respects HomeOrHoldingBranch:
  * homebranch: uses item's home library
  * holdingbranch: uses item's holding library

Tests cover both LOST and PROCESSING fee types and verify the branch
context is preserved when fees are charged via the LostItem function,
which is called both from the staff interface when manually marking
items lost and from the longoverdue cronjob.

The tests also verify correct behavior when mark_lost_from = 'cronjob'
to ensure the interface context is properly set for batch operations.

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>
2026-02-27 11:38:51 -07:00
Laura_Escamilla c7764f1140
Bug 35612: Add tests for branch context recording in OVERDUE fees
Adds comprehensive test coverage for C4::Overdues::UpdateFine to verify
that the branchcode field is correctly populated based on CircControl
and HomeOrHoldingBranch system preferences.

Tests verify:
- CircControl = PatronLibrary uses patron's home branch
- CircControl = PickupLibrary uses checkout/issue branch
- CircControl = ItemHomeLibrary respects HomeOrHoldingBranch:
  * homebranch: uses item's home library
  * holdingbranch: uses item's holding library

The tests create distinct libraries for each context (patron home,
item home, item holding, checkout branch) and verify the correct
branch is recorded in accountlines.branchcode for OVERDUE debits.

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>
2026-02-27 11:38:50 -07:00
Laura_Escamilla 60728c63bb
Bug 35612: Add unit test for Koha::Checkout->branch_for_fee_context
Adds test coverage for the new branch_for_fee_context class method.
Tests are minimal at this stage but verify the method exists and
can be called. More comprehensive testing is covered in the
C4::Accounts, C4::Circulation, and C4::Overdues test additions.

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>
2026-02-27 11:38:50 -07:00
Laura_Escamilla 82ff1e700b
Bug 35612: Recorded branch context for OVERDUE, LOST, and PROCESSING fees
This patch ensures that the accountlines.branchcode field correctly records
which branch's circulation rules were applied when creating OVERDUE, LOST,
and PROCESSING fee debits. Previously, this field was often NULL or set
to the logged-in user's branch, making it difficult to track which branch's
policies governed the fee calculation.

Changes made:

1. New helper method: Koha::Checkout->branch_for_fee_context()
   - Centralizes branch resolution logic based on system preferences
   - For LOST/PROCESSING: honors LostChargesControl preference
   - For OVERDUE: honors CircControl preference
   - Respects HomeOrHoldingBranch when choosing item's branch
   - Supports both Koha objects and plain hashrefs
   - Returns branchcode or undef if context unavailable
2. C4::Accounts::chargelostitem()
   - Now accepts optional $opts hashref parameter
   - Accepts interface, library_id, and issue_id in options
   - Determines appropriate branch using branch_for_fee_context
   - Passes resolved branch to add_debit as library_id
   - Issue_id lookup handles bundled items correctly
3. C4::Circulation::LostItem()
   - Calls branch_for_fee_context before chargelostitem
   - Passes interface context ('cron' for cronjob, otherwise current)
   - Passes resolved branch and issue_id to chargelostitem
4. C4::Overdues::UpdateFine()
   - Calls branch_for_fee_context to determine OVERDUE branch
   - Passes result as library_id to add_debit
   - Resolves long-standing FIXME about branch selection
5. misc/cronjobs/longoverdue.pl
   - Determines branch context before marking items lost
   - Passes library_id to LostItem when charging fees

The branch resolution logic respects these system preferences:
- CircControl: PatronLibrary | PickupLibrary | ItemHomeLibrary
- LostChargesControl: PatronLibrary | PickupLibrary | ItemHomeLibrary
- HomeOrHoldingBranch: homebranch | holdingbranch

Test plan:
1. Create three test libraries (e.g., FPL, CPL, PVL)
2. Create a test patron with home library PVL
3. Create an item with:
   - homebranch = FPL
   - holdingbranch = CPL
4. Set system preferences:
   - FinesMode: Calculate and charge
   - Create circulation rule that charges overdue fines
5. Test OVERDUE fees with CircControl:
   a. Set CircControl = PatronLibrary, HomeOrHoldingBranch = homebranch
   b. Check out item to patron at CPL, backdate to be overdue
   c. Run: perl /usr/share/koha/bin/cronjobs/fines.pl
   d. Check accountlines: SELECT accountlines_id, branchcode, debit_type_code
      FROM accountlines ORDER BY accountlines_id DESC LIMIT 5;
   e. Verify branchcode = PVL (patron's home library)
   f. Delete the fine, repeat with CircControl = PickupLibrary
   g. Verify branchcode = CPL (checkout library)
   h. Delete the fine, repeat with CircControl = ItemHomeLibrary
   i. Verify branchcode = FPL (item's home library)
   j. Change HomeOrHoldingBranch = holdingbranch, repeat
   k. Verify branchcode = CPL (item's holding library)

6. Test LOST fees with LostChargesControl:
   a. Set WhenLostChargeReplacementFee = Charge
   b. Set item replacement price > 0
   c. Ensure item is checked out to patron
   d. Set LostChargesControl = PatronLibrary
   e. Mark item as lost in staff interface
   f. Check accountlines for LOST debit
   g. Verify branchcode = PVL (patron's home library)
   h. Return item, re-checkout, repeat with LostChargesControl = PickupLibrary
   i. Verify branchcode = CPL (checkout library)
   j. Repeat with LostChargesControl = ItemHomeLibrary + homebranch
   k. Verify branchcode = FPL (item's home)
   l. Repeat with LostChargesControl = ItemHomeLibrary + holdingbranch
   m. Verify branchcode = CPL (item's holding)
7. Test longoverdue cronjob:
   a. Re-checkout item with due date triggering longoverdue
   b. Run: perl /usr/share/koha/bin/cronjobs/longoverdue.pl --lost 30=1 --charge 1 --confirm --verbose
   c. Verify LOST debit created with correct branchcode per LostChargesControl
8. Test edge cases:
   - WhenLostChargeReplacementFee = Don't charge → no LOST debit
   - Replacement price = 0 → no LOST debit
   - FinesMode = Don't calculate → no OVERDUE debits

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>
2026-02-27 11:38:49 -07:00
Lucas Gass 0a58a77d1d
Bug 36506: DBIC
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2026-02-27 11:38:49 -07:00
Lucas Gass 9fa84c7fd4
Bug 36506: DBRev 25.12.00.026
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2026-02-27 11:38:48 -07:00
Martin Renvoize 08409dd450
Bug 36506: (QA follow-up) Add test for missing circulation rule
This adds a test case to verify that when no lost_item_processing_fee
circulation rule exists for a branch/itemtype combination, the system
correctly defaults to 0 (no processing fee charged).

The test confirms:
- No PROCESSING account line is created when the rule is missing
- The LOST fee is still created normally (only processing fee affected)
- The // 0 fallback in C4::Accounts::chargelostitem works correctly

Test plan:
1. prove t/db_dependent/Accounts.t
2. Verify all tests pass, including the new "missing circulation rule"
   test case

Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
2026-02-27 11:38:48 -07:00
Nick Clemens 88542c7a64
Bug 36506: Unit tests
Sponsored-by: The Main Library Alliance <https://www.mainlib.org/>
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>
2026-02-27 11:38:47 -07:00
Nick Clemens cd0462b4c6
Bug 36506: Adjust existing tests and test builder
All the previously written tests should continue to pass.

Sponsored-by: The Main Library Alliance <https://www.mainlib.org/>
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>
2026-02-27 11:38:47 -07:00
Nick Clemens 2b5f89788d
Bug 36506: Use new rules in C4::Accounts
This seems to be the only place processing fee is used, in chargelostitem

This patch uses get_effective_rule_value, falling back to 0 if no rule.

We also respect the new system preference, LostChargesControl, introduced in bug 39802

To test:
 1 - Apply patch, updatedatabase, restart all
 2 - Set a default/all branches  processing rule for Books itemtype to 5
 3 - Set a rule for branch 1 to 1
 4 - Set a rule for branch 2 to 2
 5 - Set a rule for branch 3 to 3
 6 - Set a rule for branch 4 to 4
 7 - Set LostChargesControl to 'library the patron is from'
 8 - Check out a Books item from a different branch to a patron from branch 1
 9 - Mark item lost, confirm processing fee is charged as 1
10 - Check out out a Books item from a different branch to a patron from branch 5
11 - Mark item lost, confirm processing fee is 5
12 - Set LostChargesControl to 'library you are logged in at'
13 - Check out a Books item from a different branch to a patron from branch 1
14 - Ensure you are logged in at branch 2
15 - Mark item lost, confirm processing fee is 2
16 - Set LostChargesControl to 'the library the item is from'
17 - Set HomeOrHoldingBranch to 'homebranch'
18 - Still at branch 2, check out a books item from branch 3 to a patron of branch 1, this ensure holdingbranch is not 3
19 - Mark item lost, confirm processing fee is 3
20 - Set HomeOrHoldingBranch to 'holdingbranch'
21 - Ensure you are logged in at branch 4
22 - Checkout an item from branch 1 to a patron from a different branch
23 - Mark item lost, confirm processing fee is 4
24 - Test other combos as you see fit
25 - Sign off!

Sponsored-by: The Main Library Alliance <https://www.mainlib.org/>
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>
2026-02-27 11:38:46 -07:00
Nick Clemens f4634ee1c8
Bug 36506: Remove process fee from itemtype administration
Sponsored-by: The Main Library Alliance <https://www.mainlib.org/>
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>
2026-02-27 11:38:45 -07:00
Nick Clemens 02702346dc
Bug 36506: Remove API mapping
Sponsored-by: The Main Library Alliance <https://www.mainlib.org/>
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>
2026-02-27 11:38:45 -07:00
Nick Clemens 5ea97b7dd5
Bug 36506: Database update
Sponsored-by: The Main Library Alliance <https://www.mainlib.org/>
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>
2026-02-27 11:38:44 -07:00