From 091f50819b562dfe332e18a37df375227f6d2544 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Wed, 20 Dec 2017 14:26:39 -0300 Subject: [PATCH] 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 --- C4/Letters.pm | 32 +++++++++------------- .../bootstrap/en/modules/opac-alert-subscribe.tt | 2 -- .../opac-tmpl/bootstrap/en/modules/opac-detail.tt | 4 +-- .../bootstrap/en/modules/opac-serial-issues.tt | 4 +-- opac/opac-alert-subscribe.pl | 28 ++++++++----------- opac/opac-detail.pl | 2 +- opac/opac-serial-issues.pl | 4 +-- serials/viewalerts.pl | 2 +- t/db_dependent/Letters.t | 16 ++++------- 9 files changed, 39 insertions(+), 55 deletions(-) diff --git a/C4/Letters.pm b/C4/Letters.pm index ffbf0060f1..0c5ad9b7c5 100644 --- a/C4/Letters.pm +++ b/C4/Letters.pm @@ -262,24 +262,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'}; @@ -301,18 +300,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; @@ -320,13 +318,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); @@ -382,7 +376,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 83408b7da5..adba9f3306 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 @@ -25,7 +25,6 @@

[% bibliotitle %]

[% IF ( notes ) %]

[% notes %]

[% END %] - @@ -40,7 +39,6 @@

[% 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 e87d152c19..fd97133967 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt @@ -828,9 +828,9 @@ [% IF ( subscription.letter ) %] [% IF ( loggedinusername ) %] [% IF ( subscription.hasalert ) %] - You have subscribed to email notification on new issues. Cancel email notification + You have subscribed to email notification on new issues. Cancel email notification [% ELSE %] - Subscribe to email notification on new issues + Subscribe to email notification on new issues [% END %] [% ELSE %] You must log in if you want to subscribe to email notification on new issues 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 bdd8587918..45104bfb8e 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 + You have subscribed to email notification on new issues Cancel email notification [% ELSE %] - + Subscribe to email notification on new issues [% END %] diff --git a/opac/opac-alert-subscribe.pl b/opac/opac-alert-subscribe.pl index 6ed153d9bd..7733c36c26 100755 --- 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 819e42692f..5c4e2bb96d 100755 --- a/opac/opac-detail.pl +++ b/opac/opac-detail.pl @@ -584,7 +584,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 f6d7aa8a1d..b74a886698 100755 --- 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 8302c988c9..fe7242f901 100755 --- a/serials/viewalerts.pl +++ b/serials/viewalerts.pl @@ -44,7 +44,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 e20cefb8b9..da3fc21433 100644 --- a/t/db_dependent/Letters.t +++ b/t/db_dependent/Letters.t @@ -18,7 +18,7 @@ # along with Koha; if not, see . use Modern::Perl; -use Test::More tests => 77; +use Test::More tests => 75; use Test::MockModule; use Test::Warn; @@ -236,9 +236,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' ); @@ -248,16 +247,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' ); @@ -512,7 +508,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; -- 2.11.0