Bugzilla – Attachment 43786 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]
[PASSED QA] Bug 10933: The PurgeSearchHistory should be merge into the C4::Search::History module
PASSED-QA-Bug-10933-The-PurgeSearchHistory-should-.patch (text/plain), 11.71 KB, created by
Katrin Fischer
on 2015-10-22 20:56:34 UTC
(
hide
)
Description:
[PASSED QA] Bug 10933: The PurgeSearchHistory should be merge into the C4::Search::History module
Filename:
MIME Type:
Creator:
Katrin Fischer
Created:
2015-10-22 20:56:34 UTC
Size:
11.71 KB
patch
obsolete
>From 57ee213e7c9bcc6a20ae4a6ca52804399f2bf45b 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] [PASSED QA] Bug 10933: The PurgeSearchHistory should be merge > into the C4::Search::History module >MIME-Version: 1.0 >Content-Type: text/plain; charset=UTF-8 >Content-Transfer-Encoding: 8bit > >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 > >Signed-off-by: Joonas Kylmälä <j.kylmala@gmail.com> > >Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> >--- > C4/Search.pm | 8 ---- > C4/Search/History.pm | 13 +++++-- > misc/cronjobs/cleanup_database.pl | 2 +- > t/db_dependent/Search/History.t | 63 +++++++++++++++++++++++++++++++- > t/db_dependent/Search_SearchHistory.t | 69 ----------------------------------- > 5 files changed, 73 insertions(+), 82 deletions(-) > delete mode 100644 t/db_dependent/Search_SearchHistory.t > >diff --git a/C4/Search.pm b/C4/Search.pm >index e0f34bf..277ba6c 100644 >--- a/C4/Search.pm >+++ b/C4/Search.pm >@@ -69,7 +69,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; >@@ -2428,13 +2427,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 c80310e..acb68e7 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 { >@@ -66,6 +67,7 @@ sub delete { > my $sessionid = $params->{sessionid}; > my $type = $params->{type} || q{}; > my $previous = $params->{previous} || 0; >+ my $interval = $params->{interval} || 0; > > unless ( ref( $id ) ) { > $id = $id ? [ $id ] : []; >@@ -99,12 +101,17 @@ 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, {}, > ( @$id ? ( @$id ) : () ), > ( $userid ? $userid : () ), > ( $sessionid ? $sessionid : () ), >- ( $type ? $type : () ) >+ ( $type ? $type : () ), >+ ( $interval ? $interval : () ), > ); > } > >diff --git a/misc/cronjobs/cleanup_database.pl b/misc/cronjobs/cleanup_database.pl >index 0766141..41a41ae 100755 >--- a/misc/cronjobs/cleanup_database.pl >+++ b/misc/cronjobs/cleanup_database.pl >@@ -243,7 +243,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 80c2b99..6c067b3 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 => 19; >+use Test::More tests => 25; > use Test::Warn; > use URI::Escape; > use List::Util qw( shuffle ); >@@ -150,11 +150,63 @@ $ids = [ shuffle map { $_->{id} } @$all ]; > C4::Search::History::delete({ id => [ @$ids[0..4] ] }); > $all = C4::Search::History::get({ userid => $userid }); > is( scalar(@$all), 4, 'There are 4 searches after calling delete with 5 ids' ); >+ >+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, >@@ -163,6 +215,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}; >@@ -173,6 +226,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}; >@@ -183,6 +237,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}; >@@ -193,6 +248,7 @@ sub add { > query_cgi => $query_cgi_a, > total => $total, > type => 'authority', >+ time => $days_ago_4, > }; > > >@@ -205,6 +261,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}; >@@ -215,6 +272,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}; >@@ -225,6 +283,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}; >@@ -235,6 +294,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}; >@@ -245,6 +305,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 e328502..0000000 >--- a/t/db_dependent/Search_SearchHistory.t >+++ /dev/null >@@ -1,69 +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'); >- >-# 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.9.1
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=43786">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