From ba41b7da79507b39bfa59965639d92be56db2496 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Fri, 30 Aug 2013 14:01:15 +0200 Subject: [PATCH] Bug 10807: Add an authority search history for the OPAC 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 an anonymous user sessin. 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 Signed-off-by: Katrin Fischer All patches together pass QA script and tests. Also, new tests in t/db_dependent/ pass. Tested in all 4 OPAC themes, being logged in and anonymous. Anonymous search history will be appended to personal search history after logging in. Also verified that cleanup_database still purges search history, now also including the authority searchs. Signed-off-by: Galen Charlton --- C4/Auth.pm | 62 ++-- C4/Search.pm | 23 -- C4/Search/History.pm | 268 ++++++++++++++++++ installer/data/mysql/kohastructure.sql | 1 + installer/data/mysql/updatedatabase.pl | 9 + .../opac-tmpl/ccsr/en/includes/top-bar.inc | 2 +- .../opac-tmpl/ccsr/en/includes/usermenu.inc | 2 +- .../opac-tmpl/prog/en/includes/masthead.inc | 2 +- .../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, 873 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 diff --git a/C4/Auth.pm b/C4/Auth.pm index 44edf67ce2..006a990826 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 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__ diff --git a/C4/Search.pm b/C4/Search.pm index 2c63f9c7ec..cc3d09e794 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 @@ -2223,28 +2222,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 0000000000..e4e78af87b --- /dev/null +++ b/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. diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql index 73bb45d800..ad6e530138 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -1910,6 +1910,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 1fcc238474..f9c23ea617 100755 --- a/installer/data/mysql/updatedatabase.pl +++ b/installer/data/mysql/updatedatabase.pl @@ -8385,6 +8385,15 @@ 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) 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 42d9576b39..a6ff97838a 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 fcbbcba4fc..91db9e28d9 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 76bf5ea7ed..28449fc192 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 5a07d9587e..5c0383be1e 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 4d0fa8297d..fc9da828e7 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,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' %] diff --git a/opac/opac-authorities-home.pl b/opac/opac-authorities-home.pl index 115012e231..5a674ce697 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,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 ); diff --git a/opac/opac-search-history.pl b/opac/opac-search-history.pl index 760b52f959..55426a2a36 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 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; - - diff --git a/opac/opac-search.pl b/opac/opac-search.pl index e2236c2e80..00590c25b8 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 ParseSearchHistorySession SetSearchHistorySession); +use C4::Auth qw(:DEFAULT get_session); use C4::Languages qw(getLanguages); use C4::Search; +use C4::Search::History; use C4::Biblio; # GetBiblioData use C4::Koha; use C4::Tags qw(get_tags); @@ -616,38 +617,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') { diff --git a/t/db_dependent/Auth_SearchHistorySession.t b/t/Search/History.t similarity index 55% rename from t/db_dependent/Auth_SearchHistorySession.t rename to t/Search/History.t index 1e0a0ea2dd..2808e44149 100644 --- a/t/db_dependent/Auth_SearchHistorySession.t +++ b/t/Search/History.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; diff --git a/t/db_dependent/Search/History.t b/t/db_dependent/Search/History.t new file mode 100644 index 0000000000..33e834de37 --- /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; -- 2.39.5