[Bug 13526] New: Email can be mandatory and hidden in borrower self-registration
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13526 Bug ID: 13526 Summary: Email can be mandatory and hidden in borrower self-registration Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: minor Priority: P5 - low Component: Patrons Assignee: koha-bugs@lists.koha-community.org Reporter: dcook@prosentient.com.au QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com, kyle.m.hall@gmail.com When using the self-registration, "email" is a mandatory field no matter what, if the "PatronSelfRegistrationVerifyByEmail" system preference is true. However, you can still hide the "email" field, which means the form will refuse to submit (saying that you haven't filled in all mandatory fields) even though you've filled out every mandatory field on the screen. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13526 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Email can be mandatory and |Hidden fields can be |hidden in borrower |mandatory in borrower |self-registration |self-registration --- Comment #1 from David Cook <dcook@prosentient.com.au> --- Actually, now that I think about it, any mandatory field can be hidden... and that would cause trouble. So really no mandatory field should be able to be hidden. That's easy enough to do... -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13526 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Hidden fields can be |Mandatory fields should not |mandatory in borrower |be able to be hidden in |self-registration |borrower self-registration -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13526 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13526 --- Comment #2 from David Cook <dcook@prosentient.com.au> --- Created attachment 34991 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=34991&action=edit Bug 13526 - Mandatory fields should not be able to be hidden in borrower self-registration Currently, mandatory fields in the borrower self-registration can be hidden. This causes problems since the validator rejects the registration, even though all required fields on the screen have been filled out. This is especially a problem when using the system preference "PatronSelfRegistrationVerifyByEmail", since it automatically makes "email" a mandatory field. This patch makes it so that a mandatory field cannot be hidden on the self-registration page. _TEST PLAN_ Before applying 1) Hide the "email" and the "emailpro" fields using the "PatronSelfRegistrationBorrowerUnwantedField" system preference. 2) Make the "email" and the "emailpro" fields mandatory using the "PatronSelfRegistrationBorrowerMandatoryField" system preference. 3) Note that you cannot see "Primary email" or "Secondary email" on the self registration page. (The registration page which can be found by turning on "PatronSelfRegistration" and filling in "PatronSelfRegistrationDefaultCategory". The link will be on opac-main.pl on the right side of the screen under the login box.) 4) Note that you cannot submit a self-registration request as the system tells you that you have not filled in all the mandatory fields. 5) Apply the patch && refresh the self registration page 6) Note that you can now see "Primary email" and "Secondary email" on the self-registration screen". -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13526 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |dcook@prosentient.com.au |ity.org | -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13526 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |Academy --- Comment #3 from David Cook <dcook@prosentient.com.au> --- This might be a useful one for Academy members to take a look at :) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13526 --- Comment #4 from M. Tompsett <mtompset@hotmail.com> --- Comment on attachment 34991 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=34991 Bug 13526 - Mandatory fields should not be able to be hidden in borrower self-registration Review of attachment 34991: --> (http://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=13526&attachment=34991) ----------------------------------------------------------------- ::: opac/opac-memberentry.pl @@ +235,5 @@
my $BorrowerUnwantedField = C4::Context->preference("PatronSelfRegistrationBorrowerUnwantedField");
+ #Fetch the mandatory fields to compare against hidden fields + my @mandatory_fields = keys %{GetMandatoryFields($action)};
The GetMandatoryFields function is already called on line 62. This calls it again. Could this be optimized? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13526 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |veron@veron.ch -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13526 --- Comment #5 from David Cook <dcook@prosentient.com.au> --- (In reply to M. Tompsett from comment #4)
Comment on attachment 34991 [details] [review] Bug 13526 - Mandatory fields should not be able to be hidden in borrower self-registration
Review of attachment 34991 [details] [review]: -----------------------------------------------------------------
::: opac/opac-memberentry.pl @@ +235,5 @@
my $BorrowerUnwantedField = C4::Context->preference("PatronSelfRegistrationBorrowerUnwantedField");
+ #Fetch the mandatory fields to compare against hidden fields + my @mandatory_fields = keys %{GetMandatoryFields($action)};
The GetMandatoryFields function is already called on line 62. This calls it again. Could this be optimized?
Probably. I was thinking that it's pointless doing the grep as well as a person could just use the hash... I'll take another look at this pronto. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13526 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #34991|0 |1 is obsolete| | --- Comment #6 from David Cook <dcook@prosentient.com.au> --- Created attachment 35029 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=35029&action=edit Bug 13526 - Mandatory fields should not be able to be hidden in borrower self-registration Currently, mandatory fields in the borrower self-registration can be hidden. This causes problems since the validator rejects the registration, even though all required fields on the screen have been filled out. This is especially a problem when using the system preference "PatronSelfRegistrationVerifyByEmail", since it automatically makes "email" a mandatory field. This patch makes it so that a mandatory field cannot be hidden on the self-registration page. _TEST PLAN_ Before applying 1) Hide the "email" and the "emailpro" fields using the "PatronSelfRegistrationBorrowerUnwantedField" system preference. 2) Make the "email" and the "emailpro" fields mandatory using the "PatronSelfRegistrationBorrowerMandatoryField" system preference. 3) Note that you cannot see "Primary email" or "Secondary email" on the self registration page. (The registration page which can be found by turning on "PatronSelfRegistration" and filling in "PatronSelfRegistrationDefaultCategory". The link will be on opac-main.pl on the right side of the screen under the login box.) 4) Note that you cannot submit a self-registration request as the system tells you that you have not filled in all the mandatory fields. 5) Apply the patch && refresh the self registration page 6) Note that you can now see "Primary email" and "Secondary email" on the self-registration screen". -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13526 --- Comment #7 from David Cook <dcook@prosentient.com.au> --- (In reply to M. Tompsett from comment #4)
The GetMandatoryFields function is already called on line 62. This calls it again. Could this be optimized?
There we go. That should be much better! A fair number of my changes turned out to be needless, so I simplified it and it works a treat. Cheers! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13526 Abby <abbyrobertson.student@wegc.school.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #35029|0 |1 is obsolete| | --- Comment #8 from Abby <abbyrobertson.student@wegc.school.nz> --- Created attachment 35126 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=35126&action=edit Bug 13526 - Mandatory fields should not be able to be hidden in borrower self-registration Currently, mandatory fields in the borrower self-registration can be hidden. This causes problems since the validator rejects the registration, even though all required fields on the screen have been filled out. This is especially a problem when using the system preference "PatronSelfRegistrationVerifyByEmail", since it automatically makes "email" a mandatory field. This patch makes it so that a mandatory field cannot be hidden on the self-registration page. _TEST PLAN_ Before applying 1) Hide the "email" and the "emailpro" fields using the "PatronSelfRegistrationBorrowerUnwantedField" system preference. 2) Make the "email" and the "emailpro" fields mandatory using the "PatronSelfRegistrationBorrowerMandatoryField" system preference. 3) Note that you cannot see "Primary email" or "Secondary email" on the self registration page. (The registration page which can be found by turning on "PatronSelfRegistration" and filling in "PatronSelfRegistrationDefaultCategory". The link will be on opac-main.pl on the right side of the screen under the login box.) 4) Note that you cannot submit a self-registration request as the system tells you that you have not filled in all the mandatory fields. 5) Apply the patch && refresh the self registration page 6) Note that you can now see "Primary email" and "Secondary email" on the self-registration screen". Signed-off-by: Abby <abbyrobnz@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13526 Abby <abbyrobertson.student@wegc.school.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |abbyrobertson.student@wegc. | |school.nz Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13526 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@biblibre.co | |m --- Comment #9 from Jonathan Druart <jonathan.druart@biblibre.com> --- I am wondering if we need to take into account this kind of configuration problems. Maybe a check in the about page is more appropriate? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13526 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13526 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #35126|0 |1 is obsolete| | --- Comment #10 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 35331 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=35331&action=edit [PASSED QA] Bug 13526 - Mandatory fields should not be able to be hidden in borrower self-registration Currently, mandatory fields in the borrower self-registration can be hidden. This causes problems since the validator rejects the registration, even though all required fields on the screen have been filled out. This is especially a problem when using the system preference "PatronSelfRegistrationVerifyByEmail", since it automatically makes "email" a mandatory field. This patch makes it so that a mandatory field cannot be hidden on the self-registration page. _TEST PLAN_ Before applying 1) Hide the "email" and the "emailpro" fields using the "PatronSelfRegistrationBorrowerUnwantedField" system preference. 2) Make the "email" and the "emailpro" fields mandatory using the "PatronSelfRegistrationBorrowerMandatoryField" system preference. 3) Note that you cannot see "Primary email" or "Secondary email" on the self registration page. (The registration page which can be found by turning on "PatronSelfRegistration" and filling in "PatronSelfRegistrationDefaultCategory". The link will be on opac-main.pl on the right side of the screen under the login box.) 4) Note that you cannot submit a self-registration request as the system tells you that you have not filled in all the mandatory fields. 5) Apply the patch && refresh the self registration page 6) Note that you can now see "Primary email" and "Secondary email" on the self-registration screen". Signed-off-by: Abby <abbyrobnz@gmail.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13526 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com --- Comment #11 from Kyle M Hall <kyle@bywatersolutions.com> --- I think both would be good! I know I'd welcome and additional report and patch to add a warning for these conflicts to the about page. (In reply to Jonathan Druart from comment #9)
I am wondering if we need to take into account this kind of configuration problems. Maybe a check in the about page is more appropriate?
-- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13526 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master CC| |tomascohen@gmail.com --- Comment #12 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Patch pushed to master. Thanks David! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13526 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |chris@bigballofwax.co.nz Status|Pushed to Master |Pushed to Stable --- Comment #13 from Chris Cormack <chris@bigballofwax.co.nz> --- Pushed to 3.18.x will be in 3.18.3 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13526 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|Academy | -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org