As highlighted in bug 33192, we're not consistently using the Koha::Patron->notice_email_address method to get our 'to' address for notices. This bug serves to catch the last remaining cases where it's not being used and should be. Copying the list from the above bug, notice_email_address is used in: C4::Auth_With_shibboleth C4::Letters (as the default) C4::Auth - For 2FA C4::Auth_with_ldap C4::Reserves shareshelf.pl memberentry.pl opac-registration-varify.pl two_factor_auth.pl notice.pl memberentry.pl pendingreserves.pl Koha::Ticket Patron::Import.pm suggestion.pl overdue_notices.pl TalkingTech_iteive_outbound.pl notice_unprocessed_suggestions.pm first_valid_email is used in: clubs-tab.tt waiting_holds.inc transferstoreceive.tt opac-sendbasket.pl Koha::SharedContent
Created attachment 148168 [details] [review] Bug 33223: Replace 'first_valid' with 'notice' for email addresses This patch replaces the uses of first_valid_email_address with notice_email_address in waiting_holds, transferstoreceive, clubs and sendbasket so that we take EmailFieldPrimary into account for these notices too.
I'll be wanting to look more at these. Sounds very useful.
Bug 33223 - Koha::Patron->notice_email_address isn't consistently used 148168 - Bug 33223: Replace 'first_valid' with 'notice' for email addresses Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 33223: Replace 'first_valid' with 'notice' for email addresses Using index info to reconstruct a base tree... M koha-tmpl/intranet-tmpl/prog/en/modules/circ/transferstoreceive.tt M opac/opac-sendbasket.pl Falling back to patching base and 3-way merge... Auto-merging opac/opac-sendbasket.pl CONFLICT (content): Merge conflict in opac/opac-sendbasket.pl Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/circ/transferstoreceive.tt error: Failed to merge in the changes. Patch failed at 0001 Bug 33223: Replace 'first_valid' with 'notice' for email addresses hint: Use 'git am --show-current-patch' to see the failed patch When you have resolved this problem run "git bz apply --continue". If you would prefer to skip this patch, instead run "git bz apply --skip". To restore the original branch and stop patching run "git bz apply --abort". Patch left in /tmp/Bug-33223-Replace-firstvalid-with-notice-for-email-t8SwfM.patch
Created attachment 151023 [details] [review] Bug 33223: Replace 'first_valid' with 'notice' for email addresses This patch replaces the uses of first_valid_email_address with notice_email_address in waiting_holds, transferstoreceive, clubs and sendbasket so that we take EmailFieldPrimary into account for these notices too.
Created attachment 151024 [details] [review] Bug 33223: Replace 'first_valid' with 'notice' for email addresses This patch replaces the uses of first_valid_email_address with notice_email_address in waiting_holds, transferstoreceive, clubs and sendbasket so that we take EmailFieldPrimary into account for these notices too.
Created attachment 151025 [details] [review] Bug 33223: Fix sendshelf Two new occurences of first_valid crept in, this patch corrects those to also use notice_
Created attachment 151041 [details] [review] Bug 33223: Replace 'first_valid' with 'notice' for email addresses This patch replaces the uses of first_valid_email_address with notice_email_address in waiting_holds, transferstoreceive, clubs and sendbasket so that we take EmailFieldPrimary into account for these notices too. Signed-off-by: David Cook <dcook@prosentient.com.au>
Created attachment 151042 [details] [review] Bug 33223: Fix sendshelf Two new occurences of first_valid crept in, this patch corrects those to also use notice_ Signed-off-by: David Cook <dcook@prosentient.com.au>
Patches apply and change looks good upon review.
Looking here, although I feel that this patch is somewhere between bug and enh actually.
Looking at what remains for first_valid_email i would opt for removing the OFF option of EmailFieldPrimary and default it to the email field. Here it is only used as boolean: $params->{is_email_required} = 0 unless $self->first_valid_email_address(); Koha/SharedContent.pm: $mana_email = $borrower->first_valid_email_address This one could be replaced imo by notice_email. But yes, not here :)
Created attachment 151117 [details] [review] Bug 33223: Replace 'first_valid' with 'notice' for email addresses This patch replaces the uses of first_valid_email_address with notice_email_address in waiting_holds, transferstoreceive, clubs and sendbasket so that we take EmailFieldPrimary into account for these notices too. Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 151118 [details] [review] Bug 33223: Fix sendshelf Two new occurences of first_valid crept in, this patch corrects those to also use notice_ Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Pushed to master for 23.05. Nice work everyone, thanks!
Missing dependencies - not backporting to 22.11.x