Bug 6810: Fix QA failures

- remove DateTime->now()
    - use Koha::DateUtils->dt_from_string;
    - use Pod2usage for the usage
    - use Modern::Perl
    - use branches table
    - Change letter code from MEMEXP to MEMBERSHIP_EXPIRY
    - review comments implemented
    - fix qa script comments

Bug 6810 - Fix QA failures

    - MembershipExpiryDaysNotice system preferences arragned alphabetical order.

Bug 6810 - Add sample notices

    - review comments implemented
    - default value of is_html field in letter table is 0

Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

C4/Members.pm

@@ -1494,7 +1494,7 @@ sub GetExpiryDate {
 sub GetUpcomingMembershipExpires {
     my $dbh = C4::Context->dbh;
     my $days = C4::Context->preference("MembershipExpiryDaysNotice") || 0;
-    my $dateexpiry = DateTime->now()->add(days => $days)->ymd();
+    my $dateexpiry = output_pref({ dt => (dt_from_string()->add( days => $days)), dateformat => 'iso', dateonly => 1 });
 
     my $query = "
         SELECT borrowers.*, categories.description,

installer/data/mysql/atomicupdate/bug_6810-add_membershipexpiry_notification.sql

@@ -0,0 +1 @@
+INSERT IGNORE INTO letter (module, code, branchcode, name, title, content, message_transport_type) VALUES('members','MEMBERSHIP_EXPIRY','','Account expiration','Account expiration','Dear <<borrowers.title>> <<borrowers.firstname>> <<borrowers.surname>>,\r\n\r\nYour library card will expire soon, on:\r\n\r\n<<borrowers.dateexpiry>>\r\n\r\nThank you,\r\n\r\nLibrarian\r\n\r\n<<branches.branchname>>', 'email');

installer/data/mysql/de-DE/mandatory/sample_notices.sql

@@ -135,4 +135,5 @@ Ihr Biblioheksteam'
 );
 
 INSERT INTO letter(module, code, branchcode, name, title, content, message_transport_type)
-VALUES ('acquisition', 'ACQ_NOTIF_ON_RECEIV', '', 'Benachrichtigung bei Zugang', 'Bestelltes Medium ist eingetroffen', 'Liebe/r <<borrowers.firstname>> <<borrowers.surname>>,\n\nDie Bestellung <<aqorders.ordernumber>> (<<biblio.title>>) ist eingetroffen und wird bearbeitet.\n\nIhr Bibliotheksteam', 'email')
+VALUES ('acquisition', 'ACQ_NOTIF_ON_RECEIV', '', 'Benachrichtigung bei Zugang', 'Bestelltes Medium ist eingetroffen', 'Liebe/r <<borrowers.firstname>> <<borrowers.surname>>,\n\nDie Bestellung <<aqorders.ordernumber>> (<<biblio.title>>) ist eingetroffen und wird bearbeitet.\n\nIhr Bibliotheksteam', 'email'),
+('members','MEMBERSHIP_EXPIRY','','Account expiration','Account expiration','Dear <<borrowers.title>> <<borrowers.firstname>> <<borrowers.surname>>,.\r\n\r\nYour library card will expire soon, on:\r\n\r\n<<borrowers.dateexpiry>>\r\n\r\nThank you,\r\n\r\nLibrarian\r\n\r\n<<branches.branchname>>','email');

installer/data/mysql/en/mandatory/sample_notices.sql

@@ -154,4 +154,5 @@ Your library.'
 );
 
 INSERT INTO letter(module, code, branchcode, name, title, content, message_transport_type)
-VALUES ('acquisition', 'ACQ_NOTIF_ON_RECEIV', '', 'Notification on receiving', 'Order received', 'Dear <<borrowers.firstname>> <<borrowers.surname>>,\n\n The order <<aqorders.ordernumber>> (<<biblio.title>>) has been received.\n\nYour library.', 'email')
+VALUES ('acquisition', 'ACQ_NOTIF_ON_RECEIV', '', 'Notification on receiving', 'Order received', 'Dear <<borrowers.firstname>> <<borrowers.surname>>,\n\n The order <<aqorders.ordernumber>> (<<biblio.title>>) has been received.\n\nYour library.', 'email'),
+('members','MEMBERSHIP_EXPIRY','','Account expiration','Account expiration','Dear <<borrowers.title>> <<borrowers.firstname>> <<borrowers.surname>>,.\r\n\r\nYour library card will expire soon, on:\r\n\r\n<<borrowers.dateexpiry>>\r\n\r\nThank you,\r\n\r\nLibrarian\r\n\r\n<<branches.branchname>>','email');

installer/data/mysql/es-ES/mandatory/sample_notices.sql

@@ -149,4 +149,5 @@ Your library.'
 
 
 INSERT INTO letter(module, code, branchcode, name, title, content, message_transport_type)
-VALUES ('acquisition', 'ACQ_NOTIF_ON_RECEIV', '', 'Notification on receiving', 'Order received', 'Dear <<borrowers.firstname>> <<borrowers.surname>>,\n\n The order <<aqorders.ordernumber>> (<<biblio.title>>) has been received.\n\nYour library.', 'email')
+VALUES ('acquisition', 'ACQ_NOTIF_ON_RECEIV', '', 'Notification on receiving', 'Order received', 'Dear <<borrowers.firstname>> <<borrowers.surname>>,\n\n The order <<aqorders.ordernumber>> (<<biblio.title>>) has been received.\n\nYour library.', 'email'),
+('members','MEMBERSHIP_EXPIRY','','Account expiration','Account expiration','Dear <<borrowers.title>> <<borrowers.firstname>> <<borrowers.surname>>,.\r\n\r\nYour library card will expire soon, on:\r\n\r\n<<borrowers.dateexpiry>>\r\n\r\nThank you,\r\n\r\nLibrarian\r\n\r\n<<branches.branchname>>','email');

installer/data/mysql/fr-FR/1-Obligatoire/sample_notices.sql

@@ -150,4 +150,5 @@ Your library.'
 );
 
 INSERT INTO letter(module, code, branchcode, name, title, content, message_transport_type)
-VALUES ('acquisition', 'ACQ_NOTIF_ON_RECEIV', '', 'Notification on receiving', 'Order received', 'Dear <<borrowers.firstname>> <<borrowers.surname>>,\n\n The order <<aqorders.ordernumber>> (<<biblio.title>>) has been received.\n\nYour library.', 'email')
+VALUES ('acquisition', 'ACQ_NOTIF_ON_RECEIV', '', 'Notification on receiving', 'Order received', 'Dear <<borrowers.firstname>> <<borrowers.surname>>,\n\n The order <<aqorders.ordernumber>> (<<biblio.title>>) has been received.\n\nYour library.', 'email'),
+('members','MEMBERSHIP_EXPIRY','','Account expiration','Account expiration','Dear <<borrowers.title>> <<borrowers.firstname>> <<borrowers.surname>>,.\r\n\r\nYour library card will expire soon, on:\r\n\r\n<<borrowers.dateexpiry>>\r\n\r\nThank you,\r\n\r\nLibrarian\r\n\r\n<<branches.branchname>>','email');

installer/data/mysql/it-IT/necessari/notices.sql

@@ -147,4 +147,5 @@ Lo staff della biblioteca.'
 );
 
 INSERT INTO letter(module, code, branchcode, name, title, content, message_transport_type)
-VALUES ('acquisition', 'ACQ_NOTIF_ON_RECEIV', '', 'Notification on receiving', 'Order received', 'Dear <<borrowers.firstname>> <<borrowers.surname>>,\n\n The order <<aqorders.ordernumber>> (<<biblio.title>>) has been received.\n\nYour library.', 'email')
+VALUES ('acquisition', 'ACQ_NOTIF_ON_RECEIV', '', 'Notification on receiving', 'Order received', 'Dear <<borrowers.firstname>> <<borrowers.surname>>,\n\n The order <<aqorders.ordernumber>> (<<biblio.title>>) has been received.\n\nYour library.', 'email'),
+('members','MEMBERSHIP_EXPIRY','','Account expiration','Account expiration','Dear <<borrowers.title>> <<borrowers.firstname>> <<borrowers.surname>>,.\r\n\r\nYour library card will expire soon, on:\r\n\r\n<<borrowers.dateexpiry>>\r\n\r\nThank you,\r\n\r\nLibrarian\r\n\r\n<<branches.branchname>>','email');

installer/data/mysql/nb-NO/1-Obligatorisk/sample_notices.sql

@@ -169,4 +169,5 @@ Your library.'
 );
 
 INSERT INTO letter(module, code, branchcode, name, title, content, message_transport_type)
-VALUES ('acquisition', 'ACQ_NOTIF_ON_RECEIV', '', 'Notification on receiving', 'Order received', 'Dear <<borrowers.firstname>> <<borrowers.surname>>,\n\n The order <<aqorders.ordernumber>> (<<biblio.title>>) has been received.\n\nYour library.', 'email')
+VALUES ('acquisition', 'ACQ_NOTIF_ON_RECEIV', '', 'Notification on receiving', 'Order received', 'Dear <<borrowers.firstname>> <<borrowers.surname>>,\n\n The order <<aqorders.ordernumber>> (<<biblio.title>>) has been received.\n\nYour library.', 'email'),
+('members','MEMBERSHIP_EXPIRY','','Account expiration','Account expiration','Dear <<borrowers.title>> <<borrowers.firstname>> <<borrowers.surname>>,.\r\n\r\nYour library card will expire soon, on:\r\n\r\n<<borrowers.dateexpiry>>\r\n\r\nThank you,\r\n\r\nLibrarian\r\n\r\n<<branches.branchname>>','email');

installer/data/mysql/pl-PL/mandatory/sample_notices.sql

@@ -147,4 +147,5 @@ Your library.'
 );
 
 INSERT INTO letter(module, code, branchcode, name, title, content, message_transport_type)
-VALUES ('acquisition', 'ACQ_NOTIF_ON_RECEIV', '', 'Notification on receiving', 'Order received', 'Dear <<borrowers.firstname>> <<borrowers.surname>>,\n\n The order <<aqorders.ordernumber>> (<<biblio.title>>) has been received.\n\nYour library.', 'email')
+VALUES ('acquisition', 'ACQ_NOTIF_ON_RECEIV', '', 'Notification on receiving', 'Order received', 'Dear <<borrowers.firstname>> <<borrowers.surname>>,\n\n The order <<aqorders.ordernumber>> (<<biblio.title>>) has been received.\n\nYour library.', 'email'),
+('members','MEMBERSHIP_EXPIRY','','Account expiration','Account expiration','Dear <<borrowers.title>> <<borrowers.firstname>> <<borrowers.surname>>,.\r\n\r\nYour library card will expire soon, on:\r\n\r\n<<borrowers.dateexpiry>>\r\n\r\nThank you,\r\n\r\nLibrarian\r\n\r\n<<branches.branchname>>','email');

installer/data/mysql/ru-RU/mandatory/sample_notices.sql

@@ -149,4 +149,5 @@ Your library.'
 );
 
 INSERT INTO letter(module, code, branchcode, name, title, content, message_transport_type)
-VALUES ('acquisition', 'ACQ_NOTIF_ON_RECEIV', '', 'Notification on receiving', 'Order received', 'Dear <<borrowers.firstname>> <<borrowers.surname>>,\n\n The order <<aqorders.ordernumber>> (<<biblio.title>>) has been received.\n\nYour library.', 'email')
+VALUES ('acquisition', 'ACQ_NOTIF_ON_RECEIV', '', 'Notification on receiving', 'Order received', 'Dear <<borrowers.firstname>> <<borrowers.surname>>,\n\n The order <<aqorders.ordernumber>> (<<biblio.title>>) has been received.\n\nYour library.', 'email'),
+('members','MEMBERSHIP_EXPIRY','','Account expiration','Account expiration','Dear <<borrowers.title>> <<borrowers.firstname>> <<borrowers.surname>>,.\r\n\r\nYour library card will expire soon, on:\r\n\r\n<<borrowers.dateexpiry>>\r\n\r\nThank you,\r\n\r\nLibrarian\r\n\r\n<<branches.branchname>>','email');

installer/data/mysql/sysprefs.sql

@@ -211,6 +211,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, `
 ('maxoutstanding','5','','maximum amount withstanding to be able make holds','Integer'),
 ('maxRecordsForFacets','20',NULL,NULL,'Integer'),
 ('maxreserves','50','','Define maximum number of holds a patron can place','Integer'),
+('MembershipExpiryDaysNotice','14','Send an account expiration notice that a patron\'s card is about to expire after',NULL,'Integer'),
 ('minPasswordLength','3',NULL,'Specify the minimum length of a patron/staff password','free'),
 ('NewItemsDefaultLocation','','','If set, all new items will have a location of the given Location Code ( Authorized Value type LOC )',''),
 ('noissuescharge','5','','Define maximum amount withstanding before check outs are blocked','Integer'),
@@ -492,6 +493,5 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, `
 ('XSLTDetailsDisplay','default','','Enable XSL stylesheet control over details page display on intranet','Free'),
 ('XSLTResultsDisplay','default','','Enable XSL stylesheet control over results page display on intranet','Free'),
 ('z3950AuthorAuthFields','701,702,700',NULL,'Define the MARC biblio fields for Personal Name Authorities to fill biblio.author','free'),
-('z3950NormalizeAuthor','0','','If ON, Personal Name Authorities will replace authors in biblio.author','YesNo'),
-('MembershipExpiryDaysNotice' ,14,'Send an account expiration notice that a patron\'s card is about to expire after',NULL,'Integer')
+('z3950NormalizeAuthor','0','','If ON, Personal Name Authorities will replace authors in biblio.author','YesNo')
 ;

installer/data/mysql/uk-UA/mandatory/sample_notices.sql

@@ -148,4 +148,5 @@ Your library.'
 );
 
 INSERT INTO letter(module, code, branchcode, name, title, content, message_transport_type)
-VALUES ('acquisition', 'ACQ_NOTIF_ON_RECEIV', '', 'Notification on receiving', 'Order received', 'Dear <<borrowers.firstname>> <<borrowers.surname>>,\n\n The order <<aqorders.ordernumber>> (<<biblio.title>>) has been received.\n\nYour library.', 'email')
+VALUES ('acquisition', 'ACQ_NOTIF_ON_RECEIV', '', 'Notification on receiving', 'Order received', 'Dear <<borrowers.firstname>> <<borrowers.surname>>,\n\n The order <<aqorders.ordernumber>> (<<biblio.title>>) has been received.\n\nYour library.', 'email'),
+('members','MEMBERSHIP_EXPIRY','','Account expiration','Account expiration','Dear <<borrowers.title>> <<borrowers.firstname>> <<borrowers.surname>>,.\r\n\r\nYour library card will expire soon, on:\r\n\r\n<<borrowers.dateexpiry>>\r\n\r\nThank you,\r\n\r\nLibrarian\r\n\r\n<<branches.branchname>>','email');

misc/cronjobs/membership_expiry.pl

@@ -2,7 +2,7 @@
 
 # This file is part of Koha.
 #
-# Copyright (C) 2013 Amit Gupta (amitddng135@gmail.com)
+# Copyright (C) 2015 Amit Gupta (amitddng135@gmail.com)
 #
 # Koha is free software; you can redistribute it and/or modify it
 # under the terms of the GNU General Public License as published by
@@ -19,21 +19,28 @@
 
 =head1 NAME
 
-membership_expires.pl - cron script to put membership expiry reminders into message queue
+membership_expiry.pl - cron script to put membership expiry reminder into message queues
 
 =head1 SYNOPSIS
 
-./membership_expires.pl -c
+./membership_expiry.pl -c
 
 or, in crontab:
 
-0 1 * * * membership_expires.pl -c
+0 1 * * * membership_expiry.pl -c
+
+=head1 DESCRIPTION
+
+This script sends membership expiry reminder notices to patrons.
+It queues them in the message queue, which is processed by
+the process_message_queue.pl cronjob.
 
 =cut
 
-use strict;
-use warnings;
+
+use Modern::Perl;
 use Getopt::Long;
+use Pod::Usage;
 use Data::Dumper;
 BEGIN {
     # find Koha's Perl modules
@@ -47,18 +54,109 @@ use C4::Letters;
 use C4::Dates qw/format_date/;
 use C4::Log;
 
-cronlogaction();
+
+=head1 NAME
+
+membership_expiry.pl - prepare messages to be sent to membership expiry reminder notices to patrons.
+
+=head1 SYNOPSIS
+
+membership_expiry.pl [-c]
+
+=head1 OPTIONS
+
+=over 8
+
+=item B<--help>
+
+Print a brief help message and exits.
+
+=item B<--man>
+
+Prints the manual page and exits.
+
+=item B<-v>
+
+Verbose. Without this flag set, only fatal errors are reported.
+
+=item B<-n>
+
+Do not send any email. Membership expire notices that would have been sent to
+the patrons are printed to standard out.
+
+=item B<-c>
+
+Confirm flag: Add this option. The script will only print a usage
+statement otherwise.
+
+=back
+
+=head1 DESCRIPTION
+
+This script is designed to alert send membership expire notices
+
+=head2 Configuration
+
+This script pays attention to send the membership expire notices
+The content of the messages is configured in Tools -> Notices and slips. Use the MEMBERSHIP_EXPIRY template
+
+=head2 Outgoing emails
+
+Typically, messages are prepared for each patron when the memberships are going to expire
+
+
+These emails are staged in the outgoing message queue, as are messages
+produced by other features of Koha. This message queue must be
+processed regularly by the
+F<misc/cronjobs/process_message_queue.pl> program.
+
+In the event that the C<-n> flag is passed to this program, no emails
+are sent. Instead, messages are sent on standard output from this
+program.
+
+=head2 Templates
+
+Templates can contain variables enclosed in double angle brackets like
+E<lt>E<lt>thisE<gt>E<gt>. Those variables will be replaced with values
+specific to the members there membership expiry date is coming.
+Available variables are:
+
+=over
+
+=item E<lt>E<lt>borrowers.*E<gt>E<gt>
+
+any field from the borrowers table
+
+=item E<lt>E<lt>branches.*E<gt>E<gt>
+
+any field from the branches table
+
+=back
+
+=head1 SEE ALSO
+
+The F<misc/cronjobs/membership_expiry.pl> program allows you to send
+messages to patrons when the membership are going to expires.
+=cut
 
 # These are defaults for command line options.
+
 my $confirm;                              # -c: Confirm that the user has read and configured this script.
 my $nomail;                               # -n: No mail. Will not send any emails.
 my $verbose= 0;                           # -v: verbose
 
-GetOptions( 'c'              => \$confirm,
+my $help    = 0;
+my $man     = 0;
+
+GetOptions(
+            'help|?'         => \$help,
+            'man'            => \$man,
+            'c'              => \$confirm,
             'n'              => \$nomail,
             'v'              => \$verbose,
-       );
-
+       ) or pod2usage(2);
+pod2usage(1) if $help;
+pod2usage( -verbose => 2 ) if $man;;
 
 my $usage = << 'ENDUSAGE';
 This script prepares for membership expiry reminders to be sent to
@@ -67,7 +165,7 @@ the process_message_queue.pl cronjob.
 See the comments in the script for directions on changing the script.
 This script has the following parameters :
     -c Confirm and remove this help & warning
-    -n send No mail. Instead, all mail messages are printed on screen. Usefull for testing purposes.
+    -n send No mail. Instead, all mail messages are printed on screen. Useful for testing purposes.
     -v verbose
 ENDUSAGE
 
@@ -78,6 +176,8 @@ unless ($confirm) {
     exit unless (/^y/i);
 }
 
+cronlogaction();
+
 my $admin_adress = C4::Context->preference('KohaAdminEmailAddress');
 warn 'getting upcoming membership expires' if $verbose;
 my $upcoming_mem_expires = C4::Members::GetUpcomingMembershipExpires();
@@ -86,7 +186,7 @@ warn 'found ' . scalar( @$upcoming_mem_expires ) . ' issues' if $verbose;
 
 UPCOMINGMEMEXP: foreach my $recent ( @$upcoming_mem_expires ) {
     my $from_address = $recent->{'branchemail'} || $admin_adress;
-    my $letter_type = 'MEMEXP';
+    my $letter_type = 'MEMBERSHIP_EXPIRY';
     my $letter = C4::Letters::getletter( 'members', $letter_type, $recent->{'branchcode'} );
     die "no letter of type '$letter_type' found. Please see sample_notices.sql" unless $letter;
 
@@ -123,9 +223,8 @@ sub parse_letter {
     }
     my $letter =  C4::Letters::GetPreparedLetter (
             module => 'members',
-            letter_code => 'MEMEXP',
-            branchcode => $params->{'branchcode'},
-            tables => {'borrowers', $params->{'borrowernumber'},},
+            letter_code => 'MEMBERSHIP_EXPIRY',
+            tables => {'borrowers', $params->{'borrowernumber'}, 'branches', $params->{'branchcode'}},
     );
 }