Bug 24072

Summary: Typos in advance_notices.pl causes DUEDGST not to be sent
Product: Koha Reporter: Magnus Enger <magnus>
Component: NoticesAssignee: Magnus Enger <magnus>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: major    
Priority: P5 - low CC: katrin.fischer, lucas, martin.renvoize
Version: master   
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:
19.11.00
Attachments: Bug 24072 - Typos in advance_notices.pl causes DUEDGST not to be sent
Bug 24072 - Typos in advance_notices.pl causes DUEDGST not to be sent
Bug 24072 - Typos in advance_notices.pl causes DUEDGST not to be sent

Description Magnus Enger 2019-11-20 13:53:17 UTC
In misc/cronjobs/advance_notices.pl we have this code: 

 372 if ($digest_per_branch) {
 373     while (my ($branchcode, $digests) = each %$upcoming_digest) {
 374         send_digests({
 375             sth => $sth_digest,
 376             digests => $digests,
 377             letter_code => 'PREDUEDGST',
 378             branchcode => $branchcode,
 379             get_item_info => sub {
 380                 my $params = shift;
 381                 $params->{sth}->execute($params->{borrowernumber},
 382                                         $params->{borrower_preferences}->{'days_in_advance'});
 383                 return sub {
 384                     $params->{sth}->fetchrow_hashref;
 385                 };
 386             }
 387         });
 388     }
 389 
 390     while (my ($branchcode, $digests) = each %$due_digest) {
 391         send_digests({
 392             sth => $sth_digest,
 393             digest => $due_digest, ### HERE! 
 394             letter_code => 'DUEDGST',
 395             branchcode => $branchcode,
 396             get_item_info => sub {
 397                 my $params = shift;
 398                 $params->{sth}->execute($params->{borrowernumber}, 0);
 399                 return sub {
 400                     $params->{sth}->fetchrow_hashref;
 401                 };
 402             }
 403         });
 404     }
 405 } else {
 406     send_digests({
 407         sth => $sth_digest,
 408         digests => $upcoming_digest,
 409         letter_code => 'PREDUEDGST',
 410         get_item_info => sub {
 411             my $params = shift;
 412             $params->{sth}->execute($params->{borrowernumber},
 413                                     $params->{borrower_preferences}->{'days_in_advance'});
 414             return sub {
 415                 $params->{sth}->fetchrow_hashref;
 416             };
 417         }
 418     });
 419 
 420     send_digests({
 421         sth => $sth_digest,
 422         digest => $due_digest, ### AND HERE!
 423         letter_code => 'DUEDGST',
 424         get_item_info => sub {
 425             my $params = shift;
 426             $params->{sth}->execute($params->{borrowernumber}, 0);
 427             return sub {
 428                 $params->{sth}->fetchrow_hashref;
 429             };
 430         }
 431     });
 432 }

A typo has crept in on lines 393 and 422, where "digest" should be written "digests", in the same way as it is done in the other two blocks of code. 

In send_digests() the incoming data is iterated over in this line: 

 531     PATRON: while ( my ( $borrowernumber, $digest ) = each %{$params->{digests}} ) {

but when the data is actually in $params->{digest} they are not found and no DUEDGST messages are queued.
Comment 1 Magnus Enger 2019-11-20 14:26:12 UTC
Created attachment 95617 [details] [review]
Bug 24072 - Typos in advance_notices.pl causes DUEDGST not to be sent

There are two typos in advance_notics.pl that cause DUEDGST messages
not to be sent. See Bugzilla for full details.

If you think the typo is sufficiently obvious, you can just eyeball the
patch and sign off, methinks. Otherwise, testing can be done something
like this:
- Make sure you have enabled enhanced messaging preferences, and a
  patron with "Email" and "Digests only" set for "Item due" messages
- Issue an item to this patron, with due date today
- Run something like this to generate advance notices:
  $ sudo koha-shell -c "perl \
  /home/vagrant/kohaclone/misc/cronjobs/advance_notices.pl -n -c" kohadev
- See that no notices are shown (-n means messages will go to stdout,
  instead of into the message queue).
- Apply the patch and run advance_notices.pl again, as before. A DUEDGST
  message should now be displayed.
Comment 2 Martin Renvoize 2019-11-21 16:43:52 UTC Comment hidden (obsolete)
Comment 3 Martin Renvoize 2019-11-21 16:44:23 UTC
Trivial patch, works, Signing off
Comment 4 Katrin Fischer 2019-11-23 07:37:48 UTC
Created attachment 95754 [details] [review]
Bug 24072 - Typos in advance_notices.pl causes DUEDGST not to be sent

There are two typos in advance_notics.pl that cause DUEDGST messages
not to be sent. See Bugzilla for full details.

If you think the typo is sufficiently obvious, you can just eyeball the
patch and sign off, methinks. Otherwise, testing can be done something
like this:
- Make sure you have enabled enhanced messaging preferences, and a
  patron with "Email" and "Digests only" set for "Item due" messages
- Issue an item to this patron, with due date today
- Run something like this to generate advance notices:
  $ sudo koha-shell -c "perl \
  /home/vagrant/kohaclone/misc/cronjobs/advance_notices.pl -n -c" kohadev
- See that no notices are shown (-n means messages will go to stdout,
  instead of into the message queue).
- Apply the patch and run advance_notices.pl again, as before. A DUEDGST
  message should now be displayed.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 5 Katrin Fischer 2019-11-23 07:41:14 UTC
Is 18.11 the right version here? It looks like send_digests was introduced later.
Comment 6 Magnus Enger 2019-11-25 07:13:50 UTC
(In reply to Katrin Fischer from comment #5)
> Is 18.11 the right version here? It looks like send_digests was introduced
> later.

You are right, of course. It is relevant to master and 19.05.
Comment 7 Martin Renvoize 2019-11-25 08:43:59 UTC
Nice work!

Pushed to master for 19.11.00
Comment 8 Lucas Gass 2019-12-16 21:45:22 UTC
backported to 19.05.x for 19.05.06