|
Description
Lucy Vaux-Harvey
2024-11-12 11:40:54 UTC
Created attachment 174406 [details] [review] Bug 38423: Code more defensively in process_invoice We re-arrange the logic of process_invoice a little here to ensure we skip order lines in invoices that do not have corresponding bib records. Created attachment 174408 [details] [review] Bug 38423: Code more defensively in process_invoice We re-arrange the logic of process_invoice a little here to ensure we skip order lines in invoices that do not have corresponding bib records. Any resource for how to test this? Or a test plan?
After trying find resources on EDI for Koha it seems the easiest path would be:
> If you set the EdifactInvoiceImport preference to ‘Don’t’ you can load invoice files manually.
But I don't know where that should be uploaded ^^"
Or maybe it's about having them in "EDIFACT messages" and manually clicking on loading them.
Then I think I would need matching order and invoice message files to test.
There doesn't seem to be an EDIFACT test plan in the whole git history :o
To know what's the minimal setup for EDI accounts to just pick files locally from a directory.
And there is no t/db_dependent/Koha/Edifact/Invoice.t what would help test at a lower level.
Only t/db_dependent/Koha/Edifact/Order.t
Editfact is hard to test as you need test edifact files.. I tend to rely heavily on Kyle to look at these ones to date as he's one of the few remaining people here with the knowledge of such messages to review these. Created attachment 175267 [details] [review] Bug 38423: Code more defensively in process_invoice We re-arrange the logic of process_invoice a little here to ensure we skip order lines in invoices that do not have corresponding bib records. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Is it possible to keep such test messages somewhere so anyone needing to test EDIFACT stuff can use them? After changing their content if there is record data that is intellectual property of a vendor. So I could signoff and Kyle could QA. Otherwise we might be a bit stuck. Created attachment 175405 [details] [review] Bug 38423: Code more defensively in process_invoice We re-arrange the logic of process_invoice a little here to ensure we skip order lines in invoices that do not have corresponding bib records. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 175406 [details] [review] Bug 38423: Add unit tests Sponsored-by: PTFS Europe <https://ptfs-europe.com> Created attachment 175407 [details] [review] Bug 38423: Code more defensively in process_invoice We re-arrange the logic of process_invoice a little here to ensure we skip order lines in invoices that do not have corresponding bib records. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 175408 [details] [review] Bug 38423: Add unit tests Sponsored-by: PTFS Europe <https://ptfs-europe.com> Unit tests added and these include an example INVOICE.CEI file with cases for each of the failures we now skip over with this patch. Created attachment 175485 [details] [review] Bug 38423: Code more defensively in process_invoice We re-arrange the logic of process_invoice a little here to ensure we skip order lines in invoices that do not have corresponding bib records. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 175486 [details] [review] Bug 38423: Add unit tests Sponsored-by: PTFS Europe <https://ptfs-europe.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> It works! :) Thanks Martin for the tests :D When having only the test commit applied (minor conflict on the `carp "Cannot find[..]`), the test fail with: "Can't call method "biblionumber" on an undefined value" That's in line with the implementation being about defensive coding. Created attachment 176344 [details] [review] Bug 38423: Add unit tests Sponsored-by: PTFS Europe <https://ptfs-europe.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Pushed for 25.05! Well done everyone, thank you! |