Bug 17960: Rename opac_news.new with opac_news.content

The field opac_news.new is very confusing and should be renamed.
If you want to access it via Koha::NewsItem you will have trouble:

  use Koha::News;
  my $news_item = Koha::News->next;
  say $news_item->new;

=> Attempt to bless into a reference at /home/vagrant/kohaclone/Koha/Object.pm line 78.

This patchset is going to rename this DB field to opac_news_content instead.

Since the opac_news.new can be used in notice templates, we need to warn the
user during the update DB process that some templates must be updated.

Test plan:
0/ Apply the first patch "Add a test to highlight the issue" and confirm that
the test fail
1/ Apply this second patch
2/ Execute the DB entry
3/ Confirm that you get a warning if at least one of your notice templates is
using opac_news.new
4/ Confirm that the test new pass
5/ Add/update and delete a news
6/ Confirm that the RSS new feed still works as expected

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This commit is contained in:
Jonathan Druart 2017-01-17 08:29:23 +01:00 committed by Kyle M Hall
parent a6b13fbcb3
commit a233f06574
7 changed files with 33 additions and 15 deletions

View File

@ -0,0 +1,18 @@
$DBversion = 'XXX';
if( CheckVersion( $DBversion ) ) {
if ( column_exists('opac_news', 'new' ) ) {
$dbh->do(q|ALTER TABLE opac_news CHANGE COLUMN new content text NOT NULL|);
}
my ( $used_in_templates ) = $dbh->selectrow_array(q|
SELECT COUNT(*) FROM letter WHERE content LIKE "%<<opac_news.new>>%";
|);
if ( $used_in_templates ) {
print "WARNING - It seems that you are using the opac_news.new column in your notice templates\n";
print "Since it has now been renamed with opac_news.content, you should update them.\n";
}
SetVersion( $DBversion );
print "Upgrade to $DBversion done (Bug 17960 - Rename opac_news with opac_news.content)\n";
}

View File

@ -1834,7 +1834,7 @@ CREATE TABLE `opac_news` ( -- data from the news tool
`idnew` int(10) unsigned NOT NULL auto_increment, -- unique identifier for the news article
`branchcode` varchar(10) default NULL, -- branch code users to create branch specific news, NULL is every branch.
`title` varchar(250) NOT NULL default '', -- title of the news article
`new` text NOT NULL, -- the body of your news article
`content` text NOT NULL, -- the body of your news article
`lang` varchar(25) NOT NULL default '', -- location for the article (koha is the staff client, slip is the circulation receipt and language codes are for the opac)
`timestamp` timestamp NOT NULL default CURRENT_TIMESTAMP, -- pulibcation date and time
`expirationdate` date default NULL, -- date the article is set to expire or no longer be visible

View File

@ -161,8 +161,8 @@ Edit news item[% ELSE %]Add news item[% END %][% ELSE %]News[% END %]</div>
<input id="number" size="3" name="number" type="text" />
[% END %]
</li>
<li><label for="new">News: </label>
<textarea name="new" id="new" cols="75" rows="10">[% new_detail.new %]</textarea>
<li><label for="content">News: </label>
<textarea name="content" id="content" cols="75" rows="10">[% new_detail.content %]</textarea>
</li>
</ol>
</fieldset>
@ -254,7 +254,7 @@ Edit news item[% ELSE %]Add news item[% END %][% ELSE %]News[% END %]</div>
<td>[% opac_new.title %]</td>
<td>[% opac_new.author_title %] [% opac_new.author_firstname %] [% opac_new.author_surname %]</td>
<td>
[% opac_new.new %]
[% opac_new.content %]
</td>
<td class="actions">
<a href="/cgi-bin/koha/tools/koha-news.pl?op=add_form&amp;id=[% opac_new.idnew %]" class="btn btn-default btn-xs"><i class="fa fa-pencil"></i> Edit</a>

View File

@ -8,7 +8,7 @@
[% FOREACH newsitem IN koha_news %]
<item>
<title>[% newsitem.title |html %]</title>
<description>[% newsitem.new |html %]</description>
<description>[% newsitem.content |html %]</description>
<guid>[% OPACBaseURL %]/cgi-bin/koha/opac-main.pl#newsitem[% newsitem.idnew |html %]</guid>
</item>
[% END %]

View File

@ -47,7 +47,7 @@ is( Koha::News->search->count, $nb_of_news + 2, 'The 2 news should have been add
my $retrieved_news_item_1 = Koha::News->find( $new_news_item_1->idnew );
is( $retrieved_news_item_1->title, $new_news_item_1->title, 'Find a news_item by id should return the correct news_item' );
is( $retrieved_news_item_1->new, $new_news_item_1->new, 'This test is failing because ->new is not a valid accessor');
is( $retrieved_news_item_1->content, $new_news_item_1->content, 'The content method return the content of the news');
$retrieved_news_item_1->delete;
is( Koha::News->search->count, $nb_of_news + 1, 'Delete should have deleted the news_item' );

View File

@ -72,7 +72,7 @@ my ( $title1, $new1, $lang1, $expirationdate1, $number1 ) =
( 'News Title', '<p>We have some exciting news!</p>', q{}, '2999-12-30', 1 );
my $href_entry1 = {
title => $title1,
new => $new1,
content => $new1,
lang => $lang1,
expirationdate => $expirationdate1,
timestamp => $timestamp1,
@ -87,7 +87,7 @@ my ( $title2, $new2, $lang2, $expirationdate2, $number2 ) =
( 'News Title2', '<p>We have some exciting news!</p>', q{}, '2999-12-31', 1 );
my $href_entry2 = {
title => $title2,
new => $new2,
content => $new2,
lang => $lang2,
expirationdate => $expirationdate2,
timestamp => $timestamp2,
@ -102,7 +102,7 @@ my ( $title3, $new3, $lang3, $number3 ) =
( 'News Title3', '<p>News without expiration date</p>', q{}, 1 );
my $href_entry3 = {
title => $title3,
new => $new3,
content => $new3,
lang => $lang3,
timestamp => $timestamp3,
number => $number3,
@ -130,7 +130,7 @@ $rv = upd_opac_new(); # intentionally bad parmeters
is( $rv, 0, 'Correctly failed on no parameter!' );
$new2 = '<p>Update! There is no news!</p>';
$href_entry2->{new} = $new2;
$href_entry2->{content} = $new2;
$href_entry2->{idnew} = $idnew2;
$rv = upd_opac_new($href_entry2);
is( $rv, 1, 'Successfully updated second dummy news item!' );
@ -145,7 +145,7 @@ is_deeply(
get_opac_new($idnew1),
{
title => $title1,
new => $new1,
content => $new1,
lang => $lang1,
expirationdate => $expirationdate1,
timestamp => $timestamp1,
@ -168,7 +168,7 @@ is_deeply(
get_opac_new($idnew2),
{
title => $title2,
new => $new2,
content => $new2,
lang => $lang2,
expirationdate => $expirationdate2,
timestamp => $timestamp2,

View File

@ -38,7 +38,7 @@ my $cgi = new CGI;
my $id = $cgi->param('id');
my $title = $cgi->param('title');
my $new = $cgi->param('new');
my $content = $cgi->param('content');
my $expirationdate;
if ( $cgi->param('expirationdate') ) {
$expirationdate = output_pref({ dt => dt_from_string( scalar $cgi->param('expirationdate') ), dateformat => 'iso', dateonly => 1 });
@ -107,7 +107,7 @@ elsif ( $op eq 'add' ) {
add_opac_new(
{
title => $title,
new => $new,
content => $content,
lang => $lang,
expirationdate => $expirationdate,
timestamp => $timestamp,
@ -127,7 +127,7 @@ elsif ( $op eq 'edit' ) {
{
idnew => $id,
title => $title,
new => $new,
content => $content,
lang => $lang,
expirationdate => $expirationdate,
timestamp => $timestamp,