Bug 35724 - Define non-standard port numbers for SFTP upload/download in EDI accounts
Summary: Define non-standard port numbers for SFTP upload/download in EDI accounts
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Alex Buckley
QA Contact: Martin Renvoize
URL:
Keywords:
Depends on:
Blocks: 36767
  Show dependency treegraph
 
Reported: 2024-01-08 14:29 UTC by Alex Buckley
Modified: 2024-07-25 11:23 UTC (History)
0 users

See Also:
Change sponsored?: Sponsored
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
This adds configuration options for the upload port and download port to the EDI account configuration page. If no port is added, it will keep using the default port 22.
Version(s) released in:
24.05.00


Attachments
Bug 35724: Database changes (2.37 KB, patch)
2024-01-08 16:12 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 35724: Template changes (1.48 KB, patch)
2024-01-08 16:12 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 35724: Define non-standard ports for EDI SFTP in EDI accounts (2.53 KB, patch)
2024-01-08 16:13 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 35724: Database changes (2.37 KB, patch)
2024-01-09 18:57 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 35724: Template changes (1.48 KB, patch)
2024-01-09 18:57 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 35724: Define non-standard ports for EDI SFTP in EDI accounts (4.38 KB, patch)
2024-01-09 18:58 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 35724: DBIX Schema file changes (1.79 KB, patch)
2024-01-09 18:58 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 35724: Database changes (2.52 KB, patch)
2024-02-27 11:13 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 35724: Template changes (1.54 KB, patch)
2024-02-27 11:13 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 35724: Define non-standard ports for EDI SFTP in EDI accounts (4.44 KB, patch)
2024-02-27 11:13 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 35724: DBIX Schema file changes (1.80 KB, patch)
2024-02-27 11:13 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 35724: Sync database update and kohasctructure and set to int(11) (2.99 KB, patch)
2024-02-27 11:13 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 35724: (QA follow-up) Also account for port in FTP (2.56 KB, patch)
2024-02-27 11:13 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Buckley 2024-01-08 14:29:11 UTC
We have a partner library that has one EDI vendor that will only accept Koha connecting on port 10022.

Currently Koha will only connect on port 22, so this library cannot download EDI files from this vendor.

It would be helpful if libraries could define a non-standard SFTP ports for uploading and downloading, at a EDI account level (not a global level) as different vendors may require different ports to be used.
Comment 1 Alex Buckley 2024-01-08 16:12:23 UTC
Created attachment 160644 [details] [review]
Bug 35724: Database changes

Sponsored-by: Waikato Institute of Technology, New Zealand
Comment 2 Alex Buckley 2024-01-08 16:12:38 UTC
Created attachment 160645 [details] [review]
Bug 35724: Template changes

Sponsored-by: Waikato Institute of Technology, New Zealand
Comment 3 Alex Buckley 2024-01-08 16:13:04 UTC
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
Comment 4 Alex Buckley 2024-01-09 18:57:33 UTC
Created attachment 160717 [details] [review]
Bug 35724: Database changes

Sponsored-by: Waikato Institute of Technology, New Zealand
Comment 5 Alex Buckley 2024-01-09 18:57:48 UTC
Created attachment 160718 [details] [review]
Bug 35724: Template changes

Sponsored-by: Waikato Institute of Technology, New Zealand
Comment 6 Alex Buckley 2024-01-09 18:58:03 UTC
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
Comment 7 Alex Buckley 2024-01-09 18:58:16 UTC
Created attachment 160720 [details] [review]
Bug 35724: DBIX Schema file changes

Sponsored-by: Waikato Institute of Technology, New Zealand
Comment 8 Alex Buckley 2024-01-09 20:19:39 UTC
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.
Comment 9 Martin Renvoize 2024-02-27 11:13:10 UTC
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>
Comment 10 Martin Renvoize 2024-02-27 11:13:12 UTC
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>
Comment 11 Martin Renvoize 2024-02-27 11:13:14 UTC
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>
Comment 12 Martin Renvoize 2024-02-27 11:13:17 UTC
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>
Comment 13 Martin Renvoize 2024-02-27 11:13:19 UTC
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>
Comment 14 Martin Renvoize 2024-02-27 11:13:22 UTC
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>
Comment 15 Martin Renvoize 2024-02-27 11:14:14 UTC
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.
Comment 16 Alex Buckley 2024-02-27 20:26:37 UTC
(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!
Comment 17 Katrin Fischer 2024-04-19 16:29:04 UTC
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?
Comment 18 Katrin Fischer 2024-04-19 16:32:13 UTC
Pushed for 24.05!

Well done everyone, thank you!
Comment 19 Katrin Fischer 2024-04-26 21:10:21 UTC
(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*
Comment 20 Alex Buckley 2024-05-02 18:59:13 UTC
(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
Comment 21 Alex Buckley 2024-05-02 19:03:53 UTC
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!