Bugzilla – Attachment 73624 Details for
Bug 19855
Move the "alert" code to Koha::Subscription
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 19855: Remove $type from the alerts
Bug-19855-Remove-type-from-the-alerts.patch (text/plain), 16.85 KB, created by
Jonathan Druart
on 2018-04-04 13:01:48 UTC
(
hide
)
Description:
Bug 19855: Remove $type from the alerts
Filename:
MIME Type:
Creator:
Jonathan Druart
Created:
2018-04-04 13:01:48 UTC
Size:
16.85 KB
patch
obsolete
>From 7008a2c44c148bd97ab47f9df2635c9ed3d96c9b Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >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 18841fc674..de3936df85 100644 >--- a/C4/Letters.pm >+++ b/C4/Letters.pm >@@ -265,24 +265,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'}; >@@ -304,18 +303,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; >@@ -323,13 +321,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); >@@ -385,7 +379,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 @@ > <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 f4b823c2c7..e091fc7d56 100644 >--- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt >+++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt >@@ -833,9 +833,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&externalid=[% subscription.subscriptionid %]&alerttype=issue&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&externalid=[% subscription.subscriptionid %]&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 %]&alerttype=issue&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 %]&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 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 <a href="opac-alert-subscribe.pl?op=cancel&externalid=[% subscription_LOO.subscriptionid %]&alerttype=issue&referer=serial&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&externalid=[% subscription_LOO.subscriptionid %]&referer=serial&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 %]&alerttype=issue&referer=serial&biblionumber=[% subscription_LOO.biblionumber %]" class="btn" title="Subscribe to email notification on new issues"> >+ <a href="opac-alert-subscribe.pl?externalid=[% subscription_LOO.subscriptionid %]&referer=serial&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 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 ff53871773..5bd05c9be7 100755 >--- 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 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 63ed214a91..6273f19626 100755 >--- 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 1a61793d5e..69a9d30fa0 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 => 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; >-- >2.11.0
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=73624">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
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