Bugzilla – Attachment 27452 Details for
Bug 10933
Merge PurgeSearchHistory into the C4::Search::History module
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 10933: The PurgeSearchHistory should be merge into the C4::Search::History module
Bug-10933-The-PurgeSearchHistory-should-be-merge-i.patch (text/plain), 11.43 KB, created by
Jonathan Druart
on 2014-04-23 08:05:54 UTC
(
hide
)
Description:
Bug 10933: The PurgeSearchHistory should be merge into the C4::Search::History module
Filename:
MIME Type:
Creator:
Jonathan Druart
Created:
2014-04-23 08:05:54 UTC
Size:
11.43 KB
patch
obsolete
>From b9af3ac67b206e0f7b5909c3825b342973dabcd9 Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@biblibre.com> >Date: Mon, 23 Sep 2013 13:34:19 +0200 >Subject: [PATCH] Bug 10933: The PurgeSearchHistory should be merge into the > C4::Search::History module > >Since bug 10803 adds a C4::Search::History module, the >PurgeSearchHistory routine should be moved. > >Test plan: >- run misc/cronjobs/cleanup_database.pl with the searchhistory param and >verify behavior is the same as before applying this patch. >- run prove t/Search/History.t >--- > C4/Search.pm | 8 ---- > C4/Search/History.pm | 13 ++++-- > misc/cronjobs/cleanup_database.pl | 2 +- > t/db_dependent/Search/History.t | 61 +++++++++++++++++++++++++++- > t/db_dependent/Search_SearchHistory.t | 72 --------------------------------- > 5 files changed, 71 insertions(+), 85 deletions(-) > delete mode 100644 t/db_dependent/Search_SearchHistory.t > >diff --git a/C4/Search.pm b/C4/Search.pm >index cc3d09e..3f58687 100644 >--- a/C4/Search.pm >+++ b/C4/Search.pm >@@ -70,7 +70,6 @@ This module provides searching functions for Koha's bibliographic databases > &buildQuery > &GetDistinctValues > &enabled_staff_search_views >- &PurgeSearchHistory > ); > > # make all your functions, whether exported or not; >@@ -2222,13 +2221,6 @@ sub enabled_staff_search_views > ); > } > >-sub PurgeSearchHistory{ >- my ($pSearchhistory)=@_; >- my $dbh = C4::Context->dbh; >- my $sth = $dbh->prepare("DELETE FROM search_history WHERE time < DATE_SUB( NOW(), INTERVAL ? DAY )"); >- $sth->execute($pSearchhistory) or die $dbh->errstr; >-} >- > =head2 z3950_search_args > > $arrayref = z3950_search_args($matchpoints) >diff --git a/C4/Search/History.pm b/C4/Search/History.pm >index e24a65a..2f2638a 100644 >--- a/C4/Search/History.pm >+++ b/C4/Search/History.pm >@@ -18,6 +18,7 @@ sub add { > my $query_cgi = $params->{query_cgi}; > my $total = $params->{total} // 0; > my $type = $params->{type} || 'biblio'; >+ my $time = $params->{time}; > > my $dbh = C4::Context->dbh; > >@@ -26,12 +27,12 @@ sub add { > INSERT INTO search_history( > userid, sessionid, query_desc, query_cgi, type, total, time > ) VALUES( >- ?, ?, ?, ?, ?, ?, NOW() >+ ?, ?, ?, ?, ?, ?, ? > ) > }; > my $sth = $dbh->prepare($query); > $sth->execute( $userid, $sessionid, $query_desc, $query_cgi, $type, >- $total ); >+ $total, $time ); > } > > sub add_to_session { >@@ -62,6 +63,7 @@ sub delete { > my $sessionid = $params->{sessionid}; > my $type = $params->{type} || q{}; > my $previous = $params->{previous} || 0; >+ my $interval = $params->{interval} || 0; > > unless ($userid) { > warn "ERROR: userid is required for history search"; >@@ -84,10 +86,15 @@ sub delete { > $query .= q{ AND type = ?} > if $type; > >+ # FIXME DATE_SUB is a Mysql-ism. Postgres uses: datefield - INTERVAL '6 months' >+ $query .= q{ AND time < DATE_SUB( NOW(), INTERVAL ? DAY )} >+ if $interval; >+ > $dbh->do( > $query, {}, $userid, > ( $sessionid ? $sessionid : () ), >- ( $type ? $type : () ) >+ ( $type ? $type : () ), >+ ( $interval ? $interval : () ), > ); > } > >diff --git a/misc/cronjobs/cleanup_database.pl b/misc/cronjobs/cleanup_database.pl >index fa27e9b..2a1f59d 100755 >--- a/misc/cronjobs/cleanup_database.pl >+++ b/misc/cronjobs/cleanup_database.pl >@@ -209,7 +209,7 @@ if($pLogs) { > > if($pSearchhistory) { > print "Purging records older than $pSearchhistory from search_history.\n" if $verbose; >- PurgeSearchHistory($pSearchhistory); >+ C4::Search::History::delete({ interval => $pSearchhistory }); > print "Done with purging search_history.\n" if $verbose; > } > >diff --git a/t/db_dependent/Search/History.t b/t/db_dependent/Search/History.t >index 33e834d..b6583f5 100644 >--- a/t/db_dependent/Search/History.t >+++ b/t/db_dependent/Search/History.t >@@ -2,7 +2,7 @@ > > use Modern::Perl; > >-use Test::More tests => 16; >+use Test::More tests => 22; > use URI::Escape; > > use C4::Context; >@@ -127,9 +127,59 @@ $all = C4::Search::History::get({userid => $userid}); > is( scalar(@$all), 9, 'There are still 9 searches after calling delete without userid' ); > delete_all( $userid ); > >+# Test delete with interval >+add( $userid, $current_sessionid, $previous_sessionid, $total, $query_cgi_b, $query_cgi_a ); >+C4::Search::History::delete({ >+ userid => $userid, >+ interval => 10, >+}); >+$all = C4::Search::History::get({userid => $userid}); >+is( scalar(@$all), 9, 'There are still 9 searches after calling delete with an interval = 10 days' ); >+C4::Search::History::delete({ >+ userid => $userid, >+ interval => 6, >+}); >+$all = C4::Search::History::get({userid => $userid}); >+is( scalar(@$all), 8, 'There are still 8 searches after calling delete with an interval = 6 days' ); >+C4::Search::History::delete({ >+ userid => $userid, >+ interval => 2, >+}); >+$all = C4::Search::History::get({userid => $userid}); >+is( scalar(@$all), 2, 'There are still 2 searches after calling delete with an interval = 2 days' ); >+delete_all( $userid ); >+ >+add( $userid, $current_sessionid, $previous_sessionid, $total, $query_cgi_b, $query_cgi_a ); >+C4::Search::History::delete({ >+ userid => $userid, >+ interval => 5, >+ type => 'biblio', >+}); >+$all = C4::Search::History::get({userid => $userid}); >+is( scalar(@$all), 8, 'There are still 9 searches after calling delete with an interval = 5 days for biblio' ); >+C4::Search::History::delete({ >+ userid => $userid, >+ interval => 5, >+ type => 'authority', >+}); >+$all = C4::Search::History::get({userid => $userid}); >+is( scalar(@$all), 6, 'There are still 6 searches after calling delete with an interval = 5 days for authority' ); >+C4::Search::History::delete({ >+ userid => $userid, >+ interval => -1, >+}); >+$all = C4::Search::History::get({userid => $userid}); >+is( scalar(@$all), 0, 'There is no search after calling delete with an interval = -1 days' ); >+delete_all( $userid ); >+ > sub add { > my ( $userid, $current_session_id, $previous_sessionid, $total, $query_cgi_b, $query_cgi_a ) = @_; > >+ my $days_ago_2 = dt_from_string()->add_duration( DateTime::Duration->new( days => -2 ) ); >+ my $days_ago_4 = dt_from_string()->add_duration( DateTime::Duration->new( days => -4 ) ); >+ my $days_ago_6 = dt_from_string()->add_duration( DateTime::Duration->new( days => -6 ) ); >+ my $days_ago_8 = dt_from_string()->add_duration( DateTime::Duration->new( days => -8 ) ); >+ > my $query_desc_b1_p = q{first previous biblio search}; > my $first_previous_biblio_search = { > userid => $userid, >@@ -138,6 +188,7 @@ sub add { > query_cgi => $query_cgi_b, > total => $total, > type => 'biblio', >+ time => $days_ago_2, > }; > > my $query_desc_a1_p = q{first previous authority search}; >@@ -148,6 +199,7 @@ sub add { > query_cgi => $query_cgi_a, > total => $total, > type => 'authority', >+ time => $days_ago_2, > }; > > my $query_desc_b2_p = q{second previous biblio search}; >@@ -158,6 +210,7 @@ sub add { > query_cgi => $query_cgi_b, > total => $total, > type => 'biblio', >+ time => $days_ago_4, > }; > > my $query_desc_a2_p = q{second previous authority search}; >@@ -168,6 +221,7 @@ sub add { > query_cgi => $query_cgi_a, > total => $total, > type => 'authority', >+ time => $days_ago_4, > }; > > >@@ -180,6 +234,7 @@ sub add { > query_cgi => $query_cgi_b, > total => $total, > type => 'biblio', >+ time => $days_ago_4, > }; > > my $query_desc_a1_c = q{first current authority search}; >@@ -190,6 +245,7 @@ sub add { > query_cgi => $query_cgi_a, > total => $total, > type => 'authority', >+ time => $days_ago_4, > }; > > my $query_desc_b2_c = q{second current biblio search}; >@@ -200,6 +256,7 @@ sub add { > query_cgi => $query_cgi_b, > total => $total, > type => 'biblio', >+ time => $days_ago_6, > }; > > my $query_desc_a2_c = q{second current authority search}; >@@ -210,6 +267,7 @@ sub add { > query_cgi => $query_cgi_a, > total => $total, > type => 'authority', >+ time => $days_ago_6, > }; > > my $query_desc_a3_c = q{third current authority search}; >@@ -220,6 +278,7 @@ sub add { > query_cgi => $query_cgi_a, > total => $total, > type => 'authority', >+ time => $days_ago_8, > }; > > >diff --git a/t/db_dependent/Search_SearchHistory.t b/t/db_dependent/Search_SearchHistory.t >deleted file mode 100644 >index 5f21e3e..0000000 >--- a/t/db_dependent/Search_SearchHistory.t >+++ /dev/null >@@ -1,72 +0,0 @@ >-#!/usr/bin/perl >- >-# This file is part of Koha. >-# >-# Copyright 2013 Equinox Software, Inc. >-# >-# Koha is free software; you can redistribute it and/or modify it under the >-# terms of the GNU General Public License as published by the Free Software >-# Foundation; either version 3 of the License, or (at your option) any later >-# version. >-# >-# Koha is distributed in the hope that it will be useful, but WITHOUT ANY >-# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR >-# A PARTICULAR PURPOSE. See the GNU General Public License for more details. >-# >-# You should have received a copy of the GNU General Public License along >-# with Koha; if not, see <http://www.gnu.org/licenses>. >- >-use Modern::Perl; >- >-use Test::More tests => 6; >- >-use C4::Context; >-use_ok('C4::Search', qw/PurgeSearchHistory/); # GetSearchHistory is not exported >-use C4::Search::History; >- >-# Start transaction >-my $dbh = C4::Context->dbh; >-$dbh->{AutoCommit} = 0; >-$dbh->{RaiseError} = 1; >- >-# start with a clean slate >-$dbh->do('DELETE FROM search_history'); >-is(_get_history_count(), 0, 'starting off with nothing in search_history'); >- >-# Add some history rows. Note that we're ignoring the return value; >-# since the search_history table doesn't have an auto_increment >-# column, it appears that the value of $dbh->last_insert_id() is >-# useless for determining if the insert failed. >-C4::Search::History::add({userid => 12345, sessionid => 'session_1', query_desc => 'query_desc_1', query_cgi => 'query_cgi_1', total => 5}); >-C4::Search::History::add({userid => 12345, sessionid => 'session_1', query_desc => 'query_desc_2', query_cgi => 'query_cgi_3', total => 6}); >-C4::Search::History::add({userid => 12345, sessionid => 'session_1', query_desc => 'query_desc_3', query_cgi => 'query_cgi_3', total => 7}); >-C4::Search::History::add({userid => 56789, sessionid => 'session_2', query_desc => 'query_desc_4', query_cgi => 'query_cgi_4', total => 8}); >-is(_get_history_count(), 4, 'successfully added four search_history rows'); >- >-# We're not testing GetSearchHistory at present because it is broken... >-# see bug 10677 >- >-# munge some dates >-my $sth = $dbh->prepare(' >- UPDATE search_history >- SET time = DATE_SUB(time, INTERVAL ? DAY) >- WHERE query_desc = ? >-'); >-$sth->execute(46, 'query_desc_1'); >-$sth->execute(31, 'query_desc_2'); >- >-PurgeSearchHistory(45); >-is(_get_history_count(), 3, 'purged history older than 45 days'); >-PurgeSearchHistory(30); >-is(_get_history_count(), 2, 'purged history older than 30 days'); >-PurgeSearchHistory(-1); >-is(_get_history_count(), 0, 'purged all history'); >- >-sub _get_history_count { >- my $count_sth = $dbh->prepare('SELECT COUNT(*) FROM search_history'); >- $count_sth->execute(); >- my $count = $count_sth->fetchall_arrayref(); >- return $count->[0]->[0]; >-} >- >-$dbh->rollback; >-- >1.7.10.4
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=27452">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 10933
:
21364
|
22058
|
27452
|
28022
|
42114
|
42116
|
43786
|
43787
|
44095