From 78831946861ace51dca3251b3063a11448a8b870 Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Fri, 6 Mar 2026 13:40:49 +0000 Subject: [PATCH] Bug 42001: Align duplicate order email notifications with acquisitions conventions - Rename preferences to the Edifact* namespace, placing them adjacent to related Edifact* prefs on the system preferences page: EdiDuplicateOrderEmailNotice -> EdifactOrderSendBlockDuplicatesEmailNotice EdiDuplicateOrderEmailAddresses -> EdifactOrderSendBlockDuplicatesEmailAddresses - Change EdifactOrderSendBlockDuplicatesEmailNotice from YesNo to Choice, matching the EmailPurchaseSuggestions pattern. Options: Do not send / AcquisitionsDefaultEmailAddress / specific email addresses / KohaAdminEmailAddress. - EdifactOrderSendBlockDuplicatesEmailAddresses is now only used when the choice is set to 'specific email addresses'. - Pass from_address (AcquisitionsDefaultEmailAddress with fallback) and reply_address (AcquisitionsDefaultReplyTo) to EnqueueLetter for both library and vendor notifications. - Add UPDATE in atomicupdate to migrate existing YesNo installs to Choice. - Fix test mock values for EmailNotice from old YesNo (1) to the correct Choice value ('EdifactOrderSendBlockDuplicatesEmailAddresses'). - Update C4/UsageStats.pm with new pref names. --- C4/UsageStats.pm | 4 +- Koha/EDI.pm | 95 +++++++++++-------- .../data/mysql/atomicupdate/bug_42001.pl | 29 ++++-- installer/data/mysql/mandatory/sysprefs.sql | 4 +- .../admin/preferences/acquisitions.pref | 15 +-- t/db_dependent/Koha/EDI.t | 22 +++-- 6 files changed, 103 insertions(+), 66 deletions(-) diff --git a/C4/UsageStats.pm b/C4/UsageStats.pm index 6c95d9a6679..b0b25f87172 100644 --- a/C4/UsageStats.pm +++ b/C4/UsageStats.pm @@ -316,8 +316,8 @@ sub _shared_preferences { EdiBlockDuplicateInvoice EdiBlockDuplicateInvoiceEmailAddresses EdiBlockDuplicateInvoiceEmailNotice - EdiDuplicateOrderEmailAddresses - EdiDuplicateOrderEmailNotice + EdifactOrderSendBlockDuplicatesEmailAddresses + EdifactOrderSendBlockDuplicatesEmailNotice EmailFieldPrimary autoMemberNum BorrowerRenewalPeriodBase diff --git a/Koha/EDI.pm b/Koha/EDI.pm index 9cc68d89279..37e2916d42b 100644 --- a/Koha/EDI.pm +++ b/Koha/EDI.pm @@ -1556,7 +1556,8 @@ sub _send_duplicate_order_email_notice { my $logger = Koha::Logger->get( { interface => 'edi' } ); # Check if email notifications enabled - return unless C4::Context->preference('EdiDuplicateOrderEmailNotice'); + my $email_pref = C4::Context->preference('EdifactOrderSendBlockDuplicatesEmailNotice'); + return unless $email_pref; my $vendor_id = $vendor->vendor_id; my $po_number = $basket->basketname; @@ -1571,45 +1572,61 @@ sub _send_duplicate_order_email_notice { vendor_san => $vendor->san || '', }; + # Use acquisitions from/reply addresses, consistent with other acquisitions notices + my $from_address = C4::Context->preference('AcquisitionsDefaultEmailAddress') + || C4::Context->preference('KohaAdminEmailAddress'); + my $reply_address = C4::Context->preference('AcquisitionsDefaultReplyTo') || undef; + # 1. Send notification to library staff - my $library_email_addresses = C4::Context->preference('EdiDuplicateOrderEmailAddresses'); - if ($library_email_addresses) { - my @library_addresses = split /\s*,\s*/, $library_email_addresses; - - foreach my $to_address (@library_addresses) { - $to_address =~ s/^\s+|\s+$//g; # trim whitespace - next unless $to_address; - next unless Koha::Email->is_valid($to_address); - - my $letter = C4::Letters::GetPreparedLetter( - module => 'acquisition', - letter_code => 'EDI_DUP_ORD_LIBRARY', - message_transport_type => 'email', - tables => { - aqbooksellers => $vendor_id, - }, - substitute => $substitute, - ); + my @library_addresses; + if ( $email_pref eq 'AcquisitionsDefaultEmailAddress' ) { + my $addr = C4::Context->preference('AcquisitionsDefaultEmailAddress') + || C4::Context->preference('KohaAdminEmailAddress'); + push @library_addresses, $addr if $addr; + } elsif ( $email_pref eq 'KohaAdminEmailAddress' ) { + my $addr = C4::Context->preference('ReplytoDefault') + || C4::Context->preference('KohaAdminEmailAddress'); + push @library_addresses, $addr if $addr; + } elsif ( $email_pref eq 'EdifactOrderSendBlockDuplicatesEmailAddresses' ) { + my $addresses = C4::Context->preference('EdifactOrderSendBlockDuplicatesEmailAddresses'); + push @library_addresses, split /\s*,\s*/, $addresses if $addresses; + } - if ($letter) { - my $message_id = C4::Letters::EnqueueLetter( - { - letter => $letter, - to_address => $to_address, - message_transport_type => 'email', - } - ); + foreach my $to_address (@library_addresses) { + $to_address =~ s/^\s+|\s+$//g; # trim whitespace + next unless $to_address; + next unless Koha::Email->is_valid($to_address); - if ($message_id) { - $logger->info( - "Library duplicate order notification queued (message_id: $message_id) for $to_address, PO number $po_number. Message will be sent by message_queue cronjob." - ); - } else { - $logger->warn("Failed to enqueue library notification to $to_address for PO number $po_number"); + my $letter = C4::Letters::GetPreparedLetter( + module => 'acquisition', + letter_code => 'EDI_DUP_ORD_LIBRARY', + message_transport_type => 'email', + tables => { + aqbooksellers => $vendor_id, + }, + substitute => $substitute, + ); + + if ($letter) { + my $message_id = C4::Letters::EnqueueLetter( + { + letter => $letter, + to_address => $to_address, + from_address => $from_address, + ( $reply_address ? ( reply_address => $reply_address ) : () ), + message_transport_type => 'email', } + ); + + if ($message_id) { + $logger->info( + "Library duplicate order notification queued (message_id: $message_id) for $to_address, PO number $po_number. Message will be sent by message_queue cronjob." + ); } else { - $logger->warn("Could not generate library notification letter for PO number $po_number"); + $logger->warn("Failed to enqueue library notification to $to_address for PO number $po_number"); } + } else { + $logger->warn("Could not generate library notification letter for PO number $po_number"); } } @@ -1641,8 +1658,10 @@ sub _send_duplicate_order_email_notice { if ($letter) { my $message_id = C4::Letters::EnqueueLetter( { - letter => $letter, - to_address => $vendor_email, + letter => $letter, + to_address => $vendor_email, + from_address => $from_address, + ( $reply_address ? ( reply_address => $reply_address ) : () ), message_transport_type => 'email', } ); @@ -1929,10 +1948,10 @@ Koha::EDI are detected (i.e. a basket with the same PO number already exists for the vendor). Queues two types of notifications using letter templates: - 1. EDI_DUP_ORD_LIBRARY - to library staff (addresses from EdiDuplicateOrderEmailAddresses) + 1. EDI_DUP_ORD_LIBRARY - to library staff (addresses from EdifactOrderSendBlockDuplicatesEmailAddresses) 2. EDI_DUP_ORD_VENDOR - to vendor contacts with edi_error_notification set - Only runs if EdiDuplicateOrderEmailNotice preference is enabled. + Only runs if EdifactOrderSendBlockDuplicatesEmailNotice preference is enabled. Parameters: - $basket: The Aqbasket DBIC row for the basket being blocked diff --git a/installer/data/mysql/atomicupdate/bug_42001.pl b/installer/data/mysql/atomicupdate/bug_42001.pl index fd05d39b4d6..a01b8f579ab 100755 --- a/installer/data/mysql/atomicupdate/bug_42001.pl +++ b/installer/data/mysql/atomicupdate/bug_42001.pl @@ -7,35 +7,46 @@ return { my ($args) = @_; my ( $dbh, $out ) = @$args{qw(dbh out)}; - # Email notification toggle + # Email notification destination $dbh->do( q{ INSERT IGNORE INTO systempreferences (variable, value, options, explanation, type) VALUES ( - 'EdiDuplicateOrderEmailNotice', + 'EdifactOrderSendBlockDuplicatesEmailNotice', '0', - NULL, - 'Send email notification when a duplicate EDIFACT purchase order number is detected for the same supplier.', - 'YesNo' + '0|AcquisitionsDefaultEmailAddress|EdifactOrderSendBlockDuplicatesEmailAddresses|KohaAdminEmailAddress', + 'Send duplicate EDIFACT order block notifications using the EDI_DUP_ORD_LIBRARY notice template to the selected address. Vendor EDI contacts are always notified separately via EDI_DUP_ORD_VENDOR.', + 'Choice' ) } ); - say $out "Added system preference 'EdiDuplicateOrderEmailNotice'"; + $dbh->do( + q{ + UPDATE systempreferences + SET type = 'Choice', + options = '0|AcquisitionsDefaultEmailAddress|EdifactOrderSendBlockDuplicatesEmailAddresses|KohaAdminEmailAddress', + value = CASE WHEN value = '1' THEN 'AcquisitionsDefaultEmailAddress' ELSE '0' END, + explanation = 'Send duplicate EDIFACT order block notifications using the EDI_DUP_ORD_LIBRARY notice template to the selected address. Vendor EDI contacts are always notified separately via EDI_DUP_ORD_VENDOR.' + WHERE variable = 'EdifactOrderSendBlockDuplicatesEmailNotice' + AND type = 'YesNo' + } + ); + say $out "Added system preference 'EdifactOrderSendBlockDuplicatesEmailNotice'"; # Email recipient list $dbh->do( q{ INSERT IGNORE INTO systempreferences (variable, value, options, explanation, type) VALUES ( - 'EdiDuplicateOrderEmailAddresses', + 'EdifactOrderSendBlockDuplicatesEmailAddresses', '', NULL, - 'Comma-separated list of email addresses to notify when duplicate EDIFACT purchase order numbers are detected (e.g., "purchasing@library.org,edi_support@library.org"). Requires EdiDuplicateOrderEmailNotice to be enabled.', + 'Comma-separated list of acquisitions staff email addresses to use when EdifactOrderSendBlockDuplicatesEmailNotice is set to specific email addresses.', 'Textarea' ) } ); - say $out "Added system preference 'EdiDuplicateOrderEmailAddresses'"; + say $out "Added system preference 'EdifactOrderSendBlockDuplicatesEmailAddresses'"; # Add notice template - library staff notification $dbh->do( diff --git a/installer/data/mysql/mandatory/sysprefs.sql b/installer/data/mysql/mandatory/sysprefs.sql index a6a72ea0832..80b8ccad188 100644 --- a/installer/data/mysql/mandatory/sysprefs.sql +++ b/installer/data/mysql/mandatory/sysprefs.sql @@ -245,8 +245,8 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` ('EdifactInvoiceImportBlockDuplicates', '0', NULL, 'Block automatic processing of EDIFACT invoices when a duplicate invoice number is detected for the same supplier. Similar to AcqWarnOnDuplicateInvoice for manually created invoices, but applies to invoices received via EDI.', 'YesNo'), ('EdifactInvoiceImportBlockDuplicatesEmailAddresses', '', NULL, 'Comma-separated list of acquisitions staff email addresses to use when EdifactInvoiceImportBlockDuplicatesEmailNotice is set to specific email addresses.', 'Textarea'), ('EdifactInvoiceImportBlockDuplicatesEmailNotice', '0', '0|AcquisitionsDefaultEmailAddress|EdifactInvoiceImportBlockDuplicatesEmailAddresses|KohaAdminEmailAddress', 'Send duplicate EDIFACT invoice block notifications using the EDI_DUP_INV_LIBRARY notice template to the selected address. Vendor EDI contacts are always notified separately via EDI_DUP_INV_VENDOR. Requires EdifactInvoiceImportBlockDuplicates to be enabled.', 'Choice'), -('EdiDuplicateOrderEmailAddresses', '', NULL, 'Comma-separated list of email addresses to notify when duplicate EDIFACT purchase order numbers are detected (e.g., "purchasing@library.org,edi_support@library.org"). Requires EdiDuplicateOrderEmailNotice to be enabled.', 'Textarea'), -('EdiDuplicateOrderEmailNotice', '0', NULL, 'Send email notification when a duplicate EDIFACT purchase order number is detected for the same supplier.', 'YesNo'), +('EdifactOrderSendBlockDuplicatesEmailAddresses', '', NULL, 'Comma-separated list of acquisitions staff email addresses to use when EdifactOrderSendBlockDuplicatesEmailNotice is set to specific email addresses.', 'Textarea'), +('EdifactOrderSendBlockDuplicatesEmailNotice', '0', '0|AcquisitionsDefaultEmailAddress|EdifactOrderSendBlockDuplicatesEmailAddresses|KohaAdminEmailAddress', 'Send duplicate EDIFACT order block notifications using the EDI_DUP_ORD_LIBRARY notice template to the selected address. Vendor EDI contacts are always notified separately via EDI_DUP_ORD_VENDOR.', 'Choice'), ('ElasticsearchBoostFieldMatch', '0', NULL, 'Add a "match" query to es when searching, will follow indexes chosen in advanced search, or use title-cover for generic keyword or title index search', 'YesNo'), ('ElasticsearchCrossFields', '1', NULL, 'Enable "cross_fields" option for searches using Elastic search.', 'YesNo'), ('ElasticsearchIndexStatus_authorities', '0', 'Authorities index status', NULL, NULL), diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/acquisitions.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/acquisitions.pref index 92e5a79df8f..f2ec7b52d5a 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/acquisitions.pref +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/acquisitions.pref @@ -215,13 +215,14 @@ Acquisitions: type: textarea class: code - - - pref: EdiDuplicateOrderEmailNotice + - "Send duplicate EDIFACT order block notifications using the EDI_DUP_ORD_LIBRARY notice template to:" + - pref: EdifactOrderSendBlockDuplicatesEmailNotice choices: - 1: Send - 0: "Don't send" - - email notifications when a duplicate EDIFACT purchase order number is detected. - - - - "Send duplicate order notifications to these email addresses (comma-separated):" - - pref: EdiDuplicateOrderEmailAddresses + 0: "Do not send" + AcquisitionsDefaultEmailAddress: AcquisitionsDefaultEmailAddress + EdifactOrderSendBlockDuplicatesEmailAddresses: "specific email addresses" + KohaAdminEmailAddress: KohaAdminEmailAddress + - '
Vendor EDI contacts are always notified separately via the EDI_DUP_ORD_VENDOR notice using contacts flagged in the vendor''s EDI account.
If choosing specific email addresses, enter them here (comma-separated):' + - pref: EdifactOrderSendBlockDuplicatesEmailAddresses type: textarea class: code diff --git a/t/db_dependent/Koha/EDI.t b/t/db_dependent/Koha/EDI.t index 776ba4376e5..15868d2e9b1 100755 --- a/t/db_dependent/Koha/EDI.t +++ b/t/db_dependent/Koha/EDI.t @@ -2999,14 +2999,14 @@ subtest 'duplicate_order_email_notifications' => sub { return ( $bookseller, $new_basket, $ean ); }; - # Test 1: No email when EdiDuplicateOrderEmailNotice is disabled + # Test 1: No email when EdifactOrderSendBlockDuplicatesEmailNotice is disabled subtest 'email_notification_disabled' => sub { plan tests => 3; $schema->storage->txn_begin; - t::lib::Mocks::mock_preference( 'EdiDuplicateOrderEmailNotice', 0 ); - t::lib::Mocks::mock_preference( 'EdiDuplicateOrderEmailAddresses', 'library@example.com' ); + t::lib::Mocks::mock_preference( 'EdifactOrderSendBlockDuplicatesEmailNotice', 0 ); + t::lib::Mocks::mock_preference( 'EdifactOrderSendBlockDuplicatesEmailAddresses', 'library@example.com' ); my ( $bookseller, $new_basket, $ean ) = $_setup_duplicate_order->(); @@ -3018,7 +3018,7 @@ subtest 'duplicate_order_email_notifications' => sub { is( $result->{error}, 'duplicate_po_number', 'Error code is duplicate_po_number' ); my $messages = $schema->resultset('MessageQueue')->search( { to_address => 'library@example.com' } ); - is( $messages->count, 0, 'No email queued when EdiDuplicateOrderEmailNotice is disabled' ); + is( $messages->count, 0, 'No email queued when EdifactOrderSendBlockDuplicatesEmailNotice is disabled' ); $logger->clear(); $schema->storage->txn_rollback; @@ -3030,8 +3030,11 @@ subtest 'duplicate_order_email_notifications' => sub { $schema->storage->txn_begin; - t::lib::Mocks::mock_preference( 'EdiDuplicateOrderEmailNotice', 1 ); - t::lib::Mocks::mock_preference( 'EdiDuplicateOrderEmailAddresses', 'acq@library.org' ); + t::lib::Mocks::mock_preference( + 'EdifactOrderSendBlockDuplicatesEmailNotice', + 'EdifactOrderSendBlockDuplicatesEmailAddresses' + ); + t::lib::Mocks::mock_preference( 'EdifactOrderSendBlockDuplicatesEmailAddresses', 'acq@library.org' ); $schema->resultset('Letter') ->search( { module => 'acquisition', code => [ 'EDI_DUP_ORD_LIBRARY', 'EDI_DUP_ORD_VENDOR' ] } ) @@ -3082,8 +3085,11 @@ subtest 'duplicate_order_email_notifications' => sub { $schema->storage->txn_begin; - t::lib::Mocks::mock_preference( 'EdiDuplicateOrderEmailNotice', 1 ); - t::lib::Mocks::mock_preference( 'EdiDuplicateOrderEmailAddresses', '' ); + t::lib::Mocks::mock_preference( + 'EdifactOrderSendBlockDuplicatesEmailNotice', + 'EdifactOrderSendBlockDuplicatesEmailAddresses' + ); + t::lib::Mocks::mock_preference( 'EdifactOrderSendBlockDuplicatesEmailAddresses', '' ); $schema->resultset('Letter') ->search( { module => 'acquisition', code => [ 'EDI_DUP_ORD_LIBRARY', 'EDI_DUP_ORD_VENDOR' ] } ) -- 2.53.0