From 43d848c662017b9a4e59c7de134735cb5564df9c Mon Sep 17 00:00:00 2001 From: Julian Maurice Date: Wed, 18 Dec 2013 10:58:36 +0100 Subject: [PATCH] Bug 10952: comments fixes and unit tests - Remove unit tests for ParseSearchHistoryCookie, which doesn't exist anymore - Add unit tests for ParseSearchHistorySession and SetSearchHistorySession - Remove/Modify comments about search history cookie Signed-off-by: Chris Cormack Tests fixed and moved, and comments tidied up --- C4/Auth.pm | 2 +- opac/opac-search-history.pl | 7 ++-- opac/opac-search.pl | 4 +- t/db_dependent/Auth_ParseSearchHistoryCookie.t | 43 --------------------- t/db_dependent/Auth_SearchHistorySession.t | 52 ++++++++++++++++++++++++++ 5 files changed, 57 insertions(+), 51 deletions(-) delete mode 100644 t/db_dependent/Auth_ParseSearchHistoryCookie.t create mode 100644 t/db_dependent/Auth_SearchHistorySession.t diff --git a/C4/Auth.pm b/C4/Auth.pm index 0d4daec..7bc7bd9 100644 --- a/C4/Auth.pm +++ b/C4/Auth.pm @@ -260,7 +260,7 @@ sub get_template_and_user { $template->param(ShowOpacRecentSearchLink => 1); } - # And if there's a cookie with searches performed when the user was not logged in, + # 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'}); if (@recentSearches) { diff --git a/opac/opac-search-history.pl b/opac/opac-search-history.pl index 4c02ba8..760b52f 100755 --- a/opac/opac-search-history.pl +++ b/opac/opac-search-history.pl @@ -44,18 +44,17 @@ my ($template, $loggedinuser, $cookie)= get_template_and_user({template_name => debug => 1, }); -# If the user is not logged in, we deal with the cookie +# If the user is not logged in, we deal with the session if (!$loggedinuser) { # Deleting search history if ($cgi->param('action') && $cgi->param('action') eq 'delete') { - # Deleting cookie's content + # Deleting session's search history SetSearchHistorySession($cgi, []); - # Redirecting to this same url with the cookie in the headers so it's deleted immediately + # 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 { diff --git a/opac/opac-search.pl b/opac/opac-search.pl index 00cd8e3..617d67e 100755 --- a/opac/opac-search.pl +++ b/opac/opac-search.pl @@ -614,7 +614,6 @@ for (my $i=0;$i<@servers;$i++) { } # Opac search history - my $newsearchcookie; if (C4::Context->preference('EnableOpacSearchHistory')) { my @recentSearches = ParseSearchHistorySession($cgi); @@ -638,11 +637,10 @@ for (my $i=0;$i<@servers;$i++) { } shift @recentSearches if (@recentSearches > 15); - # Pushing the cookie back SetSearchHistorySession($cgi, \@recentSearches); } else { - # To the session (the user is logged in) + # 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); diff --git a/t/db_dependent/Auth_ParseSearchHistoryCookie.t b/t/db_dependent/Auth_ParseSearchHistoryCookie.t deleted file mode 100644 index 507ac91..0000000 --- a/t/db_dependent/Auth_ParseSearchHistoryCookie.t +++ /dev/null @@ -1,43 +0,0 @@ -#!/usr/bin/perl - -use strict; -use warnings; - -use Test::More tests => 3; - -use_ok('C4::Auth', qw/ParseSearchHistoryCookie/); - -my $valid_cookie = "%5B%7B%22time%22%3A1374978877%2C%22query_cgi%22%3A%22idx%3D%26q%3Dhistory%26branch_group_limit%3D%22%2C%22total%22%3A2%2C%22query_desc%22%3A%22kw%2Cwrdl%3A%20history%2C%20%22%7D%5D"; -my $expected_recent_searches = [ - { - 'time' => 1374978877, - 'query_cgi' => 'idx=&q=history&branch_group_limit=', - 'total' => 2, - 'query_desc' => 'kw,wrdl: history, ' - } -]; - -my $input = CookieSimulator->new($valid_cookie); -my @recent_searches = ParseSearchHistoryCookie($input); -is_deeply(\@recent_searches, $expected_recent_searches, 'parsed valid search history cookie value'); - -# simulate bit of a Storable-based search history cookie -my $invalid_cookie = "%04%08%0812345"; -$input = CookieSimulator->new($invalid_cookie); -@recent_searches = ParseSearchHistoryCookie($input); -is_deeply(\@recent_searches, [], 'got back empty search history list if given invalid cookie'); - -package CookieSimulator; - -sub new { - my ($class, $str) = @_; - my $val = [ $str ]; - return bless $val, $class; -} - -sub cookie { - my $self = shift; - return $self->[0]; -} - -1; diff --git a/t/db_dependent/Auth_SearchHistorySession.t b/t/db_dependent/Auth_SearchHistorySession.t new file mode 100644 index 0000000..1e0a0ea --- /dev/null +++ b/t/db_dependent/Auth_SearchHistorySession.t @@ -0,0 +1,52 @@ +#!/usr/bin/perl + +use strict; +use warnings; + +use Test::More tests => 4; + +use_ok('C4::Auth', qw/ParseSearchHistorySession SetSearchHistorySession get_session/); + +my $expected_recent_searches = [ + { + 'time' => 1374978877, + 'query_cgi' => 'idx=&q=history&branch_group_limit=', + 'total' => 2, + 'query_desc' => 'kw,wrdl: history, ' + } +]; + +# Create new session and put its id into CGISESSID cookie +my $session = get_session(""); +$session->flush; +my $input = new CookieSimulator({CGISESSID => $session->id}); + +my @recent_searches = ParseSearchHistorySession($input); +is_deeply(\@recent_searches, [], 'at start, there is no recent searches'); + +SetSearchHistorySession($input, $expected_recent_searches); +@recent_searches = ParseSearchHistorySession($input); +is_deeply(\@recent_searches, $expected_recent_searches, 'recent searches set and retrieved successfully'); + +SetSearchHistorySession($input, []); +@recent_searches = ParseSearchHistorySession($input); +is_deeply(\@recent_searches, [], 'recent searches emptied successfully'); + +# Delete session +$session->delete; +$session->flush; + +package CookieSimulator; + +sub new { + my ($class, $hashref) = @_; + my $val = $hashref; + return bless $val, $class; +} + +sub cookie { + my ($self, $name) = @_; + return $self->{$name}; +} + +1; -- 1.8.3.2