Bug 27708 misunderstood how the acquisitions link tables work and thus incorrectly assigned values from rows in the wrong table.
Created attachment 128375 [details] [review] Bug 29670: Fix EDI for AcqCreateItem = 'placing on order' The AcqCreatItem at order time functionality was broken by bug 27708. This patch resolves that.
Created attachment 128379 [details] [review] Bug 29670: Fix EDI for AcqCreateItem = 'placing on order' The AcqCreatItem at order time functionality was broken by bug 27708. This patch resolves that.
Created attachment 128391 [details] [review] Bug 29670: Unit tests This patch adds unit tests for Koha::Edifact::Order->order_line. We now check that the message segments are created as expected for both the 'ordering' and not 'ordering' case for acquisitions item creation time.
Created attachment 128392 [details] [review] Bug 29670: Fix EDI for AcqCreateItem = 'placing on order' The AcqCreatItem at order time functionality was broken by bug 27708. This patch resolves that. Test plan. 1) Run the newly created unit tests that prove both settings work
Created attachment 128393 [details] [review] Bug 29670: Unit tests This patch adds unit tests for Koha::Edifact::Order->order_line. We now check that the message segments are created as expected for both the 'ordering' and not 'ordering' case for acquisitions item creation time. Signed-off-by: Jonathan Field <jonathan.field@ptfs-europe.com>
Created attachment 128394 [details] [review] Bug 29670: Fix EDI for AcqCreateItem = 'placing on order' The AcqCreatItem at order time functionality was broken by bug 27708. This patch resolves that. Test plan. 1) Run the newly created unit tests that prove both settings work Signed-off-by: Jonathan Field <jonathan.field@ptfs-europe.com>
Tested on live environments that exhibited problems prior to the patch and signed off. The issue was that if your settings meant that items were created at order time, but 27708 made it so that the GIR segments would not get sent to the vendor. The patch fixes that problem, but leaves in place the handling for when items are not created at order time. I also corrected the itemtype assignment key for the case where items are not created as this was incorrect in bug 27708 too.
I can only partially test this, but I have made sure the unit tests pass and I have run through the test plan from bug 27708. No errors in the GUI and none in the logs.
Created attachment 128419 [details] [review] Bug 29670: Unit tests This patch adds unit tests for Koha::Edifact::Order->order_line. We now check that the message segments are created as expected for both the 'ordering' and not 'ordering' case for acquisitions item creation time. Signed-off-by: Jonathan Field <jonathan.field@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 128420 [details] [review] Bug 29670: Fix EDI for AcqCreateItem = 'placing on order' The AcqCreatItem at order time functionality was broken by bug 27708. This patch resolves that. Test plan. 1) Run the newly created unit tests that prove both settings work Signed-off-by: Jonathan Field <jonathan.field@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Thanks Katrin, the yeah, EDI is notoriously difficult to test. I'm really pleased with the unit tests this patch adds personally, I feel like it's a real steel in the right direction for keeping EDI working in future versions
Ah QA tests fail for me : FAIL t/db_dependent/Koha/Edifact/Order.t FAIL spelling segement ==> segment segement ==> segment
(In reply to Fridolin Somers from comment #12) > Ah QA tests fail for me : > FAIL t/db_dependent/Koha/Edifact/Order.t > FAIL spelling > segement ==> segment > segement ==> segment Critical bug stuck because of that?
Created attachment 129584 [details] [review] Bug 29670: Unit tests This patch adds unit tests for Koha::Edifact::Order->order_line. We now check that the message segments are created as expected for both the 'ordering' and not 'ordering' case for acquisitions item creation time. Signed-off-by: Jonathan Field <jonathan.field@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> JD amended patch: spelling segement ==> segment
Created attachment 129585 [details] [review] Bug 29670: Fix EDI for AcqCreateItem = 'placing on order' The AcqCreatItem at order time functionality was broken by bug 27708. This patch resolves that. Test plan. 1) Run the newly created unit tests that prove both settings work Signed-off-by: Jonathan Field <jonathan.field@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Pushed to master for 22.05, thanks to everybody involved [U+1F984]
This affects 21.11.00,21.05.05 and 20.11.12.. it needs backporting
Pushed to 21.11.x for 21.11.02
Pushed to 21.05.x for 21.05.11
This one is tricky for 20.11.X The patch fixes one of the tests but there are others failing. Digging the differences with 21.05.x showed that bug 27793 wasn't on 20.11.x and bingo, that's what makes the difference in output and fixes the tests :D So I'll backport it to have this one. 27793 doesn't seem too scary since no regression has been found after all this time and the missing automated tests are coming with this very ticket. And the functional change shouldn't bother users. An alternative is reverting bug 27708 Tell me in the next week (before the release) if the alternative would actually be better.
Backported: Pushed to 20.11.x branch for 20.11.15
Do I need to backport this to 19.11.x? I probably won't unless there is a need for it
It depends on bug 27708 so no need for 19.11.x
Not backported to 19.11.x