Create AJAX runtime environment setup/cleanup

[jjgrainger]

• Addresses acceptance criteria for AJAX Runtime Environment Setup/Cleanup #86

Closes #86

[jjgrainger]

Hi @felixarntz

After setting this up I've been facing errors where the wp_get_cache function do not exist. From what I can tell this is due to setting up and preparing the Runners in the object-cache.php file, in particular when getting the plugin parameter and using the Plugin_Request_Utility::get_plugin_basename_from_input which calls get_plugins() that uses wp_get_cache.

The reason these functions are called is because we need to create the Check_Context for the Checks class in order to return the array of checks to then work out if there is a runtime check and setup the environment.

I'm wondering what the best path forward is here? Whether we refactor the Checks class so we can return the array of Checks without needing a Check_Context (though I think we would still need to test if the plugin is active based on the logic we are trying to run) or if we simply could require_once wp-includes/cache.php before the get_plugins call is made?

Happy to discuss this over a call as I feel this is somewhat related to my other comment - #100 (comment)

Thanks

[felixarntz]

@jjgrainger Good catch!

Maybe we can get around this without any notable refactoring, just by executing that logic later? While we have to call the code in object-cache.php, it doesn't necessarily have to be executed right away.

Potentially, we can hook the code into the muplugins_loaded action? That action should still be in time for us to do the other things, like filter out plugins etc. If there is an earlier action that we can reasonably use, that would be even better, but I'm not sure there is.

[jjgrainger]

Thanks @felixarntz

I've updated the PR to use the muplugins_loaded action to initialize the runner and this is ready for review.

After updating this I came across a different error where the WP_PLUGIN_CHECK_MAIN_FILE and WP_PLUGIN_CHECK_PLUGIN_DIR_PATH constants were not defined in the Universal_Runtime_Preparation.

I've put in a small fix to get around this but I'm not sure this is the best approach but we can discuss as part of the code review.

[felixarntz]

@jjgrainger Left a few comments, some of them similar to what I mentioned on #100.

[felixarntz]

I see you're returning all these values from every AJAX endpoint just to be able to chain the AJAX requests in the JS file. I think that makes for a confusing architecture and requires us to include unnecessary data in the AJAX responses. No need to fix that here since it's already happening in existing code, but I think that will be something we should resolve as part of the architecture review.

[jjgrainger]

Thanks @felixarntz I agree and let's tackle this as part of the architectural review.

[felixarntz]

See my comment below, rather than chaining these and thereby forcing every AJAX response to include its request parameters to pass it on to the next AJAX request, I think we should decouple them.

We can then change the JS functions to accept dedicated parameters for the things that each function needs rather than the generic data object from the previous AJAX response.

But like my comment above, let's handle this refactoring in a separate PR.

[felixarntz]

@jjgrainger Almost good to go, just one thing that is off in the FormData set, and a few nit-picks.

[felixarntz]

@jjgrainger One last question :)

[felixarntz]

Thanks @jjgrainger!

[spacedmonkey]

This logic is inverted compared to https://github.com/10up/plugin-check/pull/101/files#diff-35a6c4633cd89c9ad66535db741d064ff3f730cf69ea87d347d8e250544fff1eR85

[spacedmonkey]

LGTM

[felixarntz]

@jjgrainger Now that #100 was merged, there's a merge conflict here, could you look into that please?

[jjgrainger]

Thanks @felixarntz @spacedmonkey I've addressed the conflicts and will merge now