Bug 36767

Summary:	Add a hint to the EDI account form that the SFTP/FTP port will fallback to port 22 if not defined
Product:	Koha	Reporter:	Alex Buckley <alexbuckley>
Component:	Acquisitions	Assignee:	Matt Blenkinsop <matt.blenkinsop>
Status:	Needs documenting ---	QA Contact:	Testopia <testopia>
Severity:	enhancement
Priority:	P5 - low	CC:	david, emily.lamancusa, lucas, matt.blenkinsop
Version:	Main	Keywords:	Academy
Hardware:	All
OS:	All
Change sponsored?:	---	Patch complexity:	---
Documentation contact:		Documentation submission:
Text to go in the release notes:	This fixes an error when creating an EDI account if you don't enter the upload and download ports. The port numbers should have defaulted to port 22, but didn't - this generated an error when saving. It also adds a hint for the input fields to say that the port will default to 22 if not set.	Version(s) released in:	24.11.00
Circulation function:
Bug Depends on:	35724
Bug Blocks:
Attachments:	Bug 36767: Default port should be set to 22 Bug 36767: Add a hint to show the new default of 22 Bug 36767: Default port should be set to 22 Bug 36767: Add a hint to show the new default of 22

Description Alex Buckley 2024-05-02 19:03:08 UTC

In bug 35724 we added the ability for users to define a non-standard port for SFTP/FTP via 'upload port' and 'download port' input boxes on the EDI account form. 

In the code, we fallback to port 22 if the user has not defined the upload and download ports. 

We should add a hint to the EDI account form that Koha will fallback to port 22 if those input fields are not set.

Comment 1 David Nind 2024-06-08 05:59:28 UTC

I was going to add a patch for this.

However, when adding a new EDI account I get an error trace if values are not entered in both the upload or download port:

DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st execute failed: Incorrect integer value: '' for column `koha_kohadev`.`vendor_edi_accounts`.`download_port` at row 1 at /kohadevbox/koha/admin/edi_accounts.pl line 100
 at /usr/share/perl5/DBIx/Class/Exception.pm line 77

So it seems that there is no "fall back" option if these fields are not defined, or something else is causing this error.

In addition, maybe some of the fields should be marked as mandatory, like the remote host?

Comment 2 Matt Blenkinsop 2024-06-14 16:13:12 UTC

Created attachment 167749 [details] [review]
Bug 36767: Default port should be set to 22

This patch sets a default value for the EDI account form for the upload and download ports

Test plan:
1) Try to create an EDI account without setting the upload or download ports
2) The form will crash
3) Apply patch and restart_all
4) Try again, the form should submit
5) In your new EDI account, you should see the value of 22 for both ports

Comment 3 Matt Blenkinsop 2024-06-14 16:13:15 UTC

Created attachment 167750 [details] [review]
Bug 36767: Add a hint to show the new default of 22

Comment 4 Matt Blenkinsop 2024-06-14 16:14:02 UTC

Spotted the same error that David mentions whilst signing off another EDI bug - this seemed the logical place to add a patch

Comment 5 Emily Lamancusa (emlam) 2024-06-14 17:30:10 UTC

Created attachment 167752 [details] [review]
Bug 36767: Default port should be set to 22

This patch sets a default value for the EDI account form for the upload and download ports

Test plan:
1) Try to create an EDI account without setting the upload or download ports
2) The form will crash
3) Apply patch and restart_all
4) Try again, the form should submit
5) In your new EDI account, you should see the value of 22 for both ports

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>

Comment 6 Emily Lamancusa (emlam) 2024-06-14 17:30:13 UTC

Created attachment 167753 [details] [review]
Bug 36767: Add a hint to show the new default of 22

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>

Comment 7 Katrin Fischer 2024-06-27 15:17:06 UTC

Picked from QA queue for direkt push to main.

Comment 8 Katrin Fischer 2024-06-27 15:26:33 UTC

Pushed for 24.11!

Well done everyone, thank you!

Comment 9 Lucas Gass (lukeg) 2024-07-19 15:49:35 UTC

Enhancement will not be backported to 24.05.x

Comment 10 David Nind 2024-07-19 23:00:43 UTC

I think this is needs backporting to 24.05.x. 

This is because it is both a bug fix and an enhancement:
1. When adding an EDIT account, it fixes a DBIx error if you don't enter a port number for the upload and download port fields (it is supposed to default to port 22, but doesn't). [1]
2. It adds some hint text for the upload and download port fields (which is an enhancement).

In the meantime, I've changed the status to needs documenting, as the manual needs updating for any screenshots and other text changes required.


[1] Error that needs fixing for 24.05.x (which is currently 24.05.01)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

1. Enable the EDIFACT system preference.
2. Add an EDI account: Administration > Acqusition parameters > EDI accounts > + New account
   . Don't add any values for the fields 'Upload port' and 'Download ports'
   . Press 'Submit'
   Result==> You get an error:
      DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st execute failed: Incorrect integer value: '' for column `koha_kohadev`.`vendor_edi_accounts`.`download_port` at row 1 at /kohadevbox/koha/admin/edi_accounts.pl line 100 at /usr/share/perl5/DBIx/Class/Exception.pm line 77
3. Note that this is supposed to default to port 22 if no values are entered (see bug 35724), but it doesn't and causes the error.