woocommerce / woocommerce-android

WooCommerce Android app
https://www.woocommerce.com/mobile
GNU General Public License v2.0
277 stars 135 forks source link

Enable Blaze feature when Blaze plugin is installed and Jetpack CP active #12146

Closed JorgeMucientes closed 4 weeks ago

JorgeMucientes commented 2 months ago

Blaze standalone plugin has been released. With it, we now don't need a full Jetpack connection to create campaigns. In Android 2 changes are needed to support this:

Update isBlazeEnabled use case logic

Enable Blaze for Jetpack CP connection when Blaze plugin is installed. Currently the logic to determine if Blaze is enabled for a given site is the following:

    suspend operator fun invoke(): Boolean = selectedSite.getIfExists()?.isAdmin ?: false &&
        selectedSite.connectionType == SiteConnectionType.Jetpack &&
        selectedSite.getIfExists()?.canBlaze ?: false &&
        isRemoteFeatureFlagEnabled(WOO_BLAZE)

We need to update the condition selectedSite.connectionType == SiteConnectionType.Jetpack to:

 selectedSite.connectionType == SiteConnectionType.Jetpack || 
 (isBlazePluginInstalled() && selectedSite.connectionType == SiteConnectionType.JetpackConnectionPackage)

Update media picker logic

When Blaze plugin is used, a Jetpack CP connection is established. This connection results in attempting to fetch media from WordPress media lib using mMediaXmlrpcClient.fetchMedia

MediaStore.Kt

    private void performFetchMedia(@NonNull MediaPayload payload) {
        if (payload.media == null) {
            // null or empty media list -or- list contains a null value
            notifyMediaError(MediaErrorType.NULL_MEDIA_ARG, MediaAction.FETCH_MEDIA, null);
            return;
        }

        if (payload.site.isUsingWpComRestApi()) {
            mMediaRestClient.fetchMedia(payload.site, payload.media);
        } else {
            mMediaXmlrpcClient.fetchMedia(payload.site, payload.media);
        }
    }

And for some reason this code is resulting in the following error:

ipptesting.mystagingwebsite.com/xmlrpc.php POST 200

<?xml version="1.0" encoding="UTF-8"?>
<methodResponse>
  <fault>
    <value>
      <struct>
        <member>
          <name>faultCode</name>
          <value><int>-32700</int></value>
        </member>
        <member>
          <name>faultString</name>
          <value><string>parse error. not well formed</string></value>
        </member>
      </struct>
    </value>
  </fault>
</methodResponse>

I'm not sure why this error happens but I don't think we need to fetch the media from WP media library when we already have the URL for the resource. Passing the URL for the resource should be enough for creating the Blaze campaign.

dangermattic commented 2 months ago

Thanks for reporting! šŸ‘

hichamboushaba commented 2 months ago

I don't think we need to fetch the media from WP media library when we already have the URL for the resource. Passing the URL for the resource should be enough for creating the Blaze campaign.

@JorgeMucientes we fetch the media because the API expects also the mimeType of the media, fetching the media returns a MediaModel which has all the needed information. I think we can quickly fix this on the FluxC part by implementing a way to fetch the media using Jetpack CP, I think it's not there just because we didn't need it in the past, it would be a change like this: https://github.com/wordpress-mobile/WordPress-FluxC-Android/pull/2193

JorgeMucientes commented 2 months ago

Oh, thanks for sharing the quick context @hichamboushaba. Yeah using /wp/v2 endpoint for fetching media should fix the problem. Cool that we already have a PR for reference šŸ‘šŸ¼.

I think we can tackle this task as part of the next Blaze i4