Bug 20556 - Marking ILL request as complete results in "Internal server error"
Summary: Marking ILL request as complete results in "Internal server error"
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: ILL (show other bugs)
Version: master
Hardware: All All
: P2 major (vote)
Assignee: Andrew Isherwood
QA Contact: Marcel de Rooy
URL:
Whiteboard:
Keywords:
Depends on: 7317
Blocks:
  Show dependency treegraph
 
Reported: 2018-04-10 15:49 UTC by Andrew Isherwood
Modified: 2018-05-23 05:22 UTC (History)
7 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 [20556] - [Marking ILL request as complete..] (1.25 KB, patch)
2018-04-10 15:59 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug [20556] - [Marking ILL request as complete..] (1.31 KB, patch)
2018-04-11 19:44 UTC, Brendan Gallagher
Details | Diff | Splinter Review
Bug 20556: Marking ILL request as complete.. (1.40 KB, patch)
2018-04-20 07:59 UTC, Marcel de Rooy
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-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 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