Attachment 108098 Details for Bug 16371 – Bug 16371: Combine get_daily_quote and get_daily_quote_for_interface

[patch] Bug 16371: Combine get_daily_quote and get_daily_quote_for_interface

0001-Bug-16371-Combine-get_daily_quote-and-get_daily_quot.patch (text/plain), 14.25 KB, created by Emmi Takkinen on 2020-08-12 11:47:44 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Emmi Takkinen

Created: 2020-08-12 11:47:44 UTC

Size: 14.25 KB

patch

obsolete

>From 4566651c830036335c71a722bbf7f370fb9e7755 Mon Sep 17 00:00:00 2001
>From: Emmi Takkinen <emmi.takkinen@outlook.com>
>Date: Fri, 31 Jul 2020 09:47:56 +0300
>Subject: [PATCH] Bug 16371: Combine get_daily_quote and
> get_daily_quote_for_interface
>
>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
>---
> Koha/Exceptions.pm           |  4 --
> Koha/Quote.pm                | 90 ------------------------------------
> Koha/Quotes.pm               | 74 ++++++++++++++++++++++++++++-
> mainpage.pl                  |  4 +-
> opac/opac-main.pl            |  4 +-
> t/db_dependent/Koha/Quotes.t | 71 +++++++++++-----------------
> 6 files changed, 103 insertions(+), 144 deletions(-)
>
>diff --git a/Koha/Exceptions.pm b/Koha/Exceptions.pm
>index c811b9775a..cf56b220ef 100644
>--- a/Koha/Exceptions.pm
>+++ b/Koha/Exceptions.pm
>@@ -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',
>diff --git a/Koha/Quote.pm b/Koha/Quote.pm
>index f189f12793..acb844d3af 100644
>--- a/Koha/Quote.pm
>+++ b/Koha/Quote.pm
>@@ -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
>diff --git a/Koha/Quotes.pm b/Koha/Quotes.pm
>index 6a9d83af58..c315b32423 100644
>--- a/Koha/Quotes.pm
>+++ b/Koha/Quotes.pm
>@@ -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
>diff --git a/mainpage.pl b/mainpage.pl
>index 090d431350..c062f5d8e1 100755
>--- a/mainpage.pl
>+++ b/mainpage.pl
>@@ -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 =
>diff --git a/opac/opac-main.pl b/opac/opac-main.pl
>index 64cd85afd4..4d742a27c8 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 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(),
> );
> 
> # If GoogleIndicTransliteration system preference is On Set parameter to load Google's javascript in OPAC search screens
>diff --git a/t/db_dependent/Koha/Quotes.t b/t/db_dependent/Koha/Quotes.t
>index 4031950547..035a2162fb 100644
>--- a/t/db_dependent/Koha/Quotes.t
>+++ b/t/db_dependent/Koha/Quotes.t
>@@ -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;
>-};
>-- 
>2.17.1
>

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