Bug 7567 - Use, display, filter News by library
authorMark Tompsett <mtompset@hotmail.com>
Fri, 20 Dec 2013 03:59:28 +0000 (22:59 -0500)
committerGalen Charlton <gmc@esilibrary.com>
Mon, 7 Apr 2014 18:14:19 +0000 (18:14 +0000)
This adds the ability to specify whether staff, OPAC,
or slip news entries apply to all libraries or just a
particular library.

With the branch parameter added to key functions in
C4/NewsChannels.pm, function calls in C4/Members.pm,
mainpage.pl, opac/opac-main.pl, tools/koha-news.pl, and
t/db_dependent/NewsChannels.t were needed.

Some license texts were updated.

Templates were modified to display, allow for entry and editing
of the branches selected.

TEST PLAN
---------
1) Having logged into the staff client, is the news displaying
   correctly? Have you entered a news item which should not
   display for this branch of logged in user?
2) Find a patron (with some items checked out?)
3) Print a slip
   - News which is labelled 'All Branches' or for the same branch
     as the one printing the slip should display on the slip.
   - THIS DOES NOT AFFECT QUICK SLIPS
4) Home -> Tools -> News
   - Can you edit a news item?
   - Does the change save correctly?
   - Can you filter based on location and branch correctly?
   - Can you add a new entry correctly?
   - Can you delete an entry correctly?
5) Open an OPAC client.
   - Does only the news for all branches display?
6) Log into the OPAC client.
   - Does the news for all branches and the specific branch display?
7) prove -v t/db_dependent/NewsChannels.t
   - Does it run and all succeed?
   - Does the code seem to catch the required cases?
8) Comparing the patched and unpatched versions of files affected,
   are the license changes missing anything?

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>

C4/Members.pm
C4/NewsChannels.pm
koha-tmpl/intranet-tmpl/prog/en/modules/tools/koha-news.tt
koha-tmpl/opac-tmpl/prog/en/modules/opac-main.tt
mainpage.pl
opac/opac-main.pl
t/db_dependent/NewsChannels.t
tools/koha-news.pl

index 486a61d..1d098c5 100644 (file)
@@ -2345,7 +2345,7 @@ sub IssueSlip {
             'news' => [ map {
                 $_->{'timestamp'} = $_->{'newdate'};
                 { opac_news => $_ }
-            } @{ GetNewsToDisplay("slip") } ],
+            } @{ GetNewsToDisplay("slip",$branch) } ],
         );
     }
 
index b9aa8ad..7c413aa 100644 (file)
@@ -125,7 +125,12 @@ sub del_opac_new {
 sub get_opac_new {
     my ($idnew) = @_;
     my $dbh = C4::Context->dbh;
-    my $query = q{ SELECT * FROM opac_news WHERE idnew = ? };
+    my $query = q{
+                  SELECT opac_news.*,branches.branchname
+                  FROM opac_news LEFT JOIN branches
+                      ON opac_news.branchcode=branches.branchcode
+                  WHERE opac_news.idnew = ?;
+                };
     my $sth = $dbh->prepare($query);
     $sth->execute($idnew);
     my $data = $sth->fetchrow_hashref;
@@ -136,14 +141,24 @@ sub get_opac_new {
 }
 
 sub get_opac_news {
-    my ($limit, $lang) = @_;
+    my ($limit, $lang, $branchcode) = @_;
     my @values;
     my $dbh = C4::Context->dbh;
-    my $query = q{ SELECT *, timestamp AS newdate FROM opac_news };
+    my $query = q{
+                  SELECT opac_news.*, branches.branchname,
+                         timestamp AS newdate
+                  FROM opac_news LEFT JOIN branches
+                      ON opac_news.branchcode=branches.branchcode
+                };
+    $query = ' WHERE 1';
     if ($lang) {
-        $query.= " WHERE (lang='' OR lang=?)";
+        $query .= " AND (opac_news.lang='' OR opac_news.lang=?)";
         push @values,$lang;
     }
+    if ($branchcode) {
+        $query .= ' AND (opac_news.branchcode IS NULL OR opac_news.branchcode=?)';
+        push @values,$branchcode;
+    }
     $query.= ' ORDER BY timestamp DESC ';
     #if ($limit) {
     #    $query.= 'LIMIT 0, ' . $limit;
@@ -163,14 +178,15 @@ sub get_opac_news {
 
 =head2 GetNewsToDisplay
 
-    $news = &GetNewsToDisplay($lang);
+    $news = &GetNewsToDisplay($lang,$branch);
     C<$news> is a ref to an array which containts
-    all news with expirationdate > today or expirationdate is null.
+    all news with expirationdate > today or expirationdate is null
+    that is applicable for a given branch.
 
 =cut
 
 sub GetNewsToDisplay {
-    my ($lang) = @_;
+    my ($lang,$branch) = @_;
     my $dbh = C4::Context->dbh;
     # SELECT *,DATE_FORMAT(timestamp, '%d/%m/%Y') AS newdate
     my $query = q{
@@ -183,13 +199,14 @@ sub GetNewsToDisplay {
      )
      AND   `timestamp` < CURRENT_DATE()+1
      AND   (lang = '' OR lang = ?)
+     AND   (branchcode IS NULL OR branchcode = ?)
      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);
+    $sth->execute($lang,$branch);
     my @results;
     while ( my $row = $sth->fetchrow_hashref ){
         $row->{newdate} = format_date($row->{newdate});
index 368b711..5613d3a 100644 (file)
@@ -78,7 +78,7 @@ Edit news item[% ELSE %]Add news item[% END %][% ELSE %]News[% END %]</div>
                        <fieldset class="rows">
             <legend>OPAC and Koha news</legend>
            <ol> <li>
-            <label for="lang">([% new_detail.lang %])([% lang %])Display location</label>
+            <label for="lang">Display location</label>
             <select id="lang" name="lang">
                 [% IF ( default_lang == "" ) %]
                 <option value=""     selected>All</option>
@@ -105,6 +105,23 @@ Edit news item[% ELSE %]Add news item[% END %][% ELSE %]News[% END %]</div>
             </select>
             </li>
             <li>
+                <label for="branch">Library: </label>
+                <select id="branch" name="branch">
+                [% IF ( new_detail.branchcode == '' ) %]
+                    <option value="" selected>All Libraries</option>
+                [% ELSE %]
+                    <option value=""         >All Libraries</option>
+                [% END %]
+                [% FOREACH branch_item IN branch_list %]
+                [% IF ( branch_item.value.branchcode == new_detail.branchcode ) %]
+                    <option value="[% branch_item.value.branchcode %]" selected>[% branch_item.value.branchname %]</option>
+                [% ELSE %]
+                    <option value="[% branch_item.value.branchcode %]">[% branch_item.value.branchname %]</option>
+                [% END %]
+                [% END %]
+                </select>
+            </li>
+            <li>
                 <label for="title">Title: </label>
                 <input id="title" size="30" type="text" name="title" value="[% new_detail.title %]" />
             </li>
@@ -157,7 +174,26 @@ Edit news item[% ELSE %]Add news item[% END %][% ELSE %]News[% END %]</div>
                 [% IF ( lang_lis.language == lang ) %]
                     <option value="[% lang_lis.language %]" selected>OPAC ([% lang_lis.language %])</option>
                 [% ELSE %]
-                    <option value="[% lang_lis.language %]">OPAC ([% lang_lis.language %])</option>
+                    <option value="[% lang_lis.language %]"         >OPAC ([% lang_lis.language %])</option>
+                [% END %]
+                [% END %]
+            </select>
+            <label for="branch">Library: </label>
+            <select id="branch" name="branch">
+                [% IF ( branchcode == "" ) %]
+                <option value="" selected>All Libraries</option>
+                [% ELSE %]
+                <option value=""         >All Libraries</option>
+                [% END %]
+                [% FOREACH branch_item IN branch_list %]
+                [% IF ( branch_item.value.branchcode == branchcode ) %]
+                    <option value="[% branch_item.value.branchcode %]"
+                            selected>[% branch_item.value.branchname %]
+                    </option>
+                [% ELSE %]
+                    <option value="[% branch_item.value.branchcode %]"
+                                    >[% branch_item.value.branchname %]
+                    </option>
                 [% END %]
                 [% END %]
             </select>
@@ -170,6 +206,7 @@ Edit news item[% ELSE %]Add news item[% END %][% ELSE %]News[% END %]</div>
                    <thead> <tr>
                         <th>&nbsp;</th>
                         <th>Location</th>
+                        <th>Library</th>
                         <th>Number</th>
                         <th>Creation date</th>
                         <th>Expiration date</th>
@@ -197,7 +234,10 @@ Edit news item[% ELSE %]Add news item[% END %][% ELSE %]News[% END %]</div>
                                     OPAC ([% opac_new.lang %])
                                 [% END %]
                              </td>
-
+                            <td>[% IF ( opac_new.branchcode == "" ) -%]
+                                All Libraries
+                                [% ELSE %][% opac_new.branchname %]
+                                [% END %]</td>
                             <td>[% opac_new.number %]</td>
                             <td><span title="[% opac_new.newdate %]">[% opac_new.newdate | $KohaDates %]</span></td>
                             <td><span title="[% opac_new.expirationdate %]">[% opac_new.expirationdate | $KohaDates %] [% IF ( opac_new.expired ) %](<span class="expired">expired</span>)[% END %]</span></td>
index eb1e600..a1be710 100644 (file)
@@ -21,8 +21,8 @@
         <div id="notloggedin" class="yui-ge">
     [% END %]
         <div class="yui-u first">
-       [% IF ( koha_news_count ) %]
 <div id="news" class="container">
+        [% IF ( koha_news_count ) %]
     <table>
     [% FOREACH koha_new IN koha_news %]
     <tr><th>[% koha_new.title %]</th></tr>
@@ -30,8 +30,8 @@
                 <p class="newsfooter"><i>(published on [% koha_new.newdate %])</i></p></td></tr>
     [% END %]
     </table>
-</div>
 [% END %]
+</div>
 
       [% IF ( display_daily_quote && daily_quote ) %]
     <div id="daily-quote" class="container"><h1>Quote of the Day</h1><div><span id="daily-quote-text">[% daily_quote.text %]</span><span id="daily-quote-sep"> ~ </span><span id="daily-quote-source">[% daily_quote.source %]</span></div></div>
index 4159618..a0de153 100755 (executable)
@@ -1,30 +1,30 @@
 #!/usr/bin/perl
 
+# This file is part of Koha.
+#
 # Copyright Paul Poulain 2002
 # Parts Copyright Liblime 2007
+# Copyright (C) 2013  Mark Tompsett
 #
-# This file is part of Koha.
+# Koha is free software; you can redistribute it and/or modify it
+# under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
 #
-# Koha is free software; you can redistribute it and/or modify it under the
-# terms of the GNU General Public License as published by the Free Software
-# Foundation; either version 2 of the License, or (at your option) any later
-# version.
+# Koha is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
 #
-# Koha is distributed in the hope that it will be useful, but WITHOUT ANY
-# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
-# A PARTICULAR PURPOSE.  See the GNU General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License along
-# with Koha; if not, write to the Free Software Foundation, Inc.,
-# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+# You should have received a copy of the GNU General Public License
+# along with Koha; if not, see <http://www.gnu.org/licenses>.
 
-use strict;
-use warnings;
+use Modern::Perl;
 use CGI;
 use C4::Output;
 use C4::Auth;
 use C4::Koha;
-use C4::NewsChannels;
+use C4::NewsChannels; # GetNewsToDisplay
 use C4::Review qw/numberofreviews/;
 use C4::Suggestions qw/CountSuggestion/;
 use C4::Tags qw/get_count_by_tag_status/;
@@ -42,7 +42,11 @@ my ( $template, $loggedinuser, $cookie, $flags ) = get_template_and_user(
     }
 );
 
-my $all_koha_news   = &GetNewsToDisplay("koha");
+my $homebranch;
+if (C4::Context->userenv) {
+    $homebranch = C4::Context->userenv->{'branch'};
+}
+my $all_koha_news   = &GetNewsToDisplay("koha",$homebranch);
 my $koha_news_count = scalar @$all_koha_news;
 
 $template->param(
index 7c6ee36..a58659c 100755 (executable)
@@ -2,26 +2,27 @@
 
 # This file is part of Koha.
 #
-# Koha is free software; you can redistribute it and/or modify it under the
-# terms of the GNU General Public License as published by the Free Software
-# Foundation; either version 2 of the License, or (at your option) any later
-# version.
+# Parts Copyright (C) 2013  Mark Tompsett
 #
-# Koha is distributed in the hope that it will be useful, but WITHOUT ANY
-# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
-# A PARTICULAR PURPOSE.  See the GNU General Public License for more details.
+# Koha is free software; you can redistribute it and/or modify it
+# under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
 #
-# You should have received a copy of the GNU General Public License along
-# with Koha; if not, write to the Free Software Foundation, Inc.,
-# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+# Koha is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with Koha; if not, see <http://www.gnu.org/licenses>.
 
 
-use strict;
-use warnings;
+use Modern::Perl;
 use CGI;
 use C4::Auth;    # get_template_and_user
 use C4::Output;
-use C4::NewsChannels;    # get_opac_news
+use C4::NewsChannels;    # GetNewsToDisplay
 use C4::Languages qw(getTranslatedLanguages accept_language);
 use C4::Koha qw( GetDailyQuote );
 
@@ -48,7 +49,11 @@ $template->param(
 # use cookie setting for language, bug default to syspref if it's not set
 my ($theme, $news_lang, $availablethemes) = C4::Templates::themelanguage(C4::Context->config('opachtdocs'),'opac-main.tt','opac',$input);
 
-my $all_koha_news   = &GetNewsToDisplay($news_lang);
+my $homebranch;
+if (C4::Context->userenv) {
+    $homebranch = C4::Context->userenv->{'branch'};
+}
+my $all_koha_news   = &GetNewsToDisplay($news_lang,$homebranch);
 my $koha_news_count = scalar @$all_koha_news;
 
 my $quote = GetDailyQuote();   # other options are to pass in an exact quote id or select a random quote each pass... see perldoc C4::Koha
index 0e34cb0..f2a29e4 100644 (file)
@@ -23,6 +23,13 @@ my $dbh = C4::Context->dbh;
 $dbh->{AutoCommit} = 0;
 $dbh->{RaiseError} = 1;
 
+# Add LIB1, if it doesn't exist.
+my $addbra = 'LIB1';
+if ( !GetBranchName($addbra) ) {
+    $dbh->do( q{ INSERT INTO branches (branchcode,branchname) VALUES (?,?) },
+        undef, ( $addbra, "$addbra branch" ) );
+}
+
 # Test add_opac_new
 my $rv = add_opac_new();    # intentionally bad
 ok( $rv == 0, 'Correctly failed on no parameter!' );
@@ -38,6 +45,7 @@ my $href_entry1 = {
     expirationdate => $expirationdate1,
     timestamp      => $timestamp1,
     number         => $number1,
+    branchcode     => 'LIB1',
 };
 
 $rv = add_opac_new($href_entry1);
@@ -52,6 +60,7 @@ my $href_entry2 = {
     expirationdate => $expirationdate2,
     timestamp      => $timestamp2,
     number         => $number2,
+    branchcode     => 'LIB1',
 };
 $rv = add_opac_new($href_entry2);
 ok( $rv == 1, 'Successfully added the second dummy news item!' );
@@ -107,11 +116,13 @@ 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, q{} );
-ok( $opac_news_count >= 2, 'Successfully tested get_opac_news!' );
+my ( $opac_news_count, $arrayref_opac_news ) = get_opac_news( 0, q{}, 'LIB1' );
+
+# using >= 2, because someone may have LIB1 news already.
+ok( $opac_news_count >= 2, 'Successfully tested get_opac_news for LIB1!' );
 
 # Test GetNewsToDisplay
-( $opac_news_count, $arrayref_opac_news ) = GetNewsToDisplay(q{});
-ok( $opac_news_count >= 2, 'Successfully tested GetNewsToDisplay!' );
+( $opac_news_count, $arrayref_opac_news ) = GetNewsToDisplay( q{}, 'LIB1' );
+ok( $opac_news_count >= 2, 'Successfully tested GetNewsToDisplay for LIB1!' );
 
 $dbh->rollback;
index 057a601..734db7c 100755 (executable)
@@ -33,6 +33,7 @@ use C4::Output;
 use C4::NewsChannels;
 use C4::Languages qw(getTranslatedLanguages);
 use Date::Calc qw/Date_to_Days Today/;
+use C4::Branch qw/GetBranches/;
 
 my $cgi = new CGI;
 
@@ -43,6 +44,10 @@ my $expirationdate = format_date_in_iso($cgi->param('expirationdate'));
 my $timestamp      = format_date_in_iso($cgi->param('timestamp'));
 my $number         = $cgi->param('number');
 my $lang           = $cgi->param('lang');
+my $branchcode     = $cgi->param('branch');
+# Foreign Key constraints work with NULL, not ''
+# NULL = All branches.
+$branchcode = undef if (defined($branchcode) && $branchcode eq '');
 
 my $new_detail = get_opac_new($id);
 
@@ -68,9 +73,13 @@ foreach my $language ( @$tlangs ) {
       };
 }
 
-$template->param( lang_list => \@lang_list );
+my $branches = GetBranches;
 
-my $op = $cgi->param('op');
+$template->param( lang_list   => \@lang_list,
+                  branch_list => $branches,
+                  branchcode  => $branchcode );
+
+my $op = $cgi->param('op') // '';
 
 if ( $op eq 'add_form' ) {
     $template->param( add_form => 1 );
@@ -94,6 +103,7 @@ elsif ( $op eq 'add' ) {
                       expirationdate => $expirationdate,
                       timestamp      => $timestamp,
                       number         => $number,
+                      branchcode     => $branchcode,
                   };
     add_opac_new( $parameters );
     print $cgi->redirect("/cgi-bin/koha/tools/koha-news.pl");
@@ -107,6 +117,7 @@ elsif ( $op eq 'edit' ) {
                       expirationdate => $expirationdate,
                       timestamp      => $timestamp,
                       number         => $number,
+                      branchcode     => $branchcode,
                   };
     upd_opac_new( $parameters );
     print $cgi->redirect("/cgi-bin/koha/tools/koha-news.pl");
@@ -119,7 +130,7 @@ elsif ( $op eq 'del' ) {
 
 else {
 
-    my ( $opac_news_count, $opac_news ) = &get_opac_news( undef, $lang );
+    my ( $opac_news_count, $opac_news ) = &get_opac_news( undef, $lang, $branchcode );
     
     foreach my $new ( @$opac_news ) {
         next unless $new->{'expirationdate'};