Bug 36767 - Add a hint to the EDI account form that the SFTP/FTP port will fallback to port 22 if not defined
Summary: Add a hint to the EDI account form that the SFTP/FTP port will fallback to po...
Status: Needs documenting
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Matt Blenkinsop
QA Contact: Testopia
URL:
Keywords: Academy
Depends on: 35724
Blocks:
  Show dependency tree / graph
 
Reported: 2024-05-02 19:03 UTC by Alex Buckley
Modified: 2024-07-20 00:34 UTC (History)
4 users (show)

See Also:
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:

Attachments
Bug 36767: Default port should be set to 22 (1.53 KB, patch)
2024-06-14 16:13 UTC, Matt Blenkinsop 		Details | Diff | Splinter Review
Bug 36767: Add a hint to show the new default of 22 (1.32 KB, patch)
2024-06-14 16:13 UTC, Matt Blenkinsop 		Details | Diff | Splinter Review
Bug 36767: Default port should be set to 22 (1.59 KB, patch)
2024-06-14 17:30 UTC, Emily Lamancusa (emlam) 		Details | Diff | Splinter Review
Bug 36767: Add a hint to show the new default of 22 (2.18 KB, patch)
2024-06-14 17:30 UTC, Emily Lamancusa (emlam) 		Details | Diff | Splinter Review
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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.