classifai icon indicating copy to clipboard operation
classifai copied to clipboard

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

Open Sidsector9 opened this issue 3 years ago • 2 comments

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

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

Sidsector9 avatar Mar 06 '23 07:03 Sidsector9

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 avatar Mar 08 '23 22:03 dkotter

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.

dkotter avatar Mar 21 '23 02:03 dkotter