Creating PDF for patron cards fails if it is getting invalid parameters for borrowernumber, template_id or layout_id
Created attachment 38854 [details] [review] Bug 14138 - Patroncard: POC for pre-testing parameters for create-pdf.pl If wrong parameters are given, create-pdf.pl displays a "Software error" or creates an invalid PDF (it is a text file containing "Software error...."). This patch is a proof of concept with the aim to prevent such a situation. It implements pre-testing the parameters provided for create-pdf.pl. If wrong parameters are given, a error message is displayed instead of creating the PDF. The user has then the possibility to go back to the page she came from. (Patron card tool, maybe later patron's detail page? ) To test: 1) Apply on top of Bug 14122 (otherwise it is not possible to test for borrower) 2) Test with links similar to the following ...cgi-bin/koha/patroncards/create-pdf.pl?borrower_number=61&template_id=24&layout_id=21&start_card=1 or: ...cgi-bin/koha/patroncards/create-pdf.pl?batch_id=8&template_id=24&layout_id=21&start_card=1 3) Change values for existing / not existing Ids. If a parameter is not valid, a new HTML page is displayed with a JavaScript alert. (This will need optimization. Ideas and/or patterns for doing this part are highly welcome). Comments are welcome - is this the way to go? Marc
Created attachment 53228 [details] [review] Bug 14138: Patroncard: Warn user if PDF creation fails First step: Change patroncards/create-pdf.pl to redirect with an an error code instead of writing an ivalid pdf. Updated patch to display error and test plan follows.
Created attachment 53233 [details] [review] Bug 14138: Patroncard: Warn user if PDF creation fails First step: Change patroncards/create-pdf.pl to redirect with an an error code instead of writing an ivalid pdf. Updated patch to display error and test plan follows.
Created attachment 53242 [details] [review] Bug 14138: Patroncard: Warn user if PDF creation fails First step: Change patroncards/create-pdf.pl to redirect with an an error code instead of writing an ivalid pdf. Updated patch to display error and test plan follows.
Created attachment 53245 [details] [review] Bug 14138: Patroncard: Warn user if PDF creation fails First step: Change patroncards/create-pdf.pl to redirect with an an error code instead of writing an ivalid pdf. Updated patch to display error and test plan follows.
Created attachment 53248 [details] [review] Bug 14138: Patroncard: Warn user if PDF creation fails Change patroncards/create-pdf.pl to redirect with an error message instead of writing an invalid pdf that does not open in pdf viewer. To test: - Apply patch - Test that pdf creator behaves as before (with valid batches and patron lists) - While testing, copy pdf link address from window with title 'Click the following link(s) to download...' - Open another staff client browser tab - Paste link to browser address field, change batch id rsp. patron list id to an invalid value and submit - The window should redirect to cgi-bin/koha/patroncards/create-pdf.pl and display an error message - Bonus test 1: Create an empty patron list and test patron card creation. You should get an error message as appropriate. - Bonus test 2: Use a link with params like the following: ...create-pdf.pl?borrower_number=61&template_id=2&layout_id=1&start_card=1 Verify that you can create a pdf with a valid borrower_number and that you get the error message with an invalid borrower number
Created attachment 53307 [details] [review] [SIGNED-OFF] Bug 14138: Patroncard: Warn user if PDF creation fails Change patroncards/create-pdf.pl to redirect with an error message instead of writing an invalid pdf that does not open in pdf viewer. To test: - Apply patch - Test that pdf creator behaves as before (with valid batches and patron lists) - While testing, copy pdf link address from window with title 'Click the following link(s) to download...' - Open another staff client browser tab - Paste link to browser address field, change batch id rsp. patron list id to an invalid value and submit - The window should redirect to cgi-bin/koha/patroncards/create-pdf.pl and display an error message - Bonus test 1: Create an empty patron list and test patron card creation. You should get an error message as appropriate. - Bonus test 2: Use a link with params like the following: ...create-pdf.pl?borrower_number=61&template_id=2&layout_id=1&start_card=1 Verify that you can create a pdf with a valid borrower_number and that you get the error message with an invalid borrower number Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 53352 [details] [review] [PASSED QA] Bug 14138: Patroncard: Warn user if PDF creation fails Change patroncards/create-pdf.pl to redirect with an error message instead of writing an invalid pdf that does not open in pdf viewer. To test: - Apply patch - Test that pdf creator behaves as before (with valid batches and patron lists) - While testing, copy pdf link address from window with title 'Click the following link(s) to download...' - Open another staff client browser tab - Paste link to browser address field, change batch id rsp. patron list id to an invalid value and submit - The window should redirect to cgi-bin/koha/patroncards/create-pdf.pl and display an error message - Bonus test 1: Create an empty patron list and test patron card creation. You should get an error message as appropriate. - Bonus test 2: Use a link with params like the following: ...create-pdf.pl?borrower_number=61&template_id=2&layout_id=1&start_card=1 Verify that you can create a pdf with a valid borrower_number and that you get the error message with an invalid borrower number Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Pushed to master for 16.11, thanks Marc!
Pushed in 16.05. Will be in 16.05.02.
Patch pushed to 3.22.x, will be in 3.22.10
Oh, a follow up patch got forgotten ;) http://git.koha-community.org/gitweb/?p=koha.git;a=blob;f=patroncards/create-pdf.pl;hb=7e1c6dddde372f622cc88f993f72b8f81d0b6252#l66 «#Note fo bug 14138: Indenting follows in separate patch to ease review» I'm not sure it's worth doing anything about this. This is a notification just in case.