Bug 21530

Summary: overdue_notices.pl aborting because error is not caught
Product: Koha Reporter: Colin Campbell <colin.campbell>
Component: NoticesAssignee: Colin Campbell <colin.campbell>
Status: RESOLVED WORKSFORME QA Contact: Testopia <testopia>
Severity: minor    
Priority: P5 - low CC: hayleypelham, josef.moravec, martin.renvoize, wizzyrea
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18086
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:
Attachments: Patch to test a valid object returned

Description Colin Campbell 2018-10-10 10:42:58 UTC
Saw this triggered by a situation where a branch had been removed but some rules remained. However when doing a lookup the return should always be tested for validity. Because it wasnt when methods were called on null the overdues_notices process aborts with a runtime error and subsequent branches overdues are not processed.
Untested returns are one of the biggest causes of errors and often hard to track down errors in the system .
Comment 1 Colin Campbell 2018-10-10 10:55:55 UTC
Created attachment 80326 [details] [review]
Patch to test a valid object returned

Adds a run-time test to ensure the assumption of the following code is true.
Without the patch process aborts on call to branchemail method.

With patch applied if find fails to return a valid library we skip to the next and issue an error listing the problematic branchcode

As we dont live in the "best of all possible worlds" code should be more pessimistic and test for unexpected results
Comment 2 Hayley Pelham 2019-03-08 02:28:31 UTC
(In reply to Colin Campbell from comment #1)
> Created attachment 80326 [details] [review] [review]
> Patch to test a valid object returned
> 
> Adds a run-time test to ensure the assumption of the following code is true.
> Without the patch process aborts on call to branchemail method.
> 
> With patch applied if find fails to return a valid library we skip to the
> next and issue an error listing the problematic branchcode
> 
> As we dont live in the "best of all possible worlds" code should be more
> pessimistic and test for unexpected results

Hi, is there some test plan for this patch?
Comment 3 Liz Rea 2019-04-30 00:47:59 UTC
Hi Colin,

I'm wondering if this situation will have been helped by the fix for bug 22679 - thoughts?
Comment 4 Owen Leonard 2019-07-31 16:27:40 UTC
I'm marking this Failed QA because comment 2 and comment 3 need to be addressed before this can be signed off.
Comment 5 Martin Renvoize (ashimema) 2024-07-09 13:09:14 UTC
Pretty sure this is caught at this point.. either way I'm closing and we can open a fresh bug when we spot it in the wild.