From c4bd20f7af74812ae07937ec5e5e19fe08764042 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). Signed-off-by: Aleisha Signed-off-by: Kyle M Hall --- 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