Bug 39180

Summary: Handle and report exception at checkout/checkin due to missing guarantor
Product: Koha Reporter: Andrew Fuerste-Henry <andrew>
Component: CirculationAssignee: Nick Clemens (kidclamp) <nick>
Status: Signed Off --- QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: david, gmcharlt, kyle.m.hall, martin.renvoize, nick
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35421
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37439
GIT URL: Change sponsored?: ---
Patch complexity: --- Documentation contact:
Documentation submission: Text to go in the release notes:
This fixes checking out, checking in, and renewing items for a patron where a guarantor is required, and they don't have one (where the ChildNeedsGuarantor system preference is enabled). These actions are now completed correctly, and a warning message is now shown on the patron's page where a guarantor is required and they don't have one: "System preference 'ChildNeedsGuarantor' is enabled and this patron does not have a guarantor.". Previously: - checking items in or out generated a 500 error message, even though the actions were successfully completed - attempting to renew items generated this message "Error: Internal Server Error" and the items were not renewed - no message was shown on the patron page warning that they needed a guarantor
Version(s) released in:
Circulation function:
Bug Depends on: 12133    
Bug Blocks:    
Attachments: Bug 39180: Catch exception when updating patron lastseen date
Bug 39180: Catch exception when updating patron lastseen date
Bug 39180: Add a warning on patron missing guaranor
Bug 39180: Unit tests
Bug 39180: Catch exception when updating patron lastseen date
Bug 39180: Add a warning on patron missing guaranor
Bug 39180: Unit tests

Description Andrew Fuerste-Henry 2025-02-20 20:17:20 UTC
When ChildNeedsGuarantor is enabled, Koha will throw an exception and fail to check an item in if the borrowing patron could have a guarantor but does not. The failed checkin gives a generic messages about configuration issues. We should check the item in and report on the screen that the patron should have a guarantor.

To recreate:
1 - have a patron in a category that can be a guarantee but does not have a guarantor on their account
2 - set TrackLastPatronActivityTriggers to update the patron on checkin of an item
3 - check an item out to your patron
4 - set ChildNeedsGuarantor to "must have"
5 - check your item in, get error

It also throws an exception at checkout if your tell TrackLastPatronActivityTriggers to update at checkout, but the checkout succeeds.

We've seen inconsistent behavior between production and test sites around the specifics of this error that we haven't quite nailed down yet. It's possible something changed here between 24.05.06 and main as they generate this error slightly differently.
Comment 1 Nick Clemens (kidclamp) 2025-05-05 18:40:04 UTC
Created attachment 181941 [details] [review]
Bug 39180: Catch exception when updating patron lastseen date

This patch adds a catch during checkin 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, I think we should add a warning on the patron display that a required guarantor
is missing, but not break circulation.

We will need to add unit tests
Comment 2 Andrew Fuerste-Henry 2025-05-05 19:16:28 UTC
(In reply to Nick Clemens (kidclamp) from comment #1)
> Created attachment 181941 [details] [review] [review]
> Bug 39180: Catch exception when updating patron lastseen date
> 
> This patch adds a catch during checkin 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, I think we should add a warning on the
> patron display that a required guarantor
> is missing, but not break circulation.
> 
> We will need to add unit tests

The general approach here is good, but the popup at checkin should tell the user more specifically what has happened. Something like "This item has been checked-in, but the following warning still needs to be addressed: [Child patron needs a guarantor]."

If we put some obvious alert in the patron record, we don't need to give a special message at checkout and renewal.
Comment 3 Martin Renvoize (ashimema) 2025-05-13 06:15:09 UTC
Are you coming back to this one Nick.. it's still marked as NEW.. I like the direction of travel but it sounds like there's a little more to go before NSO?
Comment 4 Nick Clemens (kidclamp) 2025-05-21 14:28:51 UTC
Created attachment 182691 [details] [review]
Bug 39180: Catch exception when updating patron lastseen date

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
Comment 5 Nick Clemens (kidclamp) 2025-05-21 14:28:53 UTC
Created attachment 182692 [details] [review]
Bug 39180: Add a warning on patron missing guaranor

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
Comment 6 Nick Clemens (kidclamp) 2025-05-21 14:28:56 UTC
Created attachment 182693 [details] [review]
Bug 39180: Unit tests
Comment 7 David Nind 2025-06-06 23:44:59 UTC
Created attachment 183084 [details] [review]
Bug 39180: Catch exception when updating patron lastseen date

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>
Comment 8 David Nind 2025-06-06 23:45:02 UTC
Created attachment 183085 [details] [review]
Bug 39180: Add a warning on patron missing guaranor

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>
Comment 9 David Nind 2025-06-06 23:45:05 UTC
Created attachment 183086 [details] [review]
Bug 39180: Unit tests

Signed-off-by: David Nind <david@davidnind.com>
Comment 10 David Nind 2025-06-07 00:02:01 UTC
From my testing, everything seems to work as expected.

I've attempted a release note - feel free to improve it!

Testing notes (using KTD):
1. Step 1: Child type patron with no guarantor: Lisa Charles
2. Step 2: Changed patron category Patron (PT) so that 'Can be guarantee' is set to 'Yes', use Mary Burton
3. Step 5: The column name is lastseen, not datelastseen, so the correct SQL to run is: UPDATE borrowers SET lastseen = '2025-01-01';
4. Step 7: I get a 500 error when attempting to check out the item, if I look at the patrons page again the item is actually checked out to them (tested for both Lisa and Mary)
5. Step 8: I get a 500 error when attempting to check in the item, if I look at the the patrons page again the item was checked in (tested for both Lisa and Mary)
6. Step 9: I get an "Error: Internal Server Error" in a yellow information box at the top of the patrons page when trying to renew the item.
7. After the patch: Creating a patron in the Kid category (category type = Child) or a patron category where "Can be guarantee" (Patron > Category type = "Adult"), and don't add a guarantor - you get this message:
    The following fields are wrong. Please fix them.
    
    A child patron needs a guarantor.
8. Error updating patron's last seen date - there are two H3 headings: H3 heading, the title check in message is also an H3:
   Check in message <--H3
   39999000001396: Philippics.
   
   Error updating patron's last seen date <-- H3 as well
   The item has been checked in, but the issue below must be corrected:
   The system encountered an error when updating the patron's last seen date:
   
   [Child patron needs a guarantor]
9. Tests pass: prove t/db_dependent/Circulation.t