Bug 20556

Summary: Marking ILL request as complete results in "Internal server error"
Product: Koha Reporter: Andrew Isherwood <bugzilla>
Component: ILLAssignee: Andrew Isherwood <bugzilla>
Status: CLOSED FIXED QA Contact: Marcel de Rooy <m.de.rooy>
Severity: major    
Priority: P2 CC: chloe.hofstede, fridolin.somers, joe.mcglynn, jonathan.druart, m.de.rooy, nick
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:
Bug Depends on: 7317    
Bug Blocks:    
Attachments: Bug [20556] - [Marking ILL request as complete..]
Bug [20556] - [Marking ILL request as complete..]
Bug 20556: Marking ILL request as complete..

Description Andrew Isherwood 2018-04-10 15:49:08 UTC
Moving an ILL request through the workflow to the point of marking it complete results in an error.

This bug manifests itself in different ways depending on whether the Koha instance is running Plack or not. Upon marking a request as complete, Plack installations see an "Internal server error" in the browser, whereas non-Plack installations see no error and everything seems to work fine. In both cases, however, the following error appears in the error log:

ill-requests.pl: Template process failed: file error - : not found at /usr/share/koha/C4/Templates.pm line 122.: /usr/share/koha/ill/ill-requests.pl, referer: http://myserver.com/cgi-bin/koha/ill/ill-requests.pl?method=illview&illrequest_id=3
Comment 1 Andrew Isherwood 2018-04-10 15:59:56 UTC
Created attachment 73957 [details] [review]
Bug [20556] - [Marking ILL request as complete..]

When marking a request as complete, the status is updated internally
then the user to redirected to the view of the request.

Although the redirect header is sent, there is no exit to indicate that
the program flow should stop. As a result, an attempt is made to render
the template, which fails.

This patch simply adds the missing exit.

TEST PLAN
---------

- Create an ILL request
- "Manage request" > "Confirm request"
- "Mark completed"
- TEST: Ensure that no error is written to the error log upon clicking
"Mark completed"
Comment 2 Katrin Fischer 2018-04-10 21:45:55 UTC
Plack is standard and pretty much non-optional, updating severity.
Comment 3 Brendan Gallagher 2018-04-11 19:44:49 UTC
Created attachment 74041 [details] [review]
Bug [20556] - [Marking ILL request as complete..]

When marking a request as complete, the status is updated internally
then the user to redirected to the view of the request.

Although the redirect header is sent, there is no exit to indicate that
the program flow should stop. As a result, an attempt is made to render
the template, which fails.

This patch simply adds the missing exit.

TEST PLAN
---------

- Create an ILL request
- "Manage request" > "Confirm request"
- "Mark completed"
- TEST: Ensure that no error is written to the error log upon clicking
"Mark completed"

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Comment 4 Marcel de Rooy 2018-04-20 07:52:48 UTC
QA: Looking here
Comment 5 Marcel de Rooy 2018-04-20 07:56:41 UTC
Commit message complaint ;)

Do not use
Bug [20556] - [Marking ILL request as complete..]
but use
Bug 20556: Marking ILL request as complete..

In short no brackets + hypen but a semicolon!
No fail for now, but things change when repeated heavily.
Note that the commit message could be more explicit like Add exit when completing ILL request or so?
Comment 6 Marcel de Rooy 2018-04-20 07:59:57 UTC
Created attachment 74598 [details] [review]
Bug 20556: Marking ILL request as complete..

When marking a request as complete, the status is updated internally
then the user to redirected to the view of the request.

Although the redirect header is sent, there is no exit to indicate that
the program flow should stop. As a result, an attempt is made to render
the template, which fails.

This patch simply adds the missing exit.

TEST PLAN
---------

- Create an ILL request
- "Manage request" > "Confirm request"
- "Mark completed"
- TEST: Ensure that no error is written to the error log upon clicking
"Mark completed"

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 7 Jonathan Druart 2018-04-20 15:49:46 UTC
Pushed to master for 18.05, thanks to everybody involved!
Comment 8 Nick Clemens (kidclamp) 2018-05-22 11:07:13 UTC
Pushed to stable for 17.11.6
Comment 9 Fridolin Somers 2018-05-23 05:22:35 UTC
Depends on Bug 7317 not in 17.05.x