From 5024513901a65822763c3fa02df891908b7a270a Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Wed, 20 Dec 2023 11:43:34 +0000 Subject: [PATCH] Bug 12802: Clarify system preferences I felt the switch to multi in the EmailFieldPrimary preference was a bit confusing given that type exposes a 'select all' option which doesn't make sense with the pre-existing 'first valid' option being an override in the code. This patch opts to switch it back to 'Choice', meaning that only one option can be picked and adds a 'selected addresses' option which prompts the use of a new 'EmailFieldSelection' preference which allows for the multi-select as before. To test: 1) Run though the test plan for 'Update notice_email_address method to return a list' but with the following ammendments: * 2) As aposed to being able to select multiple options under EmailFieldPrimary, you should now only be able to select one option at a time, but a new 'selected addresses' option should be present. * 8) Select the 'selected addresses' option for 'EmailFieldPrimary' and also select multiple fields for the new 'EmailFieldSelection' preference. Signed-off-by: Brendan Gallagher Signed-off-by: Axelle Clarisse Signed-off-by: Mathieu Saby Signed-off-by: Aleisha Amohia Signed-off-by: Martin Renvoize Signed-off-by: Katrin Fischer --- Koha/Patron.pm | 24 ++++++++++++------- .../data/mysql/atomicupdate/bug_12802.pl | 11 +++++++-- installer/data/mysql/mandatory/sysprefs.sql | 3 ++- .../en/modules/admin/preferences/patrons.pref | 13 ++++++++-- t/db_dependent/Koha/Patron.t | 22 ++++++++++++----- 5 files changed, 53 insertions(+), 20 deletions(-) diff --git a/Koha/Patron.pm b/Koha/Patron.pm index e8874c597d..d4b848db56 100644 --- a/Koha/Patron.pm +++ b/Koha/Patron.pm @@ -1619,19 +1619,25 @@ sub notice_email_address{ my ( $self ) = @_; my $which_address = C4::Context->preference("EmailFieldPrimary"); - my @addresses; - for my $email_field ( split ",", $which_address ) { - # if syspref is set to 'first valid' (value == OFF), look up email address - if ( $email_field eq 'OFF' ) { - return $self->first_valid_email_address; - } + # if syspref is set to 'first valid' (value == OFF), look up email address + if ( $which_address eq 'OFF' ) { + return $self->first_valid_email_address; + } - my $email_address = $self->$email_field; - push @addresses, $email_address if $email_address; + # if syspref is set to 'selected addresses' (value == MULTI), look up email addresses + if ( $which_address eq 'MULTI' ) { + my @addresses; + my $selected_fields = C4::Context->preference("EmailFieldSelection"); + for my $email_field ( split ",", $selected_fields ) { + my $email_address = $self->$email_field; + push @addresses, $email_address if $email_address; + } + return join(",",@addresses); } - return join(",",@addresses); + return $self->$which_address || ''; + } =head3 first_valid_email_address diff --git a/installer/data/mysql/atomicupdate/bug_12802.pl b/installer/data/mysql/atomicupdate/bug_12802.pl index 55def6be3f..3be7681170 100755 --- a/installer/data/mysql/atomicupdate/bug_12802.pl +++ b/installer/data/mysql/atomicupdate/bug_12802.pl @@ -7,8 +7,15 @@ return { my ($args) = @_; my ( $dbh, $out ) = @$args{qw(dbh out)}; - $dbh->do("UPDATE systempreferences SET type='multiple' WHERE variable='EmailFieldPrimary'"); + $dbh->do( + "UPDATE systempreferences SET options='email|emailpro|B_email|cardnumber|OFF|MULTI' WHERE variable='EmailFieldPrimary'" + ); + say $out "Updated system preference 'EmailFieldPrimary' to include 'selected addresses' option"; - say $out "Updated system preference 'EmailFieldPrimary' to have type 'multiple'"; + $dbh->do( + "INSERT IGNORE INTO systempreferences ( `variable`, `value`, `options`, `explanation`, `type` ) VALUES ('EmailFieldSelection','email|emailpro|B_email','','Selection list of patron email fields to use whern AutoEmailPrimaryAddress is set to selected addresses','multiple')" + ); + + say $out "Added new system preference 'EmailFieldSelection'"; }, }; diff --git a/installer/data/mysql/mandatory/sysprefs.sql b/installer/data/mysql/mandatory/sysprefs.sql index 8ea05a46ff..4c372bad78 100644 --- a/installer/data/mysql/mandatory/sysprefs.sql +++ b/installer/data/mysql/mandatory/sysprefs.sql @@ -229,7 +229,8 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` ('EmailAddressForPatronRegistrations', '', '', ' If you choose EmailAddressForPatronRegistrations you have to enter a valid email address: ', 'free'), ('EmailAddressForSuggestions','','',' If you choose EmailAddressForSuggestions you have to enter a valid email address: ','free'), ('EmailFieldPrecedence','email|emailpro|B_email','','Ordered list of patron email fields to use when AutoEmailPrimaryAddress is set to first valid','multiple'), -('EmailFieldPrimary','OFF','email|emailpro|B_email|cardnumber|OFF','Defines the default email address field where patron email notices are sent.','multiple'), +('EmailFieldPrimary','OFF','email|emailpro|B_email|cardnumber|OFF|MULTI','Defines the default email address field where patron email notices are sent.','Choice'), +('EmailFieldSelection','email|emailpro|B_email','','Selection list of patron email fields to use whern AutoEmailPrimaryAddress is set to selected addresses','multiple'), ('emailLibrarianWhenHoldIsPlaced','0',NULL,'If ON, emails the librarian whenever a hold is placed','YesNo'), ('EmailOverduesNoEmail','1',NULL,'Send send overdues of patrons without email address to staff','YesNo'), ('EmailPatronRegistrations', '0', '0|EmailAddressForPatronRegistrations|BranchEmailAddress|KohaAdminEmailAddress', 'Choose email address that new patron registrations will be sent to: ', 'Choice'), diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref index 5e34886bb1..0c280f302b 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref @@ -194,15 +194,16 @@ Patrons: - "Use the patron's" - pref: EmailFieldPrimary default: "OFF" - multiple: + choices: email: primary email emailpro: secondary email B_email: alternate email cardnumber: card number "OFF": first valid email address + "MULTI": selected addresses - 'for sending out email notices.' - '
NOTE: If set to "first valid", the order in which the email addresses are checked is set in EmailFieldPrecedence.' - - '
NOTE: To send notices to multiple email fields, ensure "first valid" is unchecked.' + - '
NOTE: If set to "selected addresses", the selection refers to the email address fields set in EmailFieldSelection.' - - "When EmailFieldPrimary is set to 'first valid', check the patron email address fields in the following order and use the first valid email address found:" - pref: EmailFieldPrecedence @@ -210,6 +211,14 @@ Patrons: - '
NOTE: All patron fields can be used, but to work correctly they must contain a valid email address or an empty string.' - "Valid options are the database columns of the borrowers table, separated by | (pipe)." - "Example: email|emailpro|B_email" + - + - "When EmailFieldPrimary is set to 'selected addresses', send email to all valid email addresses in the selected fields:" + - pref: EmailFieldSelection + default: "email" + multiple: + email: primary email + emailpro: secondary email + B_email: alternate email - - pref: TalkingTechItivaPhoneNotification choices: diff --git a/t/db_dependent/Koha/Patron.t b/t/db_dependent/Koha/Patron.t index 583d61f292..9d5e3e0c6c 100755 --- a/t/db_dependent/Koha/Patron.t +++ b/t/db_dependent/Koha/Patron.t @@ -1793,17 +1793,27 @@ subtest 'notice_email_address' => sub { plan tests => 3; $schema->storage->txn_begin; - my $patron = $builder->build_object({ class => 'Koha::Patrons' }); + my $patron = $builder->build_object( { class => 'Koha::Patrons' } ); t::lib::Mocks::mock_preference( 'EmailFieldPrecedence', 'email|emailpro' ); - t::lib::Mocks::mock_preference( 'EmailFieldPrimary', 'OFF' ); - is ($patron->notice_email_address, $patron->email, "Koha::Patron->notice_email_address returns correct value when EmailFieldPrimary is off"); + t::lib::Mocks::mock_preference( 'EmailFieldPrimary', 'OFF' ); + is( + $patron->notice_email_address, $patron->email, + "Koha::Patron->notice_email_address returns correct value when EmailFieldPrimary is off" + ); t::lib::Mocks::mock_preference( 'EmailFieldPrimary', 'emailpro' ); - is ($patron->notice_email_address, $patron->emailpro, "Koha::Patron->notice_email_address returns correct value when EmailFieldPrimary is emailpro"); + is( + $patron->notice_email_address, $patron->emailpro, + "Koha::Patron->notice_email_address returns correct value when EmailFieldPrimary is emailpro" + ); - t::lib::Mocks::mock_preference( 'EmailFieldPrimary', 'email,emailpro' ); - is ($patron->notice_email_address, $patron->email.",".$patron->emailpro, "Koha::Patron->notice_email_address returns correct value when EmailFieldPrimary is email|emailpro"); + t::lib::Mocks::mock_preference( 'EmailFieldPrimary', 'MULTI' ); + t::lib::Mocks::mock_preference( 'EmailFieldSelection', 'email,emailpro' ); + is( + $patron->notice_email_address, $patron->email . "," . $patron->emailpro, + "Koha::Patron->notice_email_address returns correct value when EmailFieldPrimary is 'MULTI' and EmailFieldSelection is 'email,emailpro'" + ); $patron->delete; $schema->storage->txn_rollback; -- 2.39.5