Bug 27145 - Patron deletion via intranet doesn't handle exceptions well
Summary: Patron deletion via intranet doesn't handle exceptions well
Status: Needs Signoff
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: Kyle M Hall
QA Contact: Testopia
URL:
Keywords:
Depends on: 27144
Blocks:
  Show dependency treegraph
 
Reported: 2020-12-03 13:35 UTC by Joonas Kylmälä
Modified: 2020-12-04 11:56 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 27145: Rethrow all other exceptions (1.59 KB, patch)
2020-12-03 14:15 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 27145: Wrap delete and move_to_deleted in a transaction (1.28 KB, patch)
2020-12-04 11:56 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Joonas Kylmälä 2020-12-03 13:35:48 UTC
Bug 14708 introduced a try catch around $patron->delete in commit "Bug 14708: (QA follow-up) Use try/catch blocks when calling
". However, in the catch block it only assumes the exception was from trying to delete anonymous patron when it can be anything else as well, the code should be modified so that it will handle the anonymous patron case and if it is anything else we log the other exception.

Kyle, can you have a go at this?
Comment 1 Kyle M Hall 2020-12-03 14:15:07 UTC
Created attachment 114149 [details] [review]
Bug 27145: Rethrow all other exceptions

Bug 14708 introduced a try catch around $patron->delete in commit "Bug 14708: (QA follow-up) Use try/catch blocks when calling
". However, in the catch block it only assumes the exception was from trying to delete anonymous patron when it can be anything else as well, the code should be modified so that it will handle the anonymous patron case and if it is anything else we log the other exception.
Comment 2 Jonathan Druart 2020-12-04 09:53:30 UTC
We should move $patron->move_to_deleted into the try and execute the two statements inside a transaction.
Comment 3 Kyle M Hall 2020-12-04 11:56:00 UTC
Created attachment 114183 [details] [review]
Bug 27145: Wrap delete and move_to_deleted in a transaction