|
Description
Jonathan Druart
2014-01-23 08:31:59 UTC
Created attachment 25021 [details] [review] Bug 11603: Preparation step, cleaning up This patch refactores and adds some good practices: - use Modern::Perl - use Pod::Usage - add POD Created attachment 25022 [details] [review] Bug 11603: Gather print notices - add a csv parameter This patch adds: - the ability to generate a csv file instead of a html file. - a letter_code parameter. From now you are able to generate a csv file for print notices. Imagine a template notice defined as: cardnumber:patron:email:item <<borrowers.cardnumber>>:<<borrowers.firstname>> <borrowers.surname>>:<<borrowers.email>>:<<items.barcode>> You would like to generate a csv file and not a html file. Test plan: - define your OVERDUE notice for the print template as: cardnumber:patron:email:item <<borrowers.cardnumber>>:<<borrowers.firstname>> <borrowers.surname>>:<<borrowers.email>>:<<items.barcode>> - define overdues rules for a patron category - check 2 items out using a due date in order to generate the overdue notices - check these 2 items in - launch the overdue_notices script - the message_queue table should now contain 2 new entries - launch the gather_print_notices cronjob with the following parameters: perl misc/cronjobs/gather_print_notices.pl /tmp/test --csv --letter_code=OVERDUE --letter_code=CHECKIN you should get an error perl misc/cronjobs/gather_print_notices.pl /tmp/test --csv you should get an error perl misc/cronjobs/gather_print_notices.pl /tmp/test --csv --letter_code=OVERDUE -d=: will produce 1 csv file in your /tmp/test directory - verify the csv file is correct and contain only 1 csv header column. Created attachment 25023 [details] [review] Bug 11603: Gather print notices - add a ods parameter This patch adds: - the ability to generate an ods file From now you are able to generate a ods file for print notices. You would like to generate a csv file and not a html file. Test plan: - same as previous patch but test the following parameters: perl misc/cronjobs/gather_print_notices.pl /tmp/test --ods --letter_code=OVERDUE -d=: you should get an error because csv2ods is not installed. Follow the installation instructions and try again the command. A ods file should be generated in your /tmp/test directory. Created attachment 25024 [details] [review] Bug 11603: Refactoring A lot of code is duplicated for the 3 supported formats html, csv and ods. This patch refactores some of the code and makes it more readable. Created attachment 25640 [details] [review] Bug 11603: Fix delimiter on launching csv2ods For instance, if the delimiter is ';', quote should be put around. Created attachment 25642 [details] [review] Bug 11603: If a letter code is given, filenames should contain it If you choose to generate print notices for a specific letter code, the generated files should be distinct. The use case is: you want to process print notice for letter codes: overdue1, overdue2 and overdue3. The cronjobs will be: perl misc/cronjobs/gather_print_notices.pl /tmp --letter_code=overdue1 --csv --ods --html --delimiter=";" perl misc/cronjobs/gather_print_notices.pl /tmp --letter_code=overdue2 --csv --ods --html --delimiter=";" perl misc/cronjobs/gather_print_notices.pl /tmp --letter_code=overdue3 --csv --ods --html --delimiter=";" without this patch, the 2 first files will be erased. This patch is blocked by bug 11679. The ods generation should be rewritten using OpenOffice::OODoc. Created attachment 30980 [details] [review] Bug 11603: Preparation step, cleaning up This patch refactores and adds some good practices: - use Modern::Perl - use Pod::Usage - add POD Created attachment 30981 [details] [review] Bug 11603: Gather print notices - add a csv parameter This patch adds: - the ability to generate a csv file instead of a html file. - a letter_code parameter. From now you are able to generate a csv file for print notices. Imagine a template notice defined as: cardnumber:patron:email:item <<borrowers.cardnumber>>:<<borrowers.firstname>> <borrowers.surname>>:<<borrowers.email>>:<<items.barcode>> You would like to generate a csv file and not a html file. Test plan: - define your OVERDUE notice for the print template as: cardnumber:patron:email:item <<borrowers.cardnumber>>:<<borrowers.firstname>> <borrowers.surname>>:<<borrowers.email>>:<<items.barcode>> - define overdues rules for a patron category - check 2 items out using a due date in order to generate the overdue notices - check these 2 items in - launch the overdue_notices script - the message_queue table should now contain 2 new entries - launch the gather_print_notices cronjob with the following parameters: perl misc/cronjobs/gather_print_notices.pl /tmp/test --csv --letter_code=OVERDUE --letter_code=CHECKIN you should get an error perl misc/cronjobs/gather_print_notices.pl /tmp/test --csv you should get an error perl misc/cronjobs/gather_print_notices.pl /tmp/test --csv --letter_code=OVERDUE -d=: will produce 1 csv file in your /tmp/test directory - verify the csv file is correct and contain only 1 csv header column. Created attachment 30982 [details] [review] Bug 11603: Gather print notices - add a ods parameter This patch adds: - the ability to generate an ods file From now you are able to generate a ods file for print notices. You would like to generate a csv file and not a html file. Test plan: - same as previous patch but test the following parameters: perl misc/cronjobs/gather_print_notices.pl /tmp/test --ods --letter_code=OVERDUE -d=: you should get an error because csv2ods is not installed. Follow the installation instructions and try again the command. A ods file should be generated in your /tmp/test directory. Created attachment 30983 [details] [review] Bug 11603: If a letter code is given, filenames should contain it If you choose to generate print notices for a specific letter code, the generated files should be distinct. The use case is: you want to process print notice for letter codes: overdue1, overdue2 and overdue3. The cronjobs will be: perl misc/cronjobs/gather_print_notices.pl /tmp --letter_code=overdue1 --csv --ods --html --delimiter=";" perl misc/cronjobs/gather_print_notices.pl /tmp --letter_code=overdue2 --csv --ods --html --delimiter=";" perl misc/cronjobs/gather_print_notices.pl /tmp --letter_code=overdue3 --csv --ods --html --delimiter=";" without this patch, the 2 first files will be erased. (In reply to Jonathan Druart from comment #7) > This patch is blocked by bug 11679. The ods generation should be rewritten > using OpenOffice::OODoc. Done in the last patch set. Thank you so much Jonathan for pushing to a better handling of print messages.
Isn't there something that could be improve conceptually or from an
architectural perspective? (always possible of course :-) On the one hand,
there is C4::Letters module, and on the other hand process_message_queue.pl and
gather_print_notices.pl using these module. Both .pl scripts are doing the
same thing: gathering messages from the queue, and process them (send them, and
switch there status from pending to sent or failed). For
process_message_queue.pl, there is just one call to
C4::Letter::SendQueuedMessages. When gather_print_notices.pl do all kind of
processing, and then call C4::Letter internal function like
_set_message_status. Shouldn't be logical just to merge both .pl scripts, and
put all code in SendQueueMessage?
SendQueueMessage interface could be extended with more parameters. For example:
SendQueueMessage({
mtt => 'print',
split => '0|1',
dir => '/tmp',
email_to => 'a@a.fr',
format => 'html|csv',
groupby => ['branch', 'letter']
});
(In reply to Frédéric Demians from comment #13) > Thank you so much Jonathan for pushing to a better handling of print > messages. > > Isn't there something that could be improve conceptually or from an > architectural perspective? (always possible of course :-) Yes of course, and that was the idea. But as you can see, I have submitted this bug and bug 11678 1 year ago and nothing has moved yet... All should be done with small steps, and these 2 bugs are 2 steps. The idea is to have only 1 script to sent notices, I would also see misc/cronjobs/printoverdues.sh merged in it. It doesn't work if --letter_code parameter isn't given to the script. There is an encoding problem with html output. I get warnings about wide character and the result isn't utf8 encoded. Maybe '>:encoding(utf8)' is required. It would be nice to have a --send|--nosend, defaulted to --nosend, in order to control whether the messages status is changed from 'pending' to 'sent'. This would ease testing. Created attachment 37799 [details] [review] Bug 11603: Fix encoding issues (bug 11944 is now pushed) (In reply to Frédéric Demians from comment #15) > It doesn't work if --letter_code parameter isn't given to the script. What do you mean? If you use --csv or --ods, you have to provide a letter_code, otherwise generating a csv file does not have sense. > There is an encoding problem with html output. I get warnings about wide > character and the result isn't utf8 encoded. Maybe '>:encoding(utf8)' is > required. Should be fixed with the last patch, please retest html, csv and ods. > It would be nice to have a --send|--nosend, defaulted to --nosend, in order > to control whether the messages status is changed from 'pending' to 'sent'. > This would ease testing. Sorry but I don't plan to provide anything else than bug fixes until it's pushed. (In reply to Jonathan Druart from comment #17) > > It doesn't work if --letter_code parameter isn't given to the script. > What do you mean? If you use --csv or --ods, you have to provide a > letter_code, otherwise generating a csv file does not have sense. It doesn't work with --html. This produces no html file: ./gather_print_notices.pl -s --html <mydir> This works properly: ./gather_print_notices.pl -s --html <mydir> --letter_code XXX I will fix that. > Should be fixed with the last patch, please retest html, csv and ods. It works. Thanks. > > It would be nice to have a --send|--nosend, defaulted to --nosend, in order > > to control whether the messages status is changed from 'pending' to 'sent'. > > This would ease testing. > Sorry but I don't plan to provide anything else than bug fixes until it's > pushed. No demand, no offense :-) (In reply to Frédéric Demians from comment #18) > (In reply to Jonathan Druart from comment #17) > > > > It doesn't work if --letter_code parameter isn't given to the script. > > > What do you mean? If you use --csv or --ods, you have to provide a > > letter_code, otherwise generating a csv file does not have sense. > > It doesn't work with --html. This produces no html file: > > ./gather_print_notices.pl -s --html <mydir> > > This works properly: > > ./gather_print_notices.pl -s --html <mydir> --letter_code XXX > > I will fix that. Ha yes, because of the "filter by letter_code" in any case :) Created attachment 37812 [details] [review] Bug 11603 Add --send|--nosend, fix stuf Fix --html without --letter_code Fix --ods which was producing a 2 lines ods file Created attachment 37813 [details] [review] [SIGNED-OFF] Bug 11603: Fix encoding issues (bug 11944 is now pushed) Signed-off-by: Frederic Demians <f.demians@tamil.fr> No more encoding issue with html file, no problem with csv|ods Created attachment 37814 [details] [review] [SIGNED-OFF] Bug 11603: If a letter code is given, filenames should contain it If you choose to generate print notices for a specific letter code, the generated files should be distinct. The use case is: you want to process print notice for letter codes: overdue1, overdue2 and overdue3. The cronjobs will be: perl misc/cronjobs/gather_print_notices.pl /tmp --letter_code=overdue1 --csv --ods --html --delimiter=";" perl misc/cronjobs/gather_print_notices.pl /tmp --letter_code=overdue2 --csv --ods --html --delimiter=";" perl misc/cronjobs/gather_print_notices.pl /tmp --letter_code=overdue3 --csv --ods --html --delimiter=";" without this patch, the 2 first files will be erased. Signed-off-by: Frederic Demians <f.demians@tamil.fr> Created attachment 37815 [details] [review] [SIGNED-OFF] Bug 11603: Gather print notices - add a ods parameter This patch adds: - the ability to generate an ods file From now you are able to generate a ods file for print notices. You would like to generate a csv file and not a html file. Test plan: - same as previous patch but test the following parameters: perl misc/cronjobs/gather_print_notices.pl /tmp/test --ods --letter_code=OVERDUE -d=: you should get an error because csv2ods is not installed. Follow the installation instructions and try again the command. A ods file should be generated in your /tmp/test directory. Signed-off-by: Frederic Demians <f.demians@tamil.fr> Created attachment 37816 [details] [review] [SIGNED-OFF] Bug 11603: Gather print notices - add a csv parameter This patch adds: - the ability to generate a csv file instead of a html file. - a letter_code parameter. From now you are able to generate a csv file for print notices. Imagine a template notice defined as: cardnumber:patron:email:item <<borrowers.cardnumber>>:<<borrowers.firstname>> <borrowers.surname>>:<<borrowers.email>>:<<items.barcode>> You would like to generate a csv file and not a html file. Test plan: - define your OVERDUE notice for the print template as: cardnumber:patron:email:item <<borrowers.cardnumber>>:<<borrowers.firstname>> <borrowers.surname>>:<<borrowers.email>>:<<items.barcode>> - define overdues rules for a patron category - check 2 items out using a due date in order to generate the overdue notices - check these 2 items in - launch the overdue_notices script - the message_queue table should now contain 2 new entries - launch the gather_print_notices cronjob with the following parameters: perl misc/cronjobs/gather_print_notices.pl /tmp/test --csv --letter_code=OVERDUE --letter_code=CHECKIN you should get an error perl misc/cronjobs/gather_print_notices.pl /tmp/test --csv you should get an error perl misc/cronjobs/gather_print_notices.pl /tmp/test --csv --letter_code=OVERDUE -d=: will produce 1 csv file in your /tmp/test directory - verify the csv file is correct and contain only 1 csv header column. Signed-off-by: Frederic Demians <f.demians@tamil.fr> Created attachment 37817 [details] [review] [SIGNED-OFF] Bug 11603: Preparation step, cleaning up This patch refactores and adds some good practices: - use Modern::Perl - use Pod::Usage - add POD Signed-off-by: Frederic Demians <f.demians@tamil.fr> For me, it functional, well documented, and add a way to do something worthwhile with 'print' notifications. I've tested all script options with success producing usable HTML, CSV, and OOffice files. Thanks Frédéric. There is something wrong the the patches order. I don't know how you did that but they are in the wrong order :) Created attachment 37829 [details] [review] [SIGNED-OFF] Bug 11603: Preparation step, cleaning up This patch refactores and adds some good practices: - use Modern::Perl - use Pod::Usage - add POD Signed-off-by: Frederic Demians <f.demians@tamil.fr> Created attachment 37830 [details] [review] [SIGNED-OFF] Bug 11603: Gather print notices - add a csv parameter This patch adds: - the ability to generate a csv file instead of a html file. - a letter_code parameter. From now you are able to generate a csv file for print notices. Imagine a template notice defined as: cardnumber:patron:email:item <<borrowers.cardnumber>>:<<borrowers.firstname>> <borrowers.surname>>:<<borrowers.email>>:<<items.barcode>> You would like to generate a csv file and not a html file. Test plan: - define your OVERDUE notice for the print template as: cardnumber:patron:email:item <<borrowers.cardnumber>>:<<borrowers.firstname>> <borrowers.surname>>:<<borrowers.email>>:<<items.barcode>> - define overdues rules for a patron category - check 2 items out using a due date in order to generate the overdue notices - check these 2 items in - launch the overdue_notices script - the message_queue table should now contain 2 new entries - launch the gather_print_notices cronjob with the following parameters: perl misc/cronjobs/gather_print_notices.pl /tmp/test --csv --letter_code=OVERDUE --letter_code=CHECKIN you should get an error perl misc/cronjobs/gather_print_notices.pl /tmp/test --csv you should get an error perl misc/cronjobs/gather_print_notices.pl /tmp/test --csv --letter_code=OVERDUE -d=: will produce 1 csv file in your /tmp/test directory - verify the csv file is correct and contain only 1 csv header column. Signed-off-by: Frederic Demians <f.demians@tamil.fr> Created attachment 37831 [details] [review] [SIGNED-OFF] Bug 11603: Gather print notices - add a ods parameter This patch adds: - the ability to generate an ods file From now you are able to generate a ods file for print notices. You would like to generate a csv file and not a html file. Test plan: - same as previous patch but test the following parameters: perl misc/cronjobs/gather_print_notices.pl /tmp/test --ods --letter_code=OVERDUE -d=: you should get an error because csv2ods is not installed. Follow the installation instructions and try again the command. A ods file should be generated in your /tmp/test directory. Signed-off-by: Frederic Demians <f.demians@tamil.fr> Created attachment 37832 [details] [review] [SIGNED-OFF] Bug 11603: If a letter code is given, filenames should contain it If you choose to generate print notices for a specific letter code, the generated files should be distinct. The use case is: you want to process print notice for letter codes: overdue1, overdue2 and overdue3. The cronjobs will be: perl misc/cronjobs/gather_print_notices.pl /tmp --letter_code=overdue1 --csv --ods --html --delimiter=";" perl misc/cronjobs/gather_print_notices.pl /tmp --letter_code=overdue2 --csv --ods --html --delimiter=";" perl misc/cronjobs/gather_print_notices.pl /tmp --letter_code=overdue3 --csv --ods --html --delimiter=";" without this patch, the 2 first files will be erased. Signed-off-by: Frederic Demians <f.demians@tamil.fr> Created attachment 37833 [details] [review] [SIGNED-OFF] Bug 11603: Fix encoding issues (bug 11944 is now pushed) Signed-off-by: Frederic Demians <f.demians@tamil.fr> No more encoding issue with html file, no problem with csv|ods Created attachment 37834 [details] [review] Bug 11603 Add --send|--nosend, fix stuf Fix --html without --letter_code Fix --ods which was producing a 2 lines ods file (In reply to Jonathan Druart from comment #27) > Thanks Frédéric. > There is something wrong the the patches order. I don't know how you did > that but they are in the wrong order :) Sorry. I git bz on me. I verified that it's fixed now. Created attachment 37837 [details] [review] Bug 11678: Gather print notices: add --email parameter This patch add the ability to send generated files by email. You can specify several emails. Test plan: - same as Bug 11603 - call the script misc/cronjobs/gather_print_notices.pl with the following parameters: perl misc/cronjobs/gather_print_notices.pl /tmp/test --csv --ods --html --letter_code=CHECKIN -d=: --email="email_one@example.org" --email="email_two@example.org" Signed-off-by: Frederic Demians <f.demians@tamil.fr> - Works as desribed. Rebased upon bug-11603. Comment on attachment 37837 [details] [review] Bug 11678: Gather print notices: add --email parameter Wrong patch attachment! Created attachment 38381 [details] [review] Bug 11603: Some minor documentation changes I like this! (But need a bit more time to play with it :( ) Some first notes from reading the docs (yay!) and code: 1) I see there are 2 copyright statements and licenses, one at the top and one at the bottom. Should this be cleaned up? 2) Is this statement about the file name still true? The generated filename will be holdnotices-TODAY.[csv|html|ods] or holdnotices-TODAY-BRANCHCODE.[csv|html|ods] if the --split parameter is given. 3) I have attached some suggestions for the documentation - please take a look and obsolete if you don't agree :) Created attachment 38385 [details] [review] Bug 11603: Preparation step, cleaning up This patch refactores and adds some good practices: - use Modern::Perl - use Pod::Usage - add POD Signed-off-by: Frederic Demians <f.demians@tamil.fr> Created attachment 38386 [details] [review] Bug 11603: Gather print notices - add a csv parameter This patch adds: - the ability to generate a csv file instead of a html file. - a letter_code parameter. From now you are able to generate a csv file for print notices. Imagine a template notice defined as: cardnumber:patron:email:item <<borrowers.cardnumber>>:<<borrowers.firstname>> <borrowers.surname>>:<<borrowers.email>>:<<items.barcode>> You would like to generate a csv file and not a html file. Test plan: - define your OVERDUE notice for the print template as: cardnumber:patron:email:item <<borrowers.cardnumber>>:<<borrowers.firstname>> <borrowers.surname>>:<<borrowers.email>>:<<items.barcode>> - define overdues rules for a patron category - check 2 items out using a due date in order to generate the overdue notices - check these 2 items in - launch the overdue_notices script - the message_queue table should now contain 2 new entries - launch the gather_print_notices cronjob with the following parameters: perl misc/cronjobs/gather_print_notices.pl /tmp/test --csv --letter_code=OVERDUE --letter_code=CHECKIN you should get an error perl misc/cronjobs/gather_print_notices.pl /tmp/test --csv you should get an error perl misc/cronjobs/gather_print_notices.pl /tmp/test --csv --letter_code=OVERDUE -d=: will produce 1 csv file in your /tmp/test directory - verify the csv file is correct and contain only 1 csv header column. Signed-off-by: Frederic Demians <f.demians@tamil.fr> Created attachment 38387 [details] [review] Bug 11603: Gather print notices - add a ods parameter This patch adds: - the ability to generate an ods file From now you are able to generate a ods file for print notices. You would like to generate a csv file and not a html file. Test plan: - same as previous patch but test the following parameters: perl misc/cronjobs/gather_print_notices.pl /tmp/test --ods --letter_code=OVERDUE -d=: you should get an error because csv2ods is not installed. Follow the installation instructions and try again the command. A ods file should be generated in your /tmp/test directory. Signed-off-by: Frederic Demians <f.demians@tamil.fr> Created attachment 38388 [details] [review] Bug 11603: If a letter code is given, filenames should contain it If you choose to generate print notices for a specific letter code, the generated files should be distinct. The use case is: you want to process print notice for letter codes: overdue1, overdue2 and overdue3. The cronjobs will be: perl misc/cronjobs/gather_print_notices.pl /tmp --letter_code=overdue1 --csv --ods --html --delimiter=";" perl misc/cronjobs/gather_print_notices.pl /tmp --letter_code=overdue2 --csv --ods --html --delimiter=";" perl misc/cronjobs/gather_print_notices.pl /tmp --letter_code=overdue3 --csv --ods --html --delimiter=";" without this patch, the 2 first files will be erased. Signed-off-by: Frederic Demians <f.demians@tamil.fr> Created attachment 38389 [details] [review] Bug 11603: Fix encoding issues (bug 11944 is now pushed) Signed-off-by: Frederic Demians <f.demians@tamil.fr> No more encoding issue with html file, no problem with csv|ods Created attachment 38390 [details] [review] Bug 11603 Add --send|--nosend, fix stuf Fix --html without --letter_code Fix --ods which was producing a 2 lines ods file Created attachment 38391 [details] [review] Bug 11603: Some minor documentation changes Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Created attachment 38392 [details] [review] Bug 11603: (follow-up) pod The filenames will change (In reply to Katrin Fischer from comment #38) > I like this! (But need a bit more time to play with it :( ) > > Some first notes from reading the docs (yay!) and code: > > 1) I see there are 2 copyright statements and licenses, one at the top and > one at the bottom. Should this be cleaned up? It depends on how you have fixed the first conflict :) > 2) Is this statement about the file name still true? > The generated filename will be holdnotices-TODAY.[csv|html|ods] or > holdnotices-TODAY-BRANCHCODE.[csv|html|ods] if the --split parameter is > given. Fixed > 3) I have attached some suggestions for the documentation - please take a > look and obsolete if you don't agree :) Ok for me, thanks! 1) Regression test Change of the file name when no options specified but the directory: From: holdnotices-<date>.html ToL notices_hold-<date>.html As by default all notices (including overdues) are in the file, maybe we could rename it to be more general just: notices_<date>.html? (trivial) 2) -s splits by branch The output is identical. 3) --nosend works - helpful in testing! 4) --letter_code generates a nicely named file with only those notices! 5) --ods creates a warn: Use of uninitialized value in lc at /usr/share/perl5/OpenOffice/OODoc/File.pm line 378. 6) --csv This one is giving me some trouble: First I thought it was the concatenation: cardnumber:patron:email:item 1:Koha <borrowers.surname>>:patron@localhost: But this is also not giving all data: cardnumber:patron:patron:email:item 1:Koha:<borrowers.surname>>:patron@localhost: Barcode and surname are missing, but set in the database. We are pretty close here - it's a great enhancement. Created attachment 38403 [details] [review] Bug 11603: Change the output filenames The output filename is notices_all_<date>.[html|csv|ods] if no letter_code parameter is given. If 1 is given: notices_<letter_code>_<date>.[html|csv|ods] If 1+ are given: noties_<letter_code1>_..._<letter_codeN>_<date>.[html|csv|ods] (In reply to Katrin Fischer from comment #48) > 1) File names make sense now. > 6) --csv > This one is giving me some trouble: This is solved, I had an error in my thinking about the template - using <<items.barcode>> in ODUE doesn't work without <item></item>. The remaining problem is now with the ods export, working on it with Joubu: > 5) --ods creates a warn: > Use of uninitialized value in lc at > /usr/share/perl5/OpenOffice/OODoc/File.pm line 378. Created attachment 38411 [details] [review] [PASED QA] Bug 11603: Preparation step, cleaning up This patch refactores and adds some good practices: - use Modern::Perl - use Pod::Usage - add POD Signed-off-by: Frederic Demians <f.demians@tamil.fr> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Created attachment 38412 [details] [review] [PASSED QA] Bug 11603: Gather print notices - add a csv parameter This patch adds: - the ability to generate a csv file instead of a html file. - a letter_code parameter. From now you are able to generate a csv file for print notices. Imagine a template notice defined as: cardnumber:patron:email:item <<borrowers.cardnumber>>:<<borrowers.firstname>> <borrowers.surname>>:<<borrowers.email>>:<<items.barcode>> You would like to generate a csv file and not a html file. Test plan: - define your ODUE notice for the print template as: cardnumber:patron:email:item <<borrowers.cardnumber>>:<<borrowers.firstname>> <<borrowers.surname>>:<<borrowers.email>>:<item><<items.barcode>></item> - define overdues rules for a patron category - check 2 items out using a due date in order to generate the overdue notices - check these 2 items in - launch the overdue_notices script - the message_queue table should now contain 2 new entries - launch the gather_print_notices cronjob with the following parameters: perl misc/cronjobs/gather_print_notices.pl /tmp/test --csv --letter_code=OVERDUE --letter_code=CHECKIN you should get an error perl misc/cronjobs/gather_print_notices.pl /tmp/test --csv you should get an error perl misc/cronjobs/gather_print_notices.pl /tmp/test --csv --letter_code=OVERDUE -d=: will produce 1 csv file in your /tmp/test directory - verify the csv file is correct and contain only 1 csv header column. Signed-off-by: Frederic Demians <f.demians@tamil.fr> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> QA note: Keep in mind that you can use all placeholders for the csv that you can use for the normal templates. If you normally get the item information from <item></item> you need to use that. If you can use <<item.barcode>> directly, you can also do so in the csv. Created attachment 38413 [details] [review] [PASSED QA] Bug 11603: Gather print notices - add a ods parameter This patch adds: - the ability to generate an ods file From now you are able to generate a ods file for print notices. You would like to generate a csv file and not a html file. Test plan: - same as previous patch but test the following parameters: perl misc/cronjobs/gather_print_notices.pl /tmp/test --ods --letter_code=OVERDUE -d=: you should get an error because csv2ods is not installed. Follow the installation instructions and try again the command. A ods file should be generated in your /tmp/test directory. Signed-off-by: Frederic Demians <f.demians@tamil.fr> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Created attachment 38414 [details] [review] [PASSED QA] Bug 11603: If a letter code is given, filenames should contain it If you choose to generate print notices for a specific letter code, the generated files should be distinct. The use case is: you want to process print notice for letter codes: overdue1, overdue2 and overdue3. The cronjobs will be: perl misc/cronjobs/gather_print_notices.pl /tmp --letter_code=overdue1 --csv --ods --html --delimiter=";" perl misc/cronjobs/gather_print_notices.pl /tmp --letter_code=overdue2 --csv --ods --html --delimiter=";" perl misc/cronjobs/gather_print_notices.pl /tmp --letter_code=overdue3 --csv --ods --html --delimiter=";" without this patch, the 2 first files will be erased. Signed-off-by: Frederic Demians <f.demians@tamil.fr> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Created attachment 38415 [details] [review] [PASSED QA] Bug 11603: Fix encoding issues (bug 11944 is now pushed) Signed-off-by: Frederic Demians <f.demians@tamil.fr> No more encoding issue with html file, no problem with csv|ods Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Created attachment 38416 [details] [review] [PASSED QA] Bug 11603 Add --send|--nosend, fix stuf Fix --html without --letter_code Fix --ods which was producing a 2 lines ods file Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Created attachment 38417 [details] [review] Bug 11603: Some minor documentation changes Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Created attachment 38418 [details] [review] [PASSED QA] Bug 11603: (follow-up) pod The filenames will change Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Created attachment 38419 [details] [review] [PASSED QA] Bug 11603: Change the output filenames The output filename is notices_all_<date>.[html|csv|ods] if no letter_code parameter is given. If 1 is given: notices_<letter_code>_<date>.[html|csv|ods] If 1+ are given: noties_<letter_code1>_..._<letter_codeN>_<date>.[html|csv|ods] Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> The warn only appeared on my system, but the file was still created. For me this is not a blocker currently. This patch is a big improvement over the existing script, while I couldn't detect any regressions. It adds useful options. The file name changes slightly, but this is a fix to a bug - the file doesn't only include holds messages nowadays and hasn't for quite a while. The file name properly reflects that now. (bug 12810) There seem to be some issues in our way to enqueue messages and in determining the mtt = print - but they are outside the scope of this script, that just deals with the results of the processes done before. Can someone rebase this one ASAP? I'm sorry... Created attachment 39033 [details] [review] Bug 11603: Preparation step, cleaning up This patch refactores and adds some good practices: - use Modern::Perl - use Pod::Usage - add POD Signed-off-by: Frederic Demians <f.demians@tamil.fr> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Created attachment 39034 [details] [review] Bug 11603: Gather print notices - add a csv parameter This patch adds: - the ability to generate a csv file instead of a html file. - a letter_code parameter. From now you are able to generate a csv file for print notices. Imagine a template notice defined as: cardnumber:patron:email:item <<borrowers.cardnumber>>:<<borrowers.firstname>> <borrowers.surname>>:<<borrowers.email>>:<<items.barcode>> You would like to generate a csv file and not a html file. Test plan: - define your ODUE notice for the print template as: cardnumber:patron:email:item <<borrowers.cardnumber>>:<<borrowers.firstname>> <<borrowers.surname>>:<<borrowers.email>>:<item><<items.barcode>></item> - define overdues rules for a patron category - check 2 items out using a due date in order to generate the overdue notices - check these 2 items in - launch the overdue_notices script - the message_queue table should now contain 2 new entries - launch the gather_print_notices cronjob with the following parameters: perl misc/cronjobs/gather_print_notices.pl /tmp/test --csv --letter_code=OVERDUE --letter_code=CHECKIN you should get an error perl misc/cronjobs/gather_print_notices.pl /tmp/test --csv you should get an error perl misc/cronjobs/gather_print_notices.pl /tmp/test --csv --letter_code=OVERDUE -d=: will produce 1 csv file in your /tmp/test directory - verify the csv file is correct and contain only 1 csv header column. Signed-off-by: Frederic Demians <f.demians@tamil.fr> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> QA note: Keep in mind that you can use all placeholders for the csv that you can use for the normal templates. If you normally get the item information from <item></item> you need to use that. If you can use <<item.barcode>> directly, you can also do so in the csv. Created attachment 39035 [details] [review] Bug 11603: Gather print notices - add a ods parameter This patch adds: - the ability to generate an ods file From now you are able to generate a ods file for print notices. You would like to generate a csv file and not a html file. Test plan: - same as previous patch but test the following parameters: perl misc/cronjobs/gather_print_notices.pl /tmp/test --ods --letter_code=OVERDUE -d=: you should get an error because csv2ods is not installed. Follow the installation instructions and try again the command. A ods file should be generated in your /tmp/test directory. Signed-off-by: Frederic Demians <f.demians@tamil.fr> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Created attachment 39036 [details] [review] Bug 11603: If a letter code is given, filenames should contain it If you choose to generate print notices for a specific letter code, the generated files should be distinct. The use case is: you want to process print notice for letter codes: overdue1, overdue2 and overdue3. The cronjobs will be: perl misc/cronjobs/gather_print_notices.pl /tmp --letter_code=overdue1 --csv --ods --html --delimiter=";" perl misc/cronjobs/gather_print_notices.pl /tmp --letter_code=overdue2 --csv --ods --html --delimiter=";" perl misc/cronjobs/gather_print_notices.pl /tmp --letter_code=overdue3 --csv --ods --html --delimiter=";" without this patch, the 2 first files will be erased. Signed-off-by: Frederic Demians <f.demians@tamil.fr> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Created attachment 39037 [details] [review] Bug 11603: Fix encoding issues (bug 11944 is now pushed) Signed-off-by: Frederic Demians <f.demians@tamil.fr> No more encoding issue with html file, no problem with csv|ods Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Created attachment 39038 [details] [review] Bug 11603 Add --send|--nosend, fix stuf Fix --html without --letter_code Fix --ods which was producing a 2 lines ods file Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Created attachment 39039 [details] [review] Bug 11603: Some minor documentation changes Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Created attachment 39040 [details] [review] Bug 11603: (follow-up) pod The filenames will change Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Created attachment 39041 [details] [review] Bug 11603: Change the output filenames The output filename is notices_all_<date>.[html|csv|ods] if no letter_code parameter is given. If 1 is given: notices_<letter_code>_<date>.[html|csv|ods] If 1+ are given: noties_<letter_code1>_..._<letter_codeN>_<date>.[html|csv|ods] Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Patches pushed to master. Good job Jonathan! |