From 18585fc8a3dc7dfb0974ef270f16529efdf4150a Mon Sep 17 00:00:00 2001 From: Jonathan Druart 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). --- 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 7e17a04..bbb3821 100644 --- a/C4/Serials.pm +++ b/C4/Serials.pm @@ -38,7 +38,7 @@ BEGIN { require Exporter; @ISA = qw(Exporter); @EXPORT = qw( - &NewSubscription &ModSubscription &DelSubscription &GetSubscriptions + &NewSubscription &ModSubscription &DelSubscription &GetSubscription &CountSubscriptionFromBiblionumber &GetSubscriptionsFromBiblionumber &SearchSubscriptions &GetFullSubscriptionsFromBiblionumber &GetFullSubscription &ModSubscriptionHistory @@ -547,85 +547,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); @@ -641,14 +562,15 @@ a table of hashref. Each hash containt the subscription. 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) @@ -714,6 +636,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 955dbf5..872c4e5 100755 --- a/catalogue/detail.pl +++ b/catalogue/detail.pl @@ -142,7 +142,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 2579a8b..0fc66e2 100755 --- a/opac/opac-ISBDdetail.pl +++ b/opac/opac-ISBDdetail.pl @@ -124,7 +124,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 77de12f..5ec97c5 100755 --- a/opac/opac-detail.pl +++ b/opac/opac-detail.pl @@ -499,7 +499,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 8dff415..66b1e14 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 e942982..5d3cb1d 100644 --- a/t/db_dependent/Serials.t +++ b/t/db_dependent/Serials.t @@ -71,16 +71,16 @@ my $subscriptionid = NewSubscription( my $subscriptioninformation = GetSubscription( $subscriptionid ); -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}); -- 1.7.10.4