Bugzilla – Attachment 194729 Details for
Bug 42001
Update duplicate purchase order number detection on EDI quote import to optionally send notifications to staff and vendors
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 42001: Align duplicate order email notifications with acquisitions conventions
7883194.patch (text/plain), 19.20 KB, created by
Martin Renvoize (ashimema)
on 2026-03-06 13:44:58 UTC
(
hide
)
Description:
Bug 42001: Align duplicate order email notifications with acquisitions conventions
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2026-03-06 13:44:58 UTC
Size:
19.20 KB
patch
obsolete
>From 78831946861ace51dca3251b3063a11448a8b870 Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@openfifth.co.uk> >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 >+ - '<br>Vendor EDI contacts are always notified separately via the EDI_DUP_ORD_VENDOR notice using contacts flagged in the vendor''s EDI account.<br>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 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 42001
:
194517
|
194523
|
194524
|
194525
|
194526
|
194725
|
194726
|
194727
|
194728
| 194729