From 831296be7a1947fd8aeed64862dbad6358c9875f Mon Sep 17 00:00:00 2001 From: Emmi Takkinen 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 Signed-off-by: Jonathan Druart --- 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 feaf327772..b5a5b284e9 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(), ); output_html_with_http_headers $input, $cookie, $template->output; 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 . 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
How the full embosomed fog
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
How the full embosomed fog
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.39.5