Attachment 106860 Details for Bug 16371 – Bug 16371: Move GetDailyQuote to get_daily_quote

[patch] Bug 16371: Move GetDailyQuote to get_daily_quote

Bug-16371-Move-GetDailyQuote-to-getdailyquote.patch (text/plain), 13.22 KB, created by David Nind on 2020-07-14 08:33:07 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: David Nind

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

Size: 13.22 KB

patch

obsolete

>From d0ebd110444fb0fe0884993590caba21ce6a88b2 Mon Sep 17 00:00:00 2001
>From: Emmi Takkinen <emmi.takkinen@outlook.com>
>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 <david@davidnind.com>
>
>Signed-off-by: David Nind <david@davidnind.com>
>---
> 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 de5ad8e945..bd11fc0830 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 <http://www.gnu.org/licenses>.
> 
> 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' );
>-- 
>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