We allow for free text instructions in the EDI FTX segment now, taken from the vendor notes field. But this is at the order level, it would be helpful to add support for the LVT segment at the line level.
Created attachment 193017 [details] [review] Bug 30144: Add servicing_instruction field to aqorders table This patch adds a new TEXT field 'servicing_instruction' to the aqorders table to store EDIFACT servicing instructions (GIR LVT/LVC segments) as JSON. The field stores an array of instruction groups, where each group is an array of instruction objects with 'type' (LVC or LVT) and 'value' fields. Structure: [[{type:"LVC",value:"BB"},{type:"LVT",value:"text"}],[...]] This allows for: - Multiple servicing instruction groups per order line - Combinations of coded (LVC) and free-text (LVT) instructions - Instructions that work together in groups Also adds EDIFACT_SI authorized value category with EDItEUR List 3B servicing instruction codes for coded (LVC) instructions. Test plan: 1. Run database update 2. Verify aqorders.servicing_instruction field exists 3. Verify EDIFACT_SI authorized value category is created 4. Verify authorized values include codes like BB, BI, etc.
Created attachment 193018 [details] [review] Bug 30144: Update DBIC schema Patch from commit 04a7b7f
Created attachment 193019 [details] [review] Bug 30144: Process and send servicing instructions in EDI messages This patch updates the EDIFACT processing to handle multiple servicing instruction groups in both incoming quotes and outgoing orders. Changes to Koha/EDI.pm (quote_item): - Reads both LVC (coded) and LVT (freetext) instructions from incoming quotes - Validates LVC codes against EDIFACT_SI authorized values - Stores instructions as JSON array structure - Groups instructions that work together Changes to Koha/Edifact/Order.pm (gir_segments): - Parses JSON servicing instruction data from order - Generates multiple GIR segments for outgoing orders - Each instruction becomes a separate GIR element - Logs JSON parse errors to EDI log using Koha::Logger - Fixed element counter reset bug when segments exceed 5 elements Structure allows vendors to specify: - Single coded instruction: [[{type:"LVC",value:"BB"}]] - Single text instruction: [[{type:"LVT",value:"Handle with care"}]] - Combined instructions: [[{type:"LVC",value:"BB"},{type:"LVT",value:"Apply on spine"}]] - Multiple services: [[{type:"LVC",value:"BB"}],[{type:"LVC",value:"BI"}]] Test plan: 1. Receive EDIFACT quote with servicing instructions 2. Verify instructions are stored correctly in aqorders 3. Create EDI order from basket 4. Verify GIR segments are generated correctly in outgoing message 5. Check EDI log for any JSON parse errors
Created attachment 193020 [details] [review] Bug 30144: Add UI for servicing instructions This patch adds the user interface for managing and displaying EDIFACT servicing instructions in the acquisitions module. Changes to acqui/neworderempty.pl: - Parses servicing_instruction JSON for editing existing orders - Fetches EDIFACT_SI authorized values for dropdown - Passes data to template in both array and JSON formats Changes to acqui/addorder.pl: - Accepts servicing_instruction_groups_json from form submission - Validates and stores JSON in database Changes to acqui/basket.pl: - Fetches EDIFACT_SI authorized values - Formats servicing instructions for display in get_order_infos() - Creates human-readable display strings with: * LVC codes replaced with descriptions * Groups separated by <br> tags * Format: "Description" or "Description: free text" Changes to neworderempty.tt: - Dynamic UI for adding/removing servicing instruction groups - Each group can contain: * One LVC (coded) instruction only, OR * One LVT (free-text) instruction only, OR * One LVC + one LVT combination - Validation prevents: * More than 2 instructions per group * Duplicate LVC or LVT in same group - LVC instructions use dropdown populated from EDIFACT_SI authorized values - LVT instructions use free-text input - Groups serialized to JSON hidden field for form submission - Smart button labels show what can be added to each group Changes to basket.tt: - Displays servicing instructions in basket table - Pre-formatted in Perl (no JavaScript parsing needed) - Shows LVC descriptions instead of codes - Multiple groups displayed on separate lines Changes to columns_settings.yml: - Adds servicing_instruction column to basket table configuration Display format examples: - LVC only: "Barcode labelling" - LVT only: "Handle with care" - Combined: "Binding: Apply to spine" - Multiple groups (on separate lines): Barcode labelling Binding: Special instructions Test plan: 1. Create/edit order 2. Add servicing instruction groups 3. Try to add more than 2 instructions to a group (should prevent) 4. Try to add duplicate LVC or LVT to group (should prevent) 5. Save order 6. View basket and verify instructions display correctly 7. Edit order and verify instructions load into form
Created attachment 193021 [details] [review] Bug 30144: Add unit tests for servicing instructions This patch adds comprehensive unit tests for the servicing instruction functionality in t/db_dependent/Koha/Edifact/Order.t. Changes: - Updated test count from 6 to 7 - Added servicing_instruction => undef to test orders to prevent TestBuilder from generating random invalid JSON - Added servicing instruction subtest with 18 tests covering: * Single LVC instruction * Single LVT instruction * Multiple LVC in one group (tests GIR segment overflow) * Mixed LVC/LVT in one group * Multiple groups * Empty servicing instruction * Invalid JSON handling (logged to EDI log) - Tests correctly handle GIR segment 5-element limit and overflow to additional segments Test plan: 1. Run: prove t/db_dependent/Koha/Edifact/Order.t 2. Verify all tests pass
Created attachment 193022 [details] [review] Bug 30144: Add tests for EDIFACT servicing instruction handling This commit adds comprehensive tests for servicing instruction processing in both incoming quotes and outgoing orders: 1. Integration tests for incoming quote processing (Koha/EDI.pm) - Tests LVC (coded) and LVT (freetext) extraction - Tests multiple GIR occurrences with different servicing instructions - Tests all combinations: LVC only, LVT only, and LVC+LVT together 2. Test EDIFACT quote file (QUOTES_SERVICING.CEQ) - Contains 3 line items with 4 total orders - Demonstrates different servicing instruction patterns 3. Fix for multi-occurrence servicing instruction extraction - Modified quote_item() to extract servicing instructions per GIR occurrence instead of across all occurrences - Ensures each order gets only its own servicing instructions - Aligns with existing multi-item processing for funds and stock rotation Test coverage: - t/db_dependent/Koha/EDI.t: 29 tests for incoming quote processing - t/db_dependent/Koha/Edifact/Order.t: 18 tests for outgoing order generation (previously added) All tests pass successfully. Sponsored-by: Martin Renvoize <martin.renvoize@gmail.com>
All tests pass with a fresh database, but if I check out current main, apply the patches, run the database update and then run the tests, they fail: Running tests (2) * Proving /kohadevbox/koha/t/db_dependent/Koha/EDI.t FAIL # Loaded QUOTE file with 1 Message that contains 2 LIN segments with the first # consistent of 1 item and the second with 3 items with 2 different funds. # Loading QUOTE file with 1 Message that contains 2 LIN segments with the first # consistent of 1 item and the second with 3 items with 2 different undefined funds. # Loading QUOTE file with 2 messages # Failed test 'no warnings' # at /usr/share/perl/5.36/Test/Builder.pm line 193. # There were 3 warning(s) # Previous test 0 '' # Violation of unique constraint in AuthorisedValue at /kohadevbox/koha/t/lib/TestBuilder.pm line 365. # at /kohadevbox/koha/t/lib/TestBuilder.pm line 365. # t::lib::TestBuilder::_buildColumnValues(t::lib::TestBuilder=HASH(0xaaab04581518), HASH(0xaaab100ad118)) called at /kohadevbox/koha/t/lib/TestBuilder.pm line 121 # t::lib::TestBuilder::build(t::lib::TestBuilder=HASH(0xaaab04581518), HASH(0xaaab100fdc18)) called at /kohadevbox/koha/t/db_dependent/Koha/EDI.t line 1995 # main::__ANON__() called at /usr/share/perl/5.36/Test/Builder.pm line 374 # eval {...} called at /usr/share/perl/5.36/Test/Builder.pm line 374 # Test::Builder::subtest(Test::Builder=HASH(0xaaab0483a2d0), "servicing_instructions_quote_processing", CODE(0xaaab0fd25d60)) called at /usr/share/perl/5.36/Test/More.pm line 809 # Test::More::subtest("servicing_instructions_quote_processing", CODE(0xaaab0fd25d60)) called at /kohadevbox/koha/t/db_dependent/Koha/EDI.t line 2105 # # ---------- # Previous test 0 '' # Violation of unique constraint in AuthorisedValue at /kohadevbox/koha/t/lib/TestBuilder.pm line 365. # at /kohadevbox/koha/t/lib/TestBuilder.pm line 365. # t::lib::TestBuilder::_buildColumnValues(t::lib::TestBuilder=HASH(0xaaab04581518), HASH(0xaaab10139a58)) called at /kohadevbox/koha/t/lib/TestBuilder.pm line 121 # t::lib::TestBuilder::build(t::lib::TestBuilder=HASH(0xaaab04581518), HASH(0xaaab101606d8)) called at /kohadevbox/koha/t/db_dependent/Koha/EDI.t line 1995 # main::__ANON__() called at /usr/share/perl/5.36/Test/Builder.pm line 374 # eval {...} called at /usr/share/perl/5.36/Test/Builder.pm line 374 # Test::Builder::subtest(Test::Builder=HASH(0xaaab0483a2d0), "servicing_instructions_quote_processing", CODE(0xaaab0fd25d60)) called at /usr/share/perl/5.36/Test/More.pm line 809 # Test::More::subtest("servicing_instructions_quote_processing", CODE(0xaaab0fd25d60)) called at /kohadevbox/koha/t/db_dependent/Koha/EDI.t line 2105 # # ---------- # Previous test 0 '' # Violation of unique constraint in AuthorisedValue at /kohadevbox/koha/t/lib/TestBuilder.pm line 365. # at /kohadevbox/koha/t/lib/TestBuilder.pm line 365. # t::lib::TestBuilder::_buildColumnValues(t::lib::TestBuilder=HASH(0xaaab04581518), HASH(0xaaab100ad118)) called at /kohadevbox/koha/t/lib/TestBuilder.pm line 121 # t::lib::TestBuilder::build(t::lib::TestBuilder=HASH(0xaaab04581518), HASH(0xaaab100fdc18)) called at /kohadevbox/koha/t/db_dependent/Koha/EDI.t line 1995 # main::__ANON__() called at /usr/share/perl/5.36/Test/Builder.pm line 374 # eval {...} called at /usr/share/perl/5.36/Test/Builder.pm line 374 # Test::Builder::subtest(Test::Builder=HASH(0xaaab0483a2d0), "servicing_instructions_quote_processing", CODE(0xaaab0fd25d60)) called at /usr/share/perl/5.36/Test/More.pm line 809 # Test::More::subtest("servicing_instructions_quote_processing", CODE(0xaaab0fd25d60)) called at /kohadevbox/koha/t/db_dependent/Koha/EDI.t line 2105 # # Looks like you failed 1 test of 9. /kohadevbox/koha/t/db_dependent/Koha/EDI.t .. Dubious, test returned 1 (wstat 256, 0x100) Failed 1/9 subtests Test Summary Report ------------------- /kohadevbox/koha/t/db_dependent/Koha/EDI.t (Wstat: 256 (exited 1) Tests: 9 Failed: 1) Failed test: 9 Non-zero exit status: 1 Files=1, Tests=9, 3 wallclock secs ( 0.02 usr 0.00 sys + 2.51 cusr 0.58 csys = 3.11 CPU) Result: FAIL * Proving /kohadevbox/koha/t/db_dependent/Koha/Edifact/Order.t PASS
Created attachment 193093 [details] [review] Bug 30144: Add servicing_instruction field to aqorders table This patch adds a new TEXT field 'servicing_instruction' to the aqorders table to store EDIFACT servicing instructions (GIR LVT/LVC segments) as JSON. The field stores an array of instruction groups, where each group is an array of instruction objects with 'type' (LVC or LVT) and 'value' fields. Structure: [[{type:"LVC",value:"BB"},{type:"LVT",value:"text"}],[...]] This allows for: - Multiple servicing instruction groups per order line - Combinations of coded (LVC) and free-text (LVT) instructions - Instructions that work together in groups Also adds EDIFACT_SI authorized value category with EDItEUR List 3B servicing instruction codes for coded (LVC) instructions. Test plan: 1. Run database update 2. Verify aqorders.servicing_instruction field exists 3. Verify EDIFACT_SI authorized value category is created 4. Verify authorized values include codes like BB, BI, etc.
Created attachment 193094 [details] [review] Bug 30144: Update DBIC schema Patch from commit 407a54a
Created attachment 193095 [details] [review] Bug 30144: Process and send servicing instructions in EDI messages This patch updates the EDIFACT processing to handle multiple servicing instruction groups in both incoming quotes and outgoing orders. Changes to Koha/EDI.pm (quote_item): - Reads both LVC (coded) and LVT (freetext) instructions from incoming quotes - Validates LVC codes against EDIFACT_SI authorized values - Stores instructions as JSON array structure - Groups instructions that work together Changes to Koha/Edifact/Order.pm (gir_segments): - Parses JSON servicing instruction data from order - Generates multiple GIR segments for outgoing orders - Each instruction becomes a separate GIR element - Logs JSON parse errors to EDI log using Koha::Logger - Fixed element counter reset bug when segments exceed 5 elements Structure allows vendors to specify: - Single coded instruction: [[{type:"LVC",value:"BB"}]] - Single text instruction: [[{type:"LVT",value:"Handle with care"}]] - Combined instructions: [[{type:"LVC",value:"BB"},{type:"LVT",value:"Apply on spine"}]] - Multiple services: [[{type:"LVC",value:"BB"}],[{type:"LVC",value:"BI"}]] Test plan: 1. Receive EDIFACT quote with servicing instructions 2. Verify instructions are stored correctly in aqorders 3. Create EDI order from basket 4. Verify GIR segments are generated correctly in outgoing message 5. Check EDI log for any JSON parse errors
Created attachment 193096 [details] [review] Bug 30144: Add UI for servicing instructions This patch adds the user interface for managing and displaying EDIFACT servicing instructions in the acquisitions module. Changes to acqui/neworderempty.pl: - Parses servicing_instruction JSON for editing existing orders - Fetches EDIFACT_SI authorized values for dropdown - Passes data to template in both array and JSON formats Changes to acqui/addorder.pl: - Accepts servicing_instruction_groups_json from form submission - Validates and stores JSON in database Changes to acqui/basket.pl: - Fetches EDIFACT_SI authorized values - Formats servicing instructions for display in get_order_infos() - Creates human-readable display strings with: * LVC codes replaced with descriptions * Groups separated by <br> tags * Format: "Description" or "Description: free text" Changes to neworderempty.tt: - Dynamic UI for adding/removing servicing instruction groups - Each group can contain: * One LVC (coded) instruction only, OR * One LVT (free-text) instruction only, OR * One LVC + one LVT combination - Validation prevents: * More than 2 instructions per group * Duplicate LVC or LVT in same group - LVC instructions use dropdown populated from EDIFACT_SI authorized values - LVT instructions use free-text input - Groups serialized to JSON hidden field for form submission - Smart button labels show what can be added to each group Changes to basket.tt: - Displays servicing instructions in basket table - Pre-formatted in Perl (no JavaScript parsing needed) - Shows LVC descriptions instead of codes - Multiple groups displayed on separate lines Changes to columns_settings.yml: - Adds servicing_instruction column to basket table configuration Display format examples: - LVC only: "Barcode labelling" - LVT only: "Handle with care" - Combined: "Binding: Apply to spine" - Multiple groups (on separate lines): Barcode labelling Binding: Special instructions Test plan: 1. Create/edit order 2. Add servicing instruction groups 3. Try to add more than 2 instructions to a group (should prevent) 4. Try to add duplicate LVC or LVT to group (should prevent) 5. Save order 6. View basket and verify instructions display correctly 7. Edit order and verify instructions load into form
Created attachment 193097 [details] [review] Bug 30144: Add unit tests for servicing instructions This patch adds comprehensive unit tests for the servicing instruction functionality in t/db_dependent/Koha/Edifact/Order.t. Changes: - Updated test count from 6 to 7 - Added servicing_instruction => undef to test orders to prevent TestBuilder from generating random invalid JSON - Added servicing instruction subtest with 18 tests covering: * Single LVC instruction * Single LVT instruction * Multiple LVC in one group (tests GIR segment overflow) * Mixed LVC/LVT in one group * Multiple groups * Empty servicing instruction * Invalid JSON handling (logged to EDI log) - Tests correctly handle GIR segment 5-element limit and overflow to additional segments Test plan: 1. Run: prove t/db_dependent/Koha/Edifact/Order.t 2. Verify all tests pass
Created attachment 193098 [details] [review] Bug 30144: Add tests for EDIFACT servicing instruction handling This commit adds comprehensive tests for servicing instruction processing in both incoming quotes and outgoing orders: 1. Integration tests for incoming quote processing (Koha/EDI.pm) - Tests LVC (coded) and LVT (freetext) extraction - Tests multiple GIR occurrences with different servicing instructions - Tests all combinations: LVC only, LVT only, and LVC+LVT together 2. Test EDIFACT quote file (QUOTES_SERVICING.CEQ) - Contains 3 line items with 4 total orders - Demonstrates different servicing instruction patterns 3. Fix for multi-occurrence servicing instruction extraction - Modified quote_item() to extract servicing instructions per GIR occurrence instead of across all occurrences - Ensures each order gets only its own servicing instructions - Aligns with existing multi-item processing for funds and stock rotation Test coverage: - t/db_dependent/Koha/EDI.t: 29 tests for incoming quote processing - t/db_dependent/Koha/Edifact/Order.t: 18 tests for outgoing order generation (previously added) All tests pass successfully. Sponsored-by: Martin Renvoize <martin.renvoize@gmail.com>
Created attachment 193099 [details] [review] Bug 30144: (QA follow-up) Add EDIFACT_SI authorised values to installer data Tests were failing on upgrade route because they tried to create EDIFACT_SI authorised values that already existed from the atomicupdate script, causing unique constraint violations. This patch: - Removes test code that created EDIFACT_SI values (tests should assume database defaults exist) - Adds EDIFACT_SI category to auth_val_cat.sql for fresh installs - Adds all 32 EDItEUR List 3B servicing instruction codes to auth_values.yml for fresh installs Now the authorised values exist in both installation paths: - Upgrade route: Created by atomicupdate script - Fresh install: Created by installer SQL/YAML files Test plan: 1. prove t/db_dependent/Koha/EDI.t 2. prove t/db_dependent/Koha/Edifact/Order.t 3. Verify both tests pass without warnings
Good catch Kyle, I had the installer files uncommited in my tree.. combined with "fixing" the test to add the data for testing against.. Not a great combination.. I changed my mind partway through and forgot. Anywho.. should all work sensible now with that follow-up.