diff --git a/C4/NewsChannels.pm b/C4/NewsChannels.pm index cc931d2d65..b9aa8ad98b 100644 --- a/C4/NewsChannels.pm +++ b/C4/NewsChannels.pm @@ -3,8 +3,7 @@ package C4::NewsChannels; # This file is part of Koha. # # Copyright (C) 2000-2002 Katipo Communications -# -# This file is part of Koha. +# Copyright (C) 2013 Mark Tompsett # # Koha is free software; you can redistribute it and/or modify it # under the terms of the GNU General Public License as published by @@ -26,12 +25,12 @@ use C4::Dates qw(format_date); use vars qw($VERSION @ISA @EXPORT); BEGIN { - $VERSION = 3.07.00.049; # set the version for version checking - @ISA = qw(Exporter); - @EXPORT = qw( - &GetNewsToDisplay - &add_opac_new &upd_opac_new &del_opac_new &get_opac_new &get_opac_news - ); + $VERSION = 3.07.00.049; # set the version for version checking + @ISA = qw(Exporter); + @EXPORT = qw( + &GetNewsToDisplay + &add_opac_new &upd_opac_new &del_opac_new &get_opac_new &get_opac_news + ); } =head1 NAME @@ -46,29 +45,69 @@ This module provides the functions needed to mange OPAC and intranet news. =cut +=head2 add_opac_new + + $retval = add_opac_new($hashref); + + $hashref should contains all the fields found in opac_news, + except idnew. The idnew field is auto-generated. + +=cut + sub add_opac_new { - my ($title, $new, $lang, $expirationdate, $timestamp, $number) = @_; - my $dbh = C4::Context->dbh; - my $sth = $dbh->prepare("INSERT INTO opac_news (title, new, lang, expirationdate, timestamp, number) VALUES (?,?,?,?,?,?)"); - $sth->execute($title, $new, $lang, $expirationdate, $timestamp, $number); - return 1; + my ($href_entry) = @_; + my $retval = 0; + + if ($href_entry) { + my @fields = keys %{$href_entry}; + my @values = values %{$href_entry}; + my $field_string = join ',',@fields; + $field_string = $field_string // q{}; + my $values_string = '?,' x ($#fields) . '?'; + my $dbh = C4::Context->dbh; + my $sth = $dbh->prepare("INSERT INTO opac_news ( $field_string ) VALUES ( $values_string )"); + $sth->execute(@values); + $retval = 1; + } + return $retval; } +=head2 upd_opac_new + + $retval = upd_opac_new($hashref); + + $hashref should contains all the fields found in opac_news, + including idnew, since it is the key for the SQL UPDATE. + +=cut + sub upd_opac_new { - my ($idnew, $title, $new, $lang, $expirationdate, $timestamp,$number) = @_; - my $dbh = C4::Context->dbh; - my $sth = $dbh->prepare(" - UPDATE opac_news SET - title = ?, - new = ?, - lang = ?, - expirationdate = ?, - timestamp = ?, - number = ? - WHERE idnew = ? - "); - $sth->execute($title, $new, $lang, $expirationdate, $timestamp,$number,$idnew); - return 1; + my ($href_entry) = @_; + my $retval = 0; + + if ($href_entry) { + # take the keys of hash entry and make a list, but... + my @fields = keys %{$href_entry}; + my @values; + $#values = -1; + my $field_string = q{}; + foreach my $field_name (@fields) { + # exclude idnew + if ( $field_name ne 'idnew' ) { + $field_string = $field_string . "$field_name = ?,"; + push @values,$href_entry->{$field_name}; + } + } + # put idnew at the end, so we know which record to update + push @values,$href_entry->{'idnew'}; + chop $field_string; # remove that excess , + + my $dbh = C4::Context->dbh; + my $sth = $dbh->prepare("UPDATE opac_news SET $field_string WHERE idnew = ?;"); + $sth->execute(@values); + $retval = 1; + } + return $retval; } sub del_opac_new { @@ -86,7 +125,8 @@ sub del_opac_new { sub get_opac_new { my ($idnew) = @_; my $dbh = C4::Context->dbh; - my $sth = $dbh->prepare("SELECT * FROM opac_news WHERE idnew = ?"); + my $query = q{ SELECT * FROM opac_news WHERE idnew = ? }; + my $sth = $dbh->prepare($query); $sth->execute($idnew); my $data = $sth->fetchrow_hashref; $data->{$data->{'lang'}} = 1 if defined $data->{lang}; @@ -97,17 +137,19 @@ sub get_opac_new { sub get_opac_news { my ($limit, $lang) = @_; + my @values; my $dbh = C4::Context->dbh; - my $query = "SELECT *, timestamp AS newdate FROM opac_news"; + my $query = q{ SELECT *, timestamp AS newdate FROM opac_news }; if ($lang) { - $query.= " WHERE lang = '" .$lang ."' "; + $query.= " WHERE (lang='' OR lang=?)"; + push @values,$lang; } - $query.= " ORDER BY timestamp DESC "; + $query.= ' ORDER BY timestamp DESC '; #if ($limit) { - # $query.= "LIMIT 0, " . $limit; + # $query.= 'LIMIT 0, ' . $limit; #} my $sth = $dbh->prepare($query); - $sth->execute(); + $sth->execute(@values); my @opac_news; my $count = 0; while (my $row = $sth->fetchrow_hashref) { @@ -128,26 +170,29 @@ sub get_opac_news { =cut sub GetNewsToDisplay { - my $lang = shift; + my ($lang) = @_; my $dbh = C4::Context->dbh; # SELECT *,DATE_FORMAT(timestamp, '%d/%m/%Y') AS newdate - my $query = " + my $query = q{ SELECT *,timestamp AS newdate FROM opac_news WHERE ( expirationdate >= CURRENT_DATE() OR expirationdate IS NULL OR expirationdate = '00-00-0000' - ) - AND `timestamp` <= CURRENT_DATE() - AND lang = ? - ORDER BY number - "; # expirationdate field is NOT in ISO format? + ) + AND `timestamp` < CURRENT_DATE()+1 + AND (lang = '' OR lang = ?) + ORDER BY number + }; # expirationdate field is NOT in ISO format? + # timestamp has HH:mm:ss, CURRENT_DATE generates 00:00:00 + # by adding 1, that captures today correctly. my $sth = $dbh->prepare($query); + $lang = $lang // q{}; $sth->execute($lang); my @results; while ( my $row = $sth->fetchrow_hashref ){ - $row->{newdate} = format_date($row->{newdate}); + $row->{newdate} = format_date($row->{newdate}); push @results, $row; } return \@results; diff --git a/t/db_dependent/NewsChannels.t b/t/db_dependent/NewsChannels.t old mode 100755 new mode 100644 index 261abd541b..0e34cb0d3a --- a/t/db_dependent/NewsChannels.t +++ b/t/db_dependent/NewsChannels.t @@ -3,10 +3,18 @@ use Modern::Perl; use C4::Dates qw(format_date); use C4::Branch qw(GetBranchName); -use Test::More tests => 8; +use Test::More tests => 10; +use Readonly; + +Readonly my $F1 => 1; +Readonly my $F2 => 2; +Readonly my $F3 => 3; +Readonly my $F4 => 4; +Readonly my $F5 => 5; +Readonly my $F6 => 6; BEGIN { - use_ok('C4::NewsChannels'); + use_ok('C4::NewsChannels'); } my $dbh = C4::Context->dbh; @@ -16,77 +24,94 @@ $dbh->{AutoCommit} = 0; $dbh->{RaiseError} = 1; # Test add_opac_new +my $rv = add_opac_new(); # intentionally bad +ok( $rv == 0, 'Correctly failed on no parameter!' ); + my $timestamp = '2000-01-01'; my ( $timestamp1, $timestamp2 ) = ( $timestamp, $timestamp ); -my ($title1, $new1, - $lang1, $expirationdate1, $number1) = - ( 'News Title', '
We have some exciting news!
', - '', '2999-12-30', 1 ); -my $rv = add_opac_new( $title1, $new1, $lang1, $expirationdate1, $timestamp1, $number1 ); -ok($rv==1,"Successfully added the first dummy news item!"); +my ( $title1, $new1, $lang1, $expirationdate1, $number1 ) = + ( 'News Title', 'We have some exciting news!
', q{}, '2999-12-30', 1 ); +my $href_entry1 = { + title => $title1, + new => $new1, + lang => $lang1, + expirationdate => $expirationdate1, + timestamp => $timestamp1, + number => $number1, +}; -my ($title2, $new2, - $lang2, $expirationdate2, $number2) = - ( 'News Title2', 'We have some exciting news!
', - '', '2999-12-31', 1 ); -$rv = add_opac_new( $title2, $new2, $lang2, $expirationdate2, $timestamp2, $number2 ); -ok($rv==1,"Successfully added the second dummy news item!"); +$rv = add_opac_new($href_entry1); +ok( $rv == 1, 'Successfully added the first dummy news item!' ); + +my ( $title2, $new2, $lang2, $expirationdate2, $number2 ) = + ( 'News Title2', 'We have some exciting news!
', q{}, '2999-12-31', 1 ); +my $href_entry2 = { + title => $title2, + new => $new2, + lang => $lang2, + expirationdate => $expirationdate2, + timestamp => $timestamp2, + number => $number2, +}; +$rv = add_opac_new($href_entry2); +ok( $rv == 1, 'Successfully added the second dummy news item!' ); # We need to determine the idnew in a non-MySQLism way. # This should be good enough. -my $sth = $dbh->prepare(q{ - SELECT idnew from opac_news - WHERE timestamp='2000-01-01' AND - expirationdate='2999-12-30'; - }); +my $query = +q{ SELECT idnew from opac_news WHERE timestamp='2000-01-01' AND expirationdate='2999-12-30'; }; +my $sth = $dbh->prepare($query); $sth->execute(); my $idnew1 = $sth->fetchrow; -$sth = $dbh->prepare(q{ - SELECT idnew from opac_news - WHERE timestamp='2000-01-01' AND - expirationdate='2999-12-31'; - }); +$query = +q{ SELECT idnew from opac_news WHERE timestamp='2000-01-01' AND expirationdate='2999-12-31'; }; +$sth = $dbh->prepare($query); $sth->execute(); my $idnew2 = $sth->fetchrow; # Test upd_opac_new -$new2 = 'Update! There is no news!
'; -$rv = upd_opac_new( $idnew2, $title2, $new2, $lang2, $expirationdate2, $timestamp2, $number2 ); -ok($rv==1,"Successfully updated second dummy news item!"); +$rv = upd_opac_new(); # intentionally bad parmeters +ok( $rv == 0, 'Correctly failed on no parameter!' ); + +$new2 = 'Update! There is no news!
'; +$href_entry2->{new} = $new2; +$href_entry2->{idnew} = $idnew2; +$rv = upd_opac_new($href_entry2); +ok( $rv == 1, 'Successfully updated second dummy news item!' ); # Test get_opac_new (single news item) -$timestamp1 = format_date( $timestamp1 ); -$expirationdate1 = format_date( $expirationdate1 ); -$timestamp2 = format_date( $timestamp2 ); -$expirationdate2 = format_date( $expirationdate2 ); +$timestamp1 = format_date($timestamp1); +$expirationdate1 = format_date($expirationdate1); +$timestamp2 = format_date($timestamp2); +$expirationdate2 = format_date($expirationdate2); my $hashref_check = get_opac_new($idnew1); -my $failure = 0; -if ($hashref_check->{title} ne $title1) { $failure = 1; } -if ($hashref_check->{new} ne $new1) { $failure = 1; } -if ($hashref_check->{lang} ne $lang1) { $failure = 1; } -if ($hashref_check->{expirationdate} ne $expirationdate1) { $failure = 1; } -if ($hashref_check->{timestamp} ne $timestamp1) { $failure = 1; } -if ($hashref_check->{number} ne $number1) { $failure = 1; } -ok($failure==0,"Successfully tested get_opac_new id1!"); +my $failure = 0; +if ( $hashref_check->{title} ne $title1 ) { $failure = $F1; } +if ( $hashref_check->{new} ne $new1 ) { $failure = $F2; } +if ( $hashref_check->{lang} ne $lang1 ) { $failure = $F3; } +if ( $hashref_check->{expirationdate} ne $expirationdate1 ) { $failure = $F4; } +if ( $hashref_check->{timestamp} ne $timestamp1 ) { $failure = $F5; } +if ( $hashref_check->{number} ne $number1 ) { $failure = $F6; } +ok( $failure == 0, "Successfully tested get_opac_new id1 ($failure)!" ); # Test get_opac_new (single news item) $hashref_check = get_opac_new($idnew2); -$failure = 0; -if ($hashref_check->{title} ne $title2) { $failure = 1; } -if ($hashref_check->{new} ne $new2) { $failure = 1; } -if ($hashref_check->{lang} ne $lang2) { $failure = 1; } -if ($hashref_check->{expirationdate} ne $expirationdate2) { $failure = 1; } -if ($hashref_check->{timestamp} ne $timestamp2) { $failure = 1; } -if ($hashref_check->{number} ne $number2) { $failure = 1; } -ok($failure==0,"Successfully tested get_opac_new id2!"); +$failure = 0; +if ( $hashref_check->{title} ne $title2 ) { $failure = $F1; } +if ( $hashref_check->{new} ne $new2 ) { $failure = $F2; } +if ( $hashref_check->{lang} ne $lang2 ) { $failure = $F3; } +if ( $hashref_check->{expirationdate} ne $expirationdate2 ) { $failure = $F4; } +if ( $hashref_check->{timestamp} ne $timestamp2 ) { $failure = $F5; } +if ( $hashref_check->{number} ne $number2 ) { $failure = $F6; } +ok( $failure == 0, "Successfully tested get_opac_new id2 ($failure)!" ); # Test get_opac_news (multiple news items) -my ($opac_news_count, $arrayref_opac_news) = get_opac_news(0,''); -ok($opac_news_count>=2,"Successfully tested get_opac_news!"); +my ( $opac_news_count, $arrayref_opac_news ) = get_opac_news( 0, q{} ); +ok( $opac_news_count >= 2, 'Successfully tested get_opac_news!' ); # Test GetNewsToDisplay -($opac_news_count, $arrayref_opac_news) = GetNewsToDisplay(''); -ok($opac_news_count>=2,"Successfully tested GetNewsToDisplay!"); +( $opac_news_count, $arrayref_opac_news ) = GetNewsToDisplay(q{}); +ok( $opac_news_count >= 2, 'Successfully tested GetNewsToDisplay!' ); $dbh->rollback; diff --git a/tools/koha-news.pl b/tools/koha-news.pl index 5578b3158e..057a6011ad 100755 --- a/tools/koha-news.pl +++ b/tools/koha-news.pl @@ -87,11 +87,28 @@ if ( $op eq 'add_form' ) { } } elsif ( $op eq 'add' ) { - add_opac_new( $title, $new, $lang, $expirationdate, $timestamp, $number ); + my $parameters = { + title => $title, + new => $new, + lang => $lang, + expirationdate => $expirationdate, + timestamp => $timestamp, + number => $number, + }; + add_opac_new( $parameters ); print $cgi->redirect("/cgi-bin/koha/tools/koha-news.pl"); } elsif ( $op eq 'edit' ) { - upd_opac_new( $id, $title, $new, $lang, $expirationdate, $timestamp ,$number ); + my $parameters = { + idnew => $id, + title => $title, + new => $new, + lang => $lang, + expirationdate => $expirationdate, + timestamp => $timestamp, + number => $number, + }; + upd_opac_new( $parameters ); print $cgi->redirect("/cgi-bin/koha/tools/koha-news.pl"); } elsif ( $op eq 'del' ) {