There's a code pattern emerging (in bug 5770 and bug 22821 at least to my knowledge) where we grab a branches email address to send notifications to the branch. However, it's prone to make mistakes in the fallback order for assigning that address so we should factor it out into a method that can be called from Koha::Library. Should be as simple as >sub to_email { > my $self = shift; > return $self->branchreplyto || $self->branchemail || C4::Context->preference('ReplytoDefault') || C4::Context->preference('KohaAdminEmailAddress'); >} along with some appropriate POD and a walk of the codebase to replace existing occurrences of the pattern.
I am more than happy to mentor someone who might like to work on this, just reach out here or on IRC (I'm ashimema there)
Created attachment 99539 [details] [review] Bug 22823: Add get_effective_email method to Koha::Library Add a `get_effective_email` method to the Koha::Library object which returns the email address for the library which they are most likely to have access to. This method falls back through preferences `branchreplyto`, `branchemail`, `ReplytoDefault`, `KohaAdminEmailAddress`.
Not entirely sure about the method name yet.. open to opinions.
Created attachment 99554 [details] [review] Bug 22823: Add unit tests for get_effective_email This patch adds unit tests for the newly introduced get_effective_email method in the Koha::Library class.
Created attachment 99555 [details] [review] Bug 22823: Add get_effective_email method to Koha::Library Add a `get_effective_email` method to the Koha::Library object which returns the email address for the library which they are most likely to have access to. This method falls back through preferences `branchreplyto`, `branchemail`, `ReplytoDefault`, `KohaAdminEmailAddress`.
This bug simply introduces a new utility function to return the most effective email address for a branch to be emailed on. Test plan 1/ Read the new test and verify it makes sense 2/ Run the tests and verify they all pass.
Created attachment 99726 [details] [review] Bug 22823: Add unit tests for get_effective_email This patch adds unit tests for the newly introduced get_effective_email method in the Koha::Library class. Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Created attachment 99728 [details] [review] Bug 22823: Add get_effective_email method to Koha::Library Add a `get_effective_email` method to the Koha::Library object which returns the email address for the library which they are most likely to have access to. This method falls back through preferences `branchreplyto`, `branchemail`, `ReplytoDefault`, `KohaAdminEmailAddress`. This bug simply introduces a new utility function to return the most effective email address for a branch to be emailed on. Test plan 1/ Read the new test and verify it makes sense 2/ Run the tests and verify they all pass. Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Tests seems ok and test pass Ok.
Sorry, didn't notice the 'Academy' keyword. Looking a bit I understand now its meaning.
Thanks for testing Bernardo :)
Should not it be named get_effective_to_email (or something saying we should not use it for from)?
(In reply to Jonathan Druart from comment #11) > Should not it be named get_effective_to_email (or something saying we should > not use it for from)? Agreed.. I just struggled to come up with a method name and wanted to submit something ;). Lets ask in the dev meeting
how about '->inbound_email_address' ?
Created attachment 100934 [details] [review] Bug 22823: Add unit tests for get_effective_email This patch adds unit tests for the newly introduced get_effective_email method in the Koha::Library class. Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 100935 [details] [review] Bug 22823: Add get_effective_email method to Koha::Library Add a `get_effective_email` method to the Koha::Library object which returns the email address for the library which they are most likely to have access to. This method falls back through preferences `branchreplyto`, `branchemail`, `ReplytoDefault`, `KohaAdminEmailAddress`. This bug simply introduces a new utility function to return the most effective email address for a branch to be emailed on. Test plan 1/ Read the new test and verify it makes sense 2/ Run the tests and verify they all pass. Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Tests seems ok and test pass Ok. Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 100936 [details] [review] Bug 22823: Rename method with ->inbound_email_address Make the name obvious it's not an email address for "from" Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 100937 [details] [review] Bug 22823: return undef if KohaAdminEmailAddress not exist I have the feeling that we should return undef here, but can be ignored if someone disagrees. Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Nice work everyone! Pushed to master for 20.05
enhancement not backported to 19.11.x