From 410a529c2bff823f01153c5f647d28539ff575eb Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Fri, 30 Aug 2013 14:01:15 +0200 Subject: [PATCH] [SIGNED-OFF] Bug 10807: Add an authority search history for the OPAC Content-Type: text/plain; charset="utf-8" Like biblio, this feature provides an authority search history. This history is available for connected and disconnected user. If the user is not logged in Koha, the history is stored in a cookie named 'KohaOpacRecentSearches'. The search history feature is now factorized in a new module. This patch adds: - 1 new db field search_history.type. It permits to distinguish the search type (biblio or authority). - 1 new module C4::Search::History. It deals with 2 different storages: db or cookie - 2 new UT files: t/Search/History.t and t/db_dependent/Search/History.t - 1 new behavior: the 'Search history' link (on the top-right corner of the screen) is always displayed. Test plan: 1/ Switch on the 'EnableOpacSearchHistory' syspref. 2/ Go on the opac and log out. 3/ Launch some biblio and authority searches. 4/ Go on your search history page. 5/ Check that all yours searches are displayed. 6/ Click on some links and check that results are consistent. 7/ Delete your biblio history searches. 8/ Delete your authority searches history searches. 9/ Launch some biblio and authority searches 10/ Delete all your history (cross on the top-right corner) 11/ Check that all your history search is empty. 12/ Launch some biblio and authority searches. 13/ Login to your account. 14/ Check that all previous searches are displayed. 15/ Launch some biblio and authority searches. 16/ Check that these previous searches are displayed under "Current session". 17/ Play with the 4 delete links (current / previous and biblio / authority). Signed-off-by: Owen Leonard --- C4/Auth.pm | 60 ++--- C4/Search.pm | 23 -- C4/Search/History.pm | 270 ++++++++++++++++++++ installer/data/mysql/kohastructure.sql | 1 + installer/data/mysql/updatedatabase.pl | 13 + koha-tmpl/opac-tmpl/ccsr/en/includes/top-bar.inc | 2 +- koha-tmpl/opac-tmpl/ccsr/en/includes/usermenu.inc | 2 +- koha-tmpl/opac-tmpl/prog/en/includes/masthead.inc | 2 +- koha-tmpl/opac-tmpl/prog/en/includes/usermenu.inc | 2 +- .../prog/en/modules/opac-search-history.tt | 224 +++++++++++----- opac/opac-authorities-home.pl | 44 ++++ opac/opac-search-history.pl | 192 +++++++------- opac/opac-search.pl | 71 ++--- t/Search/History.t | 34 +++ t/db_dependent/Auth_ParseSearchHistoryCookie.t | 43 ---- t/db_dependent/Search/History.t | 248 ++++++++++++++++++ 16 files changed, 938 insertions(+), 293 deletions(-) create mode 100644 C4/Search/History.pm create mode 100644 t/Search/History.t delete mode 100644 t/db_dependent/Auth_ParseSearchHistoryCookie.t create mode 100644 t/db_dependent/Search/History.t diff --git a/C4/Auth.pm b/C4/Auth.pm index 1e17e59..7b61da0 100644 --- a/C4/Auth.pm +++ b/C4/Auth.pm @@ -20,7 +20,7 @@ package C4::Auth; use strict; use warnings; use Digest::MD5 qw(md5_base64); -use JSON qw/encode_json decode_json/; +use JSON qw/encode_json/; use URI::Escape; use CGI::Session; @@ -28,6 +28,7 @@ require Exporter; use C4::Context; use C4::Templates; # to get the template use C4::Branch; # GetBranches +use C4::Search::History; use C4::VirtualShelves; use POSIX qw/strftime/; use List::MoreUtils qw/ any /; @@ -46,9 +47,7 @@ BEGIN { $debug = $ENV{DEBUG}; @ISA = qw(Exporter); @EXPORT = qw(&checkauth &get_template_and_user &haspermission &get_user_subpermissions); - @EXPORT_OK = qw(&check_api_auth &get_session &check_cookie_auth &checkpw &get_all_subpermissions &get_user_subpermissions - ParseSearchHistoryCookie - ); + @EXPORT_OK = qw(&check_api_auth &get_session &check_cookie_auth &checkpw &get_all_subpermissions &get_user_subpermissions); %EXPORT_TAGS = ( EditPermissions => [qw(get_all_subpermissions get_user_subpermissions)] ); $ldap = C4::Context->config('useldapserver') || 0; $cas = C4::Context->preference('casAuthentication'); @@ -127,11 +126,6 @@ Output.pm module. =cut -my $SEARCH_HISTORY_INSERT_SQL =<fetchrow_array > 0) { - # We show the link in opac - $template->param(ShowOpacRecentSearchLink => 1); + # We show the link in opac + $template->param( EnableOpacSearchHistory => 1 ); } # And if there's a cookie with searches performed when the user was not logged in, # we add them to the logged-in search history - my @recentSearches = ParseSearchHistoryCookie($in->{'query'}); + my @recentSearches = @{ + C4::Search::History::get_from_cookie({ + cookie => $in->{'query'}->cookie('KohaOpacRecentSearches') + }) + }; if (@recentSearches) { - my $sth = $dbh->prepare($SEARCH_HISTORY_INSERT_SQL); + my $dbh = C4::Context->dbh; + + my $query = q{ + INSERT INTO search_history(userid, sessionid, query_desc, query_cgi, type, total, time ) + VALUES (?, ?, ?, ?, ?, ?, ?) + }; + + my $sth = $dbh->prepare($query); $sth->execute( $borrowernumber, - $in->{'query'}->cookie("CGISESSID"), - $_->{'query_desc'}, - $_->{'query_cgi'}, - $_->{'total'}, - $_->{'time'}, + $in->{query}->cookie("CGISESSID"), + $_->{query_desc}, + $_->{query_cgi}, + $_->{type} || 'biblio', + $_->{total}, + $_->{time}, ) foreach @recentSearches; # And then, delete the cookie's content @@ -288,9 +294,13 @@ sub get_template_and_user { # Anonymous opac search history # If opac search history is enabled and at least one search has already been performed if (C4::Context->preference('EnableOpacSearchHistory')) { - my @recentSearches = ParseSearchHistoryCookie($in->{'query'}); + my @recentSearches = @{ + C4::Search::History::get_from_cookie({ + cookie => $in->{'query'}->cookie('KohaOpacRecentSearches') + }) + }; if (@recentSearches) { - $template->param(ShowOpacRecentSearchLink => 1); + $template->param(EnableOpacSearchHistory => 1); } } @@ -1715,16 +1725,6 @@ sub getborrowernumber { return 0; } -sub ParseSearchHistoryCookie { - my $input = shift; - my $search_cookie = $input->cookie('KohaOpacRecentSearches'); - return () unless $search_cookie; - my $obj = eval { decode_json(uri_unescape($search_cookie)) }; - return () unless defined $obj; - return () unless ref $obj eq 'ARRAY'; - return @{ $obj }; -} - END { } # module clean-up code here (global destructor) 1; __END__ diff --git a/C4/Search.pm b/C4/Search.pm index 0453b91..0aae9bb 100644 --- a/C4/Search.pm +++ b/C4/Search.pm @@ -68,7 +68,6 @@ This module provides searching functions for Koha's bibliographic databases &searchResults &getRecords &buildQuery - &AddSearchHistory &GetDistinctValues &enabled_staff_search_views &PurgeSearchHistory @@ -2189,28 +2188,6 @@ sub enabled_staff_search_views ); } -sub AddSearchHistory{ - my ($borrowernumber,$session,$query_desc,$query_cgi, $total)=@_; - my $dbh = C4::Context->dbh; - - # Add the request the user just made - my $sql = "INSERT INTO search_history(userid, sessionid, query_desc, query_cgi, total, time) VALUES(?, ?, ?, ?, ?, NOW())"; - my $sth = $dbh->prepare($sql); - $sth->execute($borrowernumber, $session, $query_desc, $query_cgi, $total); - return $dbh->last_insert_id(undef, 'search_history', undef,undef,undef); -} - -sub GetSearchHistory{ - my ($borrowernumber,$session)=@_; - my $dbh = C4::Context->dbh; - - # Add the request the user just made - my $query = "SELECT FROM search_history WHERE (userid=? OR sessionid=?)"; - my $sth = $dbh->prepare($query); - $sth->execute($borrowernumber, $session); - return $sth->fetchall_hashref({}); -} - sub PurgeSearchHistory{ my ($pSearchhistory)=@_; my $dbh = C4::Context->dbh; diff --git a/C4/Search/History.pm b/C4/Search/History.pm new file mode 100644 index 0000000..4a5d78b --- /dev/null +++ b/C4/Search/History.pm @@ -0,0 +1,270 @@ +package C4::Search::History; + +use Modern::Perl; + +use C4::Context; +use Koha::DateUtils; + +use JSON qw( encode_json decode_json ); +use URI::Escape; +use Encode; + +sub add { + my ( $params ) = @_; + my $userid = $params->{userid}; + my $sessionid = $params->{sessionid}; + my $query_desc = $params->{query_desc}; + my $query_cgi = $params->{query_cgi}; + my $total = $params->{total}; + my $type = $params->{type} || 'biblio'; + + my $dbh = C4::Context->dbh; + + # Add the request the user just made + my $query = q{ + 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); +} + +sub build_new_cookie_value { + my ( $params ) = @_; + my $recent_searches = $params->{recent_searches}; + my $query_desc = Encode::decode_utf8($params->{query_desc}) || "unknown"; + my $query_cgi = Encode::decode_utf8($params->{query_cgi}) || "unknown"; + my $total = $params->{total}; + my $type = $params->{type} || 'biblio'; + + my @recent_searches; + # Getting the (maybe) already sent cookie + if ( $recent_searches ){ + $recent_searches = uri_unescape($recent_searches); + if (decode_json($recent_searches)) { + @recent_searches = @{decode_json( $recent_searches )} + } + } + + # To a cookie (the user is not logged in) + push @recent_searches, { + query_desc => $query_desc, + query_cgi => $query_cgi, + total => "$total", + type => $type, + time => output_pref( dt_from_string(), 'iso' ), + }; + + shift @recent_searches if (@recent_searches > 15); + + return uri_escape( encode_json( \@recent_searches ) ); +} + +sub delete { + my ( $params ) = @_; + my $userid = $params->{userid}; + my $sessionid = $params->{sessionid}; + my $type = $params->{type} || q{}; + my $previous = $params->{previous} || 0; + + unless ( $userid ) { + warn "ERROR: userid is required for history search"; + return; + } + + my $dbh = C4::Context->dbh; + my $query = q{ + DELETE FROM search_history + WHERE userid = ? + }; + + if ( $sessionid ) { + $query .= $previous + ? q{ AND sessionid != ?} + : q{ AND sessionid = ?}; + } + + $query .= q{ AND type = ?} + if $type; + + $dbh->do( + $query, {}, + $userid, + ( $sessionid ? $sessionid : () ), + ( $type ? $type : () ) + ); +} + +sub get { + my ( $params ) = @_; + my $userid = $params->{userid}; + my $sessionid = $params->{sessionid}; + my $type = $params->{type}; + my $previous = $params->{previous}; + + unless ( $userid ) { + warn "ERROR: userid is required for history search"; + return; + } + + my $query = q{ + SELECT * + FROM search_history + WHERE userid = ? + }; + + if ( $sessionid ) { + $query .= $previous + ? q{ AND sessionid != ?} + : q{ AND sessionid = ?}; + } + + $query .= q{ AND type = ?} + if $type; + + my $dbh = C4::Context->dbh; + my $sth = $dbh->prepare( $query ); + $sth->execute( + $userid, + ( $sessionid ? $sessionid : () ), + ( $type ? $type : () ) + ); + return $sth->fetchall_arrayref({}); +} + +sub get_from_cookie { + my ( $params ) = @_; + my $search_cookie = $params->{cookie}; + return [] unless $search_cookie; + my $obj = eval { decode_json(uri_unescape($search_cookie)) }; + return [] unless defined $obj; + return [] unless ref $obj eq 'ARRAY'; + return $obj; +} + +sub get_empty_cookie { + my ( $params ) = @_; + my $cgi = $params->{cgi}; + my $name = $params->{name}; + $cgi->cookie( + -name => $name, + -value => encode_json([]), + -expires => '' + ); +} + +1; + +__END__ + +=pod + +=head1 NAME + +C4::Search::History - Manage search history + +=head1 DESCRIPTION + +This module provides some routines for the search history management. +It deals with cookie or database. + +=head1 ROUTINES + +=head2 add + + C4::Search::History::add({ + userid => $userid, + sessionid => $cgi->cookie("CGIESSID"), + query_desc => $query_desc, + query_cgi => $query_cgi, + total => $total, + type => $type, + }); + +type is "biblio" or "authority". + +Add a new search to the user's history. + +=head2 build_new_cookie_value + + my $value = C4::Search::History::build_new_cookie_value({ + cookie => $cookie, + recent_searches => $cgi->cookie('KohaOpacRecentSearches'), + query_desc => $query_desc, + query_cgi => $query_cgi, + total => $total, + type => $type, + }); + +Build a new cookie value containing previous search and the last one. + +=head2 delete + + C4::Search::History::delete({ + userid => $loggedinuser, + sessionid => $sessionid, + type => $type, + previous => $previous + }); + +Delete searches in the database. +If the sessionid is missing all searches for all sessions will be deleted. +It is possible to delete searches for current session or all previous sessions using the previous flag. +If the type ("biblio" or "authority") is missing, all type will be deleted. +To delete *all* searches for a given userid, just pass a userid. + +=head2 get + + my $searches C4::Search::History::get({ + userid => $userid, + sessionsid => $sessionid, + type => $type, + previous => $previous + }); + +Return a list of searches for a given userid. +If a sessionid is given, searches are limited to the matching session. +type and previous follow the same behavior as the delete routine. + +=head2 get_from_cookie + + my $searches = C4::Search::History::get_from_cookie({ + cookie => $cgi->cookie('KohaOpacRecentSearches') + }); + +Return all searches present in the given cookie. + +=head2 get_empty_cookie + + my $cookie = C4::Search::History::get_empty_cookie({ + cgi => $cgi, + name => $cookie_name + }); + +Return a cookie with no value. + +=head1 AUTHORS + +Jonathan Druart + +=head1 LICENSE + +Copyright 2013 BibLibre SARL + +This file is part of Koha. + +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 2 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, write to the Free Software Foundation, Inc., +51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql index 3214ad9..f782baf 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -1879,6 +1879,7 @@ CREATE TABLE IF NOT EXISTS `search_history` ( -- patron's opac search history `sessionid` varchar(32) NOT NULL, -- a system generated session id `query_desc` varchar(255) NOT NULL, -- the search that was performed `query_cgi` text NOT NULL, -- the string to append to the search url to rerun the search + `type` varchar(255) NOT NULL DEFAULT 'biblio', -- search type, must be 'biblio' or 'authority' `total` int(11) NOT NULL, -- the total of results found `time` timestamp NOT NULL default CURRENT_TIMESTAMP, -- the date and time the search was run KEY `userid` (`userid`), diff --git a/installer/data/mysql/updatedatabase.pl b/installer/data/mysql/updatedatabase.pl index ddcc455..cef18e3 100755 --- a/installer/data/mysql/updatedatabase.pl +++ b/installer/data/mysql/updatedatabase.pl @@ -7104,6 +7104,19 @@ if ( CheckVersion($DBversion) ) { SetVersion($DBversion); } + + + + + + +$DBversion = "3.13.00.XXX"; +if ( CheckVersion($DBversion) ) { + $dbh->do("ALTER TABLE search_history ADD COLUMN type VARCHAR(255) NOT NULL DEFAULT 'biblio' AFTER query_cgi"); + print "Upgrade to $DBversion done (Bug 10807 - Add db field search_history.type)\n"; + SetVersion($DBversion); +} + =head1 FUNCTIONS =head2 TableExists($table) diff --git a/koha-tmpl/opac-tmpl/ccsr/en/includes/top-bar.inc b/koha-tmpl/opac-tmpl/ccsr/en/includes/top-bar.inc index 42d9576..a6ff978 100644 --- a/koha-tmpl/opac-tmpl/ccsr/en/includes/top-bar.inc +++ b/koha-tmpl/opac-tmpl/ccsr/en/includes/top-bar.inc @@ -60,7 +60,7 @@
  • Welcome, [% FOREACH USER_INF IN USER_INFO %][% USER_INF.title %] [% USER_INF.firstname %] [% USER_INF.surname %][% END %]
    • [% END %] - [% IF ( ShowOpacRecentSearchLink ) %] + [% IF ( EnableOpacSearchHistory ) %]
  • Search history
    • [% END %] [% IF ( loggedinusername ) %]
  • [% IF persona %][% ELSE %][% END %]Log Out
    • [% END %] diff --git a/koha-tmpl/opac-tmpl/ccsr/en/includes/usermenu.inc b/koha-tmpl/opac-tmpl/ccsr/en/includes/usermenu.inc index fcbbcba..91db9e2 100644 --- a/koha-tmpl/opac-tmpl/ccsr/en/includes/usermenu.inc +++ b/koha-tmpl/opac-tmpl/ccsr/en/includes/usermenu.inc @@ -12,7 +12,7 @@ [% IF ( OpacPasswordChange ) %] [% IF ( passwdview ) %]
  • [% ELSE %]
  • [% END %]change my password
    • [% END %] - [% IF ( ShowOpacRecentSearchLink ) %] + [% IF EnableOpacSearchHistory %] [% IF ( searchhistoryview ) %]
  • [% ELSE %]
  • [% END %]my search history
    • [% END %] [% IF ( opacreadinghistory ) %] diff --git a/koha-tmpl/opac-tmpl/prog/en/includes/masthead.inc b/koha-tmpl/opac-tmpl/prog/en/includes/masthead.inc index 79eb1a1..8abfcbf 100644 --- a/koha-tmpl/opac-tmpl/prog/en/includes/masthead.inc +++ b/koha-tmpl/opac-tmpl/prog/en/includes/masthead.inc @@ -7,7 +7,7 @@
  • Welcome, [% FOREACH USER_INF IN USER_INFO %][% USER_INF.title %] [% USER_INF.firstname %] [% USER_INF.surname %][% END %]
    • [% END %] - [% IF ( ShowOpacRecentSearchLink ) %] + [% IF EnableOpacSearchHistory %]
  • Search history [x]
    • [% END %] [% IF ( loggedinusername ) %]
  • [% IF persona %][% ELSE %][% END %]Log Out
    • [% END %] diff --git a/koha-tmpl/opac-tmpl/prog/en/includes/usermenu.inc b/koha-tmpl/opac-tmpl/prog/en/includes/usermenu.inc index 5a07d95..5c0383b 100644 --- a/koha-tmpl/opac-tmpl/prog/en/includes/usermenu.inc +++ b/koha-tmpl/opac-tmpl/prog/en/includes/usermenu.inc @@ -12,7 +12,7 @@ [% IF ( OpacPasswordChange ) %] [% IF ( passwdview ) %]
  • [% ELSE %]
  • [% END %]change my password
    • [% END %] - [% IF ( ShowOpacRecentSearchLink ) %] + [% IF EnableOpacSearchHistory %] [% IF ( searchhistoryview ) %]
  • [% ELSE %]
  • [% END %]my search history
    • [% END %] [% IF ( opacreadinghistory ) %] diff --git a/koha-tmpl/opac-tmpl/prog/en/modules/opac-search-history.tt b/koha-tmpl/opac-tmpl/prog/en/modules/opac-search-history.tt index 9042543..975fd77 100644 --- a/koha-tmpl/opac-tmpl/prog/en/modules/opac-search-history.tt +++ b/koha-tmpl/opac-tmpl/prog/en/modules/opac-search-history.tt @@ -6,19 +6,21 @@ [% INCLUDE 'datatables.inc' %] @@ -34,68 +36,156 @@
    [% INCLUDE 'masthead.inc' %] -
    -
    -
    -

    Search history

    - [% IF ( recentSearches ) %]
    [% ELSE %][% IF ( previousSearches ) %]
    [% END %][% END %] +
    +
    +
    +

    Search history

    +
    + +
    + [% IF ( current_biblio_searches ) %] +

    Current session

    +
    + + + + +
    + + + + + + + + + + [% FOREACH s IN current_biblio_searches %] + + + + + + [% END %] + +
    DateSearchResults
    [% s.time |$KohaDates with_hours => 1 %][% s.query_desc |html %][% s.total %]
    + [% END %] + + [% IF ( previous_biblio_searches ) %] +

    Previous sessions

    +
    + + + + +
    + + + + + + + + + + [% FOREACH s IN previous_biblio_searches %] + + + + + + [% END %] + +
    DateSearchResults
    [% s.time |$KohaDates with_hours => 1 %][% s.query_desc |html %][% s.total %]
    + [% END %] - [% IF ( recentSearches ) %] - - [% IF ( previousSearches ) %] - - [% END %] - - - - - [% FOREACH recentSearche IN recentSearches %] - - - - - - [% END %] - -
    Current session
    DateSearchResults
    [% recentSearche.time |$KohaDates with_hours => 1 %][% recentSearche.query_desc |html %][% recentSearche.total %]
    - [% END %] + [% IF !current_biblio_searches && !previous_biblio_searches %] +

    Your biblio search history is empty.

    + [% END %] +
    - [% IF ( previousSearches ) %] - - - - - - - [% FOREACH previousSearche IN previousSearches %] - - - - - - [% END %] - -
    Previous sessions
    DateSearchResults
    [% previousSearche.time |$KohaDates with_hours => 1 %][% previousSearche.query_desc |html %][% previousSearche.total %]
    - [% END %] +
    + [% IF ( current_authority_searches ) %] +

    Current session

    +
    + + + + +
    + + + + + + + + + + [% FOREACH s IN current_authority_searches %] + + + + + + [% END %] + +
    DateSearchResults
    [% s.time |$KohaDates with_hours => 1 %][% s.query_desc |html %][% s.total %]
    + [% END %] -[% IF ( recentSearches ) %][% ELSE %][% IF ( previousSearches ) %][% ELSE %]

    Your search history is empty.

    [% END %][% END %] + [% IF ( previous_authority_searches ) %] +

    Previous sessions

    +
    + + + + +
    + + + + + + + + + + [% FOREACH s IN previous_authority_searches %] + + + + + + [% END %] + +
    DateSearchResults
    [% s.time |$KohaDates with_hours => 1 %][% s.query_desc |html %][% s.total %]
    + [% END %] -
    -
    -
    -
    + [% IF !current_authority_searches && !previous_authority_searches %] +

    Your authority search history is empty.

    + [% END %] +
    +
    +
    +
    +
    [% IF ( OpacNav ) %] -
    -[% INCLUDE 'navigation.inc' IsPatronPage=1 %] -
    +
    +
    + [% INCLUDE 'navigation.inc' IsPatronPage=1 %] +
    +
    [% ELSIF ( loggedinusername ) %] -
    -[% INCLUDE 'navigation.inc' IsPatronPage=1 %] -
    -[% ELSE %] +
    +
    + [% INCLUDE 'navigation.inc' IsPatronPage=1 %] +
    +
    [% END %] -
    [% INCLUDE 'opac-bottom.inc' %] diff --git a/opac/opac-authorities-home.pl b/opac/opac-authorities-home.pl index 115012e..4ff7923 100755 --- a/opac/opac-authorities-home.pl +++ b/opac/opac-authorities-home.pl @@ -22,6 +22,7 @@ use strict; use warnings; use CGI; + use C4::Auth; use C4::Context; @@ -29,6 +30,7 @@ use C4::Auth; use C4::Output; use C4::AuthoritiesMarc; use C4::Koha; # XXX subfield_is_koha_internal_p +use C4::Search::History; my $query = new CGI; my $op = $query->param('op') || ''; @@ -129,6 +131,48 @@ if ( $op eq "do_search" ) { my @usedauths = grep { $_->{used} > 0 } @$results; $results = \@usedauths; } + + # Opac search history + my $newsearchcookie; + if (C4::Context->preference('EnableOpacSearchHistory')) { + unless ( $startfrom ) { + my $path_info = $query->url(-path_info=>1); + my $query_cgi_history = $query->url(-query=>1); + $query_cgi_history =~ s/^$path_info\?//; + $query_cgi_history =~ s/;/&/g; + + unless ( $loggedinuser ) { + my $new_search = C4::Search::History::build_new_cookie_value({ + cookie => $cookie, + recent_searches => $query->cookie('KohaOpacRecentSearches') || q{}, + query_desc => $value[0], + query_cgi => $query_cgi_history, + total => $total, + type => "authority", + }); + + $cookie = [ + $cookie, + $query->cookie( + -name => 'KohaOpacRecentSearches', + -value => $new_search, + -expires => '' + ) + ]; + } else { + # To the session (the user is logged in) + C4::Search::History::add({ + userid => $loggedinuser, + sessionid => $query->cookie("CGISESSID"), + query_desc => $value[0], + query_cgi => $query_cgi_history, + total => $total, + type => "authority", + }); + } + } + } + $template->param( result => $results ) if $results; $template->param( FIELDS => \@fields ); $template->param( orderby => $orderby ); diff --git a/opac/opac-search-history.pl b/opac/opac-search-history.pl index 52477c5..8d0ba74 100755 --- a/opac/opac-search-history.pl +++ b/opac/opac-search-history.pl @@ -1,6 +1,6 @@ #!/usr/bin/perl -# Copyright 2009 BibLibre SARL +# Copyright 2013 BibLibre SARL # # This file is part of Koha. # @@ -17,18 +17,17 @@ # with Koha; if not, write to the Free Software Foundation, Inc., # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. -use strict; -use warnings; +use Modern::Perl; -use C4::Auth qw(:DEFAULT get_session ParseSearchHistoryCookie); +use C4::Auth qw(:DEFAULT get_session); use CGI; -use JSON qw/decode_json encode_json/; use C4::Context; use C4::Output; use C4::Log; use C4::Items; use C4::Debug; use C4::Dates; +use C4::Search::History; use URI::Escape; use POSIX qw(strftime); @@ -36,111 +35,120 @@ use POSIX qw(strftime); my $cgi = new CGI; # Getting the template and auth -my ($template, $loggedinuser, $cookie) -= get_template_and_user({template_name => "opac-search-history.tmpl", - query => $cgi, - type => "opac", - authnotrequired => 1, - flagsrequired => {borrowers => 1}, - debug => 1, - }); +my ($template, $loggedinuser, $cookie) = get_template_and_user( + { + template_name => "opac-search-history.tmpl", + query => $cgi, + type => "opac", + authnotrequired => 1, + flagsrequired => {borrowers => 1}, + debug => 1, + } +); + +my $type = $cgi->param('type'); +my $action = $cgi->param('action') || q{}; +my $previous = $cgi->param('previous'); # If the user is not logged in, we deal with the cookie -if (!$loggedinuser) { +unless ( $loggedinuser ) { # Deleting search history - if ($cgi->param('action') && $cgi->param('action') eq 'delete') { - # Deleting cookie's content - my $recentSearchesCookie = $cgi->cookie( - -name => 'KohaOpacRecentSearches', - -value => encode_json([]), - -expires => '' - ); - - # Redirecting to this same url with the cookie in the headers so it's deleted immediately - my $uri = $cgi->url(); - print $cgi->redirect(-uri => $uri, - -cookie => $recentSearchesCookie); + if ( $action eq 'delete' ) { + # Deleting cookie's content + my $current_searches_cookie = C4::Search::History::get_empty_cookie({ + cgi => $cgi, + name => 'KohaOpacRecentSearches', + }); + + # Redirecting to this same url with the cookie in the headers so it's deleted immediately + my $uri = $cgi->url(); + print $cgi->redirect( + -uri => $uri, + -cookie => $current_searches_cookie + ); # Showing search history } else { - - my @recentSearches = ParseSearchHistoryCookie($cgi); - if (@recentSearches) { - - # As the dates are stored as unix timestamps, let's do some formatting - foreach my $asearch (@recentSearches) { - - # We create an iso date from the unix timestamp - my $isodate = strftime "%Y-%m-%d", localtime($asearch->{'time'}); - - # So we can create a C4::Dates object, to get the date formatted according to the dateformat syspref - my $date = C4::Dates->new($isodate, "iso"); - my $sysprefdate = $date->output("syspref"); - - # We also get the time of the day from the unix timestamp - my $time = strftime " %H:%M:%S", localtime($asearch->{'time'}); - - # And we got our human-readable date : - $asearch->{'time'} = $sysprefdate . $time; - } - - $template->param(recentSearches => \@recentSearches); - } + # Getting the cookie + my @current_searches = @{ + C4::Search::History::get_from_cookie({ + cookie => $cgi->cookie('KohaOpacRecentSearches') + }) + }; + + my @current_biblio_searches = map { + $_->{type} eq 'biblio' ? $_ : () + } @current_searches; + + my @current_authority_searches = map { + $_->{type} eq 'authority' ? $_ : () + } @current_searches; + + $template->param( + current_biblio_searches => \@current_biblio_searches, + current_authority_searches => \@current_authority_searches, + ); } } else { -# And if the user is logged in, we deal with the database - + # And if the user is logged in, we deal with the database my $dbh = C4::Context->dbh; # Deleting search history - if ($cgi->param('action') && $cgi->param('action') eq 'delete') { - my $query = "DELETE FROM search_history WHERE userid = ?"; - my $sth = $dbh->prepare($query); - $sth->execute($loggedinuser); - - # Redirecting to this same url so the user won't see the search history link in the header - my $uri = $cgi->url(); - print $cgi->redirect($uri); - + if ( $action eq 'delete' ) { + my $sessionid = defined $previous + ? $cgi->cookie("CGISESSID") + : q{}; + C4::Search::History::delete( + { + userid => $loggedinuser, + sessionid => $sessionid, + type => $type, + previous => $previous + } + ); + # Redirecting to this same url so the user won't see the search history link in the header + my $uri = $cgi->url(); + print $cgi->redirect($uri); # Showing search history } else { - - my $date = C4::Dates->new(); - my $dateformat = $date->DHTMLcalendar() . " %H:%i:%S"; # Current syspref date format + standard time format - - # Getting the data with date format work done by mysql - my $query = "SELECT userid, sessionid, query_desc, query_cgi, total, time FROM search_history WHERE userid = ? AND sessionid = ?"; - my $sth = $dbh->prepare($query); - $sth->execute($loggedinuser, $cgi->cookie("CGISESSID")); - my $searches = $sth->fetchall_arrayref({}); - $template->param(recentSearches => $searches); - - # Getting searches from previous sessions - $query = "SELECT COUNT(*) FROM search_history WHERE userid = ? AND sessionid != ?"; - $sth = $dbh->prepare($query); - $sth->execute($loggedinuser, $cgi->cookie("CGISESSID")); - - # If at least one search from previous sessions has been performed - if ($sth->fetchrow_array > 0) { - $query = "SELECT userid, sessionid, query_desc, query_cgi, total, time FROM search_history WHERE userid = ? AND sessionid != ?"; - $sth = $dbh->prepare($query); - $sth->execute($loggedinuser, $cgi->cookie("CGISESSID")); - my $previoussearches = $sth->fetchall_arrayref({}); - $template->param(previousSearches => $previoussearches); - - } - - $sth->finish; - - + my $current_searches = C4::Search::History::get({ + userid => $loggedinuser, + sessionid => $cgi->cookie("CGISESSID") + }); + my @current_biblio_searches = map { + $_->{type} eq 'biblio' ? $_ : () + } @$current_searches; + + my @current_authority_searches = map { + $_->{type} eq 'authority' ? $_ : () + } @$current_searches; + + my $previous_searches = C4::Search::History::get({ + userid => $loggedinuser, + sessionid => $cgi->cookie("CGISESSID"), + previous => 1 + }); + + my @previous_biblio_searches = map { + $_->{type} eq 'biblio' ? $_ : () + } @$previous_searches; + + my @previous_authority_searches = map { + $_->{type} eq 'authority' ? $_ : () + } @$previous_searches; + + $template->param( + current_biblio_searches => \@current_biblio_searches, + current_authority_searches => \@current_authority_searches, + previous_biblio_searches => \@previous_biblio_searches, + previous_authority_searches => \@previous_authority_searches, + + ); } - } $template->param(searchhistoryview => 1); output_html_with_http_headers $cgi, $cookie, $template->output; - - diff --git a/opac/opac-search.pl b/opac/opac-search.pl index 2854da6..9426e5d 100755 --- a/opac/opac-search.pl +++ b/opac/opac-search.pl @@ -42,9 +42,10 @@ for ( $searchengine ) { } use C4::Output; -use C4::Auth qw(:DEFAULT get_session ParseSearchHistoryCookie); +use C4::Auth qw(:DEFAULT get_session); use C4::Languages qw(getAllLanguages); use C4::Search; +use C4::Search::History; use C4::Biblio; # GetBiblioData use C4::Koha; use C4::Tags qw(get_tags); @@ -614,47 +615,49 @@ for (my $i=0;$i<@servers;$i++) { } # Opac search history - my $newsearchcookie; if (C4::Context->preference('EnableOpacSearchHistory')) { - my @recentSearches = ParseSearchHistoryCookie($cgi); - - # Adding the new search if needed - my $path_info = $cgi->url(-path_info=>1); - my $query_cgi_history = $cgi->url(-query=>1); - $query_cgi_history =~ s/^$path_info\?//; - $query_cgi_history =~ s/;/&/g; - my $query_desc_history = "$query_desc, $limit_desc"; - - if (!$borrowernumber || $borrowernumber eq '') { - # To a cookie (the user is not logged in) - if (!$offset) { - push @recentSearches, { - "query_desc" => Encode::decode_utf8($query_desc_history) || "unknown", - "query_cgi" => Encode::decode_utf8($query_cgi_history) || "unknown", - "time" => time(), - "total" => $total - }; - $template->param(ShowOpacRecentSearchLink => 1); - } + unless ( $offset ) { + my $path_info = $cgi->url(-path_info=>1); + my $query_cgi_history = $cgi->url(-query=>1); + $query_cgi_history =~ s/^$path_info\?//; + $query_cgi_history =~ s/;/&/g; + my $query_desc_history = $query_desc; + $query_desc_history .= ", $limit_desc" + if $limit_desc; + + unless ( $borrowernumber ) { + my $new_search = C4::Search::History::build_new_cookie_value({ + recent_searches => $cgi->cookie('KohaOpacRecentSearches') || q{}, + query_desc => $query_desc_history, + query_cgi => $query_cgi_history, + total => $total, + type => "biblio", + }); - shift @recentSearches if (@recentSearches > 15); - # Pushing the cookie back - $newsearchcookie = $cgi->cookie( + $cookie = [ + $cookie, + $cgi->cookie( -name => 'KohaOpacRecentSearches', # We uri_escape the whole serialized structure so we're sure we won't have any encoding problems - -value => uri_escape( encode_json(\@recentSearches) ), + -value => $new_search, -expires => '' - ); - $cookie = [$cookie, $newsearchcookie]; - } - else { - # To the session (the user is logged in) - if (!$offset) { - AddSearchHistory($borrowernumber, $cgi->cookie("CGISESSID"), $query_desc_history, $query_cgi_history, $total); - $template->param(ShowOpacRecentSearchLink => 1); + ) + ]; + } else { + # To the session (the user is logged in) + C4::Search::History::add({ + userid => $borrowernumber, + sessionid => $cgi->cookie("CGISESSID"), + query_desc => $query_desc_history, + query_cgi => $query_cgi_history, + total => $total, + type => "biblio", + }); } } + $template->param( EnableOpacSearchHistory => 1 ); } + ## If there's just one result, redirect to the detail page if ($total == 1 && $format ne 'rss2' && $format ne 'opensearchdescription' && $format ne 'atom') { diff --git a/t/Search/History.t b/t/Search/History.t new file mode 100644 index 0000000..a340d16 --- /dev/null +++ b/t/Search/History.t @@ -0,0 +1,34 @@ +#!/usr/bin/env perl + +use Modern::Perl; + +use Test::More tests => 7; +use URI::Escape; +use JSON qw( decode_json ); + +use_ok('Koha::DateUtils'); +use_ok('C4::Search::History'); + + +# Test cookie +my $query_desc = q{first search}; +my $query_cgi = q{idx=kw&idx=ti&idx=au%2Cwrdl&q=word1é&q=word2è&q=word3à&do=Search&sort_by=author_az}; +my $total = 42; + +my $first_search = C4::Search::History::build_new_cookie_value({ + recent_searches => q{}, + query_desc => $query_desc, + query_cgi => $query_cgi, + total => $total, + type => "biblio", +}); + +my $date = output_pref( dt_from_string(), 'iso' ); + +my @values = @{ C4::Search::History::get_from_cookie({cookie => $first_search}) }; +my $values = shift @values; +is( $values->{time}, $date, 'build search time' ); +is( $values->{query_cgi}, Encode::decode_utf8($query_cgi), 'build search query_cgi' ); +is( $values->{type}, 'biblio', 'build search type' ); +is( $values->{total}, '42', 'build search total' ); +is( $values->{query_desc}, Encode::decode_utf8($query_desc), 'build search query_desc' ); diff --git a/t/db_dependent/Auth_ParseSearchHistoryCookie.t b/t/db_dependent/Auth_ParseSearchHistoryCookie.t deleted file mode 100644 index 507ac91..0000000 --- a/t/db_dependent/Auth_ParseSearchHistoryCookie.t +++ /dev/null @@ -1,43 +0,0 @@ -#!/usr/bin/perl - -use strict; -use warnings; - -use Test::More tests => 3; - -use_ok('C4::Auth', qw/ParseSearchHistoryCookie/); - -my $valid_cookie = "%5B%7B%22time%22%3A1374978877%2C%22query_cgi%22%3A%22idx%3D%26q%3Dhistory%26branch_group_limit%3D%22%2C%22total%22%3A2%2C%22query_desc%22%3A%22kw%2Cwrdl%3A%20history%2C%20%22%7D%5D"; -my $expected_recent_searches = [ - { - 'time' => 1374978877, - 'query_cgi' => 'idx=&q=history&branch_group_limit=', - 'total' => 2, - 'query_desc' => 'kw,wrdl: history, ' - } -]; - -my $input = CookieSimulator->new($valid_cookie); -my @recent_searches = ParseSearchHistoryCookie($input); -is_deeply(\@recent_searches, $expected_recent_searches, 'parsed valid search history cookie value'); - -# simulate bit of a Storable-based search history cookie -my $invalid_cookie = "%04%08%0812345"; -$input = CookieSimulator->new($invalid_cookie); -@recent_searches = ParseSearchHistoryCookie($input); -is_deeply(\@recent_searches, [], 'got back empty search history list if given invalid cookie'); - -package CookieSimulator; - -sub new { - my ($class, $str) = @_; - my $val = [ $str ]; - return bless $val, $class; -} - -sub cookie { - my $self = shift; - return $self->[0]; -} - -1; diff --git a/t/db_dependent/Search/History.t b/t/db_dependent/Search/History.t new file mode 100644 index 0000000..33e834d --- /dev/null +++ b/t/db_dependent/Search/History.t @@ -0,0 +1,248 @@ +#!/usr/bin/env perl + +use Modern::Perl; + +use Test::More tests => 16; +use URI::Escape; + +use C4::Context; +my $dbh = C4::Context->dbh; +$dbh->{AutoCommit} = 0; +$dbh->{RaiseError} = 1; + +use_ok('Koha::DateUtils'); +use_ok('C4::Search::History'); + +my $userid = 123; +my $previous_sessionid = "PREVIOUS_SESSIONID"; +my $current_sessionid = "CURRENT_SESSIONID"; +my $total = 42; +my $query_cgi_b = q{idx=kw&idx=ti&idx=au%2Cwrdl&q=word1é&q=word2è&q=word3à&do=Search&sort_by=author_az}; +my $query_cgi_a = q{op=do_search&type=opac&authtypecode=NP&operator=start&value=Harry&marclist=match&and_or=and&orderby=HeadingAsc}; + +# add +my $added = add( $userid, $current_sessionid, $previous_sessionid, $total, $query_cgi_b, $query_cgi_a ); +is ( $added, 9, '9 searches are added' ); + +# get +my $searches_for_userid = C4::Search::History::get({ + userid => $userid, +}); +is( scalar(@$searches_for_userid), 9, 'There are 9 searches in all' ); + +my $searches_for_current_session = C4::Search::History::get({ + userid => $userid, + sessionid => $current_sessionid, +}); +is( scalar(@$searches_for_current_session), 5, 'There are 5 searches for the current session' ); + +my $searches_for_previous_sessions = C4::Search::History::get({ + userid => $userid, + sessionid => $current_sessionid, + previous => 1, +}); +is( scalar(@$searches_for_previous_sessions), 4, 'There are 4 searches for previous sessions' ); + +my $authority_searches_for_current_session = C4::Search::History::get({ + userid => $userid, + sessionid => $current_sessionid, + type => 'authority', +}); +is( scalar(@$authority_searches_for_current_session), 3, 'There are 3 authority searches for the current session' ); + +my $authority_searches_for_previous_session = C4::Search::History::get({ + userid => $userid, + sessionid => $current_sessionid, + type => 'authority', + previous => 1, +}); +is( scalar(@$authority_searches_for_previous_session), 2, 'There are 2 authority searches for previous sessions' ); + +my $biblio_searches_for_userid = C4::Search::History::get({ + userid => $userid, + type => 'biblio', +}); +is( scalar(@$biblio_searches_for_userid), 4, 'There are 5 searches for the current session' ); + +my $authority_searches_for_userid = C4::Search::History::get({ + userid => $userid, + type => 'authority', +}); +is( scalar(@$authority_searches_for_userid), 5, 'There are 4 searches for previous sessions' ); + +delete_all( $userid ); + +# delete +add( $userid, $current_sessionid, $previous_sessionid, $total, $query_cgi_b, $query_cgi_a ); +C4::Search::History::delete({ + userid => $userid, + sessionid => $current_sessionid, + type => 'authority', +}); +my $all = C4::Search::History::get({userid => $userid}); +is( scalar(@$all), 6, 'There are 6 searches in all after deleting current biblio searches' ); +delete_all( $userid ); + +add( $userid, $current_sessionid, $previous_sessionid, $total, $query_cgi_b, $query_cgi_a ); +C4::Search::History::delete({ + userid => $userid, + sessionid => $current_sessionid, + type => 'biblio', + previous => 1, +}); +$all = C4::Search::History::get({userid => $userid}); +is( scalar(@$all), 7, 'There are 7 searches in all after deleting previous authority searches' ); +delete_all( $userid ); + +add( $userid, $current_sessionid, $previous_sessionid, $total, $query_cgi_b, $query_cgi_a ); +C4::Search::History::delete({ + userid => $userid, + sessionid => $current_sessionid, + previous => 1, +}); +$all = C4::Search::History::get({userid => $userid}); +is( scalar(@$all), 5, 'There are 5 searches in all after deleting all previous searches' ); +delete_all( $userid ); + +add( $userid, $current_sessionid, $previous_sessionid, $total, $query_cgi_b, $query_cgi_a ); +C4::Search::History::delete({ + userid => $userid, + sessionid => $current_sessionid, +}); +$all = C4::Search::History::get({userid => $userid}); +is( scalar(@$all), 4, 'There are 5 searches in all after deleting all searches for a sessionid' ); +delete_all( $userid ); + +add( $userid, $current_sessionid, $previous_sessionid, $total, $query_cgi_b, $query_cgi_a ); +C4::Search::History::delete({ + userid => $userid, +}); +$all = C4::Search::History::get({userid => $userid}); +is( scalar(@$all), 0, 'There are 0 search after deleting all searches for a userid' ); +delete_all( $userid ); + +add( $userid, $current_sessionid, $previous_sessionid, $total, $query_cgi_b, $query_cgi_a ); +C4::Search::History::delete({}); +$all = C4::Search::History::get({userid => $userid}); +is( scalar(@$all), 9, 'There are still 9 searches after calling delete without userid' ); +delete_all( $userid ); + +sub add { + my ( $userid, $current_session_id, $previous_sessionid, $total, $query_cgi_b, $query_cgi_a ) = @_; + + my $query_desc_b1_p = q{first previous biblio search}; + my $first_previous_biblio_search = { + userid => $userid, + sessionid => $previous_sessionid, + query_desc => $query_desc_b1_p, + query_cgi => $query_cgi_b, + total => $total, + type => 'biblio', + }; + + my $query_desc_a1_p = q{first previous authority search}; + my $first_previous_authority_search = { + userid => $userid, + sessionid => $previous_sessionid, + query_desc => $query_desc_a1_p, + query_cgi => $query_cgi_a, + total => $total, + type => 'authority', + }; + + my $query_desc_b2_p = q{second previous biblio search}; + my $second_previous_biblio_search = { + userid => $userid, + sessionid => $previous_sessionid, + query_desc => $query_desc_b2_p, + query_cgi => $query_cgi_b, + total => $total, + type => 'biblio', + }; + + my $query_desc_a2_p = q{second previous authority search}; + my $second_previous_authority_search = { + userid => $userid, + sessionid => $previous_sessionid, + query_desc => $query_desc_a2_p, + query_cgi => $query_cgi_a, + total => $total, + type => 'authority', + }; + + + my $query_desc_b1_c = q{first current biblio search}; + + my $first_current_biblio_search = { + userid => $userid, + sessionid => $current_sessionid, + query_desc => $query_desc_b1_c, + query_cgi => $query_cgi_b, + total => $total, + type => 'biblio', + }; + + my $query_desc_a1_c = q{first current authority search}; + my $first_current_authority_search = { + userid => $userid, + sessionid => $current_sessionid, + query_desc => $query_desc_a1_c, + query_cgi => $query_cgi_a, + total => $total, + type => 'authority', + }; + + my $query_desc_b2_c = q{second current biblio search}; + my $second_current_biblio_search = { + userid => $userid, + sessionid => $current_sessionid, + query_desc => $query_desc_b2_c, + query_cgi => $query_cgi_b, + total => $total, + type => 'biblio', + }; + + my $query_desc_a2_c = q{second current authority search}; + my $second_current_authority_search = { + userid => $userid, + sessionid => $current_sessionid, + query_desc => $query_desc_a2_c, + query_cgi => $query_cgi_a, + total => $total, + type => 'authority', + }; + + my $query_desc_a3_c = q{third current authority search}; + my $third_current_authority_search = { + userid => $userid, + sessionid => $current_sessionid, + query_desc => $query_desc_a3_c, + query_cgi => $query_cgi_a, + total => $total, + type => 'authority', + }; + + + my $r = 0; + $r += C4::Search::History::add( $first_current_biblio_search ); + $r += C4::Search::History::add( $first_current_authority_search ); + $r += C4::Search::History::add( $second_current_biblio_search ); + $r += C4::Search::History::add( $second_current_authority_search ); + $r += C4::Search::History::add( $first_previous_biblio_search ); + $r += C4::Search::History::add( $first_previous_authority_search ); + $r += C4::Search::History::add( $second_previous_biblio_search ); + $r += C4::Search::History::add( $second_previous_authority_search ); + $r += C4::Search::History::add( $third_current_authority_search ); + return $r; +} + +sub delete_all { + my $userid = shift; + C4::Search::History::delete({ + userid => $userid, + }); +} + +$dbh->rollback; + +done_testing; -- 1.7.9.5