Attachment 106868 Details for Bug 16371 – Bug 16371: Rewrite get_daily_quote

[patch] Bug 16371: Rewrite get_daily_quote

Bug-16371-Rewrite-getdailyquote.patch (text/plain), 7.45 KB, created by David Nind on 2020-07-14 08:33:39 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: David Nind

Created: 2020-07-14 08:33:39 UTC

Size: 7.45 KB

patch

obsolete

>From 95411d7a8288881911777d5403ea85c55b5a0195 Mon Sep 17 00:00:00 2001
>From: Emmi Takkinen <emmi.takkinen@outlook.com>
>Date: Tue, 7 Jul 2020 15:45:17 +0300
>Subject: [PATCH] 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>
>---
> Koha/Quote.pm                | 69 +++++++++++++++++++-------------------------
> t/db_dependent/Koha/Quotes.t | 16 +++++-----
> 2 files changed, 38 insertions(+), 47 deletions(-)
>
>diff --git a/Koha/Quote.pm b/Koha/Quote.pm
>index 244aea0673..f189f12793 100644
>--- 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 c935569801..4031950547 100644
>--- 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;
>-- 
>2.11.0

Actions: View | Diff | Splinter Review

Attachments on bug 16371: 102515 | 102516 | 102517 | 102969 | 103019 | 103329 | 103330 | 103331 | 103332 | 103333 | 103334 | 103732 | 103973 | 103974 | 103975 | 103976 | 103977 | 103978 | 103979 | 106818 | 106819 | 106820 | 106858 | 106859 | 106860 | 106861 | 106862 | 106863 | 106864 | 106865 | 106866 | 106867 | 106868 | 106894 | 106895 | 106896 | 106897 | 106898 | 106899 | 106900 | 106901 | 106902 | 106903 | 108097 | 108098 | 108160 | 108163