From 79ed0bcfe1b879822661242634a4ea75e6b02c12 Mon Sep 17 00:00:00 2001 From: Emmi Takkinen Date: Tue, 7 Jul 2020 15:45:17 +0300 Subject: [PATCH 1/2] 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 --- 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..2d17718891 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.17.1