|
Description
Alex Buckley
2024-01-08 14:29:11 UTC
Created attachment 160644 [details] [review] Bug 35724: Database changes Sponsored-by: Waikato Institute of Technology, New Zealand Created attachment 160645 [details] [review] Bug 35724: Template changes Sponsored-by: Waikato Institute of Technology, New Zealand Created attachment 160646 [details] [review] Bug 35724: Define non-standard ports for EDI SFTP in EDI accounts Not all EDI vendors accept Koha servers connecting on port 22, sometimes they require a different port to be used, so it would be helpful for librarians to be able to configure non-standard EDI SFTP ports themselves. Test plan: 1. Set EDIFACT syspref = 'Enable' and define a EDI account: Administration > Acquisition parameters > EDI accounts. 2. Create an EDI order: https://koha-community.org/manual/latest/en/html/acquisitions.html#ordering-via-edi 3. Confirm you can upload the EDI order and download the vendor invoice. 4. Apply patches, update database, and restart services. 5. Go to the EDI account you made in #1. 6. Confirm there are two new fields: Upload port and Download port, both have the value of 22. 7. Create a new EDI order. 8. Confirm you can upload the EDI order and download the vendor invoice. 9. Change the EDI account. Set the Download port = 10022. 10. Create a third EDI order. 11. Confirm you can upload the EDI order and download the vendor invoice. Sponsored-by: Waikato Institute of Technology, New Zealand Created attachment 160717 [details] [review] Bug 35724: Database changes Sponsored-by: Waikato Institute of Technology, New Zealand Created attachment 160718 [details] [review] Bug 35724: Template changes Sponsored-by: Waikato Institute of Technology, New Zealand Created attachment 160719 [details] [review] Bug 35724: Define non-standard ports for EDI SFTP in EDI accounts Not all EDI vendors accept Koha servers connecting on port 22, sometimes they require a different port to be used, so it would be helpful for librarians to be able to configure non-standard EDI SFTP ports themselves. Test plan: 1. Set EDIFACT syspref = 'Enable' and define a EDI account: Administration > Acquisition parameters > EDI accounts. 2. Create an EDI order: https://koha-community.org/manual/latest/en/html/acquisitions.html#ordering-via-edi 3. Confirm you can upload the EDI order and download the vendor invoice. 4. Apply patches, update database, and restart services. 5. Go to the EDI account you made in #1. 6. Confirm there are two new fields: Upload port and Download port, both have the value of 22. 7. Create a new EDI order. 8. Confirm you can upload the EDI order and download the vendor invoice. 9. Change the EDI account. Set the Download port = 10022. 10. Create a third EDI order. 11. Confirm you can upload the EDI order and download the vendor invoice. Sponsored-by: Waikato Institute of Technology, New Zealand Created attachment 160720 [details] [review] Bug 35724: DBIX Schema file changes Sponsored-by: Waikato Institute of Technology, New Zealand Hi all, This patchset is ready for testing. Please note, I hav checked and the changes I made to Koha/Edifact/Transport.pm are in functions not covered by unit tests: Koha::Edifact::Transport->sftp_upload() Koha::Edifact::Transport->sftp_download() The functions that call these subroutines are also not covered by unit tests: Koha::Edifact::Transport->upload_messages() Koha::Edifact::Transport->download_messages() I have, therefore, not written unit tests for this enhancement. However, if you believe unit tests are required please let me know. Created attachment 162494 [details] [review] Bug 35724: Database changes Sponsored-by: Waikato Institute of Technology, New Zealand Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 162495 [details] [review] Bug 35724: Template changes Sponsored-by: Waikato Institute of Technology, New Zealand Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 162496 [details] [review] Bug 35724: Define non-standard ports for EDI SFTP in EDI accounts Not all EDI vendors accept Koha servers connecting on port 22, sometimes they require a different port to be used, so it would be helpful for librarians to be able to configure non-standard EDI SFTP ports themselves. Test plan: 1. Set EDIFACT syspref = 'Enable' and define a EDI account: Administration > Acquisition parameters > EDI accounts. 2. Create an EDI order: https://koha-community.org/manual/latest/en/html/acquisitions.html#ordering-via-edi 3. Confirm you can upload the EDI order and download the vendor invoice. 4. Apply patches, update database, and restart services. 5. Go to the EDI account you made in #1. 6. Confirm there are two new fields: Upload port and Download port, both have the value of 22. 7. Create a new EDI order. 8. Confirm you can upload the EDI order and download the vendor invoice. 9. Change the EDI account. Set the Download port = 10022. 10. Create a third EDI order. 11. Confirm you can upload the EDI order and download the vendor invoice. Sponsored-by: Waikato Institute of Technology, New Zealand Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 162497 [details] [review] Bug 35724: DBIX Schema file changes Sponsored-by: Waikato Institute of Technology, New Zealand Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 162498 [details] [review] Bug 35724: Sync database update and kohasctructure and set to int(11) We use int(11) in the smtp_servers table for port, so I think it makes sense to use the same here too. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 162499 [details] [review] Bug 35724: (QA follow-up) Also account for port in FTP We were only passing port into SFTP connections.. now that we allow defining it, we should also pass it to FTP connections just in case someone is still using those. This patch adds such functionality and defaults to port 21 in this case when the port isn't defined. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Sensible defaults in place, unit tests passing, QA scripts happy. I'm going to PQA on this as one of the few people here that understand EDI. Thanks for the patch Alex. (In reply to Martin Renvoize from comment #15) > Sensible defaults in place, unit tests passing, QA scripts happy. > > I'm going to PQA on this as one of the few people here that understand EDI. > Thanks for the patch Alex. Thanks Martin! It looks like the code has fallbacks for the old ports to keep things working. Should we add these as a hint to the EDI account form? Pushed for 24.05! Well done everyone, thank you! (In reply to Katrin Fischer from comment #17) > It looks like the code has fallbacks for the old ports to keep things > working. Should we add these as a hint to the EDI account form? *ping* (In reply to Katrin Fischer from comment #19) > (In reply to Katrin Fischer from comment #17) > > It looks like the code has fallbacks for the old ports to keep things > > working. Should we add these as a hint to the EDI account form? > > *ping* Hi Katrin, Sorry for my delayed reply. Yes, I think that is a good idea. I will log a follow-up bug report for adding that. Alex I(In reply to Katrin Fischer from comment #19) > (In reply to Katrin Fischer from comment #17) > > It looks like the code has fallbacks for the old ports to keep things > > working. Should we add these as a hint to the EDI account form? > > *ping* I've logged a follow-up bug report for adding this: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36767 Thanks Katrin! |