From f32f92cb577e01d638b1adbc3f065b511eb77d4a Mon Sep 17 00:00:00 2001 From: Emmi Takkinen Date: Fri, 17 Apr 2020 11:28:55 +0300 Subject: [PATCH] Bug 16371: Move GetDailyQuote to get_daily_quote This patch moves subroutine 'GetDailyQuote' to new Koha::Quote object and adjusts tests. To test: 1. Set 'QuoteOfTheDay' as 'enable' 2. Check that quote is displayed on OPAC mainpage Prove t/db_dependent/Koha/GetDailyQuote.t Sponsored-by: Koha-Suomi Oy Signed-off-by: David Nind Signed-off-by: David Nind Signed-off-by: Katrin Fischer Signed-off-by: Jonathan Druart --- C4/Koha.pm | 75 ----------------------------- Koha/Quote.pm | 74 ++++++++++++++++++++++++++++ opac/opac-main.pl | 4 +- t/db_dependent/Koha/GetDailyQuote.t | 54 +++++++++------------ 4 files changed, 100 insertions(+), 107 deletions(-) diff --git a/C4/Koha.pm b/C4/Koha.pm index 2e51935dae..aac05c76f9 100644 --- a/C4/Koha.pm +++ b/C4/Koha.pm @@ -24,15 +24,12 @@ use Modern::Perl; use C4::Context; use Koha::Caches; -use Koha::DateUtils qw(dt_from_string); use Koha::AuthorisedValues; use Koha::Libraries; use Koha::MarcSubfieldStructures; -use DateTime::Format::MySQL; use Business::ISBN; use Business::ISSN; use autouse 'Data::cselectall_arrayref' => qw(Dumper); -use DBI qw(:sql_types); use vars qw(@ISA @EXPORT @EXPORT_OK $DEBUG); BEGIN { @@ -63,7 +60,6 @@ BEGIN { $DEBUG ); $DEBUG = 0; -@EXPORT_OK = qw( GetDailyQuote ); } =head1 NAME @@ -686,77 +682,6 @@ sub GetNormalizedOCLCNumber { return } -=head2 GetDailyQuote($opts) - -Takes a hashref of options - -Currently supported options are: - -'id' An exact quote id -'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... -# at least for default option - -sub GetDailyQuote { - my %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(); - } - 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(); - } - 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 $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(); - # 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'} - ); - } - return $quote; -} - sub _normalize_match_point { my $match_point = shift; (my $normalized_match_point) = $match_point =~ /([\d-]*[X]*)/; diff --git a/Koha/Quote.pm b/Koha/Quote.pm index a16e2dd7e4..ecfc086022 100644 --- a/Koha/Quote.pm +++ b/Koha/Quote.pm @@ -17,8 +17,11 @@ package Koha::Quote; use Modern::Perl; use Carp; +use DateTime::Format::MySQL; +use DBI qw(:sql_types); use Koha::Database; +use Koha::DateUtils qw(dt_from_string); use base qw(Koha::Object); @@ -32,6 +35,77 @@ Koha::Quote - Koha Quote object class =cut +=head2 get_daily_quote($opts) + +Takes a hashref of options + +Currently supported options are: + +'id' An exact quote id +'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... +# at least for default option + +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(); + } + 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(); + } + 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 $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(); + # 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'} + ); + } + return $quote; +} + =head3 _type =cut diff --git a/opac/opac-main.pl b/opac/opac-main.pl index 78b8d74fe7..6104d606b1 100755 --- a/opac/opac-main.pl +++ b/opac/opac-main.pl @@ -24,7 +24,7 @@ use C4::Auth; # get_template_and_user use C4::Output; use C4::NewsChannels; # GetNewsToDisplay use C4::Languages qw(getTranslatedLanguages accept_language); -use C4::Koha qw( GetDailyQuote ); +use Koha::Quote; use C4::Members; use C4::Overdues; use Koha::Checkouts; @@ -73,7 +73,7 @@ if (defined $news_id){ @all_koha_news = &GetNewsToDisplay( $template->lang, $homebranch); } -my $quote = GetDailyQuote(); # other options are to pass in an exact quote id or select a random quote each pass... see perldoc C4::Koha +my $quote = Koha::Quote->get_daily_quote(); # other options are to pass in an exact quote id or select a random quote each pass... see perldoc C4::Koha # For dashboard my $patron = Koha::Patrons->find( $borrowernumber ); diff --git a/t/db_dependent/Koha/GetDailyQuote.t b/t/db_dependent/Koha/GetDailyQuote.t index a2f5514f53..d4c979b5c3 100644 --- a/t/db_dependent/Koha/GetDailyQuote.t +++ b/t/db_dependent/Koha/GetDailyQuote.t @@ -16,34 +16,32 @@ # along with Koha; if not, see . use Modern::Perl; - +use DateTime::Format::MySQL; use Test::More tests => 12; -use C4::Koha qw( GetDailyQuote ); -use DateTime::Format::MySQL; use Koha::Database; use Koha::DateUtils qw(dt_from_string); +use Koha::Quote; +use Koha::Quotes; BEGIN { - use_ok('C4::Koha'); + use_ok('Koha::Quote'); } -can_ok('C4::Koha', qw( GetDailyQuote )); +my $quote = Koha::Quote->new(); +isa_ok( $quote, 'Koha::Quote', 'Quote class returned' ); my $schema = Koha::Database->new->schema; $schema->storage->txn_begin; my $dbh = C4::Context->dbh; -# Setup stage -$dbh->do("DELETE FROM quotes"); - # Ids not starting with 1 to reflect possible deletes, this acts as a regression test for bug 11297 -$dbh->do("INSERT INTO `quotes` VALUES -(6,'George Washington','To be prepared for war is one of the most effectual means of preserving peace.',NOW()), -(7,'Thomas Jefferson','When angry, count ten, before you speak; if very angry, an hundred.',NOW()), -(8,'Abraham Lincoln','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.',NOW()), -(9,'Abraham Lincoln','I have always found that mercy bears richer fruits than strict justice.',NOW()), -(10,'Andrew Johnson','I feel incompetent to perform duties...which have been so unexpectedly thrown upon me.',NOW());"); +my $timestamp = DateTime::Format::MySQL->format_datetime(dt_from_string()); #??? +my $quote_1 = Koha::Quote->new({ id => 6, source => 'George Washington', text => 'To be prepared for war is one of the most effectual means of preserving peace.', timestamp => $timestamp })->store; +my $quote_2 = Koha::Quote->new({ id => 7, source => 'Thomas Jefferson', text => 'When angry, count ten, before you speak; if very angry, an hundred.', timestamp => $timestamp })->store; +my $quote_3 = Koha::Quote->new({ id => 8, 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 => $timestamp })->store; +my $quote_4 = Koha::Quote->new({ id => 9, source => 'Abraham Lincoln', text => 'I have always found that mercy bears richer fruits than strict justice.', timestamp => $timestamp })->store; +my $quote_5 = Koha::Quote->new({ id => 10, source => 'Andrew Johnson', text => 'I feel incompetent to perform duties...which have been so unexpectedly thrown upon me.', timestamp => $timestamp })->store; my $expected_quote = { id => 8, @@ -52,33 +50,29 @@ my $expected_quote = { timestamp => dt_from_string, }; -my $quote = GetDailyQuote('id'=>8); +$quote = Koha::Quote->get_daily_quote('id'=>$quote_3->id); cmp_ok($quote->{'id'}, '==', $expected_quote->{'id'}, "Correctly got quote by ID"); is($quote->{'quote'}, $expected_quote->{'quote'}, "Quote is correct"); -$quote = GetDailyQuote('random'=>1); +$quote = Koha::Quote->get_daily_quote('random'=>1); ok($quote, "Got a random quote."); cmp_ok($quote->{'id'}, '>', 0, 'Id is greater than 0'); -my $timestamp = DateTime::Format::MySQL->format_datetime(dt_from_string->add( seconds => 1 )); # To make it the last one -my $query = 'UPDATE quotes SET timestamp = ? WHERE id = ?'; -my $sth = C4::Context->dbh->prepare($query); -$sth->execute( $timestamp , $expected_quote->{'id'}); - +$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 = GetDailyQuote(); # this is the "default" mode of selection +$quote = Koha::Quote->get_daily_quote(); # 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|); -$quote = eval {GetDailyQuote();}; -is( $@, '', 'GetDailyQuote does not die if no quote exist' ); -is_deeply( $quote, {}, 'GetDailyQuote return an empty hashref is no quote exist'); # Is it what we expect? -$dbh->do(q|INSERT INTO `quotes` VALUES - (6,'George Washington','To be prepared for war is one of the most effectual means of preserving peace.',NOW()) -|); +$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? + +my $quote_6 = Koha::Quote->new({ id => 6, 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 = GetDailyQuote(); -is( $quote->{id}, 6, ' GetDailyQuote returns the only existing quote' ); +$quote = Koha::Quote->get_daily_quote(); +is( $quote->{id}, 6, ' get_daily_quote returns the only existing quote' );