Bug 30144 - Add support for EDI GIR:LVT and/or GIR:LVC, Servicing instructions, segment
Summary: Add support for EDI GIR:LVT and/or GIR:LVC, Servicing instructions, segment
Status: Needs Signoff
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Martin Renvoize (ashimema)
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-02-21 17:54 UTC by Martin Renvoize (ashimema)
Modified: 2026-02-13 15:29 UTC (History)
3 users (show)

See Also:
GIT URL:
Initiative type: ---
Sponsorship status: Sponsored
Comma delimited list of Sponsors: Martin Renvoize <martin.renvoize@gmail.com>
Crowdfunding goal: 0
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 30144: Add servicing_instruction field to aqorders table (7.61 KB, patch)
2026-02-12 19:06 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 30144: Update DBIC schema (1.65 KB, patch)
2026-02-12 19:06 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 30144: Process and send servicing instructions in EDI messages (7.24 KB, patch)
2026-02-12 19:06 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 30144: Add UI for servicing instructions (32.78 KB, patch)
2026-02-12 19:06 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 30144: Add unit tests for servicing instructions (8.30 KB, patch)
2026-02-12 19:07 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 30144: Add tests for EDIFACT servicing instruction handling (14.83 KB, patch)
2026-02-12 19:07 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 30144: Add servicing_instruction field to aqorders table (7.61 KB, patch)
2026-02-13 15:27 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 30144: Update DBIC schema (1.65 KB, patch)
2026-02-13 15:27 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 30144: Process and send servicing instructions in EDI messages (7.24 KB, patch)
2026-02-13 15:27 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 30144: Add UI for servicing instructions (32.78 KB, patch)
2026-02-13 15:27 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 30144: Add unit tests for servicing instructions (8.30 KB, patch)
2026-02-13 15:27 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 30144: Add tests for EDIFACT servicing instruction handling (14.83 KB, patch)
2026-02-13 15:27 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 30144: (QA follow-up) Add EDIFACT_SI authorised values to installer data (8.85 KB, patch)
2026-02-13 15:27 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Renvoize (ashimema) 2022-02-21 17:54:16 UTC
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.
Comment 1 Martin Renvoize (ashimema) 2026-02-12 19:06:55 UTC
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.
Comment 2 Martin Renvoize (ashimema) 2026-02-12 19:06:56 UTC
Created attachment 193018 [details] [review]
Bug 30144: Update DBIC schema

Patch from commit 04a7b7f
Comment 3 Martin Renvoize (ashimema) 2026-02-12 19:06:58 UTC
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
Comment 4 Martin Renvoize (ashimema) 2026-02-12 19:06:59 UTC
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
Comment 5 Martin Renvoize (ashimema) 2026-02-12 19:07:01 UTC
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
Comment 6 Martin Renvoize (ashimema) 2026-02-12 19:07:02 UTC
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>
Comment 7 Kyle M Hall (khall) 2026-02-13 12:55:20 UTC
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
Comment 8 Martin Renvoize (ashimema) 2026-02-13 15:27:34 UTC
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.
Comment 9 Martin Renvoize (ashimema) 2026-02-13 15:27:36 UTC
Created attachment 193094 [details] [review]
Bug 30144: Update DBIC schema

Patch from commit 407a54a
Comment 10 Martin Renvoize (ashimema) 2026-02-13 15:27:37 UTC
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
Comment 11 Martin Renvoize (ashimema) 2026-02-13 15:27:38 UTC
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
Comment 12 Martin Renvoize (ashimema) 2026-02-13 15:27:40 UTC
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
Comment 13 Martin Renvoize (ashimema) 2026-02-13 15:27:42 UTC
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>
Comment 14 Martin Renvoize (ashimema) 2026-02-13 15:27:44 UTC
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
Comment 15 Martin Renvoize (ashimema) 2026-02-13 15:29:20 UTC
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.