On a failed login, the default message is harcorded into opac-auth.tt. It would be preferable to allow for a preference to override that message (for example: ...Please bring an ID to the front desk located on 3rd floor...) The changes modify -opac-auth.tt to allow for custom value -admin/preferences/opac.pref to add it to the preferences with a description -C4/Auth.pm for the loading of the preference TESTING 1) in OPAC, logged out, try login in by entering no or wrong credentials. Acknowledge the "Don't have a passord yet?" message 2) Apply the patch 3) Regression Test: Redo step 1. Same (default) message should appear. 4) Log in to intranet, - select NoLoginInstructions in system preferences. - Enter new (xml) message. Possible: <h5>Welcome to Koha, please bring your passport to the front office</h5> - and save 5) refresh the OPAC, try login again with invalid credentials. The new message should appear.
Created attachment 21460 [details] [review] Enhancement adding system preference NoLoginInstructions to be used when wrong credential are used in OPAC login
Thanks for the patch. I should point out that nowadays it is very common for this sort of minor tweak to be handled using jQuery entered in (in this case) the OPACUserJS system preference. Here is a specific example on the wiki for changing the login help, which accomplishes the intent of this patch without requiring that a new system preference be added. http://wiki.koha-community.org/wiki/JQuery_Library#Change_Login_Help
See http://wiki.koha-community.org/wiki/System_Preferences for information on adding system preferences. You forgot to modify installer/data/mysql/updatedatabase.pl and installer/data/mysql/kohastructure.sql, but you did get the appropriate .pref file. :)
I think having a syspref for this is better, as not everyone runs javascript. My only issue with the idea is that it makes it harder to be language aware.
A more long-term and internationalization-friendly (and complicated) solution would be to build a generalized framework for entering multiple text blocks in a Koha News-style interface tied to available languages.
We've had a longer discussion about this on IRC last night. I agree with both Owen and Robin. We actually should find a better way than sysprefs - one that allows you to offer the ocntent in multiple languages. But as long as we don't have that this system preference could be really helpful. I know a lot of people who turn off Javascript on websites - a system preference allows to always display a text and you can toggle it with jQuery between languages then (I am using different div blocks with a language class). It's not perfect, but a little better.
Created attachment 21510 [details] [review] Bug 10951 - Adding NoLoginInstructions to system preferences On a failed login, the default message is harcorded into opac-auth.tt. It would be preferable to allow for a preference to override that message (for example: ...Please bring an ID to the front desk located on 3rd floor...) The changes modify -opac-auth.tt to allow for custom value -admin/preferences/opac.pref to add it to the preferences with a description -C4/Auth.pm for the loading of the preference TESTING 1) in OPAC, logged out, try login in by entering no or wrong credentials. Acknowledge the "Don't have a passord yet?" message 2) Apply the patch 3) Regression Test: Redo step 1. Same (default) message should appear. 4) Log in to intranet, - select NoLoginInstructions in system preferences. - Enter new (xml) message. Possible: <h5>Welcome to Koha, please bring your passport to the front office</h5> - and save 5) refresh the OPAC, try login again with invalid credentials. The new message should appear.
Thank you for noting the updatedatabase.pl missing part, but you still forgot installer/data/mysql/kohastructure.sql which is important for fresh installs.
I think the file Mark means is syspref.sql - if you had changed the database structure it would go in kohastructure.sql, but for a system preference we only need an entry in sysprefs.sql :)
Created attachment 21515 [details] [review] Second patch to the bug, contains sysprefs.sql
Created attachment 21516 [details] [review] Bug 10951 - Second patch to the bug, contains sysprefs.sql
Live and learn. I now have to do that for my other patches. If there's a standard/preference when naming a second patch, please let me know. git-bz throws me error so I'm doing it all manually.
Created attachment 21522 [details] [review] Adding NoLoginInstructions to system preferences Fixed issue in previous patch. Merged all the patches together for simpler install. 1 > 3.
Created attachment 21524 [details] [review] Adding NoLoginInstructions to system preferences
This looks good, Blou, but I haven't tested it. I just eyeballed it. I should be able to sign this off pretty quickly later. :)
Patch applied cleanly, go forth and signoff
Comment on attachment 21524 [details] [review] Adding NoLoginInstructions to system preferences Review of attachment 21524 [details] [review]: ----------------------------------------------------------------- ::: installer/data/mysql/updatedatabase.pl @@ +7157,5 @@ > > +$DBversion ="3.13.00.XXX"; > +if ( CheckVersion($DBversion) ) { > + $dbh->do("INSERT IGNORE INTO systempreferences (variable,value,options,explanation,type) > + VALUES ('NoLoginInstructions', '<h5>Don't have a password yet?</h5> <p> If you don't have a password yet, stop by the circulation desk the next time you're in the library. We'll happily set one up for you.</p> <h5>Don't have a library card?</h5> <p> If you don't have a library card, stop by your local library to sign up. </p>', '60|10', 'Instructions when a person is not connected', 'Textarea')"); You forgot to escape the single quotes in the message and you then typed contractions like "Don't" and "We'll" in the message. Escape the single quotes, and this works fine.
Created attachment 21807 [details] [review] Neglected to escape the single quotes in the sql command. This corrects it.
Thank you! I can't believe i had done the escaping for its sister file (sysprefs.sql) and not this one. This new patch patches the big previous patch. My apologies. I'll be more careful next time. > You forgot to escape the single quotes in the message and you then typed > contractions like "Don't" and "We'll" in the message. Escape the single > quotes, and this works fine.
Oops. I forgot to mention escaping the question mark too. :) Could you patch that up too?
Comment on attachment 21524 [details] [review] Adding NoLoginInstructions to system preferences Review of attachment 21524 [details] [review]: ----------------------------------------------------------------- ::: installer/data/mysql/sysprefs.sql @@ +418,5 @@ > ('XSLTResultsDisplay','default','','Enable XSL stylesheet control over results page display on intranet','Free'), > ('yuipath','local','local|http://yui.yahooapis.com/2.5.1/build','Insert the path to YUI libraries, choose local if you use koha offline','Choice'), > ('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'), > +('NoLoginInstructions', '<h5>Don\'t have a password yet?</h5> <p> If you don\'t have a password yet, stop by the circulation desk the next time you\'re in the library. We\'ll happily set one up for you.</p> <h5>Don\'t have a library card?</h5> <p> If you don\'t have a library card, stop by your local library to sign up. </p>', '60|10', 'Instructions when a person is not connected', 'Textarea') This should probably be in the alphabetical order location. This question mark didn't need escaping, because it isn't wrapped in perl code. :)
Created attachment 21810 [details] [review] Bug 10951 - Adding NoLoginInstructions to system preferences On a failed login, the default message is harcorded into opac-auth.tt. It would be preferable to allow for a preference to override that message (for example: ...Please bring an ID to t The changes modify -opac-auth.tt to allow for custom value -admin/preferences/opac.pref to add it to the preferences with a description -C4/Auth.pm for the loading of the preference -sysprefs.sql -updatedatabase.pl TESTING 1) in OPAC, logged out, try login in by entering no or wrong credentials. Acknowledge the "Don't have a p 2) Apply the patch 3) Regression Test: Redo step 1. Same (default) message should appear. 4) Log in to intranet, - select NoLoginInstructions in system preferences. - Enter new (xml) message. Possible: <h5>Welcome to Koha, please bring your passport to the front office</h5> - and save 5) refresh the OPAC, try login again with invalid credentials. The new message should appear.
Comment on attachment 21524 [details] [review] Adding NoLoginInstructions to system preferences Last patch posted replaces this.
Created attachment 21811 [details] [review] Corrected entry which was on two lines.
Created attachment 21812 [details] [review] Corrected entry which was on two lines.
Created attachment 21813 [details] [review] changed escape \' to '' (and \? to ?)
Created attachment 21814 [details] [review] Adding NoLoginInstructions to system preferences Ok, this is a clean one, containing all the changes, from the start. All previous patches are obsolete.
Created attachment 21815 [details] [review] Bug 10951 - Adding NoLoginInstructions to system preferences On a failed login, the default message is harcorded into opac-auth.tt. It would be preferable to allow for a preference to override that message (for example: ...Please bring an ID to t The changes modify -opac-auth.tt to allow for custom value -admin/preferences/opac.pref to add it to the preferences with a description -C4/Auth.pm for the loading of the preference -sysprefs.sql -updatedatabase.pl TESTING 1) in OPAC, logged out, try login in by entering no or wrong credentials. Acknowledge the "Don't have a p 2) Apply the patch 3) Regression Test: Redo step 1. Same (default) message should appear. 4) Log in to intranet, - select NoLoginInstructions in system preferences. - Enter new (xml) message. Possible: <h5>Welcome to Koha, please bring your passport to the front office</h5> - and save 5) refresh the OPAC, try login again with invalid credentials. The new message should appear.
Comment on attachment 21814 [details] [review] Adding NoLoginInstructions to system preferences This is obsolete.
Created attachment 21816 [details] [review] [SIGNED OFF] Bug 10951 - Adding NoLoginInstructions to system preferences On a failed login, the default message is harcorded into opac-auth.tt. It would be preferable to allow for a preference to override that message (for example: ...Please bring an ID to t The changes modify -opac-auth.tt to allow for custom value -admin/preferences/opac.pref to add it to the preferences with a description -C4/Auth.pm for the loading of the preference -sysprefs.sql -updatedatabase.pl TESTING 1) in OPAC, logged out, try login in by entering no or wrong credentials. Acknowledge the "Don't have a p 2) Apply the patch 3) Regression Test: Redo step 1. Same (default) message should appear. 4) Log in to intranet, - select NoLoginInstructions in system preferences. - Enter new (xml) message. Possible: <h5>Welcome to Koha, please bring your passport to the front office</h5> - and save 5) refresh the OPAC, try login again with invalid credentials. The new message should appear. Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment on attachment 21815 [details] [review] Bug 10951 - Adding NoLoginInstructions to system preferences Obsolete (unsigned) version.
test plan: - drop kohadatabase, create blank one - go to staff client and do web install - confirm the nologin-ish system preference was created. - restore only koha database - go to staff client and do web upgrade - confirm the nologin-ish system preference was created. play with values of the system preference - does the message appear on failed log in? YES. - blank the system preference, refresh OPAC, gives old default message? YES. - set system preference to 'blah', refresh OPAC, gives 'blah'? YES. Looks great! Signed off. :)
Created attachment 21853 [details] [review] Bug 10951 - Extra Patch to fix next sentence. We noticed that an extra . (dot) was appearing in the message. And if the "self-registration" was ON, it started on a new line without an uppercase. Very minor, but style is important. I'm sorry to break a sign-off we worked so hard to get :( Code change is one line in one file.
Created attachment 21854 [details] [review] The change was causing issue with the next sentence. Adjusted the start (uppercase) and ending (dot) on self-registration invitation. Modified opac-auth.tt http://bugs.koha-community.org/show_bug.cgi?id=10951 Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Created attachment 21855 [details] [review] [ SIGNED OFF] Bug 10951 - Tweaked opac-auth.tt sentence. The change was causing issue with the next sentence. Adjusted the start (uppercase) and ending (dot) on self-registration invitation. Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
The patch was such a minor tweak. Unless someone wants to complain about a few pixels shifting up between the default message and the one that is preloaded, I see no reason for QA to fail this. Though, optimization-wise, perhaps a blank setting and not filling in the system preference is better? It would certainly have saved us the escaping headache. :)
Well, not sure I should say that, but the patch would have worked perfectly without messing the preference, indeed. There's a default value in the .tt that would have done the job. I can't tell you how we got to this. But if I can ship you some Tylenol (for the headache I caused), I'll be happy to do. I made sure to reuse the lessons on my other patches. They should be a lot easier on the signee, I promess. Thanks again and again!
QA comment: The syspref makes sense for me. It works as described. The pref should not be filled (syspref.sql and updatedb): it will be displayed in English for everybody. Since a default message is defined in the template, the pref value should be empty. I will provide a followup for that and for 4 tab characters. Marked as Passed QA.
Created attachment 22711 [details] [review] Bug 10951 - Adding NoLoginInstructions to system preferences On a failed login, the default message is harcorded into opac-auth.tt. It would be preferable to allow for a preference to override that message (for example: ...Please bring an ID to t The changes modify -opac-auth.tt to allow for custom value -admin/preferences/opac.pref to add it to the preferences with a description -C4/Auth.pm for the loading of the preference -sysprefs.sql -updatedatabase.pl TESTING 1) in OPAC, logged out, try login in by entering no or wrong credentials. Acknowledge the "Don't have a p 2) Apply the patch 3) Regression Test: Redo step 1. Same (default) message should appear. 4) Log in to intranet, - select NoLoginInstructions in system preferences. - Enter new (xml) message. Possible: <h5>Welcome to Koha, please bring your passport to the front office</h5> - and save 5) refresh the OPAC, try login again with invalid credentials. The new message should appear. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 22712 [details] [review] Bug 10951 - Tweaked opac-auth.tt sentence. The change was causing issue with the next sentence. Adjusted the start (uppercase) and ending (dot) on self-registration invitation. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 22713 [details] [review] Bug 10951: The new pref NoLoginInstructions should be empty by default. We don't want to display an English message by default for everybody. The default message is in the template, so the pref should be empty. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Note for RM: maybe "patron" is better than "person" in the pref description.
Pushed to master, along with various follow-ups, including one adding support for the Bootstrap theme. Thanks, Phillipe!
Pushed to 3.14.x, will be in 3.14.09 (very nice, thanks all)
*** Bug 6864 has been marked as a duplicate of this bug. ***