@@ -, +, @@ - 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. 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 --- 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 | 35 +++ t/db_dependent/Auth_ParseSearchHistoryCookie.t | 43 ---- t/db_dependent/Search/History.t | 248 ++++++++++++++++++ 16 files changed, 939 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 --- a/C4/Auth.pm +++ a/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__ --- a/C4/Search.pm +++ a/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; --- a/C4/Search/History.pm +++ a/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. --- a/installer/data/mysql/kohastructure.sql +++ a/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`), --- a/installer/data/mysql/updatedatabase.pl +++ a/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) --- a/koha-tmpl/opac-tmpl/ccsr/en/includes/top-bar.inc +++ a/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 %] --- a/koha-tmpl/opac-tmpl/ccsr/en/includes/usermenu.inc +++ a/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 ) %] --- a/koha-tmpl/opac-tmpl/prog/en/includes/masthead.inc +++ a/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 %] --- a/koha-tmpl/opac-tmpl/prog/en/includes/usermenu.inc +++ a/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 ) %] --- a/koha-tmpl/opac-tmpl/prog/en/modules/opac-search-history.tt +++ a/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' %] --- a/opac/opac-authorities-home.pl +++ a/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 ); --- a/opac/opac-search-history.pl +++ a/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; - - --- a/opac/opac-search.pl +++ a/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') { --- a/t/Search/History.t +++ a/t/Search/History.t @@ -0,0 +1,35 @@ +#!/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' ); + --- a/t/db_dependent/Auth_ParseSearchHistoryCookie.t +++ a/t/db_dependent/Auth_ParseSearchHistoryCookie.t @@ -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; --- a/t/db_dependent/Search/History.t +++ a/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; --