@@ -, +, @@ - 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 | 62 ++--- C4/Search.pm | 23 -- C4/Search/History.pm | 268 ++++++++++++++++++++ installer/data/mysql/kohastructure.sql | 1 + installer/data/mysql/updatedatabase.pl | 14 + 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 | 219 +++++++++++----- opac/opac-authorities-home.pl | 33 +++ opac/opac-search-history.pl | 169 ++++++------ opac/opac-search.pl | 59 +++-- .../History.t} | 27 +- t/db_dependent/Search/History.t | 248 ++++++++++++++++++ 15 files changed, 878 insertions(+), 253 deletions(-) create mode 100644 C4/Search/History.pm rename t/{db_dependent/Auth_SearchHistorySession.t => Search/History.t} (55%) 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 Koha::AuthUtils qw(hash_password); use POSIX qw/strftime/; @@ -49,7 +50,6 @@ BEGIN { @EXPORT = qw(&checkauth &get_template_and_user &haspermission &get_user_subpermissions); @EXPORT_OK = qw(&check_api_auth &get_session &check_cookie_auth &checkpw &checkpw_internal &checkpw_hash &get_all_subpermissions &get_user_subpermissions - ParseSearchHistorySession SetSearchHistorySession ); %EXPORT_TAGS = ( EditPermissions => [qw(get_all_subpermissions get_user_subpermissions)] ); $ldap = C4::Context->config('useldapserver') || 0; @@ -129,11 +129,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 are searches performed when the user was not logged in, # we add them to the logged-in search history - my @recentSearches = ParseSearchHistorySession($in->{'query'}); + my @recentSearches = C4::Search::History::get_from_session({ cgi => $in->{'query'} }); 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; # clear out the search history from the session now that # we've saved it to the database - SetSearchHistorySession($in->{'query'}, []); + C4::Search::History::set_to_session({ cgi => $in->{'query'}, search_history => [] }); } } } @@ -292,9 +295,9 @@ 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 = ParseSearchHistorySession($in->{'query'}); + my @recentSearches = C4::Search::History::get_from_session({ cgi => $in->{'query'} }); if (@recentSearches) { - $template->param(ShowOpacRecentSearchLink => 1); + $template->param(EnableOpacSearchHistory => 1); } } @@ -1791,27 +1794,6 @@ sub getborrowernumber { return 0; } -sub ParseSearchHistorySession { - my $cgi = shift; - my $sessionID = $cgi->cookie('CGISESSID'); - return () unless $sessionID; - my $session = get_session($sessionID); - return () unless $session and $session->param('search_history'); - my $obj = eval { decode_json(uri_unescape($session->param('search_history'))) }; - return () unless defined $obj; - return () unless ref $obj eq 'ARRAY'; - return @{ $obj }; -} - -sub SetSearchHistorySession { - my ($cgi, $search_history) = @_; - my $sessionID = $cgi->cookie('CGISESSID'); - return () unless $sessionID; - my $session = get_session($sessionID); - return () unless $session; - $session->param('search_history', uri_escape(encode_json($search_history))); -} - 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 @@ -2202,28 +2201,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,268 @@ +package C4::Search::History; + +use Modern::Perl; + +use C4::Auth qw( get_session ); +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} // 0; + 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 add_to_session { + my ($params) = @_; + my $cgi = $params->{cgi}; + 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 = get_from_session( { cgi => $cgi } ); + push @recent_searches, + { + query_desc => $query_desc, + query_cgi => $query_cgi, + total => "$total", + type => $type, + time => output_pref( { dt => dt_from_string(), dateformat => 'iso' } ), + }; + + shift @recent_searches if ( @recent_searches > 15 ); + set_to_session( { cgi => $cgi, search_history => \@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_session { + my ($params) = @_; + my $cgi = $params->{cgi}; + my $sessionID = $cgi->cookie('CGISESSID'); + return () unless $sessionID; + my $session = C4::Auth::get_session($sessionID); + return () unless $session and $session->param('search_history'); + my $obj = + eval { decode_json( uri_unescape( $session->param('search_history') ) ) }; + return () unless defined $obj; + return () unless ref $obj eq 'ARRAY'; + return @{$obj}; +} + +sub set_to_session { + my ($params) = @_; + my $cgi = $params->{cgi}; + my $search_history = $params->{search_history}; + my $sessionID = $cgi->cookie('CGISESSID'); + return () unless $sessionID; + my $session = C4::Auth::get_session($sessionID); + return () unless $session; + $session->param( 'search_history', + uri_escape( encode_json($search_history) ) ); +} + +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 session 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 add_to_session + + my $value = C4::Search::History::add_to_session({ + cgi => $cgi, + query_desc => $query_desc, + query_cgi => $query_cgi, + total => $total, + type => $type, + }); + +Add a search to the session. The number of searches to keep is hardcoded to 15. + +=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_session + + my $searches = C4::Search::History::get_from_session({ + cgi => $cgi + }); + +Return all searches present for the given session. + +=head2 set_to_session + + C4::Search::History::set_to_session({ + cgi => $cgi, + search_history => $search_history + }); + +Store searches into the session. + +=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 @@ -1911,6 +1911,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 @@ -7912,6 +7912,20 @@ if ( CheckVersion($DBversion) ) { SetVersion($DBversion); } + + + + + +$DBversion = "3.15.00.XXX"; +if ( CheckVersion($DBversion) ) { + $dbh->do(q| + 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,151 @@
    [% 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 ( recentSearches ) %] - - [% IF ( previousSearches ) %] - - [% END %] - - - - - [% FOREACH recentSearche IN recentSearches %] - - - - - - [% END %] - -
    Current session
    DateSearchResults
    [% recentSearche.time %][% recentSearche.query_desc |html %][% recentSearche.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 ( 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 %][% 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,37 @@ if ( $op eq "do_search" ) { my @usedauths = grep { $_->{used} > 0 } @$results; $results = \@usedauths; } + + # Opac search history + 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::add_to_session({ + cgi => $query, + query_desc => $value[0], + query_cgi => $query_cgi_history, + total => $total, + type => "authority", + }); + } 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 ParseSearchHistorySession SetSearchHistorySession); +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,100 +35,116 @@ 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, + } +); -# If the user is not logged in, we deal with the session -if (!$loggedinuser) { +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 session +unless ( $loggedinuser ) { # Deleting search history if ($cgi->param('action') && $cgi->param('action') eq 'delete') { # Deleting session's search history - SetSearchHistorySession($cgi, []); + my $type = $cgi->param('type'); + my @searches = (); + if ( $type ) { + @searches = C4::Search::History::get_from_session({ cgi => $cgi }); + @searches = map { $_->{type} ne $type ? $_ : () } @searches; + } + C4::Search::History::set_to_session({ cgi => $cgi, search_history => \@searches }); # 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 => $uri); # Showing search history } else { - - my @recentSearches = ParseSearchHistorySession($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'}); - - # 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'} = $isodate . $time; - } - - $template->param(recentSearches => \@recentSearches); - } + # Getting the searches from session + my @current_searches = C4::Search::History::get_from_session({ + cgi => $cgi, + }); + + 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 ParseSearchHistorySession SetSearchHistorySession); +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); @@ -615,38 +616,36 @@ for (my $i=0;$i<@servers;$i++) { # Opac search history if (C4::Context->preference('EnableOpacSearchHistory')) { - my @recentSearches = ParseSearchHistorySession($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 = join ", ", grep { defined $_ } $query_desc, $limit_desc; - - if (!$borrowernumber || $borrowernumber eq '') { - # To the session (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); - } - - shift @recentSearches if (@recentSearches > 15); - SetSearchHistorySession($cgi, \@recentSearches); - } - else { - # To the database (the user is logged in) - if (!$offset) { - AddSearchHistory($borrowernumber, $cgi->cookie("CGISESSID"), $query_desc_history, $query_cgi_history, $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 = join ", ", grep { defined $_ } $query_desc, $limit_desc; + + unless ( $borrowernumber ) { + my $new_searches = C4::Search::History::add_to_session({ + cgi => $cgi, + query_desc => $query_desc_history, + query_cgi => $query_cgi_history, + total => $total, + type => "biblio", + }); + } 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/db_dependent/Auth_SearchHistorySession.t +++ a/t/db_dependent/Auth_SearchHistorySession.t @@ -1,12 +1,16 @@ -#!/usr/bin/perl +#!/usr/bin/env perl -use strict; -use warnings; +use Modern::Perl; -use Test::More tests => 4; +use Test::More tests => 6; +use URI::Escape; +use JSON qw( decode_json ); -use_ok('C4::Auth', qw/ParseSearchHistorySession SetSearchHistorySession get_session/); +use_ok('Koha::DateUtils'); +use_ok('C4::Search::History'); +use_ok('C4::Auth', qw/get_session/ ); +# Test session my $expected_recent_searches = [ { 'time' => 1374978877, @@ -17,19 +21,19 @@ my $expected_recent_searches = [ ]; # Create new session and put its id into CGISESSID cookie -my $session = get_session(""); +my $session = C4::Auth::get_session(""); $session->flush; my $input = new CookieSimulator({CGISESSID => $session->id}); -my @recent_searches = ParseSearchHistorySession($input); +my @recent_searches = C4::Search::History::get_from_session({ cgi => $input }); is_deeply(\@recent_searches, [], 'at start, there is no recent searches'); -SetSearchHistorySession($input, $expected_recent_searches); -@recent_searches = ParseSearchHistorySession($input); +C4::Search::History::set_to_session({ cgi => $input, search_history => $expected_recent_searches }); +@recent_searches = C4::Search::History::get_from_session({ cgi => $input }); is_deeply(\@recent_searches, $expected_recent_searches, 'recent searches set and retrieved successfully'); -SetSearchHistorySession($input, []); -@recent_searches = ParseSearchHistorySession($input); +C4::Search::History::set_to_session({ cgi => $input, search_history => [] }); +@recent_searches = C4::Search::History::get_from_session({ cgi => $input }); is_deeply(\@recent_searches, [], 'recent searches emptied successfully'); # Delete session @@ -49,4 +53,3 @@ sub cookie { return $self->{$name}; } -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; --