| Summary: | We should allow configuration of whether EDI LSQ segments map to 'location' or 'collection' | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Martin Renvoize (ashimema) <martin.renvoize> |
| Component: | Acquisitions | Assignee: | Martin Renvoize (ashimema) <martin.renvoize> |
| Status: | CLOSED FIXED | QA Contact: | Testopia <testopia> |
| Severity: | enhancement | ||
| Priority: | P5 - low | CC: | caroline.cyr-la-rose, fridolin.somers, jon.turner, jonathan.field, kyle, sally.healey, wainuiwitikapark |
| Version: | Main | Keywords: | Manual-updated |
| Hardware: | All | ||
| OS: | All | ||
| Change sponsored?: | --- | Patch complexity: | Small patch |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: |
EDItEUR describes the LSQ segment as "A code or other designation which identifies stock which is to be shelved in a specified sequence or collection."
In Koha, this could be interpreted as either 'location' or 'ccode'; This bug makes that configurable for each EDI vendor, defaulting to location as that was the previously hard coded configuration.
|
Version(s) released in: |
22.05.00
|
| Circulation function: | |||
| Bug Depends on: | |||
| Bug Blocks: | 31134 | ||
| Attachments: |
Bug 30135: Add EdifactLSQ mapping preferenec
Bug 30135: Add EdifactLSQ mapping preference Bug 30135: Unit tests Bug 30135: Add EdifactLSQ mapping preference Bug 30135: Unit tests Bug 30135: Add EdifactLSQ mapping preference Bug 30135: Unit tests Bug 30135: (follow-up) Rephrase system preference description to use collection Bug 30135: Add EdifactLSQ mapping preference Bug 30135: Unit tests Bug 30135: (follow-up) Rephrase system preference description to use collection Bug 30135: (QA follow-up) Fix typo in update Bug 30135: (QA follow-up) Fix typo in sysprefs.sql Bug 30135: (follow-up) Rename accessor in failing test |
||
|
Description
Martin Renvoize (ashimema)
2022-02-18 14:59:05 UTC
Created attachment 130879 [details] [review] Bug 30135: Add EdifactLSQ mapping preferenec This patch adds a new system preference, EdifactLSQ, to allow configuration of the ambiguous LSQ, sequence code, field included in the EDIFACT specifications. Originally the field was hard coded to map to 'location', but as per the specification it could have been mapped to 'ccode'. From the specification: A code or other designation which identifies stock which is to be shelved in a specified sequence or collection. Created attachment 130971 [details] [review] Bug 30135: Add EdifactLSQ mapping preference This patch adds a new system preference, EdifactLSQ, to allow configuration of the ambiguous LSQ, sequence code, field included in the EDIFACT specifications. Originally the field was hard coded to map to 'location', but as per the specification it could have been mapped to 'ccode'. From the specification: A code or other designation which identifies stock which is to be shelved in a specified sequence or collection. Created attachment 130972 [details] [review] Bug 30135: Unit tests Add a unit test for EdifactLSQ preference. Spelling might be a false alarm, but the rest looks valid: FAIL Koha/EDI.pm FAIL valid Useless use of hash element in void context Useless use of hash element in void context OK Koha/Edifact/Line.pm OK Koha/Edifact/Order.pm FAIL installer/data/mysql/atomicupdate/bug_30135.pl FAIL file permissions File must have the exec flag FAIL spelling GIR ==> GIT OK installer/data/mysql/mandatory/sysprefs.sql OK t/db_dependent/Koha/Edifact/Order.t Created attachment 131006 [details] [review] Bug 30135: Add EdifactLSQ mapping preference This patch adds a new system preference, EdifactLSQ, to allow configuration of the ambiguous LSQ, sequence code, field included in the EDIFACT specifications. Originally the field was hard coded to map to 'location', but as per the specification it could have been mapped to 'ccode'. From the specification: A code or other designation which identifies stock which is to be shelved in a specified sequence or collection. Created attachment 131007 [details] [review] Bug 30135: Unit tests Add a unit test for EdifactLSQ preference. Thanks Katrin, all fixed :) Created attachment 131139 [details] [review] Bug 30135: Add EdifactLSQ mapping preference This patch adds a new system preference, EdifactLSQ, to allow configuration of the ambiguous LSQ, sequence code, field included in the EDIFACT specifications. Originally the field was hard coded to map to 'location', but as per the specification it could have been mapped to 'ccode'. From the specification: A code or other designation which identifies stock which is to be shelved in a specified sequence or collection. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 131140 [details] [review] Bug 30135: Unit tests Add a unit test for EdifactLSQ preference. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 131141 [details] [review] Bug 30135: (follow-up) Rephrase system preference description to use collection Avoids ccode in favor of collection in the system preference description. Also makes it a tiny bit easier to translate. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 132694 [details] [review] Bug 30135: Add EdifactLSQ mapping preference This patch adds a new system preference, EdifactLSQ, to allow configuration of the ambiguous LSQ, sequence code, field included in the EDIFACT specifications. Originally the field was hard coded to map to 'location', but as per the specification it could have been mapped to 'ccode'. From the specification: A code or other designation which identifies stock which is to be shelved in a specified sequence or collection. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 132695 [details] [review] Bug 30135: Unit tests Add a unit test for EdifactLSQ preference. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 132696 [details] [review] Bug 30135: (follow-up) Rephrase system preference description to use collection Avoids ccode in favor of collection in the system preference description. Also makes it a tiny bit easier to translate. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 132697 [details] [review] Bug 30135: (QA follow-up) Fix typo in update Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 133259 [details] [review] Bug 30135: (QA follow-up) Fix typo in sysprefs.sql Pushed to master for 22.05, thanks to everybody involved [U+1F984] Seems I missed a unit test update.. looking now. Created attachment 133294 [details] [review] Bug 30135: (follow-up) Rename accessor in failing test We renamed the LSQ seqment accessor from 'collection_code' to 'sequenece_code' to more clearly reflect the EDI segment name instead of the Koha field we had mapped it to. (Especially as that mapping is not optionally to ccode or location). However, I forgot to update the corresponding test. This patch does that update. *** Bug 30134 has been marked as a duplicate of this bug. *** Follow-up pushed as RM Assistant.. I was pushing something else to get peoples dev envs back up and running and thought it not a bad idea to push this one too and try and get the dashboard green ;) (In reply to Martin Renvoize from comment #20) > Follow-up pushed as RM Assistant.. I was pushing something else to get > peoples dev envs back up and running and thought it not a bad idea to push > this one too and try and get the dashboard green ;) RM likes a green dashboard ;) This patch might have introduced a flaw in the tests - when we run t/ediorder.t, it fails with this output: vagrant@kohadevbox:kohaclone((v22.05.02))$ sudo koha-shell kohadev kohadev-koha@kohadevbox:/home/vagrant/kohaclone$ prove t/Ediorder.t t/Ediorder.t .. 1/13 Use of uninitialized value $lsq_field in hash element at /home/vagrant/kohaclone/Koha/Edifact/Order.pm line 563. Use of uninitialized value $lsq_field in hash element at /home/vagrant/kohaclone/Koha/Edifact/Order.pm line 563. t/Ediorder.t .. 11/13 # Failed test 'Single Gir field OK' # at t/Ediorder.t line 104. # got: 'GIR+001+BUDGET:LFN+BRANCH:LLO+TYPE:LST+CALL:LSM' # expected: 'GIR+001+BUDGET:LFN+BRANCH:LLO+TYPE:LST+LOCATION:LSQ+CALL:LSM' Use of uninitialized value $lsq_field in hash element at /home/vagrant/kohaclone/Koha/Edifact/Order.pm line 563. Use of uninitialized value $lsq_field in hash element at /home/vagrant/kohaclone/Koha/Edifact/Order.pm line 563. $VAR1 = 'GIR+001+BUDGET:LFN+BRANCH:LLO+TYPE:LST+CALL:LSM+S_I:LVT'; $VAR2 = 'GIR+002+BUDGET:LFN+BRANCH:LLO+TYPE:LST+CALL:LSM+S_I:LVT'; Use of uninitialized value $got in string eq at (eval in cmp_ok) t/Ediorder.t line 119. # Failed test 'First part of split Gir field OK' # at t/Ediorder.t line 119. # got: undef # expected: 'GIR+002+BUDGET:LFN+BRANCH:LLO+TYPE:LST+LOCATION:LSQ+CALL:LSM' Use of uninitialized value $got in string eq at (eval in cmp_ok) t/Ediorder.t line 125. # Failed test 'Second part of split GIR field OK' # at t/Ediorder.t line 125. # got: undef # expected: 'GIR+002+S_I:LVT' # Looks like you failed 3 tests of 13. t/Ediorder.t .. Dubious, test returned 3 (wstat 768, 0x300) Failed 3/13 subtests Test Summary Report ------------------- t/Ediorder.t (Wstat: 768 Tests: 13 Failed: 3) Failed tests: 11-13 Non-zero exit status: 3 Files=1, Tests=13, 2 wallclock secs ( 0.02 usr 0.00 sys + 1.40 cusr 0.10 csys = 1.52 CPU) Result: FAIL (In reply to wainuiwitikapark from comment #22) > This patch might have introduced a flaw in the tests - when we run The unit tests are passing for me. I'm wondering how this is even possible as this test isn't db dependent! (In reply to Kyle M Hall from comment #23) > (In reply to wainuiwitikapark from comment #22) > > This patch might have introduced a flaw in the tests - when we run > > The unit tests are passing for me. I'm wondering how this is even possible > as this test isn't db dependent! Ah, I was testing on master. |