woocommerce / facebook-for-woocommerce

A first-party extension plugin built for WooCommerce. Development is managed by Ventures.
https://woocommerce.com/products/facebook/
GNU General Public License v2.0
211 stars 138 forks source link

Problems with disconnecting and reconnecting to Facebook #1822

Closed haszari closed 3 years ago

haszari commented 3 years ago

🔎 Isolate the bug

✍️ Describe the bug

While working on #1817 I wanted to disconnect and reconnect my site to confirm that I'm using the right APIs & state to determine if site is connected. I had a couple of issues, not sure if these are high-impact since most stores connect once and leave it.

Screen Shot 2021-03-24 at 2 34 29 PM

The Woo end still has a connection token, but it knows that the connection is invalid:

Screen Shot 2021-03-24 at 2 49 05 PM

🚶‍♀️ Steps to reproduce

  1. Install Facebook for WooCommerce. Click connect and get connected to Facebook.
  2. Disconnect the Woo site from Facebook.
  3. Attempt to reconnect.

Or, delete one of the referenced entities in Facebook, then try to get the connection working correctly.

✔️ Expected behavior

As easy to disconnect as it is to get connected - e.g. can trigger from Woo end by clicking a button.

When there are missing entities on Facebook end, through accident or misadventure, the merchant can find out what's going on and get things working correctly again.

haszari commented 3 years ago

I see there is an Uninstall link next to the big connect button - possibly this is part of a disconnect flow. I tried clicking it and it didn't work:

Screen Shot on 2021-03-24 at 14-58-55

🎉Clicking the click here to securely reconnect in the admin notice restarted the onboarding 🤞

haszari commented 3 years ago

1805 might help with this.

solstudioim commented 3 years ago

Based on interaction with a user, ref: 3781898, it appears that a few solutions we offered didn't work.

What the user was trying to achieve: Making a new connection to Facebook, because their Facebook Business manager account was disabled, so they wanted to do a reconnection and choose a different Business Manager account and the rest of the assets (Page, Pixel, Catalog, etc).

Few attempts to reconnect with the following failed:

Working solution:

I was able to solve this issue by deleting all options with the name 'wcfacebook*' directly from the database.

Therefore it's similar to removing those fields as well on wp-admin/options.php page.

Expected Behavior The "Uninstall" button next to "Manage Connection" should be able to force remove previous connection data, so users can click "Get Started" to start a new fresh connection, to choose a new business manager, without users manually going to wp-admin/options.php or going to the database directly to remove 'wcfacebook*' options under wp_options table.

Perhaps the "small plugin" isn't needed anymore if this available uninstall menu on the setting works as expected.

solstudioim commented 3 years ago

Based on interaction with a user, ref: 3757196

What the user was trying to achieve: They had a connection before with an old Business Manager account, and they wanted to start a new connection to connect a different Business Manager account. But Facebook for WooCommerce kept trying to reconnect to the same Business Manager (old one).

Working solution: When removing the following fields manually through the options page, the extension was able to reconnect to Facebook, where they were able to choose a new Facebook Manager account:

wc_facebook_external_business_id wc_facebook_system_user_id wc_facebook_business_manager_id wc_facebook_commerce_merchant_settings_id

Expected Behavior Same expected behavior as above, the "Uninstall" button next to "Manage Connection" should be able to force remove previous connection data, so users can click "Get Started" to start a new fresh connection

haszari commented 3 years ago

@rcstr – FYI I've started working on this. The plan is to fix the disconnect (aka "uninstall") flow so it works cleanly, ensure site can be disconnected/reconnected without needing to manually delete options.

haszari commented 3 years ago

Well in my initial testing the Uninstall seems to disconnect and reconnect fine. So there must be other situations in which things get broken, need more info on how that happens.

Screen Shot 2021-04-01 at 3 25 09 PM

We could reword Uninstall => disconnect to make the UI clearer. @j111q - thoughts on this?

budzanowski commented 3 years ago

The Uninstall button should be renamed to disconnect. It looks like the flow does everything that is necessary to start fresh:

    private function disconnect() {

        $this->update_access_token( '' );
        $this->update_merchant_access_token( '' );
        $this->update_system_user_id( '' );
        $this->update_business_manager_id( '' );
        $this->update_ad_account_id( '' );
        $this->update_instagram_business_id( '' );
        $this->update_commerce_merchant_settings_id( '' );

        update_option( \WC_Facebookcommerce_Integration::SETTING_FACEBOOK_PAGE_ID, '' );
        update_option( \WC_Facebookcommerce_Integration::SETTING_FACEBOOK_PIXEL_ID, '' );
        facebook_for_woocommerce()->get_integration()->update_product_catalog_id( '' );

        delete_transient( 'wc_facebook_business_configuration_refresh' );
    }

I would say that we should close this with #1860 and be on the lookout for more specific problems with connection and disconnection flow.

j111q commented 3 years ago

We could reword Uninstall => disconnect to make the UI clearer. @j111q - thoughts on this?

+1. Specifically for that little green notice in the screenshot, it should say Disconnection successful.

budzanowski commented 3 years ago

I have found a flow that fails with the Uninstall/Disconnect button - this happens when there is a problem with the WooCommerce Application in FB. This could be something like @haszari has experienced when the Woo App was first removed from FB site config. In this case, the operation of removing application access to the site will fail, an exception will be thrown and the code will not reach the disconnect() function. As a result, the user will be left with old tokens, ids, transients, etc. I have a simple proposal: Since the user clicks disconnect and wants to disconnect then even if some of the disconnect operations fail ( for whatever reason ) we still delete all the settings. This allows for a fresh connection. Required change: https://github.com/woocommerce/facebook-for-woocommerce/pull/1860/commits/ffdc0f07a49787b56c33c3b0ecd92c3ca172079a