Bug 10952: (follow-up) 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 <chrisc@catalyst.net.nz>
Tests fixed and moved, and comments tidied up

Signed-off-by: Charlene Criton <charlene.criton@univ-lyon2.fr>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>

diff --git a/C4/Auth.pm b/C4/Auth.pm
index 0d4daec..7bc7bd9 100644 (file)
--- 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 (executable)
--- 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 (executable)
--- 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 (file)
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 (file)
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;