Bug 21289 - Error when sending emails to partner libraries
Summary: Error when sending emails to partner libraries
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: ILL (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Target Milestone: ---
Assignee: Andrew Isherwood
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-08-30 09:44 UTC by Andrew Isherwood
Modified: 2018-10-24 09:13 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 21289: Fix "isa" bug during partner request (2.80 KB, patch)
2018-08-30 10:29 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 21289: Fix "isa" bug during partner request (2.85 KB, patch)
2018-09-25 14:38 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 21289: Fix "isa" bug during partner request (2.92 KB, patch)
2018-09-30 22:26 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Isherwood 2018-08-30 09:44:19 UTC
During the "Place request with partners" workflow, at the point of sending the request mail, an "Internal server error" is displayed and the following is logged:

Can't call method "isa" on unblessed reference at /home/koha/kohaclone/ill/ill-requests.pl line 241.

The error here is misleading. It is due to the 'catch' block being called without an instance of a Koha::Exceptions::Ill object being passed. The reason it is being called is that the handle_commit_maybe sub is being called inside it. The handle_commit_maybe sub redirects the client, the exits:

295             # Redirect to a view of the newly created request
296             print $cgi->redirect(
297                 '/cgi-bin/koha/ill/ill-requests.pl?method=illview&illrequest_id='.
298                 $request->id
299             );
300             exit;

This is correct, but because it is being called inside the try block, the "exit" causes the catch block to be called, despite the client being redirected.

A patch for this should move the call to handle_commit_maybe outside of the try block, we will still only reach it if everything inside the try block succeeded.
Comment 1 Andrew Isherwood 2018-08-30 10:29:55 UTC
Created attachment 78309 [details] [review]
Bug 21289: Fix "isa" bug during partner request

We cannot call 'handle_commit_maybe' inside a 'try' block.
handle_commit_maybe redirects the client then calls 'exit', this is
interpreted as an error and the 'catch' block is erroneously called.

This patch moves the calling of 'handle_commit_maybe' outside the try
block, it will only be reached if everything inside the try block
suceeds.

To test:
1) Enable ILL and have the FreeForm backend available
2) Create a patron category to hold ILL "partners" to whom requests can
be sent. The category can be called anything, you should note the code
you assign
3) Create a patron that belongs to your new category, the patron must
have a primary email defined.
4) In your <interlibrary_loans> block in koha-conf.xml, ensure you have
a <partner_code> element, it should contain the code you assigned in
step 2
5) Create an ILL request using the FreeForm backend
6) Once the request is created, select the "Place request with partners"
button
7) Select your "partner" from the "Select partner libraries" box
8) Click "Send email"
9) TEST: Observe no errors are displayed in the UI
Comment 2 PTFS Europe Sandboxes 2018-09-25 14:38:46 UTC
Created attachment 79380 [details] [review]
Bug 21289: Fix "isa" bug during partner request

We cannot call 'handle_commit_maybe' inside a 'try' block.
handle_commit_maybe redirects the client then calls 'exit', this is
interpreted as an error and the 'catch' block is erroneously called.

This patch moves the calling of 'handle_commit_maybe' outside the try
block, it will only be reached if everything inside the try block
suceeds.

To test:
1) Enable ILL and have the FreeForm backend available
2) Create a patron category to hold ILL "partners" to whom requests can
be sent. The category can be called anything, you should note the code
you assign
3) Create a patron that belongs to your new category, the patron must
have a primary email defined.
4) In your <interlibrary_loans> block in koha-conf.xml, ensure you have
a <partner_code> element, it should contain the code you assigned in
step 2
5) Create an ILL request using the FreeForm backend
6) Once the request is created, select the "Place request with partners"
button
7) Select your "partner" from the "Select partner libraries" box
8) Click "Send email"
9) TEST: Observe no errors are displayed in the UI

Signed-off-by: Barry Cannon <bc@interleaf.ie>
Comment 3 Jonathan Druart 2018-09-30 22:26:02 UTC
Created attachment 79663 [details] [review]
Bug 21289: Fix "isa" bug during partner request

We cannot call 'handle_commit_maybe' inside a 'try' block.
handle_commit_maybe redirects the client then calls 'exit', this is
interpreted as an error and the 'catch' block is erroneously called.

This patch moves the calling of 'handle_commit_maybe' outside the try
block, it will only be reached if everything inside the try block
suceeds.

To test:
1) Enable ILL and have the FreeForm backend available
2) Create a patron category to hold ILL "partners" to whom requests can
be sent. The category can be called anything, you should note the code
you assign
3) Create a patron that belongs to your new category, the patron must
have a primary email defined.
4) In your <interlibrary_loans> block in koha-conf.xml, ensure you have
a <partner_code> element, it should contain the code you assigned in
step 2
5) Create an ILL request using the FreeForm backend
6) Once the request is created, select the "Place request with partners"
button
7) Select your "partner" from the "Select partner libraries" box
8) Click "Send email"
9) TEST: Observe no errors are displayed in the UI

Signed-off-by: Barry Cannon <bc@interleaf.ie>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 4 Nick Clemens 2018-10-01 16:44:29 UTC
Awesome work all!

Pushed to master for 18.11
Comment 5 Martin Renvoize 2018-10-05 12:10:52 UTC
Pushed to 18.05.x for 18.05.05
Comment 6 Fridolin SOMERS 2018-10-11 12:45:41 UTC
Pushed to 17.11.x for 17.11.11