Skip to content

JITM: Detect when WooCommerce Services is installed, but not active#6382

Merged
singerb merged 3 commits intomasterfrom
fix/6283-jitm-if-wooservices-disabled
Feb 20, 2017
Merged

JITM: Detect when WooCommerce Services is installed, but not active#6382
singerb merged 3 commits intomasterfrom
fix/6283-jitm-if-wooservices-disabled

Conversation

@DanReyLop
Copy link
Copy Markdown
Contributor

Fixes #6283

Changes proposed in this Pull Request:

When the WooCommerce Services JITM is displayed and the user already has the plugin installed (but deactivated):

  • Change the button label from "Install WooCommerce Services" to "Activate WooCommerce Services" (see screenshot).
  • When clicking the button, the plugin will just be activated (not trying to installing it again).
  • When clicking the button, a different stat will be bumped (wooservices-activate-$version as opposed to the regular wooservices-install-$version). I expect this use case to be very rare, but I don't want unexplained discrepancies in our numbers :)

activate

Testing instructions:

  • Install WooCommerce, configure your store to be in the US or Canada.
  • Install WooCommerce Services, keep it deactivated.
  • Go to WooCommerce -> Settings or any of the other pages that show the JITM.
  • Note the JITM is prompting you to activate WooCommerce Services, not installing it.
  • Click the button.
  • The page will reload without errors.
  • Check that now the WooCommerce Services plugin is activated.
  • (Optional) Uninstall WooCommerce Services, then check the old behaviour of installing the plugin from the JITM still works as expected.

Daniel Rey Lopez added 2 commits February 13, 2017 11:44
…es/.bin/something" is not needed in NPM scripts.
…ted, and change the copy & behaviour of the JITM to activate only instead of install & activate
@DanReyLop DanReyLop self-assigned this Feb 13, 2017
"build-client": "./node_modules/.bin/gulp",
"build-production": "yarn clean-client && yarn build-languages && ./node_modules/.bin/gulp languages:extract && NODE_ENV=production BABEL_ENV=production yarn build",
"build-languages": "./node_modules/.bin/gulp languages",
"build-client": "gulp",
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated with this issue, but I had yarn build-client failing for me (Windows 10) because ./node_modules/... is not valid CMD syntax. On the context of NPM scripts, that full path is redundant, since all executables from ./node_modules/.bin/* are included to the $PATH automatically.

@dereksmart dereksmart added the [Status] Needs Review This PR is ready for review. label Feb 13, 2017
$install_url = wp_nonce_url( add_query_arg( array( 'wc-services-action' => 'install' ) ), 'wc-services-install' );
// If plugin dir exists, means it's installed but not activated
$all_plugins = get_plugins();
$already_installed = isset( $all_plugins[ 'woocommerce-services/woocommerce-services.php' ] );
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we use validate_plugin() here instead?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, didn't know about that one, thanks!

</p>
<p>
<a href="<?php echo esc_url( $install_url ); ?>" title="<?php esc_attr_e( 'Install WooCommerce Services', 'jetpack' ); ?>" data-module="wooservices" class="button button-jetpack show-after-enable"><?php esc_html_e( 'Install WooCommerce Services', 'jetpack' ); ?></a>
<a href="<?php echo esc_url( $install_url ); ?>" title="<?php esc_attr_e( 'Install WooCommerce Services', 'jetpack' ); ?>" data-module="wooservices" class="button button-jetpack show-after-enable">
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the title attr value doesn't account for the conditional

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, good catch! Fixed it.

@jeherve jeherve added [Pri] Normal Bug When a feature is broken and / or not performing as intended labels Feb 13, 2017
…Fixed the title attr for the JITM when the action is "activate plugin"
Copy link
Copy Markdown
Contributor

@dereksmart dereksmart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good and tests well 👍

@dereksmart dereksmart added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels Feb 20, 2017
@singerb singerb merged commit e19683d into master Feb 20, 2017
@matticbot matticbot removed the [Status] Ready to Merge Go ahead, you can push that green button! label Feb 20, 2017
@singerb
Copy link
Copy Markdown
Contributor

singerb commented Feb 20, 2017

Merged!

@singerb singerb deleted the fix/6283-jitm-if-wooservices-disabled branch February 20, 2017 15:55
jeherve added a commit that referenced this pull request Feb 21, 2017
dereksmart pushed a commit that referenced this pull request Feb 28, 2017
* Changelog: update stable tag and move changelog to changelog.txt

Also remove old releases from readme.txt to keep the changelog tab short.

* Changelog: add #5883

Also update the filter's docblock to match new version.

* Changelog: add #5938

* Changelog: add #6298

* Changelog: add #3405

* Changelog: add #5941

* Changelog: add #6239

* Changelog: add #6281

* Changelog: add #6303

* Changelog: add #6018

* Changelog: add #6300

* Changelog: add #6296

* Changelog: add #6130

* Changelog: add #6292

* Readme: remove extra "on".

* Changelog: add #6307

* Changelog: add #3297

* Changelog: add #6275

* Changelog: add #6321

* Changelog: add #6297

* Readme: update the support forum link anchor.

Anchor changed when WordPress.org forums were updated to bbPress 2

* Readme: update list of a12s, it wasn't up to date anymore!

* Changelog: add #6338

* Changelog: add #6337

* Changelog: add #6335

* Changelog: add #6333

* Testing List: first version of the 4.7 testing list.

* Changelog: add #6332

* Changelog: add #6325

* Changelog: add #6326

* Changelog: add #6339

* Changelog: add #6342

* Changelog: add #6343

* Changelog: add #6346

* Changelog: add #6347

* Changelog: add #6279

* Changelog: add #6306

* Changelog: add #6312

* Changelog: add #6316

* Changelog: add #6171

* Changelog: add #6317

* Changelog: add #6246

* Changelog: add #6263

* Changelog: add #4220

* Changelog: add #5888

* Changelog: add #3406

* Changelog: add #3637

* Changelog: add #6320

* Changelog: add #5992

* Changelog: add #6322

* Changelog: add #6324

* Changelog: add #6352

* Changelog: add #6355

* Changelog: add #6360

* Changelog: add #6362

* Changelog: add #6369, #6382

* Changelog: add #6370

* Changelog: add #6375

* Changelog: add #6383

* Changelog: add #6384

* Changelog: add #6386

* Changelog: add #6395

* Changelog: add #6403

* Changelog: add #6406

* Changelog: add #6418

* Changelog: add #6419

* Changelog: add #6434

* Changelog: add #6446

* Changelog: add #6006

* Changelog: add #6096

* Changelog: add #6399

* Changelog: fix typo.

@see #6331 (comment)

* Changelog: add #6440

* Changelog: add #6443

* Changelog: add #6445

* Changelog: add #6463

* Changelog: add #6468

* Changelog: add #6471

* Changelog: add #6474

* Changelog: add #6480

* Changelog: add #6497

* Changelog: add #6499

* Changelog: add #6514

* Changelog: add #6267

* Changelog: add #5940

* Changelog: add #6492

* Changelog: add #5281

* Changelog: add #6327

* Changelog: add #6451

* Changelog: add #6525

* Changelog: add #6530
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug When a feature is broken and / or not performing as intended General [Pri] Normal

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants