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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
Interestingly this script already had a $surname variable, just never accepted a parameter for it.
This enhancement enables the user to supply a surname parameter when creating a superlibrarian user via the commandline create_superlibrarian.pl script. Koha patrons MUST have a name, so if no surname is provided, the userid will be used for the surname instead.
To test:
1. Apply the patch and restart services
2. Attempt to create a superlibrarian with no surname provided.
In KTD: misc/devel/create_superlibrarian.pl --userid test --cardnumber test --password XXXX --branchcode CPL --categorycode S
3. Search for your patron in Koha. Confirm the surname was automatically set to the userid 'test'
4. Attempt to create a superlibrarian, this time provide a surname that is different from the userid so easy to tell apart.
In KTD: misc/devel/create_superlibrarian.pl --userid test1 --cardnumber test1 --password XXXX --branchcode CPL --categorycode S --surname testsurname
5. Search for your patron in Koha. Confirm the surname was set correctly using the given surname.
Sponsored-by: Catalyst IT
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This patch updates the table cell selector so that the
"previous" class isn't overwritte, and updates the "active" and
"previous" classes so that they use standard Bootstrap colors.
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Some libraries find it useful to have the item that was just
edited/saved visible in the items list
To test:
0 - Apply patch
1 - Edit an item, confirm the itme is highlighted yellow with title
'Currently editing' (Hover to see the title)
2 - Save the item
3 - Confirm item is now highlighted light blue with title 'Previously
edited'
4 - Sign off
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This patch adapts the work on Bug 34778 to the staff client, adding a
"Show password" checkbox to the login form.
To test, apply the batch and go to the staff client login screen.
- You should see a "Show password" checkbox under the password field.
- Type something in the password field. It should be masked (****).
- Click the "Show password" label or check the checkbox.
- The password field should now show the text you entered.
- Confirm that you can log in successfully whether or not the password
is visible.
- `prove t/db_dependent/selenium/authentication.t` should pass.
Sponsored-by: Athens County Public Libraries
Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Signed-off-by: Mikko <mikko.liimatainen@turku.fi>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This patch includes automated stylelint corrections for login.css. It
should have no visible effect.
To test, apply the patch and clear your browser cache. View the staff
interface login form and confirm that everything looks correct.
Sponsored-by: Athens County Public Libraries
Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This patch implements collapsing facets using the Bootstrap Collapse
component.
To test, apply the patch and rebuild the OPAC CSS.
1. Clear your browser cache if necessary.
2. Using the sample data, go to Advanced Search in the OPAC. Check the
"Books" checkbox under the "Item types" tab and submit.
3. In the left-hand sidebar you should see the facets you normally see,
but now each one has an arrow icon to show whether the facet is
expanded or collapsed.
4. All facets should be expanded by default. Adding expanded/collapsed
settings to system preferences is out of scope for this bug.
5. Test expanding and collapsing facets, as well as the "Show more" link
under each one.
6. Click the "Fiction" link under the "Collections" facet. When the
refined search results load you clicked should now have a red "X"
link next to "Fiction," which should no longer be a link.
- Because "Fiction" is the only uption under the "Collections" facet,
the facet is not collapsible. I reasoned that it would be important
not to let a facet limitation disappear behind hidden facets.
* The only way I know to do this in the template is to check for
facets with only one option this has the possibly-unwanted side
effect of making facets which haven't been selected but still only
have one option not collapsible. In this example search "Holding
libraries" and "Topics" show this. I'm open to suggestions on this!
7. Test with the displayFacetCount preference both enabled and disabled.
Sponsored-by: Athens County Public Libraries
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Added the following tags and subfields to the existing note index for Elasticsearch MARC21, as used by the "Notes/Comments" advanced search dropdown option: 501a, 503a, 504a, 508a, 511a, 521a, 538a, 547a
Sponsored-by: Educational Services Australia SCIS
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Thomas Klausner <domm@plix.at>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This patch adds "Serial enumeration / chronology" to the inventory table
output.
Test plan:
0. Apply the patch
1. Go to http://localhost:8081/cgi-bin/koha/tools/inventory.pl
2. Click "Submit"
3. Note that a "Serial enumeration / chronology" column appears in
the results
4. For the top result, go into the bib record and then add a "Serial enumeration /
chronology" to the item in question
5. Re-run the inventory
6. Note that the "Serial enumeration / chronology" now has a value in iy
7. Click "Columns" and hide the column
8. Try again using the CSV export and note that the new column appears again
Signed-off-by: Ludovic Julien <ludovic.julien@inLibro.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
When AlwaysLoadCheckoutsTable is enabled and a delay is configured,
we were calling setTimeout without capturing the returned id, despite a
`loadIssuesTableDelayTimeoutId` variable already being declared and targetted
by clearTimeout. This prevented the "Load now" button from properly clearing
the pending timeout.
Test plan:
1. Configure a delay (LoadCheckoutsTableDelay > 0) so checkouts table
loads after N seconds.
2. Go to a patron’s checkouts page.
3. Before the delay completes, click "Load now".
- Without this patch: an alert window will display a "Something went wrong
when loading the table" error once the timeout triggers.
- With this patch: the pending timeout is cleared; the table loads
immediately only once.
4. Reload the page with the delay set; wait for the timeout to elapse
and confirm the table loads normally.
Signed-off-by: Mike Grgurev <mike.grgurev@mainlib.org>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Test plan:
0. Apply the patch
1. Enable the SelfCheckInModule system preference
2. Go to http://localhost:8080/cgi-bin/koha/sci/sci-main.pl
3. Enter barcode 39999000004571 into input box, click "Submit",
then click "Check in"
4. Using your F12 dev tools, look at the interface with different
widths
5. Note that the page looks good on both large and small screens
6. Note that the page stays contained to the small screen size
and you don't have to scroll horizontally to see all the content
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This change makes the "Scheduled for automatic renewal" text appear
in the staff interface and OPAC only if both the issue has auto_renew
and the borrower has autorenew_checkouts set. These are the criteria that
automatic_renewals.pl uses for processing automatic renewals.
Test plan:
0. Apply the patch
1. koha-plack --restart kohadev
2. Checkout an item as an automatic renewal
(ie either set it as Automatic renewal using the checkout option or have
the circ and fines rules set to use Automatic renewals)
3. View the checkout in both the staff interface and the OPAC
4. Note that they both show "Scheduled for automatic renewal" or
"This item is scheduled for auto renewal" for the OPAC
5. In the patron record change "Allow auto-renewal of items" from
"Yes" to "No"
6. Note that the text talking about the item being scheduled
for automatic renewal no longer displays
Bonus points:
1. Run 'perl ./misc/cronjobs/automatic_renewals.pl -v'
2. Note that the automatic renewal doesn't show when "Allow auto-renewal
of items" is set to "No" but it does show when it's set to "Yes"
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
- Booking::store runs item-level and biblio-level clash
detection unconditionally on every call
- During checkout, C4::Circulation sets the booking status to
'completed' and calls ->store(), which triggers clash checks
- Combined with false positives from Biblio::check_booking
(e.g. checkouts on non-bookable sibling items inflating the
unavailable count), this throws
Koha::Exceptions::Booking::Clash resulting in a 500 error
- Running clash detection on a terminal transition is
unnecessary regardless; skip it for cancelled/completed
- Add _is_final_status_transition() helper that inspects the
dirty columns to detect completed/cancelled transitions
- Guard both clash detection blocks with the new helper
To test:
1. Have a biblio with two items: one bookable, one not
(bookable = 0).
2. Create a booking on the bookable item.
3. Check out the non-bookable item to a patron.
4. Check out the bookable item to the booking patron (this
triggers the completed transition via C4::Circulation).
5. Without the patch: 500 error from
Koha::Exceptions::Booking::Clash.
6. Apply the patch.
7. Repeat steps 2-4.
8. Verify the checkout succeeds and the booking transitions
to completed.
9. Run: prove t/db_dependent/Koha/Booking.t
and confirm the new subtest "store() skips clash
detection on terminal status transition" passes.
Sponsored-by: Büchereizentrale Schleswig-Holstein <https://www.bz-sh.de/>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
- Track pending reset timeouts in a WeakMap keyed by element
- Clear any pending timeout in the pageshow handler before re-enabling
- Prevents the resumed setTimeout from re-disabling the button after
bfcache restoration
Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
- Wrap icons in semantic container elements for visibility toggling
- Create throttled-button.inc with Template Toolkit BLOCKs to reduce duplication
- Extract throttle logic to throttledButton.js with event delegation
- Make timeout configurable via data-throttle-timeout attribute
- Replace string-based modal generation with template element to support
SSR
To test:
1) Follow the test plan from the previous commit
2) Additionally verify the throttled button behavior works in all locations:
- Reports list table: "Run" button for each saved report
- Report toolbar: "Run report" button (when viewing/editing a report)
- Parameters form: "Run the report" button
- Edit SQL form: "Update and run SQL" button
- Preview SQL modal: "Run report" button (click "Preview SQL" in dropdown)
3) For each location, confirm:
- Button shows play icon initially
- Upon click, button becomes disabled with spinner icon
- Button has aria-busy="true" when disabled
- After navigation/submission, button state is handled correctly
4) Test the modal button specifically:
- Open Preview SQL modal from any report dropdown
- Click "Run report" in the modal
- Verify throttle behavior works for this dynamically created button
5) Verify that the set timeout works well, I thought 120 seconds is too
long if you go back via bfcache (back button in the browser). You'd
have to either reload or wait 2 minutes for the button to become
interactive again.
6) Sign off
Signed-off-by: Jake Deery <jake.deery@openfifth.co.uk>
Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This patch will disable the run report button found on both the list page, and the individual operation pages (e.g. run, edit, saved successfully). This will stop librarians from unintentionally crashing their Koha system, by pressing the run report button repeatedly when there is no need to re-request it.
TO TEST:
a) go to /cgi-bin/koha/reports/guided_reports.pl?op=list
b) create three new sql test reports:
*) Name: Test report #1, SQL: SELECT count(*) FROM biblio
*) Name: Test report #2, SQL: SELECT count(*) FROM items
*) Name: Test report #3, SQL: SELECT * FROM items WHERE items.dateaccessioned = <<Date accessioned|date>>
c) run all of the above test reports, noticing how the run report button can be activated repeatedly
APPLY PATCH
d) redo step a
e) run test report #1 from the table, notice how now the button can only be activated once before it is greyed out and the play button is replaced with a spinner
f) return to the list, edit test report #2 from the table, click run report from the edit page, and notice how this run report button exhibits the same behaviour as the button in step e
g) return to the list, run test report #3, select a date, click run report from this page, and notice again how the run report button exhibits the same behaviour as the buttons in steps e and f
SIGN OFF
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
- Biblio::check_booking counts current checkouts toward the
unavailable pool to detect booking clashes
- It counts checkouts on ALL items for the biblio, including
non-bookable ones (e.g. items with an excluded item type)
- When only a subset of items are bookable, checkouts on
non-bookable siblings incorrectly reduce the available pool,
producing false clashes
- Restrict the checkout query to only count items whose
itemnumber is in the bookable items set
To test:
1. Have a biblio with at least two items, where one item is
bookable and the other is not (bookable = 0).
2. Check out the non-bookable item to a patron.
3. Attempt to create a booking for the bookable item.
4. Without the patch: a clash is detected even though the
bookable item is available.
5. Apply the patch.
6. Repeat steps 2-3.
7. Verify the booking succeeds without a false clash.
8. Run: prove t/db_dependent/Koha/Biblio.t
and confirm the new subtest
"checkouts on non-bookable items do not cause false clashes"
passes.
Sponsored-by: Büchereizentrale Schleswig-Holstein <https://www.bz-sh.de/>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
We say that you cannot limit transfers internally within a branch, but
it seems we do allow you to, and don't show you when we do
To test:
1 - Go to Admin->Library transfer limits
2 - On a branches page, uncheck the box for that branch
e.g. don't allow FIC to transfer to CPL from CPL
3 - Save
4 - Note the box is still checked
5 - Check the DB
SELECT * FROM branch_transfer_limits;
6 - There is a line to/from CPL!
7 - Apply patches
8 - updatedatabase
9 - Repeat 5 - the line is deleted!
10 - Try to create the rule again, not the box is now disabled
11 - Be sneaky, enable the box, uncheck it and save, no rule created
12 - Be sneaky, edit the HMTL for another button to make it limiting CPL to CPL, save, no rule created
13 - Sign off
Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This patch changes the incorrect usage of output_error
in acquisitions_stats.pl so that it uses a HTTP status
code instead of an arbitrary string.
Test plan:
0. Apply the patch and koha-plack --restart kohadev
1. Go to http://localhost:8081/cgi-bin/koha/reports/acquisitions_stats.pl
2. Use your F12 HTML inspector to find the "Line" radio buttons that you've
selected and remove the values for those elements
3. Click "Submit" at the bottom of the screen
4. Note that you get a 403 error (if bug 30803 applied - otherwise it's a 404 error)
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This patch removes the hardcoded "404 Not Found" status
code and reason phrase from output_error and instead
uses just the code passed into the function.
Test Plan:
0. Apply patch and koha-plack --restart kohadev
1. Go to http://localhost:8080/cgi-bin/koha/tracklinks.pl?uri=https%3A%2F%2Ftest.com&biblionumber=29
2. Note normal OPAC 404 error
Bonus points:
1. Change 404 to 403 in the first output_error in opac/tracklinks.pl
2. koha-plack --restart kohadev
3. Go to http://localhost:8080/cgi-bin/koha/tracklinks.pl?uri=https%3A%2F%2Ftest.com&biblionumber=29
4. Note normal OPAC 403 error
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
After introducing Bug 40966 opac-search.pl, when invoked in the
simple form (not as advanced search) generates warnings like:
[WARN] Odd number of elements in anonymous hash at /kohadevbox/koha/opac/opac-search.pl line 529.
Test plan:
==========
1. Perform a simple search from main OPAC page.
2. Check the content of plack-opac-error.log. You should see a
warning like:
[WARN] Odd number of elements in anonymous hash at /kohadevbox/koha/opac/opac-search.pl line 529.
3. Apply the patch ; restart_all.
4. Repeat p. 1. There should be no new warning now.
Sponsored-by: Ignatianum University in Cracow
Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
To test:
0. APPLY PATCH, restart_all
1. Create an accountline and make note of the accountline.date
2. On that accoutnline edit the note.
3. Confirm that the accountline.date does NOT update.
Signed-off-by: Juliet Heltibridle <jheltibridle@rcplib.org>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
When creating a new Koha instance using the two-step workflow
(`koha-create --request-db <instance>`, followed by
`koha-create --populate-db <instance>`), some log files under
/var/log/koha/<instance>/ end up with incorrect ownership.
Specifically, intranet-error.log and opac-error.log are owned
by root:root instead of <instance>-koha:<instance>-koha.
This patch fixes that.
Test plan (tailored for KTD):
1) Before applying this patch, run the following commands
inside your KTD Koha container:
$ sudo koha-create --request-db test1
$ SQLPASS=`sudo grep password: test1-db-request.txt | cut -c 16-`
$ mysql -hdb -uroot -ppassword -e "CREATE DATABASE koha_test1;"
$ mysql -hdb -uroot -ppassword -e "GRANT ALL PRIVILEGES ON koha_test1.* TO 'koha_test1'@'%' IDENTIFIED BY '${SQLPASS}';"
$ mysql -hdb -uroot -ppassword -e "FLUSH PRIVILEGES;"
$ sudo koha-create --populate-db test1
$ ls -l /var/log/koha/test1/
total 8
-rw-r--r-- 1 test1-koha test1-koha 0 Feb 9 15:35 indexer-error.log
-rw-r--r-- 1 test1-koha test1-koha 1278 Feb 9 15:35 indexer-output.log
-rw-r--r-- 1 root root 0 Feb 9 15:35 intranet-error.log
-rw-r--r-- 1 root root 0 Feb 9 15:35 opac-error.log
-rw-r--r-- 1 test1-koha test1-koha 0 Feb 9 15:35 worker-error.log
-rw-r--r-- 1 test1-koha test1-koha 1828 Feb 9 15:35 worker-output.log
-rw-r--r-- 1 test1-koha test1-koha 0 Feb 9 15:35 zebra-error.log
-rw-r--r-- 1 test1-koha test1-koha 0 Feb 9 15:35 zebra-output.log
Notice the incorrect user/group ownership on two
files: `intranet-error.log` and `opac-error.log`.
2) Apply this patch, remove the `test1` instance along
with the database request text file, and copy the
modified script to its system-wide location:
$ sudo koha-remove test1
$ sudo rm test1-db-request.txt
$ sudo cp debian/scripts/koha-create /usr/sbin/koha-create
3) Repeat the commands in Step 1. This time, the output of
the last command should resemble the following:
total 8
-rw-r--r-- 1 test1-koha test1-koha 0 Feb 9 15:42 indexer-error.log
-rw-r--r-- 1 test1-koha test1-koha 1520 Feb 9 15:42 indexer-output.log
-rw-r--r-- 1 test1-koha test1-koha 0 Feb 9 15:42 intranet-error.log
-rw-r--r-- 1 test1-koha test1-koha 0 Feb 9 15:42 opac-error.log
-rw-r--r-- 1 test1-koha test1-koha 0 Feb 9 15:42 worker-error.log
-rw-r--r-- 1 test1-koha test1-koha 1828 Feb 9 15:42 worker-output.log
-rw-r--r-- 1 test1-koha test1-koha 0 Feb 9 15:42 zebra-error.log
-rw-r--r-- 1 test1-koha test1-koha 0 Feb 9 15:42 zebra-output.log
All files should now have the correct user/group ownership.
Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Signed-off-by: Jan Kissig <bibliothek@th-wildau.de>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
To test:
1. prove t/db_dependent/Koha/ImportBatch.t
2. Observe success
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>
Jake Deery
Martin Renvoize
Laura_Escamilla
Lucas Gass
Nick Clemens
Aleisha Amohia
Owen Leonard
Wainui Witika-Park
David Cook
Tomás Cohen Arazi
grgurmg
Paul Derscheid
Jonathan Druart
Janusz Kaczmarek
Andreas Roussos
John Doe
Lari Taskula