Summary: | Handle and report exception at checkout/checkin due to missing guarantor | ||
---|---|---|---|
Product: | Koha | Reporter: | Andrew Fuerste-Henry <andrew> |
Component: | Circulation | Assignee: | 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
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 (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. 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? 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 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 Created attachment 182693 [details] [review] Bug 39180: Unit tests 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> 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> Created attachment 183086 [details] [review] Bug 39180: Unit tests Signed-off-by: David Nind <david@davidnind.com> 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 |