Bug 17960: Rename opac_news.new with opac_news.content
authorJonathan Druart <jonathan.druart@bugs.koha-community.org>
Tue, 17 Jan 2017 07:29:23 +0000 (08:29 +0100)
committerKyle M Hall <kyle@bywatersolutions.com>
Mon, 6 Feb 2017 17:42:12 +0000 (17:42 +0000)
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>

installer/data/mysql/atomicupdate/bug_17960.perl [new file with mode: 0644]
installer/data/mysql/kohastructure.sql
koha-tmpl/intranet-tmpl/prog/en/modules/tools/koha-news.tt
koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-news-rss.tt
t/db_dependent/Koha/News.t
t/db_dependent/NewsChannels.t
tools/koha-news.pl

diff --git a/installer/data/mysql/atomicupdate/bug_17960.perl b/installer/data/mysql/atomicupdate/bug_17960.perl
new file mode 100644 (file)
index 0000000..003787d
--- /dev/null
@@ -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";
+}
index cc57e3d..7f405c9 100644 (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
index 13d8e94..dd1ae84 100644 (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>
index ac952f5..aa18011 100644 (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 %]
index 09095b4..debae26 100644 (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' );
index 812a366..ce15247 100644 (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,
index 53b943d..61c3ec6 100755 (executable)
@@ -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,