Ensure we don't send unnecessary requests to the Watson API

[dkotter]

Describe the bug

While testing #405 I noticed I was getting HTTP requests to the Watson API even when on the OpenAI settings page. In investigating further, I was seeing requests to this API on every single page load, including the front-end. This can result in potentially thousands of extra API requests on sites with lots of traffic.

In investigating this, the issue is in the can_register method of the NLU class. The can_register method for each Provider is called on init, so this code runs on all requests. The problem in the NLU class is we make an authentication request within can_register, so we hit the API on every single request.

This may have just been an oversight when this was built or it may have been done purposely to ensure we always have a valid connection but I would classify this as a bug that we need to fix. I would suggest we change how this works to match what we do for ComputerVision and ChatGPT, where we check these credentials when settings are saved and store the result of that, which we then check in the can_register methods.

The only downside I can think of is there is a scenario where credentials are valid when they are saved but later an API key may be revoked. Checking credentials more often allows us to alert the user sooner, though I don't think we are actually outputting any error message at the moment if that happens. So I don't think there's any issue with making this change.

Steps to Reproduce

Install a plugin like Query Monitor and inspect the HTTP requests that are made on any page load

Screenshots, screen recording, code snippet

No response

Environment information

No response

WordPress information

No response

Code of Conduct

• I agree to follow this project's Code of Conduct

[benlk]

Other Providers have an is_configured() method unmodified from parent::is_configured(), but NLU replaces the parent method with a check to see whether the plugin itself is configured?

classifai/includes/Classifai/Providers/Watson/NLU.php

Lines 903 to 910 in ae7f106

	/**
	* Returns whether the provider is configured or not.
	*
	* @return bool
	*/
	public function is_configured() {
	return get_option( 'classifai_configured', false );
	}

It seems like we could get rid of this is_configured and let NLU fall back to the parent is_configured():

classifai/includes/Classifai/Providers/Provider.php

Lines 384 to 398 in ae7f106

	/**
	* Returns whether the provider is configured or not.
	*
	* @return bool
	*/
	public function is_configured() {
	$settings = $this->get_settings();
	$is_configured = false;
	if ( ! empty( $settings ) && ! empty( $settings['authenticated'] ) ) {
	$is_configured = true;
	}
	return $is_configured;
	}

Then NLU's can_register() becomes a wrapper around NLU's is_configured().

If this sounds like a good approach, I'll update my PR to match.

[dkotter]

@benlk The is_configured method was just barely added in the last release (see https://github.com/10up/classifai/pull/411/files#diff-39e68d5df2219939bc675114fe73e5922f0ef0ccd6c13dee7fc500ba34c8afb8R389) so I don't believe any providers are currently taking advantage of that (I believe only the new user onboarding is using that).

That said, one of my main goals over the last month or two has been trying to clean up the codebase and bring alignment in approaches. The way NLU is set up differs from our other providers and the goal is to make those as similar as possible. So this is a great opportunity to help with that.

I'd suggest looking to use the is_configured method in all providers can_register method. This should be fairly plug-and-play other than for NLU, as we'll need to modify how we save settings to set the authenticated property (right now looks like we store that in an option).

[benlk]

Next steps:

• one of these:
  • change how NLU stores its settings to save the authenticated property, or
  • change NLU's is_configured method to check the custom place where authenticated is saved
  • save NLU's authenticated in both the current place and the old place, then write the is_configured method to check both places, with an eye to deprecating the old method
• modify can_register() to wrap the customized is_configured()