Bug 19855: Remove $type from the alerts

It looks like this feature has never been finished. It has been
developed with more flexibility in mind, but only 'issue' is used for
this parameter. Apparently it could have been 'virtual', for virtual shelves.

Let remove this parameter and clean the code a bit.
TODO: Remove the DB column

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

diff --git a/C4/Letters.pm b/C4/Letters.pm
index ed27b1f..6bc0382 100644 (file)
--- a/C4/Letters.pm
+++ b/C4/Letters.pm
@@ -266,24 +266,23 @@ sub DelLetter {
     , undef, $branchcode, $module, $code, ( $mtt ? $mtt : () ), ( $lang ? $lang : () ) );
 }
 
-=head2 addalert ($borrowernumber, $type, $externalid)
+=head2 addalert ($borrowernumber, $subscriptionid)
 
     parameters : 
     - $borrowernumber : the number of the borrower subscribing to the alert
-    - $type : the type of alert.
-    - $externalid : the primary key of the object to put alert on. For issues, the alert is made on subscriptionid.
-    
+    - $subscriptionid
+
     create an alert and return the alertid (primary key)
 
 =cut
 
 sub addalert {
-    my ( $borrowernumber, $type, $externalid ) = @_;
+    my ( $borrowernumber, $subscriptionid) = @_;
     my $dbh = C4::Context->dbh;
     my $sth =
       $dbh->prepare(
-        "insert into alert (borrowernumber, type, externalid) values (?,?,?)");
-    $sth->execute( $borrowernumber, $type, $externalid );
+        "insert into alert (borrowernumber, externalid) values (?,?)");
+    $sth->execute( $borrowernumber, $subscriptionid );
 
     # get the alert number newly created and return it
     my $alertid = $dbh->{'mysql_insertid'};
@@ -305,18 +304,17 @@ sub delalert {
     $sth->execute($alertid);
 }
 
-=head2 getalert ([$borrowernumber], [$type], [$externalid])
+=head2 getalert ([$borrowernumber], [$subscriptionid])
 
     parameters :
     - $borrowernumber : the number of the borrower subscribing to the alert
-    - $type : the type of alert.
-    - $externalid : the primary key of the object to put alert on. For issues, the alert is made on subscriptionid.
-    all parameters NON mandatory. If a parameter is omitted, the query is done without the corresponding parameter. For example, without $externalid, returns all alerts for a borrower on a topic.
+    - $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, $type, $externalid ) = @_;
+    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;
@@ -324,13 +322,9 @@ sub getalert {
         $query .= " AND borrowernumber=?";
         push @bind, $borrowernumber;
     }
-    if ($type) {
-        $query .= " AND type=?";
-        push @bind, $type;
-    }
-    if ($externalid) {
+    if ($subscriptionid) {
         $query .= " AND externalid=?";
-        push @bind, $externalid;
+        push @bind, $subscriptionid;
     }
     my $sth = $dbh->prepare($query);
     $sth->execute(@bind);
@@ -386,7 +380,7 @@ sub SendAlerts {
 
         my %letter;
         # find the list of borrowers to alert
-        my $alerts = getalert( '', 'issue', $subscriptionid );
+        my $alerts = getalert( '', $subscriptionid );
         foreach (@$alerts) {
             my $patron = Koha::Patrons->find( $_->{borrowernumber} );
             next unless $patron; # Just in case

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 83408b7..adba9f3 100644 (file)
--- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-alert-subscribe.tt
+++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-alert-subscribe.tt
@@ -25,7 +25,6 @@
                                 <h4>[% bibliotitle %]</h4>
                                 [% IF ( notes ) %]<p>[% notes %]</p>[% END %]
                                 <input type="hidden" name="externalid" value="[% externalid %]">
-                                <input type="hidden" name="alerttype" value="[% alerttype %]">
                                 <input type="hidden" name="referer" value="[% referer %]">
                                 <input type="hidden" name="biblionumber" value="[% biblionumber | html %]">
                                 <input type="hidden" name="op" value="alert_confirmed">
@@ -40,7 +39,6 @@
                                 <h4>[% bibliotitle %]</h4>
                                 [% IF ( notes ) %]<p>[% notes %]</p>[% END %]
                                 <input type="hidden" name="externalid" value="[% externalid %]">
-                                <input type="hidden" name="alerttype" value="[% alerttype %]">
                                 <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 21c6ccd..26fed4e 100644 (file)
--- 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;alerttype=issue&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;externalid=[% 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;alerttype=issue&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?externalid=[% 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 bdd8587..45104bf 100644 (file)
--- 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;alerttype=issue&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;externalid=[% 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;alerttype=issue&amp;referer=serial&amp;biblionumber=[% subscription_LOO.biblionumber %]" class="btn" title="Subscribe to email notification on new issues">
+                                                <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">
                                                     Subscribe to email notification on new issues
                                                 </a>
                                             [% END %]

diff --git a/opac/opac-alert-subscribe.pl b/opac/opac-alert-subscribe.pl
index 6ed153d..7733c36 100755 (executable)
--- a/opac/opac-alert-subscribe.pl
+++ b/opac/opac-alert-subscribe.pl
@@ -37,7 +37,6 @@ my $dbh   = C4::Context->dbh;
 my $sth;
 my ( $template, $loggedinuser, $cookie );
 my $externalid   = $query->param('externalid');
-my $alerttype    = $query->param('alerttype') || '';
 my $referer      = $query->param('referer') || 'detail';
 my $biblionumber = $query->param('biblionumber');
 
@@ -53,7 +52,7 @@ my $biblionumber = $query->param('biblionumber');
 );
 
 if ( $op eq 'alert_confirmed' ) {
-    addalert( $loggedinuser, $alerttype, $externalid );
+    addalert( $loggedinuser, $externalid );
     if ( $referer eq 'serial' ) {
         print $query->redirect(
             "opac-serial-issues.pl?biblionumber=$biblionumber");
@@ -65,8 +64,8 @@ if ( $op eq 'alert_confirmed' ) {
     }
 }
 elsif ( $op eq 'cancel_confirmed' ) {
-    my $alerts = getalert( $loggedinuser, $alerttype, $externalid );
-    warn "CANCEL confirmed : $loggedinuser, $alerttype, $externalid".Data::Dumper::Dumper( $alerts );
+    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} );
@@ -84,17 +83,14 @@ elsif ( $op eq 'cancel_confirmed' ) {
 
 }
 else {
-    if ( $alerttype eq 'issue' ) { # alert for subscription issues
-        my $subscription = &GetSubscription($externalid);
-        $template->param(
-            alerttype      => $alerttype,
-            referer        => $referer,
-            "typeissue$op" => 1,
-            bibliotitle    => $subscription->{bibliotitle},
-            notes          => $subscription->{notes},
-            externalid     => $externalid,
-            biblionumber   => $biblionumber,
-        );
-    }
+    my $subscription = &GetSubscription($externalid);
+    $template->param(
+        referer        => $referer,
+        "typeissue$op" => 1,
+        bibliotitle    => $subscription->{bibliotitle},
+        notes          => $subscription->{notes},
+        externalid     => $externalid,
+        biblionumber   => $biblionumber,
+    );
 }
 output_html_with_http_headers $query, $cookie, $template->output;

diff --git a/opac/opac-detail.pl b/opac/opac-detail.pl
index ff53871..5bd05c9 100755 (executable)
--- a/opac/opac-detail.pl
+++ b/opac/opac-detail.pl
@@ -589,7 +589,7 @@ foreach my $subscription (@subscriptions) {
     $cell{latestserials} =
       GetLatestSerials( $subscription->{subscriptionid}, $serials_to_display );
     if ( $borrowernumber ) {
-        my $sub = getalert($borrowernumber,'issue',$subscription->{subscriptionid});
+        my $sub = getalert($borrowernumber, $subscription->{subscriptionid});
         if (@$sub[0]) {
             $cell{hasalert} = 1;
         }

diff --git a/opac/opac-serial-issues.pl b/opac/opac-serial-issues.pl
index f6d7aa8..b74a886 100755 (executable)
--- a/opac/opac-serial-issues.pl
+++ b/opac/opac-serial-issues.pl
@@ -62,7 +62,7 @@ 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,'issue',$_->{subscriptionid});
+            my $sub = getalert($loggedinuser, $_->{subscriptionid});
             if ($sub) {
                 $_->{hasalert} = 1;
             }
@@ -102,7 +102,7 @@ else {
     # now, check is there is an alert subscription for one of the subscriptions
     if ($loggedinuser){
         foreach (@$subscriptions) {
-            my $subscription = getalert($loggedinuser,'issue',$_->{subscriptionid});
+            my $subscription = getalert($loggedinuser, $_->{subscriptionid});
             if (@$subscription[0]) {
                 $_->{hasalert} = 1;
             }

diff --git a/serials/viewalerts.pl b/serials/viewalerts.pl
index 63ed214..6273f19 100755 (executable)
--- a/serials/viewalerts.pl
+++ b/serials/viewalerts.pl
@@ -43,7 +43,7 @@ my ($template, $loggedinuser, $cookie)
 
 my $subscriptionid=$input->param('subscriptionid');
 
-my $borrowers = getalert('','issue',$subscriptionid);
+my $borrowers = getalert('', $subscriptionid);
 my $subscription = GetSubscription($subscriptionid);
 
 for my $borrowernumber (@$borrowers) {

diff --git a/t/db_dependent/Letters.t b/t/db_dependent/Letters.t
index 1a61793..69a9d30 100644 (file)
--- 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 => 77;
+use Test::More tests => 75;
 use Test::MockModule;
 use Test::Warn;
 
@@ -237,9 +237,8 @@ my $letter14206_c = C4::Letters::getletter('my module', $overdue_rules->{"letter
 is( $letter14206_c->{message_transport_type}, 'print', 'Bug 14206 - correct mtt detected for call from overdue_notices.pl' );
 
 # addalert
-my $type = 'my type';
 my $externalid = 'my external id';
-my $alert_id = C4::Letters::addalert($borrowernumber, $type, $externalid);
+my $alert_id = C4::Letters::addalert($borrowernumber, $externalid);
 isnt( $alert_id, undef, 'addalert does not return undef' );
 
 
@@ -249,16 +248,13 @@ 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]->{type}, $type, 'addalert stores the type correctly' );
 is( $alerts->[0]->{externalid}, $externalid, 'addalert stores the externalid correctly' );
 
-$alerts = C4::Letters::getalert($borrowernumber, $type);
+$alerts = C4::Letters::getalert($borrowernumber);
 is( @$alerts, 1, 'getalert returns the correct number of alerts' );
-$alerts = C4::Letters::getalert($borrowernumber, $type, $externalid);
+$alerts = C4::Letters::getalert($borrowernumber, $externalid);
 is( @$alerts, 1, 'getalert returns the correct number of alerts' );
-$alerts = C4::Letters::getalert($borrowernumber, 'another type');
-is( @$alerts, 0, 'getalert returns the correct number of alerts' );
-$alerts = C4::Letters::getalert($borrowernumber, $type, 'another external id');
+$alerts = C4::Letters::getalert($borrowernumber, 'another external id');
 is( @$alerts, 0, 'getalert returns the correct number of alerts' );
 
 
@@ -513,7 +509,7 @@ my $borrowernumber = AddMember(
     dateofbirth  => $date,
     email        => 'john.smith@test.de',
 );
-my $alert_id = C4::Letters::addalert($borrowernumber, 'issue', $subscriptionid);
+my $alert_id = C4::Letters::addalert($borrowernumber, $subscriptionid);
 
 
 my $err2;