There is an include file, error-messages.inc, which is used by a few patron card creator scripts. It uses some custom JavaScript to create a closable dialog for showing error messages. This can more simply be implemented as a standard error message box and provide a more consistent experience to users. In addition, the include's name "error-messages.inc" doesn't properly convey that it is used exclusively by patron card creator pages.
Created attachment 16859 [details] [review] Bug 9924 - Simplify and rename patron card creator error message include The patron card creator error message include uses a non-standard method for displaying error messages, and is poorly-named. This patch converts the method of displaying error messages for various patron card creator options to the standard one ('<div class="dialog alert">') and renames the include file to make it clear that it relates only to patron card creator operations. To test, perform various operations: - Go to 'manage images' and try to upload a file which exceeds the 500KBfile size limit - Go to the edit batch page and manually append an error code to the URL: /cgi-bin/koha/patroncards/edit-batch.pl?op=new&error=403 - Go to one of the manage pages and manually append an error code to the URL: /cgi-bin/koha/patroncards/manage.pl?card_element=profile&error=201 Correct display of an error message indicates that the include file is being found.
Created attachment 17404 [details] [review] Bug 9924 - Simplify and rename patron card creator error message include The patron card creator error message include uses a non-standard method for displaying error messages, and is poorly-named. This patch converts the method of displaying error messages for various patron card creator options to the standard one ('<div class="dialog alert">') and renames the include file to make it clear that it relates only to patron card creator operations. To test, perform various operations: - Go to 'manage images' and try to upload a file which exceeds the 500KBfile size limit - Go to the edit batch page and manually append an error code to the URL: /cgi-bin/koha/patroncards/edit-batch.pl?op=new&error=403 - Go to one of the manage pages and manually append an error code to the URL: /cgi-bin/koha/patroncards/manage.pl?card_element=profile&error=201 Correct display of an error message indicates that the include file is being found. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Hi Owen, not sure if it's a false positive, but the QA script found something to complain about: * koha-tmpl/intranet-tmpl/prog/en/modules/patroncards/edit-batch.tt FAIL forbidden patterns OK tt_valid OK valid_template FAIL patroncards-batches-toolbar.inc: not found Could you please take a look?
Could you look at this again Katrin? I'm not getting any errors from the QA script when I run it.
Hm this is weird, I am still getting the error running the latest version of the QA script. Trying to see what is causing it.
Comment on attachment 17404 [details] [review] Bug 9924 - Simplify and rename patron card creator error message include Review of attachment 17404 [details] [review]: ----------------------------------------------------------------- ::: koha-tmpl/intranet-tmpl/prog/en/modules/patroncards/edit-batch.tt @@ +152,4 @@ > </div> > > > + [% INCLUDE 'patroncards-batches-toolbar.inc' %] Hi Owen, I think it's this line - and I can't spot a file with this name in the includes directory?
Created attachment 20542 [details] [review] [SIGNED-OFF] Bug 9924 - Simplify and rename patron card creator error message include The patron card creator error message include uses a non-standard method for displaying error messages, and is poorly-named. This patch converts the method of displaying error messages for various patron card creator options to the standard one ('<div class="dialog alert">') and renames the include file to make it clear that it relates only to patron card creator operations. To test, perform various operations: - Go to 'manage images' and try to upload a file which exceeds the 500KBfile size limit - Go to the edit batch page and manually append an error code to the URL: /cgi-bin/koha/patroncards/edit-batch.pl?op=new&error=403 - Go to one of the manage pages and manually append an error code to the URL: /cgi-bin/koha/patroncards/manage.pl?card_element=profile&error=201 Correct display of an error message indicates that the include file is being found. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(In reply to Katrin Fischer from comment #6) > > + [% INCLUDE 'patroncards-batches-toolbar.inc' %] I think this was caused by a merge error following Bug 9697. I have corrected the patch but kept the signoff. I hope that's okay.
Created attachment 20590 [details] [review] [PASSED QA] Bug 9924 - Simplify and rename patron card creator error message include The patron card creator error message include uses a non-standard method for displaying error messages, and is poorly-named. This patch converts the method of displaying error messages for various patron card creator options to the standard one ('<div class="dialog alert">') and renames the include file to make it clear that it relates only to patron card creator operations. To test, perform various operations: - Go to 'manage images' and try to upload a file which exceeds the 500KBfile size limit - Go to the edit batch page and manually append an error code to the URL: /cgi-bin/koha/patroncards/edit-batch.pl?op=new&error=403 - Go to one of the manage pages and manually append an error code to the URL: /cgi-bin/koha/patroncards/manage.pl?card_element=profile&error=201 Correct display of an error message indicates that the include file is being found. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Passes test plan, test suite and QA script.
Created attachment 20874 [details] [review] Bug 9924: (follow-up) ensure that correct error message is displayed (Apparently) unlike HTML::Template::Pro, Template Toolkit doesn't like template variables that are entirely numeric -- in conditionals, it consider them integers, most of which are Perl true. This patch changes this by setting the error variable to the error value. To test: [1] Run the test plan from the previous patch. In each case, verify that the error message is specifically applicable to the test. For example, if you try uplaoading a patron image that is larger than 500KB, the error message displayed should specifically say so. Signed-off-by: Galen Charlton <gmc@esilibrary.com>
In the course of testing this, I discovered that the patron card errors were broken, probably since the switch to Template Toolkit -- numeric template variable names are not a Good Thing. The follow-up patch I just attached tries to fix this. Setting to signed off for QA review of the follow-up.
Created attachment 20875 [details] [review] [PASSED QA] Bug 9924: (follow-up) ensure that correct error message is displayed (Apparently) unlike HTML::Template::Pro, Template Toolkit doesn't like template variables that are entirely numeric -- in conditionals, it consider them integers, most of which are Perl true. This patch changes this by setting the error variable to the error value. To test: [1] Run the test plan from the previous patch. In each case, verify that the error message is specifically applicable to the test. For example, if you try uplaoading a patron image that is larger than 500KB, the error message displayed should specifically say so. Signed-off-by: Galen Charlton <gmc@esilibrary.com> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Thank you Galen for catching this. Error messages showing up now are much more specific and according to the error codes given. I testd uploading a file larger then 500KB and triggered several error messages giving the error code in the URL: /cgi-bin/koha/patroncards/manage.pl?card_element=profile&error=201 All tests and QA script pass.
Pushed to master. Thanks, Owen!