Bug 22823 - Koha::Library needs a method for obtaining the inbound email address
Summary: Koha::Library needs a method for obtaining the inbound email address
Status: Pushed to master
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Martin Renvoize
QA Contact: Jonathan Druart
URL:
Keywords:
Depends on:
Blocks: 4461 22821 22843 24721 24723 24726
  Show dependency treegraph
 
Reported: 2019-05-02 06:39 UTC by Martin Renvoize
Modified: 2020-05-01 13:26 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Who signed the patch off:
Text to go in the release notes:
This patch adds a new `inbound_email_address` method to the Koha::Library class. This allows for a consistent way of getting a libraries branch email address for incoming mail.
Version(s) released in:
20.05.00


Attachments
Bug 22823: Add get_effective_email method to Koha::Library (1.33 KB, patch)
2020-02-24 20:47 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 22823: Add unit tests for get_effective_email (2.29 KB, patch)
2020-02-25 07:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 22823: Add get_effective_email method to Koha::Library (1.33 KB, patch)
2020-02-25 07:38 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 22823: Add unit tests for get_effective_email (2.35 KB, patch)
2020-02-27 23:54 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Bug 22823: Add get_effective_email method to Koha::Library (1.64 KB, patch)
2020-02-27 23:54 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Bug 22823: Add unit tests for get_effective_email (2.42 KB, patch)
2020-03-18 09:48 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 22823: Add get_effective_email method to Koha::Library (1.72 KB, patch)
2020-03-18 09:48 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 22823: Rename method with ->inbound_email_address (2.84 KB, patch)
2020-03-18 09:48 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 22823: return undef if KohaAdminEmailAddress not exist (2.33 KB, patch)
2020-03-18 09:48 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Renvoize 2019-05-02 06:39:59 UTC
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.
Comment 1 Martin Renvoize 2019-05-02 06:42:55 UTC
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)
Comment 2 Martin Renvoize 2020-02-24 20:47:08 UTC
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`.
Comment 3 Martin Renvoize 2020-02-24 20:48:06 UTC
Not entirely sure about the method name yet.. open to opinions.
Comment 4 Martin Renvoize 2020-02-25 07:37:57 UTC
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.
Comment 5 Martin Renvoize 2020-02-25 07:38:00 UTC
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`.
Comment 6 Martin Renvoize 2020-02-25 07:40:11 UTC
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.
Comment 7 Bernardo Gonzalez Kriegel 2020-02-27 23:54:31 UTC
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>
Comment 8 Bernardo Gonzalez Kriegel 2020-02-27 23:54:34 UTC
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.
Comment 9 Bernardo Gonzalez Kriegel 2020-02-28 01:00:04 UTC
Sorry, didn't notice the 'Academy' keyword. 
Looking a bit I understand now its meaning.
Comment 10 Martin Renvoize 2020-02-28 15:39:33 UTC
Thanks for testing Bernardo :)
Comment 11 Jonathan Druart 2020-03-03 09:25:18 UTC
Should not it be named get_effective_to_email (or something saying we should not use it for from)?
Comment 12 Martin Renvoize 2020-03-04 14:00:28 UTC
(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
Comment 13 Martin Renvoize 2020-03-06 13:34:44 UTC
how about '->inbound_email_address' ?
Comment 14 Jonathan Druart 2020-03-18 09:48:48 UTC
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>
Comment 15 Jonathan Druart 2020-03-18 09:48:52 UTC
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>
Comment 16 Jonathan Druart 2020-03-18 09:48:55 UTC
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>
Comment 17 Jonathan Druart 2020-03-18 09:48:59 UTC
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>
Comment 18 Martin Renvoize 2020-03-18 15:45:01 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 19 Martin Renvoize 2020-03-18 16:30:04 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 20 Joy Nelson 2020-04-03 20:12:27 UTC
enhancement not backported to 19.11.x