Bugzilla – Attachment 73310 Details for
Bug 20478
Advance notices: send separate digest messages per library
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 20478: Have the cronjob script advance_notices.pl send digest messages per branch.
Bug-20478-Have-the-cronjob-script-advancenoticespl.patch (text/plain), 10.43 KB, created by
Andreas Jonsson
on 2018-03-27 10:07:03 UTC
(
hide
)
Description:
Bug 20478: Have the cronjob script advance_notices.pl send digest messages per branch.
Filename:
MIME Type:
Creator:
Andreas Jonsson
Created:
2018-03-27 10:07:03 UTC
Size:
10.43 KB
patch
obsolete
>From ef41ac46e5b0d918061026b1177b734d2ad5ad9d Mon Sep 17 00:00:00 2001 >From: Andreas Jonsson <andreas.jonsson@kreablo.se> >Date: Mon, 22 Jan 2018 10:37:14 +0000 >Subject: [PATCH] Bug 20478: Have the cronjob script advance_notices.pl send > digest messages per branch. >MIME-Version: 1.0 >Content-Type: text/plain; charset=UTF-8 >Content-Transfer-Encoding: 8bit > >Desired behavior of the script advance_notices.pl is that the sender >address on the notice message is that of the branch of the issues in >question. Thus, the solution is to generate digest messages per >branch. > >To test: >1) Inspect unit test in t/db_dependent/cronjobs/advance_notices_digest.t and note that: > - There are three libraries > - There is a borrower > - The borrower is registered at library1 > - The borrower has message preference wants_digest set to 1 > - The borrower has message preference days_in_advance set to 1 > - The content of the letter PREDUEDGST is '<<count>> <<branches.branchname>>' > - There are three items > - There is one issue per item > - There is one issues at library2 > - There are two issues at library3 > - The date_due of the issues are set to tomorrow > - It is asserted that there are two messages in the message queue after running the script > - It is asserted that the item count of the message corresponding to library2 is 1 > - It is asserted that the item count of the message corresponding to library3 is 2 > - It is asserted that the branchnames are correct. >2) Run unit test: prove t/db_dependent/cronjobs/advance_notices_digest.t > >Sponsored-By: The Bibliotek Mellansjö coallition consisting of: >Gullspångs kommunbibliotek, Hjo stadsbibliotek, Karlsborgs bibliotek, >Mariestads stadsbibliotek, Skövde stadsbibliotek, Tibro bibliotek, >Tidaholms stadsbibliotek, Töreboda kommunbibliotek >--- > .../intranet-tmpl/prog/en/modules/tools/letter.tt | 2 - > misc/cronjobs/advance_notices.pl | 96 +++++++++------------- > t/db_dependent/cronjobs/advance_notices_digest.t | 45 ++++++++-- > 3 files changed, 79 insertions(+), 64 deletions(-) > >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/letter.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/letter.tt >index 25523ce..ffe9632 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/letter.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/letter.tt >@@ -493,8 +493,6 @@ $(document).ready(function() { > </div> > [% END %] > >- [% IF code.search('DGST') %] <span class="overdue">Warning, this is a template for a Digest, as such, any references to branch data ( e.g. branches.branchname ) will refer to the borrower's home branch.</span> [% END %] >- > <input type="hidden" id="redirect" name="redirect" value="" /> > <input type="hidden" name="searchfield" value="[% searchfield %]" /> > </form> >diff --git a/misc/cronjobs/advance_notices.pl b/misc/cronjobs/advance_notices.pl >index 3c6ef6c..fc8632f 100755 >--- a/misc/cronjobs/advance_notices.pl >+++ b/misc/cronjobs/advance_notices.pl >@@ -218,8 +218,8 @@ warn 'found ' . scalar( @$upcoming_dues ) . ' issues' if $verbose; > > # hash of borrowernumber to number of items upcoming > # for patrons wishing digests only. >-my $upcoming_digest; >-my $due_digest; >+my $upcoming_digest = {}; >+my $due_digest = {}; > > my $dbh = C4::Context->dbh(); > my $sth = $dbh->prepare(<<'END_SQL'); >@@ -250,8 +250,8 @@ UPCOMINGITEM: foreach my $upcoming ( @$upcoming_dues ) { > > if ( $borrower_preferences->{'wants_digest'} ) { > # cache this one to process after we've run through all of the items. >- $due_digest->{ $upcoming->{borrowernumber} }->{email} = $from_address; >- $due_digest->{ $upcoming->{borrowernumber} }->{count}++; >+ $due_digest->{ $upcoming->{branchcode} }->{ $upcoming->{borrowernumber} }->{email} = $from_address; >+ $due_digest->{ $upcoming->{branchcode} }->{ $upcoming->{borrowernumber} }->{count}++; > } else { > my $item = Koha::Items->find( $upcoming->{itemnumber} ); > my $letter_type = 'DUE'; >@@ -283,8 +283,8 @@ UPCOMINGITEM: foreach my $upcoming ( @$upcoming_dues ) { > > if ( $borrower_preferences->{'wants_digest'} ) { > # cache this one to process after we've run through all of the items. >- $upcoming_digest->{ $upcoming->{borrowernumber} }->{email} = $from_address; >- $upcoming_digest->{ $upcoming->{borrowernumber} }->{count}++; >+ $upcoming_digest->{ $upcoming->{branchcode} }->{ $upcoming->{borrowernumber} }->{email} = $from_address; >+ $upcoming_digest->{ $upcoming->{branchcode} }->{ $upcoming->{borrowernumber} }->{count}++; > } else { > my $item = Koha::Items->find( $upcoming->{itemnumber} ); > my $letter_type = 'PREDUE'; >@@ -342,33 +342,39 @@ SELECT biblio.*, items.*, issues.* > AND (TO_DAYS(date_due)-TO_DAYS(NOW()) = ?) > END_SQL > >-send_digests({ >- sth => $sth_digest, >- digests => $upcoming_digest, >- letter_code => 'PREDUEDGST', >- get_item_info => sub { >- my $params = shift; >- $params->{sth}->execute($params->{borrowernumber}, >- $params->{borrower_preferences}->{'days_in_advance'}); >- return sub { >- $params->{sth}->fetchrow_hashref; >- }; >- } >-}); >- >-send_digests({ >- sth => $sth_digest, >- digest => $due_digest, >- letter_code => 'DUEGST', >- get_item_info => sub { >- my $params = shift; >- $params->{sth}->execute($params->{borrowernumber}, 0); >- return sub { >- $params->{sth}->fetchrow_hashref; >- }; >- } >-}); >- >+while (my ($branchcode, $digests) = each %$upcoming_digest) { >+ send_digests({ >+ sth => $sth_digest, >+ digests => $digests, >+ letter_code => 'PREDUEDGST', >+ branchcode => $branchcode, >+ get_item_info => sub { >+ my $params = shift; >+ $params->{sth}->execute($params->{borrowernumber}, >+ $params->{borrower_preferences}->{'days_in_advance'}); >+ return sub { >+ $params->{sth}->fetchrow_hashref; >+ }; >+ } >+ }); >+} >+ >+while (my ($branchcode, $digests) = each %$due_digest) { >+ send_digests({ >+ sth => $sth_digest, >+ digest => $due_digest, >+ letter_code => 'DUEGST', >+ branchcode => $branchcode, >+ get_item_info => sub { >+ my $params = shift; >+ $params->{sth}->execute($params->{borrowernumber}, 0); >+ return sub { >+ $params->{sth}->fetchrow_hashref; >+ }; >+ } >+ }); >+} >+ > =head1 METHODS > > =head2 parse_letter >@@ -408,24 +414,6 @@ sub parse_letter { > ); > } > >-=head2 get_branch_info >- >-=cut >- >-sub get_branch_info { >- my ( $borrowernumber ) = @_; >- >- ## Get branch info for borrowers home library. >- my $patron = Koha::Patrons->find( $borrowernumber ); >- my $branch = $patron->library->unblessed; >- my %branch_info; >- foreach my $key( keys %$branch ) { >- $branch_info{"branches.$key"} = $branch->{$key}; >- } >- >- return %branch_info; >-} >- > =head2 send_digests > > send_digests({ >@@ -491,9 +479,6 @@ sub send_digests { > $titles .= C4::Letters::get_item_content( { item => $item_info, item_content_fields => \@item_content_fields } ); > } > >- ## Get branch info for borrowers home library. >- my %branch_info = get_branch_info( $borrowernumber ); >- > foreach my $transport ( keys %{ $borrower_preferences->{'transports'} } ) { > my $letter = parse_letter( > { >@@ -502,9 +487,8 @@ sub send_digests { > substitute => { > count => $count, > 'items.content' => $titles, >- %branch_info, > }, >- branchcode => $branch_info{"branches.branchcode"}, >+ branchcode => $params->{branchcode}, > message_transport_type => $transport, > } > ) >diff --git a/t/db_dependent/cronjobs/advance_notices_digest.t b/t/db_dependent/cronjobs/advance_notices_digest.t >index d422e59..457c9c0 100644 >--- a/t/db_dependent/cronjobs/advance_notices_digest.t >+++ b/t/db_dependent/cronjobs/advance_notices_digest.t >@@ -19,7 +19,7 @@ > > use Modern::Perl; > >-use Test::More tests => 3; >+use Test::More tests => 5; > use t::lib::TestBuilder; > use DateTime; > use File::Spec; >@@ -113,6 +113,9 @@ my $item1 = $builder->build({ > my $item2 = $builder->build({ > source => 'Item' > }); >+my $item3 = $builder->build({ >+ source => 'Item' >+}); > my $now = DateTime->now(); > my $tomorrow = $now->add(days => 1)->strftime('%F'); > >@@ -121,7 +124,7 @@ my $issue1 = $builder->build({ > value => { > date_due => $tomorrow, > itemnumber => $item1->{itemnumber}, >- branchcode => $library1->{branchcode}, >+ branchcode => $library2->{branchcode}, > borrowernumber => $borrower->{borrowernumber}, > returndate => undef > } >@@ -132,7 +135,16 @@ my $issue2 = $builder->build({ > value => { > date_due => $tomorrow, > itemnumber => $item2->{itemnumber}, >- branchcode => $library2->{branchcode}, >+ branchcode => $library3->{branchcode}, >+ borrowernumber => $borrower->{borrowernumber}, >+ returndate => undef >+ } >+}); >+my $issue3 = $builder->build({ >+ source => 'Issue', >+ value => { >+ date_due => $tomorrow, >+ itemnumber => $item3->{itemnumber}, > branchcode => $library3->{branchcode}, > borrowernumber => $borrower->{borrowernumber}, > returndate => undef >@@ -160,14 +172,35 @@ $sthmq->execute($borrower->{borrowernumber}); > > my $messages = $sthmq->fetchall_hashref('message_id'); > >-is(scalar(keys %$messages), 1, 'There is one message in the queue'); >+is(scalar(keys %$messages), 2, 'There are two messages in the queue'); >+ >+my %expected = ( >+ $library2->{branchname} => { >+ count => 1, >+ }, >+ $library3->{branchname} => { >+ count => 2, >+ } >+ ); >+ >+my %expected_branchnames = ( >+ $library2->{branchname} => 1, >+ $library3->{branchname} => 1 >+); >+ >+my $i = 0; > for my $message (keys %$messages) { > $messages->{$message}->{content} =~ /(\d+) (.*)/; > my $count = $1; > my $branchname = $2; > >- is ($count, '2', 'Issue count is 2'); >- is ($branchname, $library1->{branchname}, 'Branchname is that of borrowers home branch.'); >+ ok ($expected_branchnames{$branchname}, 'Branchname is that of expected issuing branch.'); >+ >+ $expected_branchnames{$branchname} = 0; >+ >+ is ($count, $expected{$branchname}->{count}, 'Issue count is ' . $expected{$branchname}->{count}); >+ >+ $i++; > } > > $dbh->rollback; >-- >2.7.4
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 20478
:
73308
|
73309
|
73310
|
73722
|
73723
|
73724
|
80867
|
80901
|
80902
|
80903
|
86381
|
86382
|
86383
|
86384