10up / classifai

Supercharge WordPress Content Workflows and Engagement with Artificial Intelligence.
https://classifaiplugin.com
GNU General Public License v2.0
581 stars 53 forks source link

Refactor ClassifAI so that it's easier to add more Providers under a Service. #404

Closed Sidsector9 closed 8 months ago

Sidsector9 commented 1 year ago

Is your enhancement related to a problem? Please describe.

At the moment, the plugin offers services like Image Processing, Language Processing, etc. But the problem is when you add multiple Providers to a Service, accessing data for that Provider is not convenient.

See this line for example:

https://github.com/10up/classifai/blob/1c1a180913f1c2f03f8bae4dd510d56e45ec9522/includes/Classifai/Helpers.php#L47

The direct index access of 0 makes it impossible to access settings for Providers other than the one at 0.

Helper functions such as:

https://github.com/10up/classifai/blob/1c1a180913f1c2f03f8bae4dd510d56e45ec9522/includes/Classifai/Helpers.php#L264

...have utility outside of Watson's Language Understanding feature. In future, it we add let say, Microsoft's Text to Speech feature which is a Provider for the Language Processing Service, then the current helper get_supported_post_statuses() will not work even though it is required by the new Provider. This will force developers to add helpers for each Provider which will lead to code duplication.

Designs

No response

Describe alternatives you've considered

  1. Identify existing helper functions that can be useful for multiple Providers.
  2. Refactor (if possible) or deprecate in favour of functions that are generic and available to every Provider that needs it.

Code of Conduct

dkotter commented 1 year ago

I started some work on this in #405, see https://github.com/10up/classifai/pull/405/files#diff-502f71f01635ed43aac831e9a91f0097f7bd09bec4e36b7dc6b52af2b694a7a0R30. My plan is to clean this up even more once that PR is merged

dkotter commented 1 year ago

Note that PR is now merged and I've opened up a simple first step follow up in #413. That said, I think there will still be more needed, as I've seen other areas of the code that assume we only have a single provider.