From 5c7ff786d5e755ff2452ec41c4385150b93d3ea9 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Wed, 20 Dec 2017 17:01:08 -0300 Subject: [PATCH] Bug 19855: Move getalert, addalert and delalert to Koha::Subscription This patch removes 3 subroutines from C4::Letters: - getalert - addalert - delalert And add 3 methods to Koha::Subscription: - subscribers - add_subscriber - remove_subscriber It makes the code cleaner for future cleanup. TODO - we should remove alert.alertid and alert.type, and rename alert.externalid with alert.subscriptionid That way alert will be renamed borrowers_subscriptions (or similar) and will become a simple join table between borrowers and subscriptions. We will need to deal with FK that could not be satisfied. Let's do that after this patch is pushed. Test plan: Subscribe and unsubscribe to email notifications sent when a new issues is available. Make sure everything works as before and you receive the emails. Signed-off-by: Kyle M Hall Signed-off-by: Katrin Fischer Signed-off-by: Jonathan Druart --- C4/Letters.pm | 81 ++----------------- Koha/Subscription.pm | 46 +++++++++++ .../prog/en/modules/serials/viewalerts.tt | 7 +- .../en/modules/opac-alert-subscribe.tt | 4 +- .../bootstrap/en/modules/opac-detail.tt | 4 +- .../en/modules/opac-serial-issues.tt | 4 +- opac/opac-alert-subscribe.pl | 19 +++-- opac/opac-detail.pl | 7 +- opac/opac-serial-issues.pl | 16 ++-- serials/viewalerts.pl | 33 +++----- t/db_dependent/Koha/Subscription.t | 35 +++++++- t/db_dependent/Letters.t | 43 ++-------- 12 files changed, 134 insertions(+), 165 deletions(-) diff --git a/C4/Letters.pm b/C4/Letters.pm index 6bc03823a2..52db4d0ac6 100644 --- a/C4/Letters.pm +++ b/C4/Letters.pm @@ -39,6 +39,7 @@ use Koha::Email; use Koha::Notice::Messages; use Koha::DateUtils qw( format_sqldatetime dt_from_string ); use Koha::Patrons; +use Koha::Subscriptions; use vars qw(@ISA @EXPORT @EXPORT_OK %EXPORT_TAGS); @@ -46,7 +47,7 @@ BEGIN { require Exporter; @ISA = qw(Exporter); @EXPORT = qw( - &GetLetters &GetLettersAvailableForALibrary &GetLetterTemplates &DelLetter &GetPreparedLetter &GetWrappedLetter &addalert &getalert &delalert &SendAlerts &GetPrintMessages &GetMessageTransportTypes + &GetLetters &GetLettersAvailableForALibrary &GetLetterTemplates &DelLetter &GetPreparedLetter &GetWrappedLetter &SendAlerts &GetPrintMessages &GetMessageTransportTypes ); } @@ -266,71 +267,6 @@ sub DelLetter { , undef, $branchcode, $module, $code, ( $mtt ? $mtt : () ), ( $lang ? $lang : () ) ); } -=head2 addalert ($borrowernumber, $subscriptionid) - - parameters : - - $borrowernumber : the number of the borrower subscribing to the alert - - $subscriptionid - - create an alert and return the alertid (primary key) - -=cut - -sub addalert { - my ( $borrowernumber, $subscriptionid) = @_; - my $dbh = C4::Context->dbh; - my $sth = - $dbh->prepare( - "insert into alert (borrowernumber, externalid) values (?,?)"); - $sth->execute( $borrowernumber, $subscriptionid ); - - # get the alert number newly created and return it - my $alertid = $dbh->{'mysql_insertid'}; - return $alertid; -} - -=head2 delalert ($alertid) - - parameters : - - alertid : the alert id - deletes the alert - -=cut - -sub delalert { - my $alertid = shift or die "delalert() called without valid argument (alertid)"; # it's gonna die anyway. - $debug and warn "delalert: deleting alertid $alertid"; - my $sth = C4::Context->dbh->prepare("delete from alert where alertid=?"); - $sth->execute($alertid); -} - -=head2 getalert ([$borrowernumber], [$subscriptionid]) - - parameters : - - $borrowernumber : the number of the borrower subscribing to the alert - - $subscriptionid - all parameters NON mandatory. If a parameter is omitted, the query is done without the corresponding parameter. For example, without $subscriptionid, returns all alerts for a borrower on a topic. - -=cut - -sub getalert { - my ( $borrowernumber, $subscriptionid ) = @_; - my $dbh = C4::Context->dbh; - my $query = "SELECT a.*, b.branchcode FROM alert a JOIN borrowers b USING(borrowernumber) WHERE 1"; - my @bind; - if ($borrowernumber and $borrowernumber =~ /^\d+$/) { - $query .= " AND borrowernumber=?"; - push @bind, $borrowernumber; - } - if ($subscriptionid) { - $query .= " AND externalid=?"; - push @bind, $subscriptionid; - } - my $sth = $dbh->prepare($query); - $sth->execute(@bind); - return $sth->fetchall_arrayref({}); -} - =head2 SendAlerts my $err = &SendAlerts($type, $externalid, $letter_code); @@ -379,22 +315,21 @@ sub SendAlerts { return; my %letter; - # find the list of borrowers to alert - my $alerts = getalert( '', $subscriptionid ); - foreach (@$alerts) { - my $patron = Koha::Patrons->find( $_->{borrowernumber} ); - next unless $patron; # Just in case + # find the list of subscribers to notify + my $subscription = Koha::Subscriptions->find( $subscriptionid ); + my $subscribers = $subscription->subscribers; + while ( my $patron = $subscribers->next ) { my $email = $patron->email or next; # warn "sending issues..."; my $userenv = C4::Context->userenv; - my $library = Koha::Libraries->find( $_->{branchcode} ); + my $library = $patron->library; my $letter = GetPreparedLetter ( module => 'serial', letter_code => $letter_code, branchcode => $userenv->{branch}, tables => { - 'branches' => $_->{branchcode}, + 'branches' => $library->branchcode, 'biblio' => $biblionumber, 'biblioitems' => $biblionumber, 'borrowers' => $patron->unblessed, diff --git a/Koha/Subscription.pm b/Koha/Subscription.pm index d585ba829a..512e018353 100644 --- a/Koha/Subscription.pm +++ b/Koha/Subscription.pm @@ -60,6 +60,52 @@ sub vendor { return scalar Koha::Acquisition::Booksellers->find($self->aqbooksellerid); } +=head3 subscribers + +my $subscribers = $subscription->subscribers; + +return a Koha::Patrons object + +=cut + +sub subscribers { + my ($self) = @_; + my $schema = Koha::Database->new->schema; + my @borrowernumbers = $schema->resultset('Alert')->search({ externalid => $self->subscriptionid })->get_column( 'borrowernumber' )->all; + return Koha::Patrons->search({ borrowernumber => {-in => \@borrowernumbers } }); +} + +=head3 add_subscriber + +$subscription->add_subscriber( $patron ); + +Add a new subscriber (Koha::Patron) to this subscription + +=cut + +sub add_subscriber { + my ( $self, $patron ) = @_; + my $schema = Koha::Database->new->schema; + my $rs = $schema->resultset('Alert'); + $rs->create({ externalid => $self->subscriptionid, borrowernumber => $patron->borrowernumber }); +} + +=head3 remove_subscriber + +$subscription->remove_subscriber( $subscriber ); + +Remove a subscriber (Koha::Patron) from this subscription + +=cut + +sub remove_subscriber { + my ($self, $patron) = @_; + my $schema = Koha::Database->new->schema; + my $rs = $schema->resultset('Alert'); + my $subscriber = $rs->find({ externalid => $self->subscriptionid, borrowernumber => $patron->borrowernumber }); + $subscriber->delete if $subscriber; +} + =head3 type =cut diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/serials/viewalerts.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/serials/viewalerts.tt index 5ad1866eca..00a3f5be4a 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/serials/viewalerts.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/serials/viewalerts.tt @@ -19,14 +19,15 @@ Subscription: [% bibliotitle %] #[% subscriptionid %]

-[% IF ( alertloop ) %] +[% IF subscribers.count %] - [% FOREACH alertloo IN alertloop %] + [% FOREACH subscriber IN subscribers %] - + [%# FIXME use patron-title when 18403 will be pushed %] + [% END %]
Patron name
[% alertloo.name %][% subscriber.firstname %] [% subscriber.surname %]
diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-alert-subscribe.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-alert-subscribe.tt index adba9f3306..e616888bfc 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-alert-subscribe.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-alert-subscribe.tt @@ -24,7 +24,7 @@

Do you want to receive an email when a new issue for this subscription arrives?

[% bibliotitle %]

[% IF ( notes ) %]

[% notes %]

[% END %] - + @@ -38,7 +38,7 @@

Please confirm that you do not want to receive email when a new issue arrives for this subscription.

[% bibliotitle %]

[% IF ( notes ) %]

[% notes %]

[% END %] - + diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt index 26fed4e9ce..121fe6d074 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt @@ -834,9 +834,9 @@ [% IF ( subscription.letter ) %]