Skip to content

Core Profiler - replace install-async with install-activate-async #38434

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.

Sign up for GitHub

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

Jump to bottom
Merged
moon0326 merged 5 commits into from
May 26, 2023
Merged

Core Profiler - replace install-async with install-activate-async #38434

moon0326 merged 5 commits into from
May 26, 2023

Conversation

@moon0326
Copy link
Contributor

@moon0326 moon0326 commented May 24, 2023 •
edited
Loading

Changes proposed in this Pull Request:

Closes #38410

This PR replaces install-async endpoint with install-and-activate-async to activate installed plugins.

How to test the changes in this Pull Request:

  1. Make a request to POST /wp-json/wc-admin/onboarding/plugins/install-and-activate-async with the following JSON request
{
    "plugins": ["test"]
}
  1. The response should contain job_id field. Copy it.
  2. Make a request to GET /wp-json/wc-admin/onboarding/plugins/scheduled-installs/:job_id. It should return the following response.
{
    "job_id": ":job_id",
    "status": "complete",
    "plugins": {
        "test": {
            "status": "failed",
            "errors": [
                "The requested plugin `test` could not be installed. Plugin API call failed."
            ],
            "install_duration": 0
        }
    }
}
  1. Repeat steps 1 ~ 3, but with a valid plug-in name this time. When everything goes well, plugins.:name.status field value should be activated

@moon0326 moon0326 requested review from a team, chihsuan and rjchow May 24, 2023 19:56
@github-actions github-actions bot added focus: react admin [team:Ghidorah] plugin: woocommerce Issues related to the WooCommerce Core plugin. labels May 24, 2023
@github-actions
Copy link
Contributor

github-actions bot commented May 24, 2023

Hi @chihsuan, @rjchow, @woocommerce/ghidorah

Apart from reviewing the code changes, please make sure to review the testing instructions as well.

You can follow this guide to find out what good testing instructions should look like:
https://github.com/woocommerce/woocommerce/wiki/Writing-high-quality-testing-instructions

1 similar comment
@github-actions
Copy link
Contributor

github-actions bot commented May 24, 2023

Hi @chihsuan, @rjchow, @woocommerce/ghidorah

Apart from reviewing the code changes, please make sure to review the testing instructions as well.

You can follow this guide to find out what good testing instructions should look like:
https://github.com/woocommerce/woocommerce/wiki/Writing-high-quality-testing-instructions

@github-actions
Copy link
Contributor

github-actions bot commented May 24, 2023 •
edited
Loading

Test Results Summary

Commit SHA: 1e09a4d

Test 🧪Passed ✅Failed 🚨Broken 🚧Skipped ⏭️Unknown ❔Total 📊Duration ⏱️
API Tests25900202610m 57s
E2E Tests1940010020415m 55s
To view the full API test report, click here.
To view the full E2E test report, click here.
To view all test reports, visit the WooCommerce Test Reports Dashboard.

@moon0326 moon0326 mentioned this pull request May 24, 2023
Closed
chihsuan
chihsuan reviewed May 25, 2023
View reviewed changes
plugins/woocommerce/src/Admin/PluginsHelper.php Outdated
* @return bool
*/
public function install_plugins_async_callback( array $plugins, string $job_id ) {
$option_name = 'woocommerce_onboarding_plugins_install_async_' . $job_id;
Copy link
Member

@chihsuan chihsuan May 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also rename this function to install_and_activate_plugins_async_callback?

And I'm seeing this error in error.log. Should we make it static?

[25-May-2023 08:06:50 UTC] PHP Fatal error:  Uncaught TypeError: call_user_func_array(): Argument #1 ($callback) must be a valid callback, non-static method Automattic\WooCommerce\Admin\PluginsHelper::install_plugins_async_callback() cannot be called statically in /var/www/html/wp-includes/class-wp-hook.php:308
Stack trace:
#0 /var/www/html/wp-includes/class-wp-hook.php(332): WP_Hook->apply_filters('', Array)
#1 /var/www/html/wp-includes/plugin.php(565): WP_Hook->do_action(Array)
#2 /var/www/html/wp-content/plugins/woocommerce/packages/action-scheduler/classes/actions/ActionScheduler_Action.php(43): do_action_ref_array('woocommerce_plu...', Array)
#3 /var/www/html/wp-content/plugins/woocommerce/packages/action-scheduler/classes/abstracts/ActionScheduler_Abstract_QueueRunner.php(65): ActionScheduler_Action->execute()
#4 /var/www/html/wp-content/plugins/woocommerce/packages/action-scheduler/classes/ActionScheduler_QueueRunner.php(166): ActionScheduler_Abstract_QueueRunner->process_action(102, 'WP Cron')
#5 /var/www/html/wp-content/plugins/woocommerce/packages/action-scheduler/classes/ActionScheduler_QueueRunner.php(136): ActionScheduler_QueueRunner->do_batch(25, 'WP Cron')
#6 /var/www/html/wp-includes/class-wp-hook.php(308): ActionScheduler_QueueRunner->run('WP Cron')
#7 /var/www/html/wp-includes/class-wp-hook.php(332): WP_Hook->apply_filters('', Array)
#8 /var/www/html/wp-includes/plugin.php(565): WP_Hook->do_action(Array)
#9 /var/www/html/wp-cron.php(188): do_action_ref_array('action_schedule...', Array)
#10 /var/www/html/wp-includes/cron.php(886): include_once('/var/www/html/w...')
#11 /var/www/html/wp-includes/cron.php(1002): spawn_cron(1685002010.5754)
#12 /var/www/html/wp-includes/class-wp-hook.php(308): _wp_cron('')
#13 /var/www/html/wp-includes/class-wp-hook.php(332): WP_Hook->apply_filters(NULL, Array)
#14 /var/www/html/wp-includes/plugin.php(517): WP_Hook->do_action(Array)
#15 /var/www/html/wp-settings.php(645): do_action('wp_loaded')
#16 /var/www/html/wp-config.php(258): require_once('/var/www/html/w...')
#17 /var/www/html/wp-load.php(50): require_once('/var/www/html/w...')
#18 /var/www/html/wp-blog-header.php(13): require_once('/var/www/html/w...')
#19 /var/www/html/index.php(17): require('/var/www/html/w...')
#20 {main}

Copy link
Contributor Author

@moon0326 moon0326 May 25, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chihsuan Thanks!

I've renamed action and callback in 1e09a4d

Interesting..I did not get the error in my local. I've changed callback to static in the commit 👍

chihsuan
chihsuan approved these changes May 26, 2023
View reviewed changes
Copy link
Member

@chihsuan chihsuan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM and tested well. 👍

@moon0326 moon0326 merged commit f3a5158 into trunk May 26, 2023
@moon0326 moon0326 deleted the update/38410-update-install-async-to-activate-plugins branch May 26, 2023 20:34
@github-actions github-actions bot added this to the 7.9.0 milestone May 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@chihsuan chihsuan chihsuan approved these changes

@rjchow rjchow Awaiting requested review from rjchow

Assignees

No one assigned

Labels

plugin: woocommerce Issues related to the WooCommerce Core plugin.

Projects

None yet

Milestone

7.9.0

Development

Successfully merging this pull request may close these issues.

Core Profiler - update install-async endpoint to activate plugins

3 participants

@moon0326 @chihsuan