Bug 10513: display a warning/message when returning a chosen item type
authorMagnus Enger <magnus@enger.priv.no>
Thu, 27 Jun 2013 20:22:18 +0000 (22:22 +0200)
committerGalen Charlton <gmc@esilibrary.com>
Mon, 16 Sep 2013 17:45:31 +0000 (17:45 +0000)
This patch adds a new column to item types. Text in this column is
displayed as a warning when an item of the given type is checked in.
The type of message can also be chosen, affecting how the message is
displayed.

Use case: Items that are on inter-library loan can have a separate
item type, and when items of this type are checked in a message
saying something like "ILL! Remember to return it to the owning
library!" can be displayed.

To test:
- Apply the patch
- Go to Home > Administration > Item types administration
- Check that there is a new column, called "Check in message"
- Edit an item type and add a check in message
- Check that the check in message you added is displayed in the table
- Check in an item with an item type that has a check in message
- Check that the message is displayed
- Repeat the steps above, but select "Alert" instead of the default
  "Message" as the "Check in message type". Check that the message
  is displayed in a yellow alert box, not a blue message box.
- Check in an item with an item type that does *not* have a check
  in message, and make sure no false messages are displayed
- Create a new item type from scratch and check that it works
  the way it is supposed to
- Run the tests in t/ItemType.t, which are updated by this patch

This patch also removes backticks around column names in the
itemtypes table in installer/data/mysql/kohastructure.sql

UPDATE 2013-07-22
- Rebased on current master (no changes)
- Added "AFTER summary" to the SQL statement in updatedatabase.pl
- Added another placeholder on line 170 of admin/itemtypes.pl
Thanks Katrin!

UPDATE 2013-07-29
- Make this message independent of all other messages - thanks Owen!
- Make it possible to choose the type of message ("alert" or
  "message")

Sponsored-by: Kultur i Halland - Regionbibliotek
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Fixed some tabs to make the QA script happy.
All old and new tests pass.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>

C4/ItemType.pm
admin/itemtypes.pl
circ/returns.pl
installer/data/mysql/kohastructure.sql
installer/data/mysql/updatedatabase.pl
koha-tmpl/intranet-tmpl/prog/en/modules/admin/itemtypes.tt
koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt
t/ItemType.t

index 648cff9..ad253a7 100644 (file)
@@ -90,6 +90,29 @@ sub all {
 
 
 
+=head3 C4::ItemType->get
+
+Return the itemtype indicated by the itemtype given as argument, as
+an object.
+
+=cut
+
+sub get {
+    my ($class, $itemtype) = @_;
+    my $dbh = C4::Context->dbh;
+
+    my $data = $dbh->selectrow_hashref(
+        "SELECT * FROM itemtypes WHERE itemtype = ?", undef, $itemtype
+    );
+    if ( $data->{description} ) {
+        utf8::encode($data->{description});
+    }
+    return $class->new($data);
+}
+
+
+
+
 =head2 Object Methods
 
 These are read-only accessors for attributes of a C4::ItemType object.
@@ -118,6 +141,10 @@ These are read-only accessors for attributes of a C4::ItemType object.
 
 =cut
 
+=head3 $itemtype->checkinmsg
+
+=cut
+
 =head3 $itemtype->summary
 
 =cut
index 841672d..ee70238 100755 (executable)
@@ -117,6 +117,8 @@ if ( $op eq 'add_form' ) {
         imageurl        => $data->{'imageurl'},
         template        => C4::Context->preference('template'),
         summary         => $data->{summary},
+        checkinmsg      => $data->{'checkinmsg'},
+        checkinmsgtype  => $data->{'checkinmsgtype'},
         imagesets       => $imagesets,
         remote_image    => $remote_image,
     );
@@ -141,6 +143,8 @@ elsif ( $op eq 'add_validate' ) {
                  , notforloan = ?
                  , imageurl = ?
                  , summary = ?
+                 , checkinmsg = ?
+                 , checkinmsgtype = ?
             WHERE itemtype = ?
         ';
         $sth = $dbh->prepare($query2);
@@ -156,15 +160,17 @@ elsif ( $op eq 'add_validate' ) {
                 )
             ),
             $input->param('summary'),
+            $input->param('checkinmsg'),
+            $input->param('checkinmsgtype'),
             $input->param('itemtype')
         );
     }
     else {    # add a new itemtype & not modif an old
         my $query = "
             INSERT INTO itemtypes
-                (itemtype,description,rentalcharge, notforloan, imageurl,summary)
+                (itemtype,description,rentalcharge, notforloan, imageurl, summary, checkinmsg, checkinmsgtype)
             VALUES
-                (?,?,?,?,?,?);
+                (?,?,?,?,?,?,?,?);
             ";
         my $sth = $dbh->prepare($query);
                my $image = $input->param('image');
@@ -177,6 +183,8 @@ elsif ( $op eq 'add_validate' ) {
             $image eq 'remoteImage' ? $input->param('remoteImage') :
             $image,
             $input->param('summary'),
+            $input->param('checkinmsg'),
+            $input->param('checkinmsgtype'),
         );
     }
 
index 618190b..3a235e9 100755 (executable)
@@ -225,6 +225,16 @@ if ($barcode) {
     # fix up item type for display
     $biblio->{'itemtype'} = C4::Context->preference('item-level_itypes') ? $biblio->{'itype'} : $biblio->{'itemtype'};
 
+    # Check if we should display a check in message, based on the the item
+    # type of the checked in item
+    my $itemtype = C4::ItemType->get( $biblio->{'itemtype'} );
+    if ( $itemtype->{'checkinmsg'} ) {
+        $template->param(
+            checkinmsg     => $itemtype->{'checkinmsg'},
+            checkinmsgtype => $itemtype->{'checkinmsgtype'},
+        );
+    }
+
     $template->param(
         title            => $biblio->{'title'},
         homebranch       => $biblio->{'homebranch'},
index b22134b..fdab879 100644 (file)
@@ -1204,12 +1204,14 @@ CREATE TABLE `items` ( -- holdings/item information
 
 DROP TABLE IF EXISTS `itemtypes`;
 CREATE TABLE `itemtypes` ( -- defines the item types
-  `itemtype` varchar(10) NOT NULL default '', -- unique key, a code associated with the item type
-  `description` mediumtext, -- a plain text explanation of the item type
-  `rentalcharge` double(16,4) default NULL, -- the amount charged when this item is checked out/issued
-  `notforloan` smallint(6) default NULL, -- 1 if the item is not for loan, 0 if the item is available for loan
-  `imageurl` varchar(200) default NULL, -- URL for the item type icon
-  `summary` text, -- information from the summary field, may include HTML
+  itemtype varchar(10) NOT NULL default '', -- unique key, a code associated with the item type
+  description mediumtext, -- a plain text explanation of the item type
+  rentalcharge double(16,4) default NULL, -- the amount charged when this item is checked out/issued
+  notforloan smallint(6) default NULL, -- 1 if the item is not for loan, 0 if the item is available for loan
+  imageurl varchar(200) default NULL, -- URL for the item type icon
+  summary text, -- information from the summary field, may include HTML
+  checkinmsg VARCHAR(255), -- message that is displayed when an item with the given item type is checked in
+  checkinmsgtype CHAR(16) DEFAULT 'message' NOT NULL, -- type (CSS class) for the checkinmsg, can be "alert" or "message"
   PRIMARY KEY  (`itemtype`),
   UNIQUE KEY `itemtype` (`itemtype`)
 ) ENGINE=InnoDB DEFAULT CHARSET=utf8;
index d8b5818..a3acd7a 100755 (executable)
@@ -7122,6 +7122,13 @@ if ( CheckVersion($DBversion) ) {
     SetVersion($DBversion);
 }
 
+$DBversion = "3.13.00.XXX";
+if ( CheckVersion($DBversion) ) {
+    $dbh->do("ALTER TABLE itemtypes ADD COLUMN checkinmsg VARCHAR(255) AFTER summary;");
+    $dbh->do("ALTER TABLE itemtypes ADD COLUMN checkinmsgtype CHAR(16) DEFAULT 'message' NOT NULL AFTER checkinmsg;");
+    print "Upgrade to $DBversion done (Bug 10513 - Light up a warning/message when returning a chosen item type)\n";
+    SetVersion($DBversion);
+}
 
 =head1 FUNCTIONS
 
index 210b575..7d8f781 100644 (file)
@@ -229,6 +229,25 @@ Item types administration
                  <input type="text" id="rentalcharge" name="rentalcharge" size="10" value="[% rentalcharge %]" />
          </li>
       <li>
+          <label for="checkinmsg">Check in message: </label>
+          <input type="text" id="checkinmsg" name="checkinmsg" size="48" value="[% checkinmsg %]" />
+      </li>
+      <li>
+          <label for="checkinmsgtype">Check in message type: </label>
+          <select type="text" id="checkinmsgtype" name="checkinmsgtype">
+              [% IF ( checkinmsgtype == 'message' ) %]
+              <option value="message" selected="selected">Message</option>
+              [% ELSE %]
+                 <option value="message">Message</option>
+              [% END %]
+              [% IF ( checkinmsgtype == 'alert' ) %]
+              <option value="alert" selected="selected">Alert</option>
+              [% ELSE %]
+                  <option value="alert">Alert</option>
+              [% END %]
+          <select>
+      </li>
+      <li>
           <label for="summary">Summary: </label>
          <textarea id="summary" name="summary" cols="55" rows="5">[% summary %]</textarea>
           <p>Enter a summary that will overwrite the default one in search results lists. Example, for a website itemtype : </p>
@@ -282,6 +301,7 @@ Item types administration
     <th>Description</th>
     <th>Not for loan</th>
     <th>Charge</th>
+    <th>Check in message</th>
     <th>Actions</th>
   </thead>
   [% FOREACH loo IN loop %]
@@ -303,6 +323,7 @@ Item types administration
       [% loo.rentalcharge %]
     [% END %]
     </td>
+    <td>[% loo.checkinmsg %]</td>
     <td>
       <a href="[% loo.script_name %]?op=add_form&amp;itemtype=[% loo.itemtype |html %]">Edit</a>
       <a href="[% loo.script_name %]?op=delete_confirm&amp;itemtype=[% loo.itemtype |html %]">Delete</a>
index 775454f..1ee9efe 100644 (file)
@@ -361,6 +361,16 @@ $(document).ready(function () {
         [% END %]
     </div>
 
+[% IF ( checkinmsg ) %]
+    [% IF ( checkinmsgtype == 'alert' ) %]
+        <div class="dialog alert">
+    [% ELSE %]
+        <div class="dialog message">
+    [% END %]
+            <p class="problem">[% checkinmsg %]</p>
+        </div>
+[% END%]
+
     <div id="exemptfines" class="dialog message" style="display:none;">
         <p>Fines for returned items are forgiven.</p>
     </div>
index 87e090e..558bba9 100755 (executable)
@@ -1,11 +1,9 @@
 #!/usr/bin/perl
-#
-# Add more tests here!!!
 
 use strict;
 use warnings;
 use DBI;
-use Test::More tests => 15;
+use Test::More tests => 26;
 use Test::MockModule;
 
 BEGIN {
@@ -26,16 +24,16 @@ $module->mock(
 my $itemtypes = [
     [
         'itemtype', 'description', 'rentalcharge', 'notforloan',
-        'imageurl', 'summary'
+        'imageurl', 'summary', 'checkinmsg'
     ],
-    [ 'BK', 'Books', 0, 0, '', '' ],
-    [ 'CD', 'CDRom', 0, 0, '', '' ]
+    [ 'BK', 'Books', 0, 0, '', '', 'foo' ],
+    [ 'CD', 'CDRom', 0, 0, '', '', 'bar' ]
 ];
 
 my $itemtypes_empty = [
     [
         'itemtype', 'description', 'rentalcharge', 'notforloan',
-        'imageurl', 'summary'
+        'imageurl', 'summary', 'checkinmsg'
     ],
 ];
 
@@ -53,6 +51,10 @@ is( scalar( @{$history} ), 1, 'Correct number of statements executed' );
 $dbh->{mock_add_resultset} = $itemtypes;
 
 @itemtypes = C4::ItemType->all();
+
+$history = $dbh->{mock_all_history};
+is( scalar( @{$history} ), 2, 'Correct number of statements executed' );
+
 is( @itemtypes, 2, 'ItemType->all should return an array with 2 elements' );
 
 is( $itemtypes[0]->fish, undef, 'Calling a bad descriptor gives undef' );
@@ -73,6 +75,33 @@ is( $itemtypes[0]->notforloan, '0', 'first not for loan is 0' );
 
 is( $itemtypes[1]->notforloan, '0', 'second not for loan is 0' );
 
-is( $itemtypes[0]->imageurl, '', 'first not for loan is undef' );
+is( $itemtypes[0]->imageurl, '', 'first imageurl is undef' );
+
+is( $itemtypes[1]->imageurl, '', 'second imageurl is undef' );
+
+is( $itemtypes[0]->checkinmsg, 'foo', 'first checkinmsg is foo' );
+
+is( $itemtypes[1]->checkinmsg, 'bar', 'second checkinmsg is bar' );
+
+# Mock the data again
+$dbh->{mock_add_resultset} = $itemtypes;
+
+# Test get(), which should return one itemtype
+my $itemtype = C4::ItemType->get( 'BK' );
+
+$history = $dbh->{mock_all_history};
+is( scalar( @{$history} ), 3, 'Correct number of statements executed' );
+
+is( $itemtype->fish, undef, 'Calling a bad descriptor gives undef' );
+
+is( $itemtype->itemtype, 'BK', 'itemtype is bk' );
+
+is( $itemtype->description, 'Books', 'description is books' );
+
+is( $itemtype->rentalcharge, '0', 'rental charge is 0' );
+
+is( $itemtype->notforloan, '0', 'not for loan is 0' );
+
+is( $itemtype->imageurl, '', ' not for loan is undef' );
 
-is( $itemtypes[1]->imageurl, '', 'second not for loan is undef' );
+is( $itemtype->checkinmsg, 'foo', 'checkinmsg is foo' );