Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add inbox notice promoting the Facebook extension. #2798

Merged
merged 2 commits into from Aug 16, 2019

Conversation

@jeffstieler
Copy link
Contributor

commented Aug 15, 2019

Fixes #2538.

This PR introduces an inbox notice to prompt stores with at least 1 product that have had WooCommerce admin installed for at least 3 days to install the Facebook extension if they already haven't.

The install mechanism reuses code written for the Onboarding flow.

cc: @pmcpinto

Screenshots

Screen Shot 2019-08-14 at 7 34 29 PM

Detailed test instructions:

  • Ensure you do not have the Facebook for WooCommerce Extension installed
  • Ensure your store has a published product (or more)
  • Use Crontrol to run the wc_admin_daily job
  • Open the WooCommerce > Dashboard
  • Verify the facebook note appears in the inbox
  • Verify the "learn more" button function
  • Verify the "install now" button function (check Plugins > Installed after)

Changelog Note:

Enhancement: add Facebook extension inbox note.

@jeffstieler jeffstieler requested a review from woocommerce/wc-admin Aug 15, 2019

@jeffstieler jeffstieler added this to In Progress PRs (for automation purposes) in wc-admin via automation Aug 15, 2019

@joshuatf
Copy link
Contributor

left a comment

Nice work @jeffstieler! Left an optional comment and a slightly bigger issue with notes in general that we can handle in a follow-up, but pre-approving pending any changes from @pmcpinto.

Also I believe unrelated to this PR, but I am not getting an unread indicator when this note is added. Does this happen for you as well?

Screen Shot 2019-08-15 at 1 28 26 PM

*/
public static function possibly_add_facebook_note() {
$wc_admin_installed = get_option( 'wc_admin_install_timestamp', false );
if ( false === $wc_admin_installed ) {

This comment has been minimized.

Copy link
@joshuatf

joshuatf Aug 15, 2019

Contributor

Optional: I notice we use this a few times across different notes; might be nice to reuse some of this logic (terrible name, but general idea):

	public static function is_wc_admin_installed_for( $seconds ) {
		$wc_admin_installed = get_option( 'wc_admin_install_timestamp', false );
		if ( false === $wc_admin_installed ) {
			$wc_admin_installed = time();
			update_option( 'wc_admin_install_timestamp', $wc_admin_installed );
		}

		$current_time        = time();

		return $current_time - $wc_admin_installed < $seconds );
	}

This comment has been minimized.

Copy link
@jeffstieler

jeffstieler Aug 15, 2019

Author Contributor

Good idea - how about introducing a Trait class for notes?

This comment has been minimized.

Copy link
@jeffstieler

jeffstieler Aug 15, 2019

Author Contributor

Done in a810cc9.

$installer = new \Automattic\WooCommerce\Admin\API\OnboardingPlugins();
$result = $installer->install_plugin( $plugin );
if ( is_wp_error( $result ) ) {

This comment has been minimized.

Copy link
@joshuatf

joshuatf Aug 15, 2019

Contributor

I think along with this, we need a better way to handle custom note interactions. Definitely outside the scope of this PR, but it would be great to set a loading indicator while the install is happening and return success/error messages.

I wasn't sure if anything was happening after clicking "Install Now" so I imagine this may lead to some confusion for impatient users.

This comment has been minimized.

Copy link
@jeffstieler

jeffstieler Aug 15, 2019

Author Contributor

For the time being I can experiment with moving the plugin install work to the shutdown hook and see if that speed up the response.

This comment has been minimized.

Copy link
@jeffstieler

jeffstieler Aug 15, 2019

Author Contributor

we need a better way to handle custom note interactions. Definitely outside the scope of this PR, but it would be great to set a loading indicator while the install is happening and return success/error messages

The shutdown hook didn't change the behavior. +1 to adding loading and feedback to note actions.

@jeffstieler

This comment has been minimized.

Copy link
Contributor Author

commented Aug 15, 2019

Also I believe unrelated to this PR, but I am not getting an unread indicator when this note is added. Does this happen for you as well?

Nope, I'm getting the indicator.

@joshuatf

This comment has been minimized.

Copy link
Contributor

commented Aug 16, 2019

a810cc9 Nice - thanks for this 😄

Nope, I'm getting the indicator.

Working well for me today too. Must have been something wonky with my setup.

@jeffstieler jeffstieler merged commit 9c97ed3 into master Aug 16, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

wc-admin automation moved this from In Progress PRs (for automation purposes) to Done Sprint 23 (August 13 - August 26) Aug 16, 2019

@jeffstieler jeffstieler deleted the add/2538-facebook-inbox-note branch Aug 16, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants
You can’t perform that action at this time.