Bug 10933: The PurgeSearchHistory should be merge into the C4::Search::History module
authorJonathan Druart <jonathan.druart@biblibre.com>
Mon, 23 Sep 2013 11:34:19 +0000 (13:34 +0200)
committerTomas Cohen Arazi <tomascohen@theke.io>
Tue, 27 Oct 2015 14:03:03 +0000 (11:03 -0300)
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>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
C4/Search.pm
C4/Search/History.pm
misc/cronjobs/cleanup_database.pl
t/db_dependent/Search/History.t
t/db_dependent/Search_SearchHistory.t [deleted file]

index 1a8bba0..c9d2529 100644 (file)
@@ -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;
@@ -2427,13 +2426,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)
index c80310e..acb68e7 100644 (file)
@@ -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  : () ),
     );
 }
 
index 0766141..41a41ae 100755 (executable)
@@ -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;
 }
 
index 80c2b99..6c067b3 100644 (file)
@@ -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 (file)
index e328502..0000000
+++ /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;