When using strict_sql_modes, the Hold ratios page causes an Incorrect
GROUP BY error and returns an Error 500.
This patch removes uncessary columns from the database query and adds
the biblio columns to the GROUP BY statement, so they can be selected.
Since it's already grouped by biblionumber, it should not cause any
additional rows to be created.
Test plan:
1. Make sure that strict_sql_modes is enabled in the koha-conf.xml
file.
2. Place at least one hold on a book.
3. Go to Circulation and select Hold ratios.
4. Observe that you get an Error 500.
5. Apply patch.
6. Repeat step 3.
7. Observe that you can view the page.
8. If the table is empty, change the ratio to 1.
9. Sign off.
Sponsored-by: Lund University Library
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit ea4600476e)
Signed-off-by: Jacob O'Mara <Jacob.omara@openfifth.co.uk>
Signed-off-by: Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz>
The warning
[WARN] Use of uninitialized value in string eq at
/kohadevbox/koha/circ/del_message.pl line 54.
appears in plack-intranet-error.log
Test plan:
==========
1. Observe plack-intranet-error.log
2. Search for any patron and go to "Check out" tab.
3. Add message for patron. Added message is shown in "Message" area
with "Edit" and "Delete" links.
4. Click "Delete" and confirm modal.
5. Warning appears in plack-intranet-error.log
6. Apply the patch; restart_all
7. Repeat 3-4. No warning appears.
Sponsored-by: Ignatianum University in Cracow
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com>
When changing library in circ/set-library.pl, desk and register values
were not being properly cleared from the session. This caused stale
desk/register data from the previous library to persist, resulting in:
- Incorrect cashup displays showing wrong library's register
- POS defaulting to registers from different libraries
- Session state inconsistent with visual UI
Three issues fixed:
1. Template: Changed register "None" option ID from "set-library-noregister"
to "noregister" to match JavaScript expectations (consistent with desk handling)
2. Desk logic: Changed condition to use defined() and explicitly clear session
when "No desk" is selected
3. Register logic: Added session clearing when "No register" is selected and
fixed condition to handle empty initial state
Test plan:
1. Select Library A with Register 5
2. Change to Library B without selecting a register
3. Verify register is cleared from session (check POS page)
4. Repeat for desk selection
5. Verify explicit "-- None --" selection clears session values
Signed-off-by: Jason Robb <jrobb@sekls.org>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 92d6fcf2bd)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
This patch does two things:
1:
It prevents rebulding the real time holds queue during a checkin if a reserve was found.
The reserve will either:
a - be set to waiting if at correct location and no confirmation - this is fine because we will rebuild the queue when
filling the hold
b - trigger a popup whihc will rebuild the queue when confirming the transfer, or be ignored
2:
It adds a form to ignoring a hold so that we can rebuild the holds queue if we decided not to use the current item
to fill the found hold
To test:
0 - Enable RealTimeHoldsQueue
1 - Place several holds for different libraries on a title
2 - Checkout a copy to a patron at current library (ignore current holds)
3 - Clean up DB so we can see outcomes from next steps:
delete from branchtransfers; delete from background_jobs; update reserves SET found=NULL where found='T';
4 - Check in item, confirm hold
5 - Check background jobs, note two rebuilds of the holds queue
6 - Check item out to previous patron, repeat 3
7 - Check in item, ignore the hold
8 - Check background jobs, note only one rebuild
9 - Apply patch, restart all
10 - Checkout item, repeat 3
11 - Checkin item, confirm hold
12 - Check background jobs, onlt one rebuild of holds queue
13 - Checkout item, repeat 3
14 - Checkin item, ignore hold
15 - Check background jobs, only one rebuild of holds queue
Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit f9d096df27)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
This patch removes the duplicate logging code from circ/returns.pl that
was part of the original Bug 9762 implementation.
The original implementation added a separate logaction() call when
ignoring holds on return. This code created a custom JSON log entry
with the message "return an element that is reserved".
However, this logging appears to have been orphaned - there is no
corresponding Alternative implementation for return override logging
in the current codebase. The code was creating log entries but they
were not properly integrated with any override confirmation workflow.
Changes:
circ/returns.pl:
- Removed entire else block that created custom override log entries
- Removed unused variables: $message, @message, $user, $branchcode, $barcode
- Removed C4::Log qw(logaction) import as it's no longer needed
This eliminates the orphaned logging code and ensures returns.pl only
uses standard Koha logging mechanisms.
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 9ce4533b75)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
This patch removes the duplicate logging code from renewal scripts
(circ/renew.pl and svc/renew) that was part of the original Bug 9762
implementation.
The original implementation created separate logaction() calls with
custom JSON messages whenever renewal overrides occurred. This caused
double logging alongside the Alternative implementation which properly
logs overrides via the confirmations parameter in AddRenewal.
Changes:
circ/renew.pl:
- Removed unused variables: $user, $branchcode, @message
- Removed separate logaction() calls for override logging
- Changed to use confirmations array with proper codes:
* 'RENEWAL_LIMIT' for override_limit
* 'ON_RESERVE' for override_holds
- Removed C4::Log qw(logaction) import
svc/renew:
- Removed unused variables: $borrower, $user, $message, @message
- Removed separate logaction() calls for override logging
- Changed to use confirmations array with proper codes:
* 'ON_RESERVE' for on_reserve override
* 'RENEWAL_LIMIT' for too_many override
- Removed C4::Log qw(logaction) import
The Alternative implementation in AddRenewal will now handle all
override logging consistently through the confirmations parameter.
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit ca47310a3b)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
This patch removes the duplicate logging code from the original
Bug 9762 implementation that was not properly removed when the
Alternative implementation was merged.
The original implementation added separate logaction() calls in
CanBookBeIssued and circulation.pl that created additional log
entries with a different JSON format. This caused double logging
when the Alternative implementation (which logs via AddIssue) was
merged on top.
Changes in this patch:
C4/Circulation.pm:
- Removed @message_log array and all pushes to it
- Removed the separate logaction() call at the end of CanBookBeIssued
- Removed 5th return value (@message_log) from CanBookBeIssued
- Removed all if ($issueconfirmed) blocks that were populating @message_log
circ/circulation.pl:
- Removed the problematic line that added OVERRIDDEN to every confirmed
checkout: $needsconfirmation->{'OVERRIDDEN'} = $issueconfirmed
This addresses the double logging issue where circulation overrides
were being logged twice with different formats.
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 2fea44ff25)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
The $message variable was declared in circ/circulation.pl but never
actually set, making the "No patron matched" template block unreachable.
This is a remnant from old code that was incompletely cleaned when that
functionality was changed/removed.
This patch removes the unused variable, template parameter, and the
unreachable template block.
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit de962e15cf)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
- The message 'No patron matched Restriction overridden temporarily' was appearing on every checkout, even when no restriction override occurred.
- This was caused by the message being set unconditionally after AddIssue in circ/circulation.pl.
- The message should not appear in circulation.pl, but in the Log Viewer.
To test:
1. Check out an item to a patron normally.
2. 'Restriction overridden temporarily' message appears.
3. Apply patches
4. Repeat 1.
---> Verify that no 'Restriction overridden temporarily' message appears.
5. Check out an item that requires an override (e.g., not for loan item with AllowNotForLoanOverride enabled).
---> Verify that no 'Restriction overridden temporarily' message appears.
6. Go to Tool -> Log viewer.
7. Enter the borrowernumber in Object field and submit.
---> Verify that the 'Restriction overridden temporarily' message is logged correctly.
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 3c41dfbe3f)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
1. Create several random patron circ messages with different dates. I used the following sql for my ktd:
-- INSERT INTO messages (borrowernumber, branchcode, message_type, message, message_date)
SELECT
51 AS borrowernumber,
'CPL' AS branchcode,
ELT(FLOOR(1 + (RAND() * 3)), 'L','B','E') AS message_type, -- L=Staff note, B=OPAC, E=Email (adjust to your codes)
CONCAT('Random test message ', FLOOR(RAND()*1000)) AS message,
DATE_ADD(CURDATE(), INTERVAL -FLOOR(RAND() * 1825) DAY) AS message_date -- random day in last ~5 years (5*365)
FROM (
SELECT 1 UNION ALL SELECT 2 UNION ALL SELECT 3 UNION ALL SELECT 4 UNION ALL SELECT 5
UNION ALL SELECT 6 UNION ALL SELECT 7 UNION ALL SELECT 8 UNION ALL SELECT 9 UNION ALL SELECT 10
) AS x;
2. View the patron account and notice that messages are sorted by type, but not by date.
3. Apply the patch and restart_all
4. Refresh the patron account page and clear the browser cache if necessary.
5. View the patron account again and notice that the messages are still sorted by type, but they are also sorted in descending order (newest to oldest).
6. Sign off and have a great day!
Signed-off-by: Jen Tormey <jntormey@dmpl.org>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 7be401fcbd)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
When implemented, the message here was believed to either be about Duplicate IDs, or guarantor issues.
We are now seeing a wider variety of issues possible from SQL errors, lock timeout or missings rows as examples
This patch adds an 'ErrorMessage' message to the return from AddReturn and displays it to the user.
To test:
1 - Issue an item to a patron, note the itemnumber
2 - On the command line, lock the row:
sudo koha-mysql kohadev
START TRANSACTION;
SELECT * FROM issues WHERE itemnumber=975 FOR UPDATE;
leave mysql open at this point
3 - On the interface, check in the item, wait a bit
4 - After a bit you get the error to check the config page, but nothing is there
5 - close mysql, apply patch, restart all
6 - Repeat 2
7 - Check in the item, wait a bit
8 - Note you now get the error from the DB:
Error: (DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st execute failed: Lock wait timeout exceeded; try restarting transaction at /kohadevbox/koha/Koha/Object.pm line 174 )
9 - Sign off!
Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 3f61f6088f)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 489780f3f9)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 255c0897f2)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
An alternative approach to the original submission for bug 9762. Instead
of recording a distinct action log line for a set of override messages
in addition to the standard issue record, this patch updates the
existing issue log to include any confirmation or impossible responses
from the preceeding CanBookBeIssued call.
We continue to record these as a JSON structure as the prior patch did,
but instead of human messages we use the predefined codes as returned
from the existing CanBookBeIssued response.
To test:
After each step, a single action log message should be displayed under
`Tools > Log viewer`. The message itself will now consist of a JSON
string with an array of 'confirmations' and 'forced' strings
representing the overriden feedback from CanBookBeIssued.
Circulation -- Checking out
1) Check out an item that is age-restricted (AgeRestrictionOverride syspref)
2) Check out an item that has a “not for loan” status (AllowNotForLoanOverride
syspref)
3) Check out an item that has a “lost” status (IssueLostItem syspref)
4) Check out an item to a patron who has reached the checkout limit
(AllowTooManyOverride syspref)
5) Check out an item to a patron who is not allowed to borrow this item type
6) Check out an item to a patron who has unpaid fines (AllFinesNeedOverride
and/or AllowFineOverride + noissuescharge sysprefs)
7) Check out an item on hold for someone else
8) Check out an item on hold ready for pickup by someone else
9) Check out an item already checked out to someone else
(AutoReturnCheckedOutItems syspref)
10) Check out to a patron who has restrictions
Circulation -- Checking in
11) Ignore a hold upon check in
12) Ignore a hold and transfer upon check in
Circulation – Renewing
13) Renew an item on hold for someone else
(AllowRenewalOnHoldOverride syspref)
14) Renew an item that has reached the maximum number of renewals
(AllowRenewalLimitOverride syspref)
Holds
15) Place a hold for a patron who is not allowed to place this item
type on hold (AllowHoldPolicyOverride syspref)
16) Place a hold for a patron who has reached the maximum number of
holds
17) Place an item-level hold when rules dictate that this is not
allowed
Sponsored-by: OpenFifth <https://openfifth.co.uk/>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 5350a9276e)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
We never actually pass `$message` back to anywhere so there's no reason
to create it in the first place.
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 86f4eb93aa)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 9183fca07d)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
This patch allows you to save commands that have been overridden by someone.
The patch has been updated but is not yet completed. Tests 10, 13, 14, and 17 are still not functional. Tests 15 and 16 need to be fixed as they do not display the barcode. All other tests should be functional.
To test:
--After each step, a message will be displayed in the action_logs table and in Tools > Log viewer --
Circulation -- Checking out
1) Check out an item that is age-restricted (AgeRestrictionOverride syspref)
2) Check out an item that has a “not for loan” status (AllowNotForLoanOverride syspref)
3) Check out an item that has a “lost” status (IssueLostItem syspref)
4) Check out an item to a patron who has reached the checkout limit (AllowTooManyOverride syspref)
5) Check out an item to a patron who is not allowed to borrow this item type
6) Check out an item to a patron who has unpaid fines (AllFinesNeedOverride and/or AllowFineOverride + noissuescharge sysprefs)
7) Check out an item on hold for someone else
8) Check out an item on hold ready for pickup by someone else
9) Check out an item already checked out to someone else (AutoReturnCheckedOutItems syspref)
10) Check out to a patron who has restrictions
Circulation -- Checking in
11) Ignore a hold upon check in
12) Ignore a hold and transfer upon check in
Circulation – Renewing
13) Renew an item on hold for someone else (AllowRenewalOnHoldOverride syspref)
14) Renew an item that has reached the maximum number of renewals (AllowRenewalLimitOverride syspref)
Holds
15) Place a hold for a patron who is not allowed to place this item type on hold (AllowHoldPolicyOverride syspref)
16) Place a hold for a patron who has reached the maximum number of holds
17) Place an item-level hold when rules dictate that this is not allowed (In circulation rules put "On shelf holds allowed" to "If any unavailable")
Sponsored-by: Solutions inLibro inc. <https://inlibro.com/>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 89811768b6)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: David Flater <flaterdavid@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 6cfac72f7e)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
When displaying messages on a patron account, group the messages by
type and label which are OPAC messages and which are internal staff
messages.
To test:
1. Find a patron and add several messages to their account, alternating
between internal staff messages and OPAC messages
--> Note that the messages display in the order that they are added and
not explicitly labeled
2. Apply patch and refresh page
--> Confirm that all staff messages display first and all OPAC messages
display underneath
--> Confirm that messages are now labeled "Staff note" or "OPAC"
3. Play around on the patron account and confirm that the message
display is consistent after taking different actions:
- On the Details page
- On the Check out page
- After adding a new staff message
- After adding a new OPAC message
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Laura_Escamilla <laura.escamilla@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit f30bed0d79)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 007d66e314)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
To test:
1 - Create a patron in a category that can be a guarantee and do not assign any guarantor
2 - Create a patron in a 'Child' account type and do not assign a guarantor
3 - Enable system preference 'ChildNeedsGuarantor'
4 - View 'Check out' and 'Details' tabs for both patrons, note there is no warning
5 - Apply patch
6 - Repeat 4, note warning on all pages
7 - Add a patron gaurantor on the child - repeat 4 - warning is gone
8 - Add non-patron guarantor information to other patron - repeat 4 - warning is gone
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>
(cherry picked from commit 2025ed7fdc)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
This patch adds a catch during circulatoin actions and passes a message back to the caller
For checkouts and renewals we don't have a method for passing messages so we simply catch the exception
and warn - this seems reasonable, as we have now added a warning on the patron display that they are missing
a guarantor
To test:
1 - Create a Child type patron with no guarantor
2 - Create a patron in a non-child category that can_be_guarantee with no guarantor
3 - Check out an item to each patron above
4 - Enable system preference 'ChildNeedsGuarantor' and TrackLastPatronActivityTriggers - select all
5 - In mysql
UPDATE borrowers SET datelastseen = '2025-01-01';
6 - Restart all to clear cache (so update last seen will update)
7 - Attempt to checkout a book to either patron - 500 error
8 - Attempt to returnt he books the patrons have checked out - 500 error
9 - Attempt to renew their items - 500 error
10 - Apply patches, restart all
11 - Repeat checkout, renewal, checkin
12 - On checkout and renewal there should be a warning on the patron page, but actions succeed with no additional problems
13 - On return you get a warning letting you know the action succeeded but there is still a problem to resolve
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>
(cherry picked from commit bd32425271)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
The first motivation of this patch is to remove the following warnings
from t/db_dependent/Auth.t:
Use of uninitialized value $anonymous_patron in string eq at /kohadevbox/koha/C4/Auth.pm line 1497
Actually a correct fix seems to add a new `is_anonymous` method to
Koha::Patron in order to remove the warnings from everywhere else.
Test plan:
Confirm that you cannot log in using the anonymous patron and that the 2
tests are passing
prove t/db_dependent/Auth.t t/db_dependent/Koha/Patron.t
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>
(cherry picked from commit e49602a87e)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
In order to remove false positives with the QA script we should replace some tabs with spaces.
There are more but I wanted to make sure no regression would be
introduced
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>
(cherry picked from commit 492e6814a1)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
To test:
1. Put a few items on hold and make note of the barcodes.
2. Check out one of the items to a patron who doesn't have the hold
3. Check the cancel hold radio button
4. Also check the Remember for the session for this patron
5. Click 'Yes, check out'
6. Notice the item is checked out and the hold is cancelled
7. No checkout another on hold item to the same patron
8. No confirm screen ( good )
9. The item is checked out ( good )
10. The hold is not cancelled. ( bad )
11. APPLY PATCH and restart all
12. Try steps 1 - 9 again. This time the 2nd hold should be cancelled.
Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Update the patch to use Koha::Account and it's associated 'outstanding'
methods to ensure we accurately portray outstanding amounts rather than
'all credits' totals.
I also remove the unused `$balance` variable from the
members/moremember.pl controller.
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
To test:
1: Have patrons with a variety of credit/debit combinations
2: Confirm Koha shows the total debit amount in patron_messages if any debits exist
3: Confirm Koha does not show anything about creditd in patron_messages if any debits exist
4: Apply patch, restart_all
5: Confirm all patrons show both debit total and credit total
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
To test:
1. Place multiple holds, good to have at least 4, for pickup at a location, I used Nick's handy script
2. Perl misc/cronjobs/holds/build_holds_queue.pl
3. Check in at least 4 items for pickup at one location so they'll be wating
4. Update the expiration date on those holds using ktd --dbshell
update reserves set expirationdate = DATE('2024-01-01') where branchcode='CPL';
5. On Circulation > Holds waiting past their expiration date tab use the cancel button that is in the actions column to cancel a hold
The page loads on the Holds waiting tab, although in the url you see /circ/waitingreserves.pl?tab=holdsover
6. APPLY PATCH
7. Try 1 - 5 again, this time after step 5 you should be redirected.
8. Try cancelling holds with a transfer, ensuring you get redirected correctly.
Signed-off-by: Andrew Auld <andrew.auld@openfifth.co.uk>
Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
because of automatic changes in circ/transferstoreceive.tt
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
This patch ignores limits when updating a wrong transfer to ensure we can return a book
home when checked in at the wrong branch
To test:
1 - Enable UseBranchTransferLimits based on 'Collection code'
2 - Disable transfers for FIC from:
CPL -> FFL
CPL -> IPT
FFL -> CPL
FFL -> IPT
IPT -> CPL
IPT -> FFL
3 - Set library to IPT
4 - Checkin an FFL item
5 - It generates a transfer home
6 - Set library to CPL
7 - Check the item in
8 - 500 error, and in logs:
==> /var/log/koha/kohadev/plack-error.log <==
Exception 'Koha::Exceptions::Item::Transfer::Limit' thrown 'Transfer not allowed'
9 - Apply patch, restart all
10 - Check item in again
11 - Success! Transfer updated
Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
This patch adds information about whether an entry in the overdue report
is overdue so we can style dates accordingly. We can't assume that each
result is overdue because the report can be changed to include all
checked out items.
To test, apply the patch and go to Circulation.
- Check out to a patron.
- Use the "Specify due date" option to check out some items with due
dates in the past, due dates today, and due dates in the future.
- Go to Circulation -> Overdues.
- You should see the items which have due dates in the past. The date in
the "Due date" column should be red text. The "Checked out on" date
should be regular text.
- In the sidebar, check the "Show any items currently checked out"
checkbox and submit the form.
- You should now see all the items you just checked out. Only the
overdue items should show their due date in red.
Sponsored-by: Athens County Public Libraries
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Checkins can disappear from the checkin list if transfer modal is
triggered.
There is a flaw in the logic of the checkin code: we keep 20 (default
value for numReturnedItemsToShow) checkins in the table, but there can
be shift in the "counter" logic when the transfer modal is triggered.
This patch is removing the different hashes and use a single @checkins
array to pass to the template.
When a item is checked in, it is inserted at the beginning of the list
(unshift). Koha::Objects are passed for patron, item and biblio to
improve the readability (and maintenance cost) of the code.
Test plan:
Generate several checkouts (see below)
Some should generate the transfer modal
Check them in
=> Without this patch notice that at some point there will be a problem
in the list, some checkins will be missing.
=> With this patch applied the checkin list always contain the last 20
checkins (or whatever is the value of numReturnedItemsToShow, or "8"
if pref is empty, don't ask why 8!)
Code to generate 50 checkouts (all overdue):
```
use C4::Circulation qw( AddIssue );
use Koha::Items;
use Koha::Patrons;
use t::lib::Mocks;
use Koha::DateUtils qw(dt_from_string);
my $patron = Koha::Patrons->find(51);
t::lib::Mocks::mock_userenv({patron => $patron});
my $items = Koha::Items->search;
my $patron_unblessed = $patron->unblessed;
my $i;
while ( my $item = $items->next ) {
my $due = dt_from_string->subtract(days => 45 + rand(45))->set_hour(23)->set_minute(59)->set_second(00);
AddIssue($patron, $item->barcode, $due);
last if ++$i >= 50;
}
```
Then retrieve the barcodes:
> SELECT items.barcode FROM issues LEFT JOIN items ON items.itemnumber=issues.itemnumber;
QA:
1. Do not be afraid by the length of this patch, most of the changes are
variable renames.
2. return_overdue is now only calculated once per checkin (per page
load, could be improved)
3. date formatting is done template-side
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
To test:
1. Set up sysprefs:
- Set any non-empty value for ClaimReturnedLostValue
- Set AutoClaimReturnStatusOnCheckout to "found in library"
- Set AutoClaimReturnStatusOnCheckin to "returned by patron"
2. Check out an item to a patron
3. On the patron's account page, click the "Claim Return" button next to
that checkout
4. Check the item in. Note the message that the claim was auto-resolved.
5. Return to the patron's account page and open the Claims Returned tab
--> Note that the claim is resolved and hidden
6. Click the "Show all 1 claims" link to view the hidden claim. Make a
note of the resolution code and timestamp.
7. Check the item out again
--> Note the message saying that the claim was resolved, even though it
was already resolved previously
8. View the return claim again
--> Note that the timestamp and resolution code have changed
9. Apply patch and restart_all
10. Repeat steps 2-8 with a new item
--> Note you do not get a claim resolved message on checkout this time
--> Note that the timestamp and resolution code remain the same before
and after the checkout
Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
To test:
1. Create a new notice template for a custom patron message
i. Go to Tools > Notices and Slips
ii. Create a new notice and choose Patrons (custom message) module
iii. Add the following text to the email notice:
You have a new notification from [% branches.branchname %]
iv. Fill in other fields and save
2. Find a patron and go to their details page
3. Send the custom notice you just created
i. Click Add message
ii. Under "add a message for", select the email option
iii. Under "Patron notice", select the notice you just created
iv. Click "Save"
4. Open the patron's notices tab and look at the notice that was sent
--> The branch name is left blank
5. Apply patch and refresh the patron page
6. Repeat steps 3-4
--> The branch code is now included in the notice
Signed-off-by: Janusz Kaczmarek <januszop@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
The FIXME correctly stated that the CheckReserves call and follow-up
only should be executed within the cancel hold scope.
NOTE: Actually, I do not see how to trigger it in current code. Since
checking in a waiting hold triggers the NotIssued error. And the
cancel hold button is not available for holds in queue.
Test plan:
Place a hold on an item.
Checkin at pickup location. Confirm. Hold waiting now?
Try again but checkin at another branch. Transfer triggered?
The hard part (see my note). Try to get at the form where you can
cancel the hold (waiting or first in queue).
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Test plan:
Enable future holds. Put a positive value in ConfirmFutureHolds.
Add a hold for tomorrow and pickup at B. Check item in at A.
Confirm hold.
Without this patch you should see a warn on L186:
[WARN] Use of uninitialized value in string ne at /usr/share/koha/circ/returns.pl line 186.
With this patch, you should not see it anymore.
NOTE: Before bug 34972 no transfer was created for future holds.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
This fixes an internal server error when renewing an expired
child patron without a guarantor, when the ChildNeedsGuarantor
is set to "must have".
Test plan:
1. Create or edit a child patron:
- Don't add a guarantor.
- Set the "Expiry date" to a date in the past.
2. Set the ChildNeedsGuarantor system preference to "must have".
3. Refresh or view the patron's check out or details page, and
select the "Renew" link under the attention heading -
this generates an internal server error message.
4. Apply patch.
5. Repeat step 3, you will now get a standard error message
"This patron could not be renewed: they need a guarantor."
6. Add a guarantor for the patron.
7. Repeat step 3, and click "Renew" - the patron is now renewed.
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
This commit is generated using:
% perl misc/devel/tidy.pl
*within* ktd, to get the same version of perltidy than what will be used
by our CI (currently v20230309).
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
To test:
- Apply the patch and enable the CurbsidePickup system preference if
necessary.
- Properly testing requires testing data. You can run this command to
REPLACE your curbside pickup data with sample data:
bash <(curl -s https://gitlab.com/-/snippets/2572579/raw/main/test_curbside_pickups.sh)
- Go to Circulation -> Curbside pickups.
- Confirm that the tabs look correct. The first tab should be active
by default.
- Confirm that the right content is displayed when you click each tab.
- Under the "Schedule pickup" tab, search for a patron.
- After you submit the search the page should reload and the "Schedule
pickup" tab should be open.
- Go on each tab and try each button to verify you're staying on the
same tab.
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
The call expects a library object, not a branchcode, likely rebasing issue
To test:
1 - Place a hold for an item at a different library than current library
2 - Check item in and confirm transfer
3 - Check item in again
4 - Kaboom!
5 - Apply patch/restart_all
6 - Check item in again
7 - Ok!
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
With a over-sufficient number of items, when more than one patron has
placed hold, and the holds have been partially filled (checked-in =
waiting for pick up), the number of items to pull in the Holds
to pull table shows the total number of holds, including those waiting.
This erroneously suggests to the librarian to pull an excessive number
of items from the shelves.
Test plan:
==========
1. For a bibliographic record with more than two items (in ktd, e.g.
"Lanark a life in four books"), place hold for two patrons.
2. On the Holds to pull page control that there are two items to pull.
3. As a librarian from the library of one of the patrons, Check-in one
item.
4. Note that in Holds to pull table you still see two items to pull,
which is misleading.
5. Apply the patch; restart_all.
6. Now you should see only one item to be pulled.
Sponsored-by: Ignatianum University in Cracow
Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
To test:
0. In the Circulation Rules, set the default return policy to "item
returns home (default settings have this already)
1. Find an item belonging to a branch other than the logged-in branch
2. Place a hold on that biblio record for pickup at the logged-in branch
3. Check in the item to set the hold to waiting
4. Set the expiration date to a date in the past
To do this in KTD:
ktd --shell
koha-mysql kohadev
UPDATE reserves SET expirationdate = < yesterday's date >;
5. Set a lost status on the item
6. Go to Circulation > Holds awaiting pickup
--> The hold should appear on the "holds waiting past their expiration
date" tab
7. Click the "Cancel and return to <homebranch>" button next to the hold
8. Open the biblio record for the item
--> Note that the lost status is gone and the item shows as in-transit
9. Apply patch
10. Repeat steps 2-8 on the same item
--> This time, the item is still lost and is not in-transit
Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
The system is vulnerable to Formula Injection attacks as the data
stored within the database and exported as CSV/Excel is not being
sanitized or validated against implanted formula payloads
This patch modifies all uses of Text::CSV and derived classes to pass
the "formula" parameter with value of "empty" which replaces formulas
by empty string.
Test Plan:
1) Apply this patch
2) For guided_reports.pl, attempt to export CSV where you've set a column to a formula somehow
( such as "=1+3" )
3) Export that CSV file
4) Note the formula has not been exported
5) Repeat this plan for the remaining scripts that export CSV files
where users can define the outputted data
Signed-off-by: Magnus Enger <magnus@libriotech.no>
Fixed two conflicts. I have tested that this works as advertised on:
- Reports (Download > Comma separated text (.csv)) [Text::CSV::Encoded]
- Circulation > Overdues > Download file of all overdues [Text::CSV_XS]
- misc/export_borrowers.pl [Text::CSV]
This covers all modules used, and both GUI and command line.
Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
[EDIT] Change none to empty in the commit message ! None is the default,
doing nothing. Empty clears the formulas.
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
[WARN] Use of uninitialized value $op in string eq at /usr/share/koha/intranet/cgi-bin/circ/circulation.pl line 144.
[WARN] Use of uninitialized value $op in string eq at /usr/share/koha/intranet/cgi-bin/circ/returns.pl line 253.
These happen in cases when $op is undef and circulation.pl or returns.pl tries to directly compare it to a string.
To reproduce:
1. Go to "Check Out" and "Check In" circulation pages, or refresh already opened one.
2. Check your logs for "Use of uninitialized value $op" warning.
3. Apply patch.
4. Repeat step 1 and check that no new "Use of uninitialized value $op" error were logged.
Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
Signed-off-by: Thomas Klausner <domm@plix.at>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
This patch removes the last remaining use of updateWrongTransfer and the
method itself.
We replace it with a call to request_transfer that passes the 'replace'
reason of 'WrongTransfer' through to Koha::Item::Transfer->cancel.
Test plan
1/ git grep updateWrongTransfer to confirm there are no more uses of the
method.
2/ Confirm an incorrect transfer still behaves as expected at
circulation returns.
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Kevin Carnes
Roman Dolny
Tomas Cohen Arazi
Martin Renvoize
Nick Clemens
Hammat Wele
Laura_Escamilla
Matthias Le Gac
emilyrose
Baptiste Wojtkowski
Emily Lamancusa
Jonathan Druart
Lucas Gass
Andrew Fuerste Henry
Owen Leonard
Marcel de Rooy
Koha Development Team
Sukhmandeep Benipal
Janusz Kaczmarek
Kyle M Hall
Andrii Nugged
Martin Renvoize