Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ExtensionInstallCommand more consistent with administrator #43700

Open
wants to merge 5 commits into
base: 5.2-dev
Choose a base branch
from

Conversation

brbrbr
Copy link

@brbrbr brbrbr commented Jun 25, 2024

Maybe mark this with RFC?

Summary of Issues

Deletion of package

with --path processPathInstallation would delete the package file if it's placed in the tmp_path.
There are two flaws with this:

  1. If the file is in tmp_path it is deleted, otherwise it's not. This is inconsistent.
  2. Extension:install --path` never downloads the package itself, so it should not delete with is not its.

So --path should only clean up the extracted files. (--path does not accept a URL as parameter, it never downloads a file)

Inconsistent names with the web interface

In the web interface we have 'URL', 'folder' and 'package'  (and web, that one is not applicable for the CLI)
'URL' can be found in extension:install, no problems there.

--path is slightly ambiguous, at a first glance you might expect this to point to a folder. Only after careful reading of the help information, it is clear that it must be a package. 

An option to install from a folder is missing at all

To be more consistent, extension:install should support 'folder' and 'package' as well

Summary of Changes

 - adding an option --package to point to a zip file (processPackageInstallation)
 - Adding an option --folder to point to a folder with extracted files (processFolderInstallation)
 - Modifying --path to accept a folder or a file (B/C). Delegating the work to processPackageInstallation or processFolderInstallation
 - Package installation only deletes the extracted files. Not the original package.
 - Minor changes to the help info, extension:install can update as well.- Added short versions of the parameters

Testing Instructions

--url is omitted.
Ensure the CLI command is executed with the right permissions. Installation fails otherwise without a clear messsage.

Actual result BEFORE applying this Pull Request

 - php joomla extension:install --path <zipfile> - works
 - php joomla extension:install --path <tmp_dir/zipfile> - works - file deleted
 - php joomla extension:install --path <folder> - fails

Expected result AFTER applying this Pull Request

 - php joomla extension:install --path <zipfile> - works (B/C)
 - php joomla extension:install --path <folder> - works
 - php joomla extension:install --path <tmp_dir/zipfile> - works - zipfile not deleted 
 
 - php joomla extension:install --package | -p <folder> - works
 - php joomla extension:install --folder | -f <folder> - works
 
 - php joomla extension:install --package | -p <folder> - fails
 - php joomla extension:install --folder | -f <zipfile> - fails

…nstallation)

 - Adding an option --folder to point to a folder with extracted files (processFolderInstallation)
 - Modifying --path to accept a folder or a file (B/C). Delegating the work to  processPackageInstallation or processFolderInstallation
 - Package installation only deletes the extracted files. Not the original package.
 - Changes to the help info, `extension:install` can update as well.- Added short versions of the parameters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants