Bugzilla – Attachment 37739 Details for
Bug 11337
calls to GetSubscriptions can be replaced by Searchsubscriptions
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 11337: Remove the C4::Serials::GetSubscriptions routine
Bug-11337-Remove-the-C4SerialsGetSubscriptions-rou.patch (text/plain), 9.95 KB, created by
Jonathan Druart
on 2015-04-13 13:27:43 UTC
(
hide
)
Description:
Bug 11337: Remove the C4::Serials::GetSubscriptions routine
Filename:
MIME Type:
Creator:
Jonathan Druart
Created:
2015-04-13 13:27:43 UTC
Size:
9.95 KB
patch
obsolete
>From c4bd20f7af74812ae07937ec5e5e19fe08764042 Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@biblibre.com> >Date: Wed, 4 Dec 2013 15:23:17 +0100 >Subject: [PATCH] Bug 11337: Remove the C4::Serials::GetSubscriptions routine > >Since SearchSubscriptions is the way to search subscriptions, each call >to GetSubscriptions could be replaced by a call to SearchSubscriptions. > >Test plan: >Verify following pages display the same thing as before this patch: >- catalogue/detail.pl >- opac/opac-ISBDdetail.pl >- opac/opac-detail.pl > >Verify the following page returns correct results: >- serials/checkexpiration.pl > >Verify the Serials UT file still passes: >- prove t/db_dependent/Serials.t > >Note: The title filter on checkexpiration now only searches on the title >DB field. I don't think it is a regression, it should be the way to use >this field. Maybe should we add new search fields on this form. >Bug 5337 reintroduces a bug fixed by bug 5864, this patch restore the >right way to search subscription (based on biblionumber). > >Signed-off-by: Aleisha <aleishaamohia@hotmail.com> > >Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> >--- > C4/Serials.pm | 88 ++++------------------------------------------ > catalogue/detail.pl | 2 +- > opac/opac-ISBDdetail.pl | 2 +- > opac/opac-detail.pl | 2 +- > serials/checkexpiration.pl | 2 +- > t/db_dependent/Serials.t | 8 ++--- > 6 files changed, 14 insertions(+), 90 deletions(-) > >diff --git a/C4/Serials.pm b/C4/Serials.pm >index 538d4bd..c3f7fbc 100644 >--- a/C4/Serials.pm >+++ b/C4/Serials.pm >@@ -39,7 +39,7 @@ BEGIN { > require Exporter; > @ISA = qw(Exporter); > @EXPORT = qw( >- &NewSubscription &ModSubscription &DelSubscription &GetSubscriptions >+ &NewSubscription &ModSubscription &DelSubscription > &GetSubscription &CountSubscriptionFromBiblionumber &GetSubscriptionsFromBiblionumber > &SearchSubscriptions > &GetFullSubscriptionsFromBiblionumber &GetFullSubscription &ModSubscriptionHistory >@@ -496,85 +496,6 @@ sub GetFullSubscriptionsFromBiblionumber { > return $subscriptions; > } > >-=head2 GetSubscriptions >- >-@results = GetSubscriptions($title,$ISSN,$ean,$biblionumber); >-this function gets all subscriptions which have title like $title,ISSN like $ISSN,EAN like $ean and biblionumber like $biblionumber. >-return: >-a table of hashref. Each hash containt the subscription. >- >-=cut >- >-sub GetSubscriptions { >- my ( $string, $issn, $ean, $biblionumber ) = @_; >- >- #return unless $title or $ISSN or $biblionumber; >- my $dbh = C4::Context->dbh; >- my $sth; >- my $sql = qq( >- SELECT subscriptionhistory.*, subscription.*, biblio.title,biblioitems.issn,biblio.biblionumber >- FROM subscription >- LEFT JOIN subscriptionhistory USING(subscriptionid) >- LEFT JOIN biblio ON biblio.biblionumber = subscription.biblionumber >- LEFT JOIN biblioitems ON biblio.biblionumber = biblioitems.biblionumber >- ); >- my @bind_params; >- my $sqlwhere = q{}; >- if ($biblionumber) { >- $sqlwhere = " WHERE biblio.biblionumber=?"; >- push @bind_params, $biblionumber; >- } >- if ($string) { >- my @sqlstrings; >- my @strings_to_search; >- @strings_to_search = map { "%$_%" } split( / /, $string ); >- foreach my $index (qw(biblio.title subscription.callnumber subscription.location subscription.notes subscription.internalnotes)) { >- push @bind_params, @strings_to_search; >- my $tmpstring = "AND $index LIKE ? " x scalar(@strings_to_search); >- $debug && warn "$tmpstring"; >- $tmpstring =~ s/^AND //; >- push @sqlstrings, $tmpstring; >- } >- $sqlwhere .= ( $sqlwhere ? " AND " : " WHERE " ) . "((" . join( ") OR (", @sqlstrings ) . "))"; >- } >- if ($issn) { >- my @sqlstrings; >- my @strings_to_search; >- @strings_to_search = map { "%$_%" } split( / /, $issn ); >- foreach my $index ( qw(biblioitems.issn subscription.callnumber)) { >- push @bind_params, @strings_to_search; >- my $tmpstring = "OR $index LIKE ? " x scalar(@strings_to_search); >- $debug && warn "$tmpstring"; >- $tmpstring =~ s/^OR //; >- push @sqlstrings, $tmpstring; >- } >- $sqlwhere .= ( $sqlwhere ? " AND " : " WHERE " ) . "((" . join( ") OR (", @sqlstrings ) . "))"; >- } >- if ($ean) { >- my @sqlstrings; >- my @strings_to_search; >- @strings_to_search = map { "$_" } split( / /, $ean ); >- foreach my $index ( qw(biblioitems.ean) ) { >- push @bind_params, @strings_to_search; >- my $tmpstring = "OR $index = ? " x scalar(@strings_to_search); >- $debug && warn "$tmpstring"; >- $tmpstring =~ s/^OR //; >- push @sqlstrings, $tmpstring; >- } >- $sqlwhere .= ( $sqlwhere ? " AND " : " WHERE " ) . "((" . join( ") OR (", @sqlstrings ) . "))"; >- } >- >- $sql .= "$sqlwhere ORDER BY title"; >- $debug and warn "GetSubscriptions query: $sql params : ", join( " ", @bind_params ); >- $sth = $dbh->prepare($sql); >- $sth->execute(@bind_params); >- my $subscriptions = $sth->fetchall_arrayref( {} ); >- for my $subscription ( @$subscriptions ) { >- $subscription->{cannotedit} = not can_edit_subscription( $subscription ); >- } >- return @$subscriptions; >-} >- > =head2 SearchSubscriptions > > @results = SearchSubscriptions($args); >@@ -604,14 +525,15 @@ subscription expiration date. > sub SearchSubscriptions { > my ( $args ) = @_; > >- my $query = qq{ >+ my $query = q{ > SELECT > subscription.notes AS publicnotes, >- subscription.*, > subscriptionhistory.*, >+ subscription.*, > biblio.notes AS biblionotes, > biblio.title, > biblio.author, >+ biblio.biblionumber, > biblioitems.issn > FROM subscription > LEFT JOIN subscriptionhistory USING(subscriptionid) >@@ -677,6 +599,8 @@ sub SearchSubscriptions { > $query .= " WHERE " . join(" AND ", @where_strs); > } > >+ $query .= " ORDER BY " . $args->{orderby} if $args->{orderby}; >+ > my $dbh = C4::Context->dbh; > my $sth = $dbh->prepare($query); > $sth->execute(@where_args); >diff --git a/catalogue/detail.pl b/catalogue/detail.pl >index adf6823..2ac5324 100755 >--- a/catalogue/detail.pl >+++ b/catalogue/detail.pl >@@ -141,7 +141,7 @@ my $dat = &GetBiblioData($biblionumber); > > #coping with subscriptions > my $subscriptionsnumber = CountSubscriptionFromBiblionumber($biblionumber); >-my @subscriptions = GetSubscriptions( $dat->{title}, $dat->{issn}, undef, $biblionumber ); >+my @subscriptions = SearchSubscriptions({ biblionumber => $biblionumber, orderby => 'title' }); > my @subs; > > foreach my $subscription (@subscriptions) { >diff --git a/opac/opac-ISBDdetail.pl b/opac/opac-ISBDdetail.pl >index 33cf4f3..4a52db5 100755 >--- a/opac/opac-ISBDdetail.pl >+++ b/opac/opac-ISBDdetail.pl >@@ -121,7 +121,7 @@ my $subscriptionsnumber = CountSubscriptionFromBiblionumber($biblionumber); > my $dbh = C4::Context->dbh; > my $dat = TransformMarcToKoha( $dbh, $record ); > >-my @subscriptions = GetSubscriptions( $dat->{title}, $dat->{issn}, undef, $biblionumber ); >+my @subscriptions = SearchSubscriptions({ biblionumber => $biblionumber, orderby => 'title' }); > my @subs; > foreach my $subscription (@subscriptions) { > my %cell; >diff --git a/opac/opac-detail.pl b/opac/opac-detail.pl >index c5f9a6f..c8ee1f5 100755 >--- a/opac/opac-detail.pl >+++ b/opac/opac-detail.pl >@@ -530,7 +530,7 @@ my $copynumbers = GetKohaAuthorisedValues('items.copynumber',$dat->{'frameworkco > > #coping with subscriptions > my $subscriptionsnumber = CountSubscriptionFromBiblionumber($biblionumber); >-my @subscriptions = GetSubscriptions($dat->{'title'}, $dat->{'issn'}, $ean, $biblionumber ); >+my @subscriptions = SearchSubscriptions({ biblionumber => $biblionumber, orderby => 'title' }); > > my @subs; > $dat->{'serial'}=1 if $subscriptionsnumber; >diff --git a/serials/checkexpiration.pl b/serials/checkexpiration.pl >index 9feeaad..7eae5f9 100755 >--- a/serials/checkexpiration.pl >+++ b/serials/checkexpiration.pl >@@ -70,7 +70,7 @@ my $issn = $query->param('issn'); > my $date = format_date_in_iso($query->param('date')); > > if ($date) { >- my @subscriptions = GetSubscriptions( $title, $issn ); >+ my @subscriptions = SearchSubscriptions({ title => $title, issn => $issn, orderby => 'title' }); > my @subscriptions_loop; > > foreach my $subscription ( @subscriptions ) { >diff --git a/t/db_dependent/Serials.t b/t/db_dependent/Serials.t >index d04dd02..4777116 100644 >--- a/t/db_dependent/Serials.t >+++ b/t/db_dependent/Serials.t >@@ -85,16 +85,16 @@ my $subscription_history = C4::Serials::GetSubscriptionHistoryFromSubscriptionId > is( $subscription_history->{opacnote}, '', 'NewSubscription should not set subscriptionhistory opacnotes' ); > is( $subscription_history->{librariannote}, '', 'NewSubscription should not set subscriptionhistory librariannotes' ); > >-my @subscriptions = GetSubscriptions( $$subscriptioninformation{bibliotitle} ); >+my @subscriptions = SearchSubscriptions({string => $subscriptioninformation->{bibliotitle}, orderby => 'title' }); > isa_ok( \@subscriptions, 'ARRAY' ); > >-@subscriptions = GetSubscriptions( undef, $$subscriptioninformation{issn} ); >+@subscriptions = SearchSubscriptions({ issn => $subscriptioninformation->{issn}, orderby => 'title' }); > isa_ok( \@subscriptions, 'ARRAY' ); > >-@subscriptions = GetSubscriptions( undef, undef, $$subscriptioninformation{ean} ); >+@subscriptions = SearchSubscriptions({ ean => $subscriptioninformation->{ean}, orderby => 'title' }); > isa_ok( \@subscriptions, 'ARRAY' ); > >-@subscriptions = GetSubscriptions( undef, undef, undef, $$subscriptioninformation{bibnum} ); >+@subscriptions = SearchSubscriptions({ biblionumber => $subscriptioninformation->{bibnum}, orderby => 'title' }); > isa_ok( \@subscriptions, 'ARRAY' ); > > my $frequency = GetSubscriptionFrequency($subscriptioninformation->{periodicity}); >-- >2.1.0
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=37739">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 11337
:
23297
|
35482
|
35514
| 37739