Bug 16371: Combine get_daily_quote and get_daily_quote_for_interface
authorEmmi Takkinen <emmi.takkinen@outlook.com>
Fri, 31 Jul 2020 06:47:56 +0000 (09:47 +0300)
committerJonathan Druart <jonathan.druart@bugs.koha-community.org>
Thu, 13 Aug 2020 08:15:33 +0000 (10:15 +0200)
This patch combines get_daily_quote and get_daily_quote_for_interface
methods and moves them from Koha::Quote to Koha::Quotes. Also removes
some unused code and adjusts datetime parsing.

To test apply this patch and confirm 'QuoteOfTheDay' syspref still
works as expected (quote is shown in correct mainpage(s)).

Also prove t/db_dependent/Koha/Quotes.t

Sponsored-by: Koha-Suomi Oy

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

Koha/Exceptions.pm
Koha/Quote.pm
Koha/Quotes.pm
mainpage.pl
opac/opac-main.pl
t/db_dependent/Koha/Quotes.t

index c811b97..cf56b22 100644 (file)
@@ -50,10 +50,6 @@ use Exception::Class (
         isa => 'Koha::Exceptions::Exception',
         description => 'Koha is under maintenance.'
     },
-    'Koha::Exceptions::UnknownProgramState' => {
-        isa => 'Koha::Exceptions::Exception',
-        description => 'The running program has done something terribly unpredicatable',
-    },
     # Virtualshelves exceptions
     'Koha::Exceptions::Virtualshelves::DuplicateObject' => {
         isa => 'Koha::Exceptions::DuplicateObject',
index f189f12..acb844d 100644 (file)
@@ -17,12 +17,8 @@ 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 Koha::Exceptions::UnknownProgramState;
 use Koha::Quotes;
 
 use base qw(Koha::Object);
@@ -37,92 +33,6 @@ 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
-
-=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 $quote = undef;
-
-    if ($opts{'id'}) {
-        $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 {
-        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
-        my $range = Koha::Quotes->search->count;
-        my $offset = int(rand($range));
-        $quote = Koha::Quotes->search(
-            {},
-            {
-                order_by => 'id',
-                rows => 1,
-                offset => $offset,
-            }
-        )->single;
-
-        unless($quote){
-            return;
-        }
-
-        # update the timestamp for that quote
-        my $dt = DateTime::Format::MySQL->format_datetime(dt_from_string());
-        $quote->update({ timestamp => $dt });
-    }
-    return $quote;
-}
-
-=head2 get_daily_quote_for_interface
-
-    my $quote = Koha::Quote->get_daily_quote_for_interface();
-
-Is a wrapper for get_daily_quote(), with an extra check for using the correct
-interface defined in the syspref 'QuoteOfTheDay'.
-If the current interface is not allowed to display quotes, then returns nothing.
-
-=cut
-
-sub get_daily_quote_for_interface {
-    my ($self, %opts) = @_;
-    my $qotdPref = C4::Context->preference('QuoteOfTheDay');
-    my $interface = C4::Context->interface();
-    unless ($interface) {
-        my @cc = caller(3);
-        Koha::Exceptions::UnknownProgramState->throw(error => $cc[3]."()> C4::Context->interface() is not set! Don't know are you in OPAC or staff client?");
-    }
-    unless ($qotdPref =~ /$interface/) {
-        return;
-    }
-
-    return $self->get_daily_quote(%opts);
-}
-
 =head3 _type
 
 =cut
index 6a9d83a..c315b32 100644 (file)
@@ -17,15 +17,17 @@ package Koha::Quotes;
 
 use Modern::Perl;
 use Carp;
+use DateTime::Format::MySQL;
 
 use Koha::Database;
+use Koha::DateUtils qw(dt_from_string);
 use Koha::Quote;
 
 use base qw(Koha::Objects);
 
 =head1 NAME
 
-Koha::Quote - Koha Quote object class
+Koha::Quotes - Koha Quote object class
 
 =head1 API
 
@@ -33,6 +35,76 @@ 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
+
+=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 $qotdPref = C4::Context->preference('QuoteOfTheDay');
+    my $interface = C4::Context->interface();
+
+    my $dtf  = Koha::Database->new->schema->storage->datetime_parser;
+
+    unless ($qotdPref =~ /$interface/) {
+        return;
+    }
+
+    my $quote = undef;
+
+    if ($opts{'id'}) {
+        $quote = $self->find({ id => $opts{'id'} });
+    }
+    elsif ($opts{'random'}) {
+        # Fall through... we also return a random quote as a catch-all if all else fails
+    }
+    else {
+        my $dt = $dtf->format_date(dt_from_string);
+        $quote = $self->search(
+            {
+                timestamp => { -between => => [ "$dt 00:00:00", "$dt 23:59:59" ] },
+            },
+            {
+                order_by => { -desc => 'timestamp' },
+                rows => 1,
+            }
+        )->single;
+    }
+    unless ($quote) {        # if there are not matches, choose a random quote
+        my $range = $self->search->count;
+        my $offset = int(rand($range));
+        $quote = $self->search(
+            {},
+            {
+                order_by => 'id',
+                rows => 1,
+                offset => $offset,
+            }
+        )->single;
+
+        unless($quote){
+            return;
+        }
+
+        # update the timestamp for that quote
+        my $dt = $dtf->format_datetime(dt_from_string);
+        $quote->update({ timestamp => $dt });
+    }
+    return $quote;
+}
+
 =head3 type
 
 =cut
index 090d431..c062f5d 100755 (executable)
@@ -32,7 +32,7 @@ use Koha::Patron::Discharge;
 use Koha::Reviews;
 use Koha::ArticleRequests;
 use Koha::ProblemReports;
-use Koha::Quote;
+use Koha::Quotes;
 
 my $query = new CGI;
 
@@ -56,7 +56,7 @@ my $koha_news_count = scalar @$all_koha_news;
 $template->param(
     koha_news       => $all_koha_news,
     koha_news_count => $koha_news_count,
-    daily_quote     => Koha::Quote->get_daily_quote_for_interface(),
+    daily_quote     => Koha::Quotes->get_daily_quote(),
 );
 
 my $branch =
index feaf327..b5a5b28 100755 (executable)
@@ -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 Koha::Quote;
+use Koha::Quotes;
 use C4::Members;
 use C4::Overdues;
 use Koha::Checkouts;
@@ -99,7 +99,7 @@ if ( $patron ) {
 $template->param(
     koha_news           => @all_koha_news,
     branchcode          => $homebranch,
-    daily_quote         => Koha::Quote->get_daily_quote_for_interface(),
+    daily_quote         => Koha::Quotes->get_daily_quote(),
 );
 
 output_html_with_http_headers $input, $cookie, $template->output;
index 4031950..035a216 100644 (file)
@@ -16,8 +16,7 @@
 # along with Koha; if not, see <http://www.gnu.org/licenses>.
 
 use Modern::Perl;
-use DateTime::Format::MySQL;
-use Test::More tests => 13;
+use Test::More tests => 15;
 
 use Koha::Database;
 use Koha::DateUtils qw(dt_from_string);
@@ -29,6 +28,7 @@ use t::lib::Mocks;
 
 BEGIN {
     use_ok('Koha::Quote');
+    use_ok('Koha::Quotes');
 }
 
 my $quote = Koha::Quote->new();
@@ -39,7 +39,8 @@ $schema->storage->txn_begin;
 my $dbh = C4::Context->dbh;
 
 # Ids not starting with 1 to reflect possible deletes, this acts as a regression test for bug 11297
-my $timestamp = DateTime::Format::MySQL->format_datetime(dt_from_string());
+my $dtf = Koha::Database->new->schema->storage->datetime_parser;
+my $timestamp = $dtf->format_datetime(dt_from_string());
 my $quote_1 = Koha::Quote->new({ 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({ 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({ 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;
@@ -53,67 +54,47 @@ my $expected_quote = {
     timestamp   => $timestamp,
 };
 
-$quote = Koha::Quote->get_daily_quote('id'=>$quote_3->id);
+#First test with QuoteOfTheDay disabled
+t::lib::Mocks::mock_preference('QuoteOfTheDay', 0);
+
+##Set interface and get nothing because syspref is not set.
+C4::Context->interface('opac');
+$quote = Koha::Quotes->get_daily_quote(id => $quote_1->id);
+ok(not($quote), "'QuoteOfTheDay'-syspref not set so nothing returned");
+
+##Set 'QuoteOfTheDay'-syspref to not include current interface 'opac'
+t::lib::Mocks::mock_preference('QuoteOfTheDay', 'intranet');
+$quote = Koha::Quotes->get_daily_quote(id => $quote_1->id);
+ok(not($quote), "'QuoteOfTheDay'-syspref doesn't include 'opac'");
+
+##Set 'QuoteOfTheDay'-syspref to include current interface 'opac'
+t::lib::Mocks::mock_preference('QuoteOfTheDay', 'opac,intranet');
+
+$quote = Koha::Quotes->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 = Koha::Quote->get_daily_quote('random'=>1);
+$quote = Koha::Quotes->get_daily_quote('random'=>1);
 ok($quote, "Got a random quote.");
 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
+$timestamp = $dtf->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()->unblessed; # this is the "default" mode of selection
+$quote = Koha::Quotes->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");
 
 Koha::Quotes->search()->delete();
-$quote = eval {Koha::Quote->get_daily_quote();};
+$quote = eval {Koha::Quotes->get_daily_quote();};
 is( $@, '', 'get_daily_quote does not die if no quote exist' );
 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();
+$quote = Koha::Quotes->get_daily_quote();
 is( $quote->id, $quote_6->id, ' get_daily_quote returns the only existing quote' );
 
 $schema->storage->txn_rollback;
-
-subtest "get_daily_quote_for_interface" => sub {
-
-    plan tests => 3;
-
-    $schema->storage->txn_begin;
-
-    my ($quote);
-    my $quote_1 = Koha::Quote->new({ source => 'Dusk And Her Embrace', text => 'Unfurl thy limbs breathless succubus<br/>How the full embosomed fog<br/>Imparts the night to us....', timestamp =>  dt_from_string })->store;
-
-    my $expected_quote = {
-        id          => $quote_1->id,
-        source      => 'Dusk And Her Embrace',
-        text        => 'Unfurl thy limbs breathless succubus<br/>How the full embosomed fog<br/>Imparts the night to us....',
-        timestamp   => DateTime::Format::MySQL->format_datetime(dt_from_string),
-    };
-
-    t::lib::Mocks::mock_preference('QuoteOfTheDay', 0);
-
-    ##Set interface and get nothing because syspref is not set.
-    C4::Context->interface('opac');
-    $quote = Koha::Quote->get_daily_quote_for_interface(id => $quote_1->id);
-    ok(not($quote), "'QuoteOfTheDay'-syspref not set so nothing returned");
-
-    ##Set 'QuoteOfTheDay'-syspref to not include current interface 'opac'
-    t::lib::Mocks::mock_preference('QuoteOfTheDay', 'intranet');
-    $quote = Koha::Quote->get_daily_quote_for_interface(id => $quote_1->id);
-    ok(not($quote), "'QuoteOfTheDay'-syspref doesn't include 'opac'");
-
-    ##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)->unblessed;
-    is_deeply($quote, $expected_quote, "Got the expected quote");
-
-    $schema->storage->txn_rollback;
-};