Bug 17470

Summary: overdue_notices.pl produces X emails with the SAME list of ALL overdue items if a patron has overdue items from X branches
Product: Koha Reporter: Theodoros Theodoropoulos <theod>
Component: NoticesAssignee: Koha Team University Lyon 3 <koha>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P3 CC: jonathan.druart, jpalmer, jschmidt, katrin.fischer, koha, kyle, theod
Version: master   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17649
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11761
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: Bug 17470 Make overdue notices according to branch item only
Bug 17470 Make overdue notices according to branch item only
Bug 17470 Make overdue notices according to branch item only
Bug 17470 Make overdue notices according to branch item only

Description Theodoros Theodoropoulos 2016-10-20 07:24:57 UTC
If patron P has two overdue items: 
I1 from branch B1 and 
I2 from branch B2, 

then overdue_notices.pl will produce two emails coming from B1 and B2 that have the same list of BOTH items I1 AND I2 in BOTH emails.

I believe that this is not an intended behavior and that each email coming from each branch should show overdue items belonging to that respective branch only.

Steps to reproduce:

0/ Have (or create) at least two branches B1 and B2
1/ Make sure you're logged in with an account belonging to B1, or switch to branch B1
2/ checkout item I1 (belonging to B1) to patron P (preferably while specifying a due date < today)
3/ switch to branch B2
4/ checkout item I2 (belonging to B2) to patron P (preferably while specifying a due date < today)
5/ make sure you have defined "Overdue notice/status triggers" for that patron category, ie with Delay=1 so that "Overdue Notice" is selected and "Email" is checked.
6/ from cli run 'perl ./misc/cronjobs/overdue_notices.pl' (optionally specifying also '-library B1 -library B2')
7/ (at least) two new entries will be generated in message_queue DB table, coming from different branches that have the SAME list of overdue items
8/ either see the 'content' field in these entries, or run 'perl ./cronjobs/process_message_queue.pl' to receive the actual emails (provided the email is properly setup)

I have verified the same behavior with versions 3.20.x, 3.22.x and master (but must also apply to all recent Koha versions).
Comment 1 Koha Team University Lyon 3 2016-11-08 11:11:05 UTC
Created attachment 57298 [details] [review]
Bug 17470 Make overdue notices according to  branch item only

As a test plan I redirect you to the steps described by Theodoros :
1) do these steps
2) apply the patch
3) redo them.

Olivier Crouzet
Comment 2 Theodoros Theodoropoulos 2016-11-08 15:38:00 UTC
Thank you Olivier for the patch!
I can verify that the patch works as expected for 3.20 and master.

Having said that, I leave it as 'Needs Signoff'. Maybe @Kyle or someone from QA team could/should sign it off?
Comment 3 Katrin Fischer 2016-11-08 20:25:31 UTC
Hi Theodorus, if it works for you on master, please sign off! The QA team will take over from there.
Comment 4 Koha Team University Lyon 3 2016-11-10 16:48:59 UTC
Created attachment 57429 [details] [review]
Bug 17470 Make overdue notices according to branch item only

I replaced the "items.homebranch" chunk in the sql query by "issues.branchcode" which I think is a better choice considered the fact that the holdingbranch is more likely to do the claim.

I must add that we can observe the same behaviour described by Theodoros with the library option and this patch deals with it as well :
if you have 2 branches, say A et B, and a claim of same delay on them, by now you get 2 messages. it's Ok, but both of them list also the claimed item of the other one.

Olivier Crouzet
Comment 5 Jonathan Druart 2016-11-25 08:35:31 UTC
The patch that has been signoff is not the one that is attached now.
Another signoff is required.
Please provide a complete test plan to make sure the tester will test all the different cases.
Comment 6 Koha Team University Lyon 3 2016-11-25 11:34:33 UTC
Test plan 

0/ Have (or create) at least two branches B1 and B2
1/ Make sure you're logged in with an account belonging to B1, or switch to branch B1
2/ checkout item I1 (belonging to B1) to patron P  while specifying a due date < today)
3/ switch to branch B2
4/ checkout item I2 (belonging to B2) to patron P  while specifying a due date < today)
5/ make sure you have defined "Overdue notice/status triggers" for that patron category, ie with Delay=1 so that "Overdue Notice" is selected and "Email" is checked.
6/ from cli run 'perl ./misc/cronjobs/overdue_notices.pl' 
7/ two new entries will be generated in message_queue DB table, coming from different branches that have the SAME list of overdue items
8/ either see the 'content' field in these entries, or run 'perl ./cronjobs/process_message_queue.pl' to receive the actual emails (provided the email is properly setup)
9) run again overdue_notices.pl with library option for both B1 and B2 (you must give the branchcode as parameter) like this :
./overdue_notices.pl -t -library B1branchcode -library B2branchcode 
10) see again that the generated messages have the same list of overdue items

11) apply the patch
12) run again the script first without library option and secondly with it : in both cases, 2 messages will be created. One from B1 claiming the B1 item, another from B2 claiming the B2 item.
Comment 7 Theodoros Theodoropoulos 2016-11-28 11:53:02 UTC
Created attachment 57791 [details] [review]
Bug 17470 Make overdue notices according to branch item only

When a patron had overdues on items from different branches
he received from each corresponding branch a notice claiming
the complete list of these items.

Signed-off-by: sbujam <sbujam@users.noreply.github.com>
Comment 8 Jonathan Druart 2016-12-12 12:38:08 UTC
Created attachment 58116 [details] [review]
Bug 17470 Make overdue notices according to branch item only

When a patron had overdues on items from different branches
he received from each corresponding branch a notice claiming
the complete list of these items.

Signed-off-by: sbujam <sbujam@users.noreply.github.com>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 9 Kyle M Hall 2016-12-16 11:09:56 UTC
Pushed to master for 17.05, thanks Olivier!
Comment 10 Katrin Fischer 2016-12-18 21:47:15 UTC
I am hesitating to push this to stable as it's a change in behavior and might come as a surprise. I think this would be better in a feature release with information added to the field for the release notes documentation.

I am changing this to an enhancement.