View | Details | Raw Unified | Return to bug 20253
Collapse All | Expand All

(-)a/Koha/EDI.pm (-3 / +53 lines)
Lines 95-100 sub create_edi_order { Link Here
95
        }
95
        }
96
    )->single;
96
    )->single;
97
97
98
    # Check for duplicate PO numbers when po_is_basketname is enabled
99
    if ( $vendor && $vendor->po_is_basketname ) {
100
        my $basket = $orderlines[0]->basketno;
101
        if ( $basket->basketname ) {
102
            my $existing_basket = $schema->resultset('Aqbasket')->search(
103
                {
104
                    basketname   => $basket->basketname,
105
                    booksellerid => $basket->booksellerid,
106
                    basketno     => { '!=' => $basketno },    # Exclude current basket
107
                }
108
            )->first;
109
110
            if ($existing_basket) {
111
                $logger->error( "EDI order blocked for basket $basketno due to duplicate PO number '"
112
                        . $basket->basketname
113
                        . "' (existing basket: "
114
                        . $existing_basket->basketno
115
                        . ")" );
116
                return {
117
                    error     => "duplicate_po_number", existing_basket => $existing_basket->basketno,
118
                    po_number => $basket->basketname
119
                };
120
            }
121
        }
122
    }
123
98
    my $ean_search_keys = { ean => $ean, };
124
    my $ean_search_keys = { ean => $ean, };
99
    if ($branchcode) {
125
    if ($branchcode) {
100
        $ean_search_keys->{branchcode} = $branchcode;
126
        $ean_search_keys->{branchcode} = $branchcode;
Lines 647-653 sub process_quote { Link Here
647
    my $schema         = Koha::Database->new()->schema();
673
    my $schema         = Koha::Database->new()->schema();
648
    my $message_count  = 0;
674
    my $message_count  = 0;
649
    my $split_message;
675
    my $split_message;
650
    my @added_baskets;    # if auto & multiple baskets need to order all
676
    my @added_baskets;                # if auto & multiple baskets need to order all
677
    my %baskets_with_po_conflicts;    # track baskets that have PO number conflicts
651
678
652
    # Get vendor EDI account configuration
679
    # Get vendor EDI account configuration
653
    my $v = $schema->resultset('VendorEdiAccount')->search(
680
    my $v = $schema->resultset('VendorEdiAccount')->search(
Lines 663-675 sub process_quote { Link Here
663
690
664
            # Determine basket name based on vendor configuration
691
            # Determine basket name based on vendor configuration
665
            my $basket_name = $quote_message->filename;
692
            my $basket_name = $quote_message->filename;
693
            my $purchase_order_number;
694
            my $existing_basket;
695
666
            if ( $v && $v->po_is_basketname ) {
696
            if ( $v && $v->po_is_basketname ) {
667
                my $purchase_order_number = $msg->purchase_order_number;
697
                $purchase_order_number = $msg->purchase_order_number;
668
                if ($purchase_order_number) {
698
                if ($purchase_order_number) {
669
                    $basket_name = $purchase_order_number;
699
                    $basket_name = $purchase_order_number;
670
700
671
                    # Check for duplicate purchase order numbers for this vendor (including closed baskets)
701
                    # Check for duplicate purchase order numbers for this vendor (including closed baskets)
672
                    my $existing_basket = $schema->resultset('Aqbasket')->search(
702
                    $existing_basket = $schema->resultset('Aqbasket')->search(
673
                        {
703
                        {
674
                            basketname   => $basket_name,
704
                            basketname   => $basket_name,
675
                            booksellerid => $quote_message->vendor_id,
705
                            booksellerid => $quote_message->vendor_id,
Lines 701-706 sub process_quote { Link Here
701
                q{} . q{}
731
                q{} . q{}
702
            );
732
            );
703
            push @added_baskets, $basketno;
733
            push @added_baskets, $basketno;
734
735
            # Record if this basket has a PO conflict (for auto_orders blocking)
736
            if ($existing_basket) {
737
                $baskets_with_po_conflicts{$basketno} = {
738
                    po_number       => $purchase_order_number,
739
                    existing_basket => $existing_basket->basketno
740
                };
741
            }
704
            if ( $message_count > 1 ) {
742
            if ( $message_count > 1 ) {
705
                my $m_filename = $quote_message->filename;
743
                my $m_filename = $quote_message->filename;
706
                $m_filename .= "_$message_count";
744
                $m_filename .= "_$message_count";
Lines 743-748 sub process_quote { Link Here
743
        for my $i ( 0 .. $#added_baskets ) {
781
        for my $i ( 0 .. $#added_baskets ) {
744
            my $basketno = $added_baskets[$i];
782
            my $basketno = $added_baskets[$i];
745
783
784
            # Check if this basket has a PO number conflict
785
            if ( exists $baskets_with_po_conflicts{$b} ) {
786
                my $conflict_info = $baskets_with_po_conflicts{$b};
787
                $logger->warn( "Auto-order blocked for basket $b due to duplicate PO number '"
788
                        . $conflict_info->{po_number}
789
                        . "' (existing basket: "
790
                        . $conflict_info->{existing_basket}
791
                        . ")" );
792
                $logger->info("Basket $b remains open due to PO number conflict");
793
                next;
794
            }
795
746
            # Use the correct buyer_ean for each message/basket
796
            # Use the correct buyer_ean for each message/basket
747
            my $buyer_ean = $messages->[$i] ? $messages->[$i]->buyer_ean : $messages->[0]->buyer_ean;
797
            my $buyer_ean = $messages->[$i] ? $messages->[$i]->buyer_ean : $messages->[0]->buyer_ean;
748
798
(-)a/acqui/basket.pl (-1 / +13 lines)
Lines 652-658 sub edi_close_and_order { Link Here
652
        if ( $basket->{branch} ) {
652
        if ( $basket->{branch} ) {
653
            $edi_params->{branchcode} = $basket->{branch};
653
            $edi_params->{branchcode} = $basket->{branch};
654
        }
654
        }
655
        if ( create_edi_order($edi_params) ) {
655
        my $edi_result = create_edi_order($edi_params);
656
        if ( ref $edi_result eq 'HASH' && $edi_result->{error} ) {
657
            if ( $edi_result->{error} eq 'duplicate_po_number' ) {
658
                push @messages, {
659
                    type            => 'error',
660
                    code            => 'edi_duplicate_po_number',
661
                    po_number       => $edi_result->{po_number},
662
                    existing_basket => $edi_result->{existing_basket}
663
                };
664
                $op = 'list';    # Stay on basket page to show error
665
                return;
666
            }
667
        } elsif ($edi_result) {
656
668
657
            #$template->param( edifile => 1 );
669
            #$template->param( edifile => 1 );
658
        }
670
        }
(-)a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tt (+5 lines)
Lines 332-337 Link Here
332
                                <span>There is no notice template with code ACQORDER defined.</span>
332
                                <span>There is no notice template with code ACQORDER defined.</span>
333
                            [% CASE 'email_sent' %]
333
                            [% CASE 'email_sent' %]
334
                                <span>Order e-mail was sent to the vendor.</span>
334
                                <span>Order e-mail was sent to the vendor.</span>
335
                            [% CASE 'edi_duplicate_po_number' %]
336
                                <span
337
                                    >EDI order blocked: Duplicate purchase order number '[% m.po_number | html %]' found. This purchase order number is already used by basket [% m.existing_basket | html %]. Please check with your vendor or
338
                                    use a different purchase order number.</span
339
                                >
335
                            [% CASE %]
340
                            [% CASE %]
336
                                <span>ERROR! - [% m.code | html %]</span>
341
                                <span>ERROR! - [% m.code | html %]</span>
337
                            [% END %]
342
                            [% END %]
(-)a/t/db_dependent/Koha/EDI.t (-2 / +234 lines)
Lines 21-27 use Modern::Perl; Link Here
21
use FindBin qw( $Bin );
21
use FindBin qw( $Bin );
22
22
23
use Test::NoWarnings;
23
use Test::NoWarnings;
24
use Test::More tests => 5;
24
use Test::More tests => 6;
25
use Test::MockModule;
25
use Test::MockModule;
26
26
27
use t::lib::Mocks;
27
use t::lib::Mocks;
Lines 1255-1260 subtest 'process_invoice_without_tax_rate' => sub { Link Here
1255
    $schema->storage->txn_rollback;
1255
    $schema->storage->txn_rollback;
1256
};
1256
};
1257
1257
1258
subtest 'duplicate_po_number_blocking' => sub {
1259
    plan tests => 2;
1260
1261
    $schema->storage->txn_begin;
1262
1263
    my $test_san      = '5013546098818';
1264
    my $dirname       = ( $Bin =~ /^(.*\/t\/)/ ? $1 . 'edi_testfiles/' : q{} );
1265
    my $active_period = $builder->build(
1266
        {
1267
            source => 'Aqbudgetperiod',
1268
            value  => { budget_period_active => 1 }
1269
        }
1270
    );
1271
1272
    # Test 1: Auto-orders blocking for duplicate PO numbers
1273
    subtest 'auto_orders_blocking' => sub {
1274
        plan tests => 9;
1275
1276
        $schema->storage->txn_begin;
1277
1278
        # Create vendor EDI account with auto_orders and po_is_basketname enabled
1279
        my $account = $builder->build(
1280
            {
1281
                source => 'VendorEdiAccount',
1282
                value  => {
1283
                    description      => 'auto_orders duplicate test vendor',
1284
                    transport        => 'FILE',
1285
                    plugin           => '',
1286
                    san              => $test_san,
1287
                    auto_orders      => 1,
1288
                    po_is_basketname => 1,
1289
                }
1290
            }
1291
        );
1292
1293
        my $ean = $builder->build(
1294
            {
1295
                source => 'EdifactEan',
1296
                value  => {
1297
                    description => 'test ean',
1298
                    branchcode  => undef,
1299
                    ean         => $test_san
1300
                }
1301
            }
1302
        );
1303
1304
        # Create existing basket with PO number that will conflict
1305
        my $existing_basket = $builder->build(
1306
            {
1307
                source => 'Aqbasket',
1308
                value  => {
1309
                    basketname   => 'orders 23/1',           # Same as in QUOTES_SMALL.CEQ
1310
                    booksellerid => $account->{vendor_id},
1311
                    closedate    => undef,
1312
                }
1313
            }
1314
        );
1315
1316
        # Setup the fund
1317
        $builder->build(
1318
            {
1319
                source => 'Aqbudget',
1320
                value  => {
1321
                    budget_code      => 'REF',
1322
                    budget_period_id => $active_period->{budget_period_id}
1323
                }
1324
            }
1325
        );
1326
1327
        my $filename = 'QUOTES_SMALL.CEQ';
1328
        my $trans    = Koha::Edifact::Transport->new( $account->{id} );
1329
        $trans->working_directory($dirname);
1330
1331
        my $mhash = $trans->message_hash();
1332
        $mhash->{message_type} = 'QUOTE';
1333
        $trans->ingest( $mhash, $filename );
1334
1335
        my $quote = $schema->resultset('EdifactMessage')->find( { filename => $filename } );
1336
1337
        # Clear logger before processing
1338
        $logger->clear();
1339
1340
        # Process quote - should create basket but NOT auto-order due to duplicate PO
1341
        my $die;
1342
        eval {
1343
            process_quote($quote);
1344
            1;
1345
        } or do {
1346
            $die = $@;
1347
        };
1348
        ok( !$die, 'Quote with duplicate PO processed without dying' );
1349
1350
        # Verify duplicate PO error was logged during quote processing
1351
        my $errors = $quote->edifact_errors;
1352
        ok( $errors->count >= 1, 'Error logged for duplicate PO number' );
1353
1354
        my $duplicate_error = $errors->search( { section => 'RFF+ON:orders 23/1' } )->first;
1355
        ok( $duplicate_error, 'Duplicate PO error found in quote errors' );
1356
1357
        # Check that new basket was created
1358
        my $baskets = Koha::Acquisition::Baskets->search(
1359
            {
1360
                booksellerid => $account->{vendor_id},
1361
                basketno     => { '!=' => $existing_basket->{basketno} }
1362
            }
1363
        );
1364
        is( $baskets->count, 1, "New basket created despite duplicate PO" );
1365
1366
        my $new_basket = $baskets->next;
1367
        is( $new_basket->basketname, 'orders 23/1', "New basket uses duplicate PO number as name" );
1368
1369
        # Critical test: Verify basket was NOT closed (auto-order was blocked)
1370
        ok( !$new_basket->closedate, 'New basket with duplicate PO was NOT closed (auto-order blocked)' );
1371
1372
        # Verify no EDI order was created for the conflicting basket
1373
        my $edi_orders = $schema->resultset('EdifactMessage')->search(
1374
            {
1375
                message_type => 'ORDERS',
1376
                basketno     => $new_basket->basketno,
1377
            }
1378
        );
1379
        is( $edi_orders->count, 0, 'No EDI order created for basket with duplicate PO' );
1380
1381
        # Check that the blocking is working by verifying no EDI orders were created
1382
        # This is the most important test - the basket should NOT be auto-ordered
1383
        ok( $edi_orders->count == 0, 'Auto-order was blocked - no EDI order created for duplicate PO' );
1384
1385
        # Verify that duplicate PO error was logged (this shows detection is working)
1386
        ok( $duplicate_error, 'Duplicate PO number detection and logging is working' );
1387
1388
        $schema->storage->txn_rollback;
1389
    };
1390
1391
    # Test 2: No blocking when po_is_basketname is disabled
1392
    subtest 'no_blocking_when_feature_disabled' => sub {
1393
        plan tests => 5;
1394
1395
        $schema->storage->txn_begin;
1396
1397
        # Create vendor EDI account with po_is_basketname DISABLED
1398
        my $account = $builder->build(
1399
            {
1400
                source => 'VendorEdiAccount',
1401
                value  => {
1402
                    description      => 'feature disabled test vendor',
1403
                    transport        => 'FILE',
1404
                    plugin           => '',
1405
                    san              => $test_san,
1406
                    auto_orders      => 1,
1407
                    po_is_basketname => 0,                                # Feature disabled
1408
                }
1409
            }
1410
        );
1411
1412
        my $ean = $builder->build(
1413
            {
1414
                source => 'EdifactEan',
1415
                value  => {
1416
                    description => 'test ean',
1417
                    branchcode  => undef,
1418
                    ean         => $test_san
1419
                }
1420
            }
1421
        );
1422
1423
        # Create existing basket with any name
1424
        my $existing_basket = $builder->build(
1425
            {
1426
                source => 'Aqbasket',
1427
                value  => {
1428
                    basketname   => 'orders 23/1',
1429
                    booksellerid => $account->{vendor_id},
1430
                    closedate    => undef,
1431
                }
1432
            }
1433
        );
1434
1435
        # Setup the fund
1436
        $builder->build(
1437
            {
1438
                source => 'Aqbudget',
1439
                value  => {
1440
                    budget_code      => 'REF',
1441
                    budget_period_id => $active_period->{budget_period_id}
1442
                }
1443
            }
1444
        );
1445
1446
        my $filename = 'QUOTES_SMALL.CEQ';
1447
        my $trans    = Koha::Edifact::Transport->new( $account->{id} );
1448
        $trans->working_directory($dirname);
1449
1450
        my $mhash = $trans->message_hash();
1451
        $mhash->{message_type} = 'QUOTE';
1452
        $trans->ingest( $mhash, $filename );
1453
1454
        my $quote = $schema->resultset('EdifactMessage')->find( { filename => $filename } );
1455
1456
        $logger->clear();
1457
        process_quote($quote);
1458
1459
        # Verify new basket was created and auto-ordered (no blocking)
1460
        my $baskets = Koha::Acquisition::Baskets->search(
1461
            {
1462
                booksellerid => $account->{vendor_id},
1463
                basketno     => { '!=' => $existing_basket->{basketno} }
1464
            }
1465
        );
1466
        is( $baskets->count, 1, "New basket created" );
1467
1468
        my $new_basket = $baskets->next;
1469
1470
        # When po_is_basketname is disabled, basket uses filename not PO number
1471
        is( $new_basket->basketname, $filename, "Basket uses filename when po_is_basketname disabled" );
1472
1473
        # The key test is that normal processing occurred without duplicate detection
1474
        # Since po_is_basketname is disabled, basket should use filename not PO number
1475
        ok( $new_basket->basketname eq $filename, 'Normal basket naming when feature disabled' );
1476
1477
        # With po_is_basketname disabled, quote processing should work normally
1478
        # The absence of duplicate errors in the database is the key indicator
1479
        my $quote_errors           = $schema->resultset('EdifactError')->search( {} );
1480
        my $has_duplicate_db_error = grep { $_->details =~ /Duplicate purchase order/ } $quote_errors->all;
1481
        ok( !$has_duplicate_db_error, 'No duplicate PO database errors when feature disabled' );
1482
1483
        pass('Feature correctly disabled - normal processing occurred');
1484
1485
        $schema->storage->txn_rollback;
1486
    };
1487
1488
    $schema->storage->txn_rollback;
1489
};
1490
1258
subtest 'create_edi_order_logging' => sub {
1491
subtest 'create_edi_order_logging' => sub {
1259
    plan tests => 8;
1492
    plan tests => 8;
1260
1493
1261
- 

Return to bug 20253