-
Notifications
You must be signed in to change notification settings - Fork 9
Add admin-tools Command #2
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
Conversation
Signed-off-by: Riddhesh Sanghvi <riddheshsanghvi96@gmail.com>
Signed-off-by: Riddhesh Sanghvi <riddheshsanghvi96@gmail.com>
Signed-off-by: Riddhesh Sanghvi <riddheshsanghvi96@gmail.com>
Signed-off-by: Riddhesh Sanghvi <riddheshsanghvi96@gmail.com>
Signed-off-by: Riddhesh Sanghvi <riddheshsanghvi96@gmail.com>
Signed-off-by: Riddhesh Sanghvi <riddheshsanghvi96@gmail.com>
Signed-off-by: Riddhesh Sanghvi <riddheshsanghvi96@gmail.com>
Signed-off-by: Riddhesh Sanghvi <riddheshsanghvi96@gmail.com>
Signed-off-by: Riddhesh Sanghvi <riddheshsanghvi96@gmail.com>
Update move config function
Signed-off-by: Riddhesh Sanghvi <riddheshsanghvi96@gmail.com>
Switch from composer install to update Signed-off-by: Riddhesh Sanghvi <riddheshsanghvi96@gmail.com>
Signed-off-by: Riddhesh Sanghvi <riddheshsanghvi96@gmail.com>
Signed-off-by: Riddhesh Sanghvi <riddheshsanghvi96@gmail.com>
Signed-off-by: Riddhesh Sanghvi <riddheshsanghvi96@gmail.com>
Signed-off-by: Riddhesh Sanghvi <riddheshsanghvi96@gmail.com>
Signed-off-by: Riddhesh Sanghvi <riddheshsanghvi96@gmail.com>
.gitignore
Outdated
| @@ -0,0 +1,11 @@ | |||
| .DS_Store | |||
| wp-cli.local.yml | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can remove this now from all repo. We don't have wp-cli.local.yml. Maybe check if we can have ee-cli.local.yml.
src/Admin_Tools_Command.php
Outdated
|
|
||
| class Admin_Tools_Command extends EE_Command { | ||
|
|
||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extra line?
src/Admin_Tools_Command.php
Outdated
| $this->fs->mkdir( ADMIN_TOOL_DIR ); | ||
| } | ||
|
|
||
| $tools = json_decode( file_get_contents( ADMIN_TOOLS_FILE ), true ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a couple of things here we can take care of.
- if we are expecting the ADMIN_TOOLS_FILE file to be .json file we can check the file extension.
- check if
file_get_contents( ADMIN_TOOLS_FILE )is not empty if it is do not proceed further just add an error message. - Check if json_decode is done correctly ( it will fail if there is invalid JSON formate data ) After this line you can check
if ( json_last_error() != JSON_ERROR_NONE )and send a error message/exception. - And one last thing if json data is valid but empty
{}then you can add a check for$toolsif it's empty add a message/exception.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will update according to this 👍
src/Admin_Tools_Command.php
Outdated
| if ( ! $this->is_installed( $tool ) ) { | ||
| EE::log( "Installing $tool" ); | ||
| $tool_path = ADMIN_TOOL_DIR . '/' . $tool; | ||
| call_user_func_array( [ $this, "install_$tool" ], [ $data, $tool_path ] ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think here we need to check method_exists since the $tool can come from json file which can be updated from ADMIN_TOOLS_FILE constant.
| $this->fs->dumpFile( $config_file, file_get_contents( ADMIN_TEMPLATE_ROOT . '/' . $template_file ) ); | ||
| } | ||
|
|
||
| private function composer_install( $tool_path ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
function doc?
| @@ -0,0 +1,70 @@ | |||
| <?php | |||
|
|
|||
| function populate_site_info() { | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add comments in this file where needed. I believe this will be copied paste from mustache template as .php file and it won't have any comment.
templates/index.mustache
Outdated
|
|
||
| function select( $columns = array(), $where = array(), $table_name = 'services', $limit = null ) { | ||
|
|
||
| $db_path = '/opt/easyengine/ee.sqlite'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can't we read this path from env variables?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No. EE_CONF_ROOT is a constant defined in EE core. It is not present as an env variable which can be used from index.php of admin-tools.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean we can keep it in Env variable and then use it when needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay 👍
templates/index.mustache
Outdated
| unset( $services['site_url'] ); | ||
|
|
||
|
|
||
| $scan = scandir( '/var/www/htdocs/ee-admin' ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can't we read this path from env variables?
Signed-off-by: Riddhesh Sanghvi <riddheshsanghvi96@gmail.com>
Signed-off-by: Riddhesh Sanghvi <riddheshsanghvi96@gmail.com>
Signed-off-by: Riddhesh Sanghvi <riddheshsanghvi96@gmail.com>
Signed-off-by: Riddhesh Sanghvi <riddheshsanghvi96@gmail.com>
Signed-off-by: Riddhesh Sanghvi <riddheshsanghvi96@gmail.com>
Signed-off-by: Riddhesh Sanghvi <riddheshsanghvi96@gmail.com>
Signed-off-by: Riddhesh Sanghvi <riddheshsanghvi96@gmail.com>
Signed-off-by: Riddhesh Sanghvi <riddheshsanghvi96@gmail.com>
Signed-off-by: Riddhesh Sanghvi <riddheshsanghvi96@gmail.com>
Closes: #1
Closes: EasyEngine/easyengine#1118
Functions implemented so far: