Bug 16371: Rewrite get_daily_quote

An updated test plan:
1. Apply the patch(es).
2. Run the database update (updatedatabase on koha-testing-docker).
3. Check that the 'QuoteOfTheDay' system preference options
   work as expected:
   - OPAC: QOTD only appears in the OAPC
   - Staff interface: QOTD only appears in the staff interface
   - Both (Select all): QOTD appears in the staff interface and OPAC
4. Run the tests and make sure they pass:
   prove t/db_dependent/Koha/Quotes.t
5. Sign off!

Sponsored-by: Koha-Suomi Oy

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

diff --git a/Koha/Quote.pm b/Koha/Quote.pm
index 244aea0..f189f12 100644 (file)
--- a/Koha/Quote.pm
+++ b/Koha/Quote.pm
@@ -23,6 +23,7 @@ use DBI qw(:sql_types);
 use Koha::Database;
 use Koha::DateUtils qw(dt_from_string);
 use Koha::Exceptions::UnknownProgramState;
+use Koha::Quotes;
 
 use base qw(Koha::Object);
 
@@ -46,15 +47,6 @@ Currently supported options are:
 'random'    Select a random quote
 noop        When no option is passed in, this sub will return the quote timestamped for the current day
 
-The function returns an anonymous hash following this format:
-
-        {
-          'source' => 'source-of-quote',
-          'timestamp' => 'timestamp-value',
-          'text' => 'text-of-quote',
-          'id' => 'quote-id'
-        };
-
 =cut
 
 # This is definitely a candidate for some sort of caching once we finally settle caching/persistence issues...
@@ -62,47 +54,46 @@ The function returns an anonymous hash following this format:
 
 sub get_daily_quote {
     my ($self, %opts) = @_;
-    my $dbh = C4::Context->dbh;
-    my $query = '';
-    my $sth = undef;
+
     my $quote = undef;
+
     if ($opts{'id'}) {
-        $query = 'SELECT * FROM quotes WHERE id = ?';
-        $sth = $dbh->prepare($query);
-        $sth->execute($opts{'id'});
-        $quote = $sth->fetchrow_hashref();
+        $quote = Koha::Quotes->find({ id => $opts{'id'} });
     }
     elsif ($opts{'random'}) {
         # Fall through... we also return a random quote as a catch-all if all else fails
     }
     else {
-        $query = 'SELECT * FROM quotes WHERE timestamp LIKE CONCAT(CURRENT_DATE,\'%\') ORDER BY timestamp DESC LIMIT 0,1';
-        $sth = $dbh->prepare($query);
-        $sth->execute();
-        $quote = $sth->fetchrow_hashref();
+        my $dt = dt_from_string()->ymd();
+        $quote = Koha::Quotes->search(
+            {
+                timestamp => { -like => "$dt%" },
+            },
+            {
+                order_by => { -desc => 'timestamp' },
+                rows => 1,
+            }
+        )->single;
     }
     unless ($quote) {        # if there are not matches, choose a random quote
-        # get a list of all available quote ids
-        $sth = C4::Context->dbh->prepare('SELECT count(*) FROM quotes;');
-        $sth->execute;
-        my $range = ($sth->fetchrow_array)[0];
-        # chose a random id within that range if there is more than one quote
+        my $range = Koha::Quotes->search->count;
         my $offset = int(rand($range));
-        # grab it
-        $query = 'SELECT * FROM quotes ORDER BY id LIMIT 1 OFFSET ?';
-        $sth = C4::Context->dbh->prepare($query);
-        # see http://www.perlmonks.org/?node_id=837422 for why
-        # we're being verbose and using bind_param
-        $sth->bind_param(1, $offset, SQL_INTEGER);
-        $sth->execute();
-        $quote = $sth->fetchrow_hashref();
+        $quote = Koha::Quotes->search(
+            {},
+            {
+                order_by => 'id',
+                rows => 1,
+                offset => $offset,
+            }
+        )->single;
+
+        unless($quote){
+            return;
+        }
+
         # update the timestamp for that quote
-        $query = 'UPDATE quotes SET timestamp = ? WHERE id = ?';
-        $sth = C4::Context->dbh->prepare($query);
-        $sth->execute(
-            DateTime::Format::MySQL->format_datetime( dt_from_string() ),
-            $quote->{'id'}
-        );
+        my $dt = DateTime::Format::MySQL->format_datetime(dt_from_string());
+        $quote->update({ timestamp => $dt });
     }
     return $quote;
 }

diff --git a/t/db_dependent/Koha/Quotes.t b/t/db_dependent/Koha/Quotes.t
index c935569..4031950 100644 (file)
--- a/t/db_dependent/Koha/Quotes.t
+++ b/t/db_dependent/Koha/Quotes.t
@@ -50,35 +50,35 @@ my $expected_quote = {
     id          => $quote_3->id,
     source      => 'Abraham Lincoln',
     text        => 'Four score and seven years ago our fathers brought forth on this continent, a new nation, conceived in Liberty, and dedicated to the proposition that all men are created equal.',
-    timestamp   => dt_from_string,
+    timestamp   => $timestamp,
 };
 
 $quote = Koha::Quote->get_daily_quote('id'=>$quote_3->id);
-cmp_ok($quote->{'id'}, '==', $expected_quote->{'id'}, "Correctly got quote by ID");
+cmp_ok($quote->id, '==', $expected_quote->{'id'}, "Correctly got quote by ID");
 is($quote->{'quote'}, $expected_quote->{'quote'}, "Quote is correct");
 
 $quote = Koha::Quote->get_daily_quote('random'=>1);
 ok($quote, "Got a random quote.");
-cmp_ok($quote->{'id'}, '>', 0, 'Id is greater than 0');
+cmp_ok($quote->id, '>', 0, 'Id is greater than 0');
 
 $timestamp = DateTime::Format::MySQL->format_datetime(dt_from_string->add( seconds => 1 )); # To make it the last one
 Koha::Quotes->search({ id => $expected_quote->{'id'} })->update({ timestamp => $timestamp });
 $expected_quote->{'timestamp'} = $timestamp;
 
-$quote = Koha::Quote->get_daily_quote(); # this is the "default" mode of selection
+$quote = Koha::Quote->get_daily_quote()->unblessed; # this is the "default" mode of selection
 cmp_ok($quote->{'id'}, '==', $expected_quote->{'id'}, "Id is correct");
 is($quote->{'source'}, $expected_quote->{'source'}, "Source is correct");
 is($quote->{'timestamp'}, $expected_quote->{'timestamp'}, "Timestamp $timestamp is correct");
 
-$dbh->do(q|DELETE FROM quotes|);
+Koha::Quotes->search()->delete();
 $quote = eval {Koha::Quote->get_daily_quote();};
 is( $@, '', 'get_daily_quote does not die if no quote exist' );
-is_deeply( $quote, {}, 'get_daily_quote return an empty hashref is no quote exist'); # Is it what we expect?
+is_deeply( $quote, undef, 'return undef if quotes do not exists'); # Is it what we expect?
 
 my $quote_6 = Koha::Quote->new({ source => 'George Washington', text => 'To be prepared for war is one of the most effectual means of preserving peace.', timestamp =>  dt_from_string() })->store;
 
 $quote = Koha::Quote->get_daily_quote();
-is( $quote->{id}, $quote_6->id, ' get_daily_quote returns the only existing quote' );
+is( $quote->id, $quote_6->id, ' get_daily_quote returns the only existing quote' );
 
 $schema->storage->txn_rollback;
 
@@ -112,7 +112,7 @@ subtest "get_daily_quote_for_interface" => sub {
 
     ##Set 'QuoteOfTheDay'-syspref to include current interface 'opac'
     t::lib::Mocks::mock_preference('QuoteOfTheDay', 'opac,intranet');
-    $quote = Koha::Quote->get_daily_quote_for_interface(id => $quote_1->id);
+    $quote = Koha::Quote->get_daily_quote_for_interface(id => $quote_1->id)->unblessed;
     is_deeply($quote, $expected_quote, "Got the expected quote");
 
     $schema->storage->txn_rollback;