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 140 forks source link

Deprecate Messenger feature #2719

Closed layoutd closed 7 months ago

layoutd commented 7 months ago

Changes proposed in this Pull Request:

This disables and deprecates all Messenger functionality following https://github.com/woocommerce/facebook-for-woocommerce/issues/2622#issuecomment-2004533578.

This PR should:

It doesn't delete any files or methods – simply marks them as deprecated for posterity. If this seems too safe, the unused files could be deleted in a subsequent PR or release.

Detailed test instructions:

For all tests, confirm no errors in log or browser console. Install and upgrade...

  1. Start by installing Facebook for WooCommerce fresh in version 3.1.11: 39ee6d306207c5b3fe4301031fe40f0a65513e03
    # clone repo
    gh repo clone woocommerce/facebook-for-woocommerce
    # check out 3.1.11
    git checkout 39ee6d306207c5b3fe4301031fe40f0a65513e03
    # build extension
    nvm use && composer i && npm i && npm run build:dev
  2. Activate and connect to a Facebook account
  3. Confirm "Messenger" tab appears in extension settings and loads (wp-admin/admin.php?page=wc-facebook&tab=messenger):

image

  1. Enable messenger and confirm the chat appears on shop (probably with a slight delay, and with has been blocked by CORS policy: No 'Access-Control-Allow-Origin' header is present on the requested resource. error in the console): image

  2. Check for .fb-customerchat DOM element or the xfbml.customerchat.js script to see that the chat plugin is being injected: image image

  3. Upgrade to current version:

    git checkout develop
  4. Visit extension settings (wp-admin/admin.php?page=wc-facebook) and confirm the deprecation notice appears (Don't dismiss!!): image (It should also appear on the messenger settings and other places, like Plugins or the Orders page) image

  5. Check for Facebook Messenger options:

    SELECT * FROM `wp_options` WHERE option_name LIKE 'wc_facebook%' AND option_name LIKE '%messenger%';
    # Should have 1 (but maybe up to 4) records
  6. Finally, upgrade to this PR:

    git checkout remove/deprecate-facebook-messenger-feature
    # build if necessary
  7. Force migration by updating the version to 3.2.0 in facebook-for-woocommerce.php:

    * Version: 3.2.0
    …
    const PLUGIN_VERSION = '3.2.0'; // WRCS: DEFINED_VERSION.

Confirm Messenger settings page is gone

  1. Navigate to the Facebook extension /wp-admin/admin.php?page=wc-facebook
  2. Confirm the "Messenger" settings tab no longer appears.
  3. Navigate to /wp-admin/admin.php?page=wc-facebook&tab=messenger
  4. Confirm no settings page is loaded (same behavior as if you put &tab=nopagehere)

Confirm settings DB records erased

  1. Check that no messenger options are left in the DB:
    SELECT * FROM `wp_options` WHERE option_name LIKE 'wc_facebook%' AND option_name LIKE '%messenger%';
    # Should not return any records

    Confirm messenger scripts are no longer injected

  2. In shop, confirm no errors in console.
  3. Confirm messenger chat no longer appears
  4. Confirm .fb-customerchat DOM element and the xfbml.customerchat.js script are no longer present.

Confirm messenger deprecation notice removed

  1. The messenger deprecation notice should no longer appear anywhere (Facebook for WooCommerce settings, Plugins, Orders, etc.)

Bonus

Changelog entry

Update - Remove the sunsetted Messenger Chat feature.

layoutd commented 7 months ago

Thanks @rawdreeg

Along with the one nit pick I left in the comments. I wonder since we're only marking the class as deprecated without removing it, we shouldn't have a _doing_it_wrong call when try to access the class method.

Yeah, I've gone back and forth on that. I doubt there are many people extending or invoking the Messenger class or methods directly, but the extension has a huge user base and I'm a bit gun shy about breaking things now 😳

I'll look into removing the code altogether (I want to see some prior art just to be sure I'm doing it right).

rawdreeg commented 7 months ago

Thanks @rawdreeg

Along with the one nit pick I left in the comments. I wonder since we're only marking the class as deprecated without removing it, we shouldn't have a _doing_it_wrong call when try to access the class method.

Yeah, I've gone back and forth on that. I doubt there are many people extending or invoking the Messenger class or methods directly, but the extension has a huge user base and I'm a bit gun shy about breaking things now 😳

I'll look into removing the code altogether (I want to see some prior art just to be sure I'm doing it right).

I am happy to deprecate gracefully. but if the feature won't work from FB then it will probably break one way or another :-D

I can approve this, and we can have a separate task for totally deleting the class if it helps.

budzanowski commented 7 months ago

I strongly suggest removing the code, tests, anything related to chat at this point.

budzanowski commented 7 months ago

@layoutd I have created #2721 for visibility and tagged you there.

layoutd commented 7 months ago

I've gone through and removed almost everything. I left a few public properties and methods that can be fully removed a few releases after 3.2.0 (just in case there is anyone using them, they'll have a few release cycles to discover the "graceful" errors):

layoutd commented 7 months ago

Thanks for the 👀 @rawdreeg. I'm going to leave it awaiting merge until April 26, so it can be released on the 30th or on May 7.