[Bug 35398] New: EDI: Fix support for LRP (Library Rotation Plan) for Koha with Stock Rotation enabled
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35398 Bug ID: 35398 Summary: EDI: Fix support for LRP (Library Rotation Plan) for Koha with Stock Rotation enabled Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Acquisitions Assignee: koha-bugs@lists.koha-community.org Reporter: martin.renvoize@ptfs-europe.com QA Contact: testopia@bugs.koha-community.org Depends on: 20595 Bug 20595 introduced support for LRP fields in EDI with StockRotation enabled. However, it missed a case where each order line only contains one item per line. This wasn't spotted in the existing uses as they structure their orders somewhat differently, but is being highlighted at a sight where they regularly order in this way. Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20595 [Bug 20595] EDI: Add support for LRP (Library Rotation Plan) for Koha with Stock Rotation enabled -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35398 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |martin.renvoize@ptfs-europe |ity.org |.com -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35398 --- Comment #1 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 159240 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159240&action=edit Bug 35398: Fix LRP support for EDI orders with single items This patch corrects a mistake in the original implementaiton of bug 20595 where we introduced support for the LRP segments being used to automatically assign items ordered via EDI to stock rotation plans. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35398 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35398 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35398 --- Comment #2 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- This is now in production at a number of PTFS-E sites. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35398 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #159240|0 |1 is obsolete| | --- Comment #3 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 160350 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=160350&action=edit Bug 35398: Fix LRP support for EDI orders with single items This patch corrects a mistake in the original implementaiton of bug 20595 where we introduced support for the LRP segments being used to automatically assign items ordered via EDI to stock rotation plans. Signed-off-by: ophie Halden <sophie.halden@cheshiresharedservices.gov.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35398 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off --- Comment #4 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Signoff line added on behalf of customer using fix in production. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35398 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #160350|0 |1 is obsolete| | --- Comment #5 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 160352 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=160352&action=edit Bug 35398: Fix LRP support for EDI orders with single items This patch corrects a mistake in the original implementaiton of bug 20595 where we introduced support for the LRP segments being used to automatically assign items ordered via EDI to stock rotation plans. Signed-off-by: Sophie Halden <sophie.halden@cheshiresharedservices.gov.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35398 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35398 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #160352|0 |1 is obsolete| | --- Comment #6 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 160406 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=160406&action=edit Bug 35398: Fix LRP support for EDI orders with single items This patch corrects a mistake in the original implementaiton of bug 20595 where we introduced support for the LRP segments being used to automatically assign items ordered via EDI to stock rotation plans. Signed-off-by: Sophie Halden <sophie.halden@cheshiresharedservices.gov.uk> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35398 --- Comment #7 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 160407 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=160407&action=edit Bug 35398: (QA follow-up) Tidy code Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35398 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Failed QA --- Comment #8 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I'd feel much better if we had some unit tests here. What do you think? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35398 --- Comment #9 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- It's not at all trivial to write a test for this.. it's in production at all PTFS-E sites backported to 22.11.x and beyond. Whilst I still agree tests are good, EDI has been notoriously difficult to write tests for from day one. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35398 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|enhancement |minor Status|Failed QA |Passed QA --- Comment #10 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I asked and as it's a bug fix, I am open to make an exception here. I cannot really test EDI unless I am provided with data and a detailed test plan. I think it's similar for others, so we might rely more on code review here than elsewhere. I think in the long term unit tests would be really helpful to avoid breaking things and make us more confident about making changes. I know EDIFACT is a critical feature for a lot of libraries. As you know, pushing might still take a bit for reasons, but I'll switch back to PQA. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35398 --- Comment #11 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 162541 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=162541&action=edit Bug 35398: Add unit test for extraction of library_rotation_plan This unit test addition adds an LRP segment to the test EDI message file and adds the corresponding test to confirm is it extracted into the field as expected. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35398 --- Comment #12 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 162542 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=162542&action=edit Bug 35398: Unit test This patch adds the begginings of unit tests for Koha::EDI. Right now we only test that a simple QUOTE message creates a basket, adds the item to said basket and assigns that single item to a corresponding stock rotation rota as defined by the LRP segment in the QUOTE message. It lays the foundations for much more rigorous tests to be written to cover the whole of EDI.pm however. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35398 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #160406|0 |1 is obsolete| | --- Comment #13 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 162543 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=162543&action=edit Bug 35398: Fix LRP support for EDI orders with single items This patch corrects a mistake in the original implementaiton of bug 20595 where we introduced support for the LRP segments being used to automatically assign items ordered via EDI to stock rotation plans. Signed-off-by: Sophie Halden <sophie.halden@cheshiresharedservices.gov.uk> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35398 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #160407|0 |1 is obsolete| | --- Comment #14 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 162544 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=162544&action=edit Bug 35398: (QA follow-up) Tidy code Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35398 --- Comment #15 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Two full days of writing code later I've finally managed to get to grips with writing the very basics of a unit test for this. Considering the original patch took 20 minutes I'm a bit bewildered by that.. but at least it's a start of unit tests for this area. I now also have a parse/deparse pair of scripts for taking an EDI message and making it a bit more human readable and then doing the opposite to package up an edited version.. that's how I created the test file added here. Those scripts are not included here for now. Anyway.. I think we are good to go here now. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35398 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |kyle@bywatersolutions.com |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35398 --- Comment #16 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Fixed the file permission error and tidied the new file EDI.t (amended commits). Decided to ignore the one line on t/Edifact.t for now. WARN t/Edifact.t WARN tidiness The file is less tidy than before (bad/messy lines before: 15, now: 16) FAIL t/db_dependent/Koha/EDI.t FAIL file permissions File must have the exec flag WARN tidiness The file is less tidy than before (bad/messy lines before: 0, now: 7) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35398 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |24.05.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35398 --- Comment #17 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Pushed for 24.05! Well done everyone, thank you! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35398 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |Pushed to stable CC| |fridolin.somers@biblibre.co | |m Version(s)|24.05.00 |24.05.00,23.11.04 released in| | --- Comment #18 from Fridolin Somers <fridolin.somers@biblibre.com> --- Pushed to 23.11.x for 23.11.04 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35398 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to stable |Pushed to oldstable CC| |lucas@bywatersolutions.com Version(s)|24.05.00,23.11.04 |24.05.00,23.11.04,23.05.10 released in| | --- Comment #19 from Lucas Gass <lucas@bywatersolutions.com> --- Backported to 23.05.x for upcoming 23.05.10. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org