User tests: Successful: Unsuccessful:
Pull Request for Issue # .
This PR removes FTP support for com_joomlaupdate:
Status | New | ⇒ | Pending |
Category | ⇒ | Administration com_joomlaupdate Language & Strings JavaScript Repository NPM Change |
Labels |
Added:
?
NPM Resource Changed
?
|
@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.
@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',
Labels |
Added:
?
|
@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.
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
@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.
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
Labels |
Added:
?
Removed: ? |
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).
@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
Labels |
Added:
?
Removed: ? |
I have tested this item
Both tests made on win10.
Labels |
Added:
?
Removed: ? |
I have tested this item
Testet again both variants on win10, xampp with php8 and error-reporting "maximum". No errors / warnings / notices.
I have tested this item
Tested with Linux server. Knowing that @chmst has used Windows, we have tested both kinds of environments now.
Status | Pending | ⇒ | Ready to Commit |
RTC
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 .
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.
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.
@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
@joomdonation @richard67 can we open an issue for refactoring this file
@dgrammatiko Which file? The build/media_source/com_joomlaupdate/js/default.es6.js
?
@dgrammatiko Which file?
The restore.php
The
restore.php
@dgrammatiko Thanks. Me or @joomdonation will make an issue when this PR has been merged.
@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.
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: ? |
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: