To Test: - Set GDPR_Policy to either Enforced or Permissive - Set PatronSelfRegistrationBorrowerMandatoryField to empty - Set OPACPatronDetails to Allow - Log into the OPAC - Go to opac-memberentry.pl, change something, submit modification request - Request is refused with message "You have not filled out all required fields. Please fill in all missing fields and resubmit."
Created attachment 90817 [details] [review] Bug 23150: Do not consider gdpr_proc_consent a mandatory field It introduced a regression in the patron's self modification feature. Patron's details are no longer modificable, the screen displays "You have not filled out all required fields. Please fill in all missing fields and resubmit." Test plan: - Set GDPR_Policy to either Enforced or Permissive - Set PatronSelfRegistrationBorrowerMandatoryField to empty - Set OPACPatronDetails to Allow - Log into the OPAC - Accept the GDPR screen - Go to opac-memberentry.pl, change something, submit modification request
Not sure about this fix, I'd like Marcel review :)
(In reply to Andrew from comment #0) > - Request is refused with message "You have not filled out all required > fields. Please fill in all missing fields and resubmit." I am not sure if I understand. You only need to tick the GDPR consent field, and you can save the record? Note that if you do not agree with the GDPR policy, we do not want you to proceed and create an account. So the title GDPR breaks self registration, is completely unclear to me.
(In reply to Jonathan Druart from comment #2) > Not sure about this fix, I'd like Marcel review :) At least for mode Enforced, we should treat it as mandatory. But I think for Permissive it should better be handled as mandatory too. But that is more arbitrary. This change actually breaks GDPR ;)
This is actually an issue on patron self *modification* rather than registration. If GDPR_policy is set to enforced or permissive, patron self modification will always give a "missing required field" error, even if the patron has already accepted the GDPR terms. There is nothing GDPR-related on the self-modification page, so there is nothing in the patron can do to move past this error and nothing indicating what the error is referring to.
(In reply to Andrew from comment #5) > This is actually an issue on patron self *modification* rather than > registration. > > If GDPR_policy is set to enforced or permissive, patron self modification > will always give a "missing required field" error, even if the patron has > already accepted the GDPR terms. There is nothing GDPR-related on the > self-modification page, so there is nothing in the patron can do to move > past this error and nothing indicating what the error is referring to. Ahh. Bad reader ;) Probably this fix then still needs a little tweak. Looking now.
I sort of anticipated confusion because I pointed at opac-memberentry.pl, which is confusingly both self reg and self mod :P
Created attachment 90846 [details] [review] Bug 23150: Do not consider gdpr_proc_consent a mandatory field It introduced a regression in the patron's self modification feature. Patron's details are no longer modificable, the screen displays "You have not filled out all required fields. Please fill in all missing fields and resubmit." Test plan: - Set GDPR_Policy to either Enforced or Permissive - Set PatronSelfRegistrationBorrowerMandatoryField to empty - Set OPACPatronDetails to Allow - Log into the OPAC - Accept the GDPR screen - Go to opac-memberentry.pl, change something, submit modification request Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 90847 [details] [review] Bug 23150: (follow-up) Make gdpr_proc_consent mandatory for create When updating (patron modification), the field should not be required. Test plan: Check patron modification on opac. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
(In reply to Andrew from comment #7) > I sort of anticipated confusion because I pointed at opac-memberentry.pl, > which is confusingly both self reg and self mod :P Fixed
GDPR is a bit painful, but still changing the title slightly :) The law is not to blame here.
Created attachment 90889 [details] [review] Bug 23150: Do not consider gdpr_proc_consent a mandatory field It introduced a regression in the patron's self modification feature. Patron's details are no longer modificable, the screen displays "You have not filled out all required fields. Please fill in all missing fields and resubmit." Test plan: - Set GDPR_Policy to either Enforced or Permissive - Set PatronSelfRegistrationBorrowerMandatoryField to empty - Set OPACPatronDetails to Allow - Log into the OPAC - Accept the GDPR screen - Go to opac-memberentry.pl, change something, submit modification request Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 90890 [details] [review] Bug 23150: (follow-up) Make gdpr_proc_consent mandatory for create When updating (patron modification), the field should not be required. Test plan: Check patron modification on opac. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Nice work! Pushed to master for 19.11.00
As the GDPR feature is in Koha down to at least 18.11... can we have this backported?
Pushed to 19.05.x for 19.05.02
I am not sure, but this appears seems to be ok in 18.11: my @fields = split( /\|/, $BorrowerMandatoryField ); push @fields, 'gdpr_proc_consent' if C4::Context->preference('GDPR_Policy') && $action eq 'create'; foreach (@fields) {
not sure = please double check :)