Bug 8434 - Notice generation fails for Advanced Notices, Item Due, and Overdues when run in shell (due to error in Letters.pm)
Summary: Notice generation fails for Advanced Notices, Item Due, and Overdues when run...
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: 3.8
Hardware: All All
: P5 - low normal (vote)
Assignee: David Cook
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-07-13 01:47 UTC by David Cook
Modified: 2013-12-05 19:59 UTC (History)
4 users (show)

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


Attachments
Bug 8434: Notice generation fails for Advanced Notices, Item Due, and Overdues when run in shell (due to error in Letters.pm) (1.10 KB, patch)
2012-07-13 01:53 UTC, David Cook
Details | Diff | Splinter Review
Bug 8434 - Notice generation fails for Advanced Notices, Item Due, and Overdues when run in shell (due to error in Letters.pm) (1.10 KB, patch)
2012-07-13 07:15 UTC, Chris Cormack
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 8434 - Notice generation fails for Advanced Notices, Item Due, and Overdues when run in shell (due to error in Letters.pm) (1.16 KB, patch)
2012-07-13 16:53 UTC, Kyle M Hall
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 8434 - Notice generation fails for Advanced Notices, Item Due, and Overdues when run in shell (due to error in Letters.pm) (1.20 KB, patch)
2012-07-23 11:58 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 David Cook 2012-07-13 01:47:03 UTC
Notice generation fails for Advanced Notices, Item Due, and Overdues when run in shell.

This is due to the call to C4::Context->userenv->{'branch'} in Letters.pm (line 124) coming back as undefined. Since there is no "userenv" context in the shell, it fails everytime.

Introduce a check for "userenv", so it doesn't try changing the branch (which is already coming in from elsewhere) to the userenv branch. With this check, notice generation works perfectly. 

Related to Bug 8198 and Bug 8197.
Comment 1 David Cook 2012-07-13 01:53:56 UTC Comment hidden (obsolete)
Comment 2 Chris Cormack 2012-07-13 02:09:08 UTC
David

How do you run the notice generation?
Comment 3 David Cook 2012-07-13 04:05:06 UTC
(In reply to comment #2)
> David
> 
> How do you run the notice generation?

I'm not personally involved with it (besides testing 3.8), but we have a shell script set up to run the overdue_notices and advanced_notices scripts and then to process_message_queue. 

Personally, it seems a bit strange that we do it this way. How is it ordinarily accomplished?
Comment 4 Chris Cormack 2012-07-13 04:57:02 UTC
Cron jobs normally.
Comment 5 David Cook 2012-07-13 06:50:16 UTC
(In reply to comment #4)
> Cron jobs normally.

This would still be a cron job, but perhaps its purpose is to group together the different nightly scripts.

Even as a regular cron job, wouldn't the context for userenv still be missing?
Comment 6 Chris Cormack 2012-07-13 07:14:12 UTC
Yep the reason it hasn't been spotted is it only happens with independent branches on.

(You can set the userenv by calling C4::Context->set_userenv if you want)

But the if statement is harmless in that case, and stops an error in other cases.
Signing off
Comment 7 Chris Cormack 2012-07-13 07:15:34 UTC Comment hidden (obsolete)
Comment 8 Kyle M Hall 2012-07-13 16:53:07 UTC Comment hidden (obsolete)
Comment 9 Jonathan Druart 2012-07-23 11:58:19 UTC
Created attachment 11078 [details] [review]
[SIGNED-OFF] Bug 8434 - Notice generation fails for Advanced Notices, Item Due, and Overdues when run in shell (due to error in Letters.pm)

QA Comments:

The patch fixes the error if getletter is called by a command line script.

(This one fixes a bad indentation)

Marking as Passed QA
Comment 10 Paul Poulain 2012-07-24 15:34:58 UTC
Patch pushed, severity updated to "normal" instead of ENH
Comment 11 Chris Cormack 2012-07-25 09:43:57 UTC
Pushed to 3.8.x, will be in 3.8.4