? NPM Resource Changed ? ? ? Pending

User tests: Successful: Unsuccessful:

avatar joomdonation
joomdonation
30 Apr 2021

Pull Request for Issue # .

Summary of Changes

This PR removes FTP support for com_joomlaupdate:

  • Remove Installation method option on Joomla update screen. We decided to drop support for FTP, so Installation method is now default to Write files directly and no need to be selected during update process.
  • Remove code related to FTP from within com_joomlaupdate
  • Remove some classes from restore.php due to removal of FTP support.

Testing Instructions

Test Upload & Update

  1. Use Joomla 4 nightly build.
  2. Download update package for this PR at https://ci.joomla.org/artifacts/joomla/joomla-cms/4.0-dev/33437/downloads/43198/Joomla_4.0.0-beta8-dev+pr.33437-Development-Update_Package.zip go to System -> Update --> Joomla and install that update package to apply changes from this PR to your site.
  3. Download Joomla 4 nightly build update package from this link https://developer.joomla.org/nightlies/Joomla_4.0.0-beta8-dev-Development-Update_Package.zip , go to System -> Update -> Joomla , upload and install that update package, make sure update processed properly (without any error)

Test Live Update

  1. Download update package for this PR at https://ci.joomla.org/artifacts/joomla/joomla-cms/4.0-dev/33437/downloads/43198/Joomla_4.0.0-beta8-dev+pr.33437-Development-Update_Package.zip, go to System -> Update --> Joomla and install that update package to re-apply changes from this PR to your site (because the changes were lost when you install Joomla 4 nightly build update package from previous test)
  2. Login to administrator area of the site, access to System -> Update -> Joomla. Click on Options button in the toolbar. On the next screen, Set:
  1. Look at Live Update tab. There should be a new update found. At the bottom, you will see Install the Update button. Click on that button to process the update and make sure it is success.
avatar joomdonation joomdonation - open - 30 Apr 2021
avatar joomdonation joomdonation - change - 30 Apr 2021
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 30 Apr 2021
Category Administration com_joomlaupdate Language & Strings JavaScript Repository NPM Change
4d0da9a 30 Apr 2021 avatar joomdonation CS
avatar joomdonation joomdonation - change - 30 Apr 2021
Labels Added: ? NPM Resource Changed ?
avatar dgrammatiko
dgrammatiko - comment - 30 Apr 2021

At some point, someone needs to bite the bullet and rewrite restore.php. The funky syntax ###{someJSON: []}### frankly is unacceptable, we could ensure the JSON responses by either:

  • own exception/error handlers
  • try/catch
  • @ mute everything
  • set error level to none
  • ob_start, ob_clean
  • and probably a good amount of other ways that I'm not even aware...
avatar joomdonation joomdonation - change - 1 May 2021
The description was changed
avatar joomdonation joomdonation - edited - 1 May 2021
avatar joomdonation
joomdonation - comment - 1 May 2021

@dgrammatiko We could do that at a later time. For this PR, I only removed some classes which are not needed anymore in restore.php file only. This PR is now ready for testing.

avatar joomdonation joomdonation - change - 1 May 2021
The description was changed
avatar joomdonation joomdonation - edited - 1 May 2021
avatar dgrammatiko
dgrammatiko - comment - 1 May 2021

@joomdonation please also remove the redundant strings ~3108:

		'WRITE_FTP'                       => 'Use FTP for all files',
		'WRITE_SFTP'                      => 'Use SFTP for all files',
		'FTP_HOST'                        => '(S)FTP host name:',
		'FTP_PORT'                        => '(S)FTP port:',
		'FTP_FTPS'                        => 'Use FTP over SSL (FTPS)',
		'FTP_PASSIVE'                     => 'Use FTP Passive Mode',
		'FTP_USER'                        => '(S)FTP user name:',
		'FTP_PASS'                        => '(S)FTP password:',
		'FTP_DIR'                         => '(S)FTP directory:',
		'FTP_TEMPDIR'                     => 'Temporary directory:',
		'FTP_CONNECTION_OK'               => 'FTP Connection Established',
		'SFTP_CONNECTION_OK'              => 'SFTP Connection Established',
		'FTP_CONNECTION_FAILURE'          => 'The FTP Connection Failed',
		'SFTP_CONNECTION_FAILURE'         => 'The SFTP Connection Failed',
		'FTP_TEMPDIR_WRITABLE'            => 'The temporary directory is writable.',
		'FTP_TEMPDIR_UNWRITABLE'          => 'The temporary directory is not writable. Please check the permissions.',
		'FTPBROWSER_ERROR_HOSTNAME'       => "Invalid FTP host or port",
		'FTPBROWSER_ERROR_USERPASS'       => "Invalid FTP username or password",
		'FTPBROWSER_ERROR_NOACCESS'       => "Directory doesn't exist or you don't have enough permissions to access it",
		'FTPBROWSER_ERROR_UNSUPPORTED'    => "Sorry, your FTP server doesn't support our FTP directory browser.",
		'FTPBROWSER_LBL_GOPARENT'         => "<up one level>",
		'FTPBROWSER_LBL_INSTRUCTIONS'     => 'Click on a directory to navigate into it. Click on OK to select that directory, Cancel to abort the procedure.',
		'FTPBROWSER_LBL_ERROR'            => 'An error occurred',
		'SFTP_NO_SSH2'                    => 'Your web server does not have the SSH2 PHP module, therefore can not connect to SFTP servers.',
		'SFTP_NO_FTP_SUPPORT'             => 'Your SSH server does not allow SFTP connections',
		'SFTP_WRONG_USER'                 => 'Wrong SFTP username or password',
		'SFTP_WRONG_STARTING_DIR'         => 'You must supply a valid absolute path',
		'SFTPBROWSER_ERROR_NOACCESS'      => "Directory doesn't exist or you don't have enough permissions to access it",
		'SFTP_COULDNT_UPLOAD'             => 'Could not upload %s',
		'SFTP_CANT_CREATE_DIR'            => 'Could not create directory %s',
		'UI-ROOT'                         => '<root>',
		'CONFIG_UI_FTPBROWSER_TITLE'      => 'FTP Directory Browser',
		'FTP_BROWSE'                      => 'Browse',
avatar joomdonation joomdonation - change - 1 May 2021
Labels Added: ?
avatar joomdonation
joomdonation - comment - 1 May 2021

@dgrammatiko Thanks. I removed the suggested language items/constants and more. There are other items which could be removed, too, but to be safe, I just remove FTP related language items for now.

avatar joomdonation joomdonation - change - 1 May 2021
The description was changed
avatar joomdonation joomdonation - edited - 1 May 2021
avatar dgrammatiko
dgrammatiko - comment - 1 May 2021

I just remove FTP related language items for now.

Since you removed the JPA support you can also safely remove the AES part AFAIK the Joomla zip files are not password protected... (but it's up to you how much redundancy you want to eliminate here ? )

avatar joomdonation
joomdonation - comment - 1 May 2021

@dgrammatiko Guess I should stop modify that file for now. In the future, when we have more time, we can review it again and remove other un-used code from there.

avatar PhilETaylor
PhilETaylor - comment - 2 May 2021

This branch has conflicts that must be resolved
Conflicting files
administrator/components/com_joomlaupdate/tmpl/joomlaupdate/default_update.php
administrator/language/en-GB/com_joomlaupdate.ini

avatar joomdonation joomdonation - change - 3 May 2021
Labels Added: ?
Removed: ?
9502eeb 3 May 2021 avatar joomdonation CS
avatar Bakual
Bakual - comment - 3 May 2021

That file certainly has unused stuff in it. As you see in its header, it comes from Akeeba Backup and was "donated" und supervised back then by Nicholas.
In the last years nobody wanted to touch it, and when we once touched it - something broke afair :-)

So if someone is brave enough to improve it, go ahead. But make sure you test it in all the many possible ways an upgrade can be done (and break).

avatar joomdonation
joomdonation - comment - 3 May 2021

@Bakual As of right now, I only removed the un-used classes and do not make any modifications to the code, so it is safe to remove. If you worry that it is not working, I can restore it to how it was. But I would say that we should remove un-used classes to make the file smaller, easier to maintain and also faster for PHP to parse.

For testing, we only support update during direct method now, so if update works, then we are fine

avatar joomdonation joomdonation - change - 4 May 2021
The description was changed
avatar joomdonation joomdonation - edited - 4 May 2021
avatar joomdonation joomdonation - change - 4 May 2021
The description was changed
avatar joomdonation joomdonation - edited - 4 May 2021
avatar joomdonation joomdonation - change - 5 May 2021
Labels Added: ?
Removed: ?
avatar joomdonation joomdonation - change - 5 May 2021
The description was changed
avatar joomdonation joomdonation - edited - 5 May 2021
avatar chmst chmst - test_item - 5 May 2021 - Tested successfully
avatar chmst
chmst - comment - 5 May 2021

I have tested this item successfully on f78f877

Both tests made on win10.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33437.

avatar joomdonation joomdonation - change - 7 May 2021
Labels Added: ?
Removed: ?
avatar joomdonation joomdonation - change - 7 May 2021
The description was changed
avatar joomdonation joomdonation - edited - 7 May 2021
avatar chmst chmst - test_item - 7 May 2021 - Tested successfully
avatar chmst
chmst - comment - 7 May 2021

I have tested this item successfully on 1856c52

Testet again both variants on win10, xampp with php8 and error-reporting "maximum". No errors / warnings / notices.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33437.

avatar joomdonation joomdonation - change - 8 May 2021
The description was changed
avatar joomdonation joomdonation - edited - 8 May 2021
avatar richard67 richard67 - test_item - 8 May 2021 - Tested successfully
avatar richard67
richard67 - comment - 8 May 2021

I have tested this item successfully on 1856c52

Tested with Linux server. Knowing that @chmst has used Windows, we have tested both kinds of environments now.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33437.

avatar richard67 richard67 - change - 8 May 2021
Status Pending Ready to Commit
avatar richard67
richard67 - comment - 8 May 2021

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33437.

avatar richard67
richard67 - comment - 8 May 2021

P.S.: In addition to the testing instructions, I've also tested that updating works when the administrator/components/com_joomlaupdate/src/Helper/Select.php deleted by this PR really has been removed from the file system. The described testing with use of the update package built for this PR for the starting condition will not do that because on update file is not removed. This will be added with my PR #33646 .

avatar joomdonation
joomdonation - comment - 8 May 2021

Thanks @richard67 . Just for information, that Select.php just uses to render Installation Method options (has 3 options before FTP removal) and it is not needed now.

avatar richard67
richard67 - comment - 8 May 2021

Thanks @richard67 . Just for information, that Select.php just uses to render Installation Method options (has 3 options before FTP removal) and it is not needed now.

I know. With my test I wanted to make sure that it is really not needed anymore.

avatar dgrammatiko
dgrammatiko - comment - 8 May 2021

@joomdonation @richard67 can we open an issue for refactoring this file as a follow up once this PR is merged? The reason is that after the removal of the support for .jpa files there's a lot of dead code (all the AES stuff), the JSON encoding is at least weird and also most of the functions here can be enormously simplified. Thanks

avatar richard67
richard67 - comment - 8 May 2021

@joomdonation @richard67 can we open an issue for refactoring this file

@dgrammatiko Which file? The build/media_source/com_joomlaupdate/js/default.es6.js?

avatar dgrammatiko
dgrammatiko - comment - 8 May 2021

@dgrammatiko Which file?

The restore.php

avatar richard67
richard67 - comment - 8 May 2021

The restore.php

@dgrammatiko Thanks. Me or @joomdonation will make an issue when this PR has been merged.

avatar joomdonation
joomdonation - comment - 8 May 2021

@dgrammatiko We actually only support restore from zip format, see https://github.com/joomla/joomla-cms/blob/4.0-dev/administrator/components/com_joomlaupdate/src/Model/UpdateModel.php#L536 (before this PR)

The class AKUnarchiverJPA however, still be there. We do not use it, but class AKUnarchiverZIP extends AKUnarchiverJPA, thus AKUnarchiverJPA could not be removed.

About refactoring restore.php file, let's do it in a later time when we are ready. The code there is quite complicated, I haven't read and understand the how workflow/process there yet. Also, it is hard for us to test it on different environments with all possible cases, so it is quite dangerous to make the change. Maybe we can try to do that with 4.1 when we are less busy.

avatar rdeutz rdeutz - change - 9 May 2021
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2021-05-09 11:31:18
Closed_By rdeutz
Labels Added: ? ?
Removed: ?
avatar rdeutz rdeutz - close - 9 May 2021
avatar rdeutz rdeutz - merge - 9 May 2021

Add a Comment

Login with GitHub to post a comment