Attachment 74692 Details for Bug 19855 – Bug 19855: Move getalert, addalert and delalert to Koha::Subscription

[patch] Bug 19855: Move getalert, addalert and delalert to Koha::Subscription

Bug-19855-Move-getalert-addalert-and-delalert-to-K.patch (text/plain), 25.62 KB, created by Katrin Fischer on 2018-04-22 09:46:43 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Katrin Fischer

Created: 2018-04-22 09:46:43 UTC

Size: 25.62 KB

patch

obsolete

>From 21bc8a50d06263ed399ab27c992b58ce401f3427 Mon Sep 17 00:00:00 2001
>From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
>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 <kyle@bywatersolutions.com>
>
>Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
>---
> C4/Letters.pm                                      | 81 +++-------------------
> Koha/Subscription.pm                               | 46 ++++++++++++
> .../prog/en/modules/serials/viewalerts.tt          |  7 +-
> .../bootstrap/en/modules/opac-alert-subscribe.tt   |  4 +-
> .../opac-tmpl/bootstrap/en/modules/opac-detail.tt  |  4 +-
> .../bootstrap/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 6bc0382..52db4d0 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 d585ba8..512e018 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 5ad1866..00a3f5b 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 @@
>     <span class="label">Subscription:</span> <a href="subscription-detail.pl?subscriptionid=[% subscriptionid %]">[% bibliotitle %] #[% subscriptionid %]</a>
> </p>
> 
>-[% IF ( alertloop ) %]
>+[% IF subscribers.count %]
>     <table>
>         <tr>
>             <th>Patron name</th>
>         </tr>
>-        [% FOREACH alertloo IN alertloop %]
>+        [% FOREACH subscriber IN subscribers %]
>         <tr>
>-            <td><a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% alertloo.borrowernumber %]">[% alertloo.name %]</a></td>
>+            [%# FIXME use patron-title when 18403 will be pushed %]
>+            <td><a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% subscriber.borrowernumber %]">[% subscriber.firstname %] [% subscriber.surname %]</a></td>
>         </tr>
>         [% END %]
>     </table>
>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 adba9f3..e616888 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 @@
>                                 <p>Do you want to receive an email when a new issue for this subscription arrives?</p>
>                                 <h4>[% bibliotitle %]</h4>
>                                 [% IF ( notes ) %]<p>[% notes %]</p>[% END %]
>-                                <input type="hidden" name="externalid" value="[% externalid %]">
>+                                <input type="hidden" name="subscriptionid" value="[% subscriptionid %]">
>                                 <input type="hidden" name="referer" value="[% referer %]">
>                                 <input type="hidden" name="biblionumber" value="[% biblionumber | html %]">
>                                 <input type="hidden" name="op" value="alert_confirmed">
>@@ -38,7 +38,7 @@
>                                 <p>Please confirm that you do not want to receive email when a new issue arrives for this subscription.</p>
>                                 <h4>[% bibliotitle %]</h4>
>                                 [% IF ( notes ) %]<p>[% notes %]</p>[% END %]
>-                                <input type="hidden" name="externalid" value="[% externalid %]">
>+                                <input type="hidden" name="subscriptionid" value="[% subscriptionid %]">
>                                 <input type="hidden" name="referer" value="[% referer %]">
>                                 <input type="hidden" name="biblionumber" value="[% biblionumber | html %]">
>                                 <input type="hidden" name="op" value="cancel_confirmed">
>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 26fed4e..121fe6d 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 ) %]<span class="email_notifications">
>                                     [% IF ( loggedinusername ) %]
>                                         [% IF ( subscription.hasalert ) %]
>-                                            <span>You have subscribed to email notification on new issues. </span><a style="color:#000;" class="btn" title="Cancel email notification" href="/cgi-bin/koha/opac-alert-subscribe.pl?op=cancel&amp;externalid=[% subscription.subscriptionid %]&amp;biblionumber=[% subscription.biblionumber %]">Cancel email notification</a>
>+                                            <span>You have subscribed to email notification on new issues. </span><a style="color:#000;" class="btn" title="Cancel email notification" href="/cgi-bin/koha/opac-alert-subscribe.pl?op=cancel&amp;subscriptionid=[% subscription.subscriptionid %]&amp;biblionumber=[% subscription.biblionumber %]">Cancel email notification</a>
>                                         [% ELSE %]
>-                                            <a style="color:#000;" class="btn" title="Subscribe to email notification on new issues" href="/cgi-bin/koha/opac-alert-subscribe.pl?externalid=[% subscription.subscriptionid %]&amp;biblionumber=[% subscription.biblionumber %]">Subscribe to email notification on new issues</a>
>+                                            <a style="color:#000;" class="btn" title="Subscribe to email notification on new issues" href="/cgi-bin/koha/opac-alert-subscribe.pl?subscriptionid=[% subscription.subscriptionid %]&amp;biblionumber=[% subscription.biblionumber %]">Subscribe to email notification on new issues</a>
>                                         [% END %]
>                                     [% ELSE %]
>                                         <span>You must log in if you want to subscribe to email notification on new issues</span>
>diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-serial-issues.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-serial-issues.tt
>index 45104bf..7cee2fd 100644
>--- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-serial-issues.tt
>+++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-serial-issues.tt
>@@ -137,11 +137,11 @@
>                                     [% IF ( subscription_LOO.letter ) %]
>                                         [% IF ( loggedinusername ) %]
>                                             [% IF ( subscription_LOO.hasalert ) %]
>-                                                You have subscribed to email notification on new issues <a href="opac-alert-subscribe.pl?op=cancel&amp;externalid=[% subscription_LOO.subscriptionid %]&amp;referer=serial&amp;biblionumber=[% subscription_LOO.biblionumber %]" class="btn" title="Cancel email notification">
>+                                                You have subscribed to email notification on new issues <a href="opac-alert-subscribe.pl?op=cancel&amp;subscriptionid=[% subscription_LOO.subscriptionid %]&amp;referer=serial&amp;biblionumber=[% subscription_LOO.biblionumber %]" class="btn" title="Cancel email notification">
>                                                     Cancel email notification
>                                                 </a>
>                                             [% ELSE %]
>-                                                <a href="opac-alert-subscribe.pl?externalid=[% subscription_LOO.subscriptionid %]&amp;referer=serial&amp;biblionumber=[% subscription_LOO.biblionumber %]" class="btn" title="Subscribe to email notification on new issues">
>+                                                <a href="opac-alert-subscribe.pl?subscriptionid=[% subscription_LOO.subscriptionid %]&amp;referer=serial&amp;biblionumber=[% subscription_LOO.biblionumber %]" class="btn" title="Subscribe to email notification on new issues">
>                                                     Subscribe to email notification on new issues
>                                                 </a>
>                                             [% END %]
>diff --git a/opac/opac-alert-subscribe.pl b/opac/opac-alert-subscribe.pl
>index 7733c36..e8809e4 100755
>--- a/opac/opac-alert-subscribe.pl
>+++ b/opac/opac-alert-subscribe.pl
>@@ -36,7 +36,7 @@ my $dbh   = C4::Context->dbh;
> 
> my $sth;
> my ( $template, $loggedinuser, $cookie );
>-my $externalid   = $query->param('externalid');
>+my $subscriptionid = $query->param('subscriptionid');
> my $referer      = $query->param('referer') || 'detail';
> my $biblionumber = $query->param('biblionumber');
> 
>@@ -51,8 +51,11 @@ my $biblionumber = $query->param('biblionumber');
>     }
> );
> 
>+my $subscription = Koha::Subscriptions->find( $subscriptionid );
>+my $logged_in_patron = Koha::Patrons->find( $loggedinuser );
>+
> if ( $op eq 'alert_confirmed' ) {
>-    addalert( $loggedinuser, $externalid );
>+    $subscription->add_subscriber( $logged_in_patron );
>     if ( $referer eq 'serial' ) {
>         print $query->redirect(
>             "opac-serial-issues.pl?biblionumber=$biblionumber");
>@@ -64,12 +67,8 @@ if ( $op eq 'alert_confirmed' ) {
>     }
> }
> elsif ( $op eq 'cancel_confirmed' ) {
>-    my $alerts = getalert( $loggedinuser, $externalid );
>-    warn "CANCEL confirmed : $loggedinuser, $externalid".Data::Dumper::Dumper( $alerts );
>-    foreach (@$alerts)
>-    {    # we are supposed to have only 1 result, but just in case...
>-        delalert( $_->{alertid} );
>-    }
>+    $subscription->remove_subscriber( $logged_in_patron );
>+    warn "CANCEL confirmed : $loggedinuser, $subscriptionid";
>     if ( $referer eq 'serial' ) {
>         print $query->redirect(
>             "opac-serial-issues.pl?biblionumber=$biblionumber");
>@@ -83,13 +82,13 @@ elsif ( $op eq 'cancel_confirmed' ) {
> 
> }
> else {
>-    my $subscription = &GetSubscription($externalid);
>+    my $subscription = &GetSubscription($subscriptionid);
>     $template->param(
>         referer        => $referer,
>         "typeissue$op" => 1,
>         bibliotitle    => $subscription->{bibliotitle},
>         notes          => $subscription->{notes},
>-        externalid     => $externalid,
>+        subscriptionid     => $subscriptionid,
>         biblionumber   => $biblionumber,
>     );
> }
>diff --git a/opac/opac-detail.pl b/opac/opac-detail.pl
>index 5bd05c9..1fad0ab 100755
>--- a/opac/opac-detail.pl
>+++ b/opac/opac-detail.pl
>@@ -589,10 +589,9 @@ foreach my $subscription (@subscriptions) {
>     $cell{latestserials} =
>       GetLatestSerials( $subscription->{subscriptionid}, $serials_to_display );
>     if ( $borrowernumber ) {
>-        my $sub = getalert($borrowernumber, $subscription->{subscriptionid});
>-        if (@$sub[0]) {
>-            $cell{hasalert} = 1;
>-        }
>+        my $subscription_object = Koha::Subscriptions->find( $subscription->{subscriptionid} );
>+        my $subscriber = $subscription_object->subscribers->find( $borrowernumber );
>+        $cell{hasalert} = 1 if $subscriber;
>     }
>     push @subs, \%cell;
> }
>diff --git a/opac/opac-serial-issues.pl b/opac/opac-serial-issues.pl
>index b74a886..9fe6fc3 100755
>--- a/opac/opac-serial-issues.pl
>+++ b/opac/opac-serial-issues.pl
>@@ -62,10 +62,9 @@ if ( $selectview eq "full" ) {
>     # now, check is there is an alert subscription for one of the subscriptions
>     if ($loggedinuser) {
>         foreach (@$subscriptions) {
>-            my $sub = getalert($loggedinuser, $_->{subscriptionid});
>-            if ($sub) {
>-                $_->{hasalert} = 1;
>-            }
>+            my $subscription = Koha::Subscriptions->find( $_->{subscriptionid} );
>+            my $subscriber = $subscription->subscribers->find( $loggedinuser );
>+            $_->{hasalert} = 1 if $subscriber;
>         }
>     }
> 
>@@ -100,12 +99,11 @@ else {
> 
>     my $subscriptions = GetSubscriptionsFromBiblionumber($biblionumber);
>     # now, check is there is an alert subscription for one of the subscriptions
>-    if ($loggedinuser){
>+    if ($loggedinuser) {
>         foreach (@$subscriptions) {
>-            my $subscription = getalert($loggedinuser, $_->{subscriptionid});
>-            if (@$subscription[0]) {
>-                $_->{hasalert} = 1;
>-            }
>+            my $subscription = Koha::Subscriptions->find( $_->{subscriptionid} );
>+            my $subscriber = $subscription->subscribers->find( $loggedinuser );
>+            $_->{hasalert} = 1 if $subscriber;
>         }
>     }
> 
>diff --git a/serials/viewalerts.pl b/serials/viewalerts.pl
>index 6273f19..39539e1 100755
>--- a/serials/viewalerts.pl
>+++ b/serials/viewalerts.pl
>@@ -23,14 +23,10 @@ use CGI qw ( -utf8 );
> use C4::Auth;
> use C4::Context;
> use C4::Output;
>-use C4::Koha;
>-use C4::Letters;
>-use C4::Serials;
> 
>-my $dbh = C4::Context->dbh;
>+use Koha::Subscriptions;
> 
> my $input = new CGI;
>-my $print = $input->param('print');
> 
> my ($template, $loggedinuser, $cookie)
>     = get_template_and_user({template_name => 'serials/viewalerts.tt',
>@@ -41,20 +37,17 @@ my ($template, $loggedinuser, $cookie)
>                  debug => 1,
>                  });
> 
>-my $subscriptionid=$input->param('subscriptionid');
>-
>-my $borrowers = getalert('', $subscriptionid);
>-my $subscription = GetSubscription($subscriptionid);
>-
>-for my $borrowernumber (@$borrowers) {
>-    my $patron = Koha::Patrons->find( $borrowernumber );
>-    next unless $borrowernumber; # Just in case...
>-    $borrowers->{name} = join( ' ', $patron->firstname, $patron->surname );
>-}
>-$template->param(alertloop => $borrowers,
>-                bibliotitle => $subscription->{bibliotitle},
>-                subscriptionid => $subscriptionid,
>-                (uc(C4::Context->preference("marcflavour"))) => 1
>-                );
>+my $subscriptionid = $input->param('subscriptionid');
>+
>+my $subscription = Koha::Subscriptions->find( $subscriptionid );
>+# FIXME raise a message if subscription does not exist (easy with 18403)
>+
>+my $subscribers = $subscription->subscribers;
>+
>+$template->param(
>+    subscribers    => $subscribers,
>+    bibliotitle    => $subscription->biblio->title,
>+    subscriptionid => $subscriptionid,
>+);
> 
> output_html_with_http_headers $input, $cookie, $template->output;
>diff --git a/t/db_dependent/Koha/Subscription.t b/t/db_dependent/Koha/Subscription.t
>index 81c5563..a8c1a4c 100644
>--- a/t/db_dependent/Koha/Subscription.t
>+++ b/t/db_dependent/Koha/Subscription.t
>@@ -19,7 +19,7 @@
> 
> use Modern::Perl;
> 
>-use Test::More tests => 2;
>+use Test::More tests => 3;
> 
> use Koha::Subscriptions;
> use Koha::Biblio;
>@@ -40,7 +40,38 @@ subtest 'Koha::Subscription->biblio' => sub {
>     })->store();
> 
>     my $b = $subscription->biblio;
>-    is($b->biblionumber, $biblio->biblionumber, 'Koha::Subscription::biblio returns the correct biblio');
>+    is($b->biblionumber, $biblio->biblionumber, 'Koha::Subscription->biblio returns the correct biblio');
>+};
>+
>+subtest 'Notifications on new issues - add_subscriber|remove_subscriber|subscribers' => sub {
>+    plan tests => 5;
>+    my $subscriber_1 = $builder->build_object( { class => 'Koha::Patrons' });
>+    my $subscriber_2 = $builder->build_object( { class => 'Koha::Patrons' });
>+
>+    my $subscription = Koha::Subscription->new({
>+        biblionumber => Koha::Biblio->new->store->biblionumber,
>+    })->store();
>+
>+    my $subscribers = $subscription->subscribers;
>+    is( $subscribers->count, 0, '->subscribers should return 0 if there are no subscribers');
>+    is( ref($subscribers), 'Koha::Patrons', '->subscribers should return a Koha::Patrons object');
>+
>+    $subscription->add_subscriber( $subscriber_1 );
>+    $subscription->add_subscriber( $subscriber_2 );
>+
>+    $subscribers = $subscription->subscribers;
>+    is( $subscribers->count, 2, '->subscribers should return 2 if there are 2 subscribers' );
>+
>+    $subscription->remove_subscriber( $subscriber_1 );
>+
>+    $subscribers = $subscription->subscribers;
>+    is( $subscribers->count, 1, '->remove_subscriber should have remove the subscriber' );
>+
>+    $subscription->remove_subscriber( $subscriber_1 ); # We do not explode if the patron is not a subscriber
>+
>+    my $is_subscriber = $subscribers->find( $subscriber_2->borrowernumber );
>+    ok( $is_subscriber, 'This structure is used in the code and should work as expected' );
>+
> };
> 
> subtest 'Koha::Subscription->vendor' => sub {
>diff --git a/t/db_dependent/Letters.t b/t/db_dependent/Letters.t
>index 69a9d30..6440dc5 100644
>--- a/t/db_dependent/Letters.t
>+++ b/t/db_dependent/Letters.t
>@@ -18,7 +18,7 @@
> # along with Koha; if not, see <http://www.gnu.org/licenses>.
> 
> use Modern::Perl;
>-use Test::More tests => 75;
>+use Test::More tests => 64;
> use Test::MockModule;
> use Test::Warn;
> 
>@@ -48,6 +48,8 @@ use Koha::Acquisition::Bookseller::Contacts;
> use Koha::Acquisition::Orders;
> use Koha::Libraries;
> use Koha::Notice::Templates;
>+use Koha::Patrons;
>+use Koha::Subscriptions;
> my $schema = Koha::Database->schema;
> $schema->storage->txn_begin();
> 
>@@ -236,41 +238,6 @@ my $branchcode = 'FFL';
> my $letter14206_c = C4::Letters::getletter('my module', $overdue_rules->{"letter$i"}, $branchcode);
> is( $letter14206_c->{message_transport_type}, 'print', 'Bug 14206 - correct mtt detected for call from overdue_notices.pl' );
> 
>-# addalert
>-my $externalid = 'my external id';
>-my $alert_id = C4::Letters::addalert($borrowernumber, $externalid);
>-isnt( $alert_id, undef, 'addalert does not return undef' );
>-
>-
>-# getalert
>-my $alerts = C4::Letters::getalert();
>-is( @$alerts, 1, 'getalert should not fail without parameter' );
>-$alerts = C4::Letters::getalert($borrowernumber);
>-is( @$alerts, 1, 'addalert adds an alert' );
>-is( $alerts->[0]->{alertid}, $alert_id, 'addalert returns the alert id correctly' );
>-is( $alerts->[0]->{externalid}, $externalid, 'addalert stores the externalid correctly' );
>-
>-$alerts = C4::Letters::getalert($borrowernumber);
>-is( @$alerts, 1, 'getalert returns the correct number of alerts' );
>-$alerts = C4::Letters::getalert($borrowernumber, $externalid);
>-is( @$alerts, 1, 'getalert returns the correct number of alerts' );
>-$alerts = C4::Letters::getalert($borrowernumber, 'another external id');
>-is( @$alerts, 0, 'getalert returns the correct number of alerts' );
>-
>-
>-# delalert
>-eval {
>-    C4::Letters::delalert();
>-};
>-isnt( $@, undef, 'delalert without argument returns an error' );
>-$alerts = C4::Letters::getalert($borrowernumber);
>-is( @$alerts, 1, 'delalert without argument does not remove an alert' );
>-
>-C4::Letters::delalert($alert_id);
>-$alerts = C4::Letters::getalert($borrowernumber);
>-is( @$alerts, 0, 'delalert removes an alert' );
>-
>-
> # GetPreparedLetter
> t::lib::Mocks::mock_preference('OPACBaseURL', 'http://thisisatest.com');
> 
>@@ -509,8 +476,8 @@ my $borrowernumber = AddMember(
>     dateofbirth  => $date,
>     email        => 'john.smith@test.de',
> );
>-my $alert_id = C4::Letters::addalert($borrowernumber, $subscriptionid);
>-
>+my $subscription = Koha::Subscriptions->find( $subscriptionid );
>+$subscription->add_subscriber( scalar Koha::Patrons->find( $borrowernumber ) );
> 
> my $err2;
> warning_is {
>-- 
>2.1.4

Actions: View | Diff | Splinter Review

Attachments on bug 19855: 69970 | 69971 | 69972 | 69973 | 69974 | 73621 | 73622 | 73623 | 73624 | 73625 | 74611 | 74612 | 74613 | 74614 | 74615 | 74688 | 74689 | 74690 | 74691 | 74692 | 74750