WooCommerce Services JITM #6183

jeffstieler · 2017-01-26T20:18:49Z

Note: this will remain "in progress" until our WordPress.org plugin slug has been finalized. This PR was opened to start the code review process.

Changes proposed in this Pull Request:

Add JITMs for WooCommerce Services.

USA and Canada-based WooCommerce merchants will be shown messages informing them of new USPS or CanadaPost shipping functionality available through WooCommerce Services.

The call to action button on these messages will handle automatic installation and activation of the plugin from the WordPress.org repository.

Messages will be displayed on the following WooCommerce pages:

WooCommerce > Orders
WooCommerce > Edit Order
WooCommerce > Settings

Testing instructions:

On a site with WooCommerce active, but without WooCommerce Services, with a store location in the USA or Canada:

Visit one of the pages described above
Verify that the JITM is displayed (with relevant USPS or CanadaPost copy)
Click "install" button
Verify that the JITM no longer displays
Verify that the plugin has been installed and activated

…od after install.

…ge entry hook to `admin_init`.

beaulebens · 2017-01-27T00:17:33Z

When I tried this out, clicking the button to install launched me into a new window, which felt a bit weird. Is it possible to just do this in the same window?

I was also presented with the banner below, but I believe that's being changed in WooCommerce Services? It was a bit confusing to see that, because it didn't feel related to what I'd just clicked:

I think you probably want to remove the JITM (note it's still onscreen) if I've already installed (even if I haven't connected/activated) the thing you want me to install ;)

jeffstieler · 2017-01-27T00:22:42Z

@beaulebens

When I tried this out, clicking the button to install launched me into a new window, which felt a bit weird. Is it possible to just do this in the same window?

Agreed - I'll make the change.

I was also presented with the banner below, but I believe that's being changed in WooCommerce Services?

This is only occurring because the released version of the plugin hasn't had the name change pushed yet (see note about plugin repo slug finalization). Our version in master has the correct naming.

I think you probably want to remove the JITM (note it's still onscreen) if I've already installed (even if I haven't connected/activated) the thing you want me to install ;)

This, again, is due to the not-yet-finalized plugin slug. In order to actually install something, we're downloading connect-for-woocommerce, but the JITM check is looking for woocommerce-services. A little janky right now, I know.

georgestephanis

I've got a couple concerns listed here -- the most serious of which is not checking capabilities for install_plugins before providing the UI and Code to do just that.

georgestephanis · 2017-01-27T15:46:14Z

3rd-party/woocommerce-services.php

+	}
+
+	public function try_install() {
+		$action = filter_input( INPUT_GET, 'wc-services-action' );


Is there a reason we're doing this, instead of simply $_GET['wc-services-action'] ?

I believe filter_input() was chosen because it handles the array key not existing - removing the need for isset( $_GET[ 'wc-services-action' ] ).

For non-string types, it can provide a mechanism for filtering the value upon retrieval as well.

In this case, we only care about it if it's 'install', so it'd probably be simpler to combine the two lines into

if ( isset( $_GET['wc-services-action'] ) && 'install' === $_GET['wc-services-action'] ) {

and would be more in keeping with the existing codebase for readability.

georgestephanis · 2017-01-27T15:48:19Z

3rd-party/woocommerce-services.php

+		$action = filter_input( INPUT_GET, 'wc-services-action' );
+
+		if ( 'install' === $action ) {
+			check_admin_referer( 'wc-services-install' );


Security concern: Nonces are for verifying intent, not capabilities. This doesn't seem to in any way test for the install_plugins capability. :\

Good catch - we'll add the check for the install_plugins capability as well.

georgestephanis · 2017-01-27T15:50:31Z

3rd-party/woocommerce-services.php

+			$result = $this->install();
+
+			wp_safe_redirect( add_query_arg( array(
+				'wc-services-action'        => false,


Instead of manually deleting these from the query string, why don't we just add_query_arg() to the base page url, as gotten via menu_page_url()? Are there arbitrary get arguments that we want to allow to keep being passed around?

menu_page_url() requires the $menu_slug to be specified - the JITM that kicks off this flow will be shown on 3 different pages.

I suppose we could programmatically determine $menu_slug and add only the wc-services-install-error query arg - just let us know if this is a merge blocker.

We can also do a wp_get_referer() to find out where to send them back to.

jetpack/class.jetpack-admin.php

Line 206 in 764a699

wp_safe_redirect( wp_get_referer() );

georgestephanis · 2017-01-27T15:51:41Z

3rd-party/woocommerce-services.php

+
+		if ( is_wp_error( $result ) ) {
+			return false;
+		}


Should this method return something truthy if the activation is successful?

As activate_plugin() returns NULL on success, couldn't we simply do a return is_null( $result ); -- and that would handle both the WP_Error and NULL returns?

georgestephanis · 2017-01-27T15:52:59Z

3rd-party/woocommerce-services.php

+	}
+
+	public function add_error_notice() {
+		$error = filter_input( INPUT_GET, 'wc-services-install-error' );


Couldn't this just be if ( ! empty( $_GET['wc-services-install-error'] ) ) { instead? I'm a bit confused by the usage of filter_input as it seems to be out of keeping with typical usage.

georgestephanis · 2017-01-27T16:02:44Z

class.jetpack-jitm.php

+		if ( $store_in_usa ) {
+			$message = __( 'Try our new service for USPS shipping & label-printing.', 'jetpack' );
+		}
+		elseif ( $store_in_canada ) {


This should be on the previous line -- } elseif ( true ) {

georgestephanis · 2017-01-27T16:03:54Z

class.jetpack-jitm.php

+		$store_in_usa    = ( 'US' == $base_location['country'] );
+		$store_in_canada = ( 'CA' == $base_location['country'] );
+
+		if ( $store_in_usa ) {


I'm not sure I see the point of a throwaway variable when the 'US' === $base_location['country'] could be tested inline here?

Agreed - this initially was part of a more complex conditional (for background).

It wouldn't be a horrible idea to change it to a switch() statement instead, for easier extensibility to more countries down the road.

georgestephanis · 2017-01-27T16:04:10Z

class.jetpack-jitm.php

+		}
+
+		$base_location   = wc_get_base_location();
+		$store_in_usa    = ( 'US' == $base_location['country'] );


Probably should use === here

georgestephanis · 2017-01-27T16:05:47Z

class.jetpack-jitm.php

+			return;
+		}
+
+		$install_url = wp_nonce_url( add_query_arg( array( 'wc-services-action' => 'install' ) ), 'wc-services-install' );


Probably should provide a base page to the link here, instead of just coasting off current.

Hmm.. okay - after a quick discussion, I think we'll take the user to the plugins page upon installation.

Could also do wp_get_referer() to send them back after

georgestephanis · 2017-01-27T16:07:53Z

class.jetpack-jitm.php

+		//jitm is being viewed, track it
+		$jetpack = Jetpack::init();
+		$jetpack->stat( 'jitm', 'woocommerce_services-viewed-' . JETPACK__VERSION );
+		$jetpack->do_stats( 'server_side' );


I don't think you need to explicitly call this? Jetpack itself batches and fires all the stats at the end of page rendering.

jetpack/class.jetpack.php

Lines 644 to 651 in f76d6bf

/**

* If there are any stats that need to be pushed, but haven't been, push them now.

*/

function __destruct() {

if ( ! empty( $this->stats ) ) {

$this->do_stats( 'server_side' );

}

}

Ah - good to know. We were just following the pattern of the other JITMs in the file.

Yup! I hadn't reviewed them, or seen them during this review, so I'm just hitting it with clean eyes.

georgestephanis · 2017-01-27T16:17:04Z

Also, if it's possible to add PHPDoc comments to the functions, that'd be awesome! ⭐️⭐️⭐️⭐️⭐️

… attachment to constructor.

…ng for easier addition of future target markets.

georgestephanis

Looks a lot better -- a couple random thoughts as I go through again, but I wouldn't have any reservations at the moment about seeing it merged.

Thanks!

georgestephanis · 2017-01-27T21:39:52Z

3rd-party/woocommerce-services.php

+			$result   = $this->install();
+			$redirect = wp_get_referer();
+
+			if ( false === $result ) {


May be easier to just do if ( ! $this->install() ) { -- no need for the variable to store it for a couple lines.

georgestephanis · 2017-01-27T21:41:40Z

3rd-party/woocommerce-services.php

+
+			if ( false === $result ) {
+				$redirect = add_query_arg( 'wc-services-install-error', true, $redirect );
+			}


Would it make sense to add an else clause so we could append a get argument confirming the installation happened? Or do we not care about showing a confirmation message on the page we're directing them to (which is fine)?

We're not concerned initially since the first activation of the plugin will cause a welcome/TOS message.

georgestephanis · 2017-01-27T21:43:21Z

3rd-party/woocommerce-services.php

+			$redirect = wp_get_referer();
+
+			if ( false === $result ) {
+				$redirect = add_query_arg( 'wc-services-install-error', true, $redirect );


I assume true works correctly here? I know false removes it, but I don't think I've ever tried passing in a boolean of true.

Yep - the value end up going through urlencode() which converts the boolean true to an integer 1.

georgestephanis · 2017-01-27T21:44:52Z

3rd-party/woocommerce-services.php

+		}
+
+		$upgrader = new Plugin_Upgrader( new Automatic_Upgrader_Skin() );
+		$result   = $upgrader->install( $api->download_link );


How does this work on environments where WordPress doesn't have write access to the filesystem? Is it just the catchall error in error_notice ?

I know we have a lot of caveats in our plugin installation rest api endpoint -- https://github.com/Automattic/jetpack/blob/master/json-endpoints/jetpack/class.jetpack-json-api-plugins-install-endpoint.php#L12-L54 // cc: @roccotripaldi

I just tested this, and it is caught by our generic error handler.

jeffstieler · 2017-01-31T20:12:15Z

I'm removing the "in progress" label as this is ready for a final look over and merge.

(cc: @georgestephanis @dereksmart)

georgestephanis

One tiny itty bitty change, but we can fix that after ~ is merged in, so I'm not blocking with this review.

georgestephanis · 2017-01-31T21:26:56Z

class.jetpack-jitm.php

+					<path d="M18,8h-2V7c0-1.105-0.895-2-2-2H4C2.895,5,2,5.895,2,7v10h2c0,1.657,1.343,3,3,3s3-1.343,3-3h4c0,1.657,1.343,3,3,3s3-1.343,3-3h2v-5L18,8z M7,18.5c-0.828,0-1.5-0.672-1.5-1.5s0.672-1.5,1.5-1.5s1.5,0.672,1.5,1.5S7.828,18.5,7,18.5z M4,14V7h10v7H4z M17,18.5c-0.828,0-1.5-0.672-1.5-1.5s0.672-1.5,1.5-1.5s1.5,0.672,1.5,1.5S17.828,18.5,17,18.5z" />
+				</svg>
+			</div>
+			<p class="msg">


Should this be wrapped in a <?php if ( ! empty( $message ) ) : ?> ? Otherwise outside of the US and Canada, it would print an empty p.msg

It's not the most straightforward logic perhaps, but if you don't fall into either the US or CA cases, the default simply returns. See line 457.

dereksmart · 2017-01-31T22:30:52Z

Tested, works fine on hosts that allow access to the plugin API. Maybe down the road we can add some more informative error messages, but for now let's

* Changelog: move 4.5 changelog to changelog.txt * Changelog: add #5603 * Changelog: add #6242 * Changelog: add #6104 * Changelog: add #6109 * Changelog: add #6118 * Changelog: adf #6122 * Changelog: add #6115 * Changelog: add #6126 * Changelog: add #6131 * Changelog: add #6140 * Testing list: add testing instructions for Widget fixes. * Changelog: add #6142 * Changelog: add #6149 * Changelog: add #6151 * Changelog: add #6153 * Changelog: add #6154 * Changelog: add #6155 * Changelog: add #6158 * Changelog: add #6170 * Changelog: add #6182 * Changelog: add #6183 * Changelog: add #5821 * Changelog: add #5953 * Changelog: add #5988 * Changelog: add #6002 * Changelog: add #6021 * Changelog: add #6038 * Changelog: add #6040 * Changelog: add #6060 * Changelog: add #6068 * Changelog: add #6083 * Changelog: add #6098 * Changelog: add #6186 * Testing list: add Publicize instructions. * Changelog: add #6190 * Changelog: add #6194 * Changelog: add #6230 * Changelog: add #6232 * Changelog: add #6234 * Testing list: add instructions to test Woo JITM. * Testing list: add PHP 7.1 testing. * Testing list: add compat tests for widgets and shortcodes. * Testing list: add wpcom REST API testing. * Missing word in testing list.

@see

* update google analytics description (#6250) * Add user tracking for disconnecting site (#6248) * Minor whitespace cleanups * Changelog and Testing list for Jetpack 4.6 (#6245) * Changelog: move 4.5 changelog to changelog.txt * Changelog: add #5603 * Changelog: add #6242 * Changelog: add #6104 * Changelog: add #6109 * Changelog: add #6118 * Changelog: adf #6122 * Changelog: add #6115 * Changelog: add #6126 * Changelog: add #6131 * Changelog: add #6140 * Testing list: add testing instructions for Widget fixes. * Changelog: add #6142 * Changelog: add #6149 * Changelog: add #6151 * Changelog: add #6153 * Changelog: add #6154 * Changelog: add #6155 * Changelog: add #6158 * Changelog: add #6170 * Changelog: add #6182 * Changelog: add #6183 * Changelog: add #5821 * Changelog: add #5953 * Changelog: add #5988 * Changelog: add #6002 * Changelog: add #6021 * Changelog: add #6038 * Changelog: add #6040 * Changelog: add #6060 * Changelog: add #6068 * Changelog: add #6083 * Changelog: add #6098 * Changelog: add #6186 * Testing list: add Publicize instructions. * Changelog: add #6190 * Changelog: add #6194 * Changelog: add #6230 * Changelog: add #6232 * Changelog: add #6234 * Testing list: add instructions to test Woo JITM. * Testing list: add PHP 7.1 testing. * Testing list: add compat tests for widgets and shortcodes. * Testing list: add wpcom REST API testing. * Missing word in testing list. * generate new module headers (#6264) * Tracks: don't track during CI runs * WPCOM MERGE Infinite Scroll (#6246) * VIP: Query errors generated for HoopsHype are caused by the infinite scroll functionality. This filter will allow to use rewrite rules so that the infinity functions can be called by rewrite rules that will be cached by batcache. Merges r120201-wpcom. * Infinite Scroll: only disable in the Customizer when previewing a non-active theme. Fixes #7507 See [115743] #6795 Merges r122634-wpcom. * Infinite Scroll: allow `get_settings` to be filtered at later points than just `__construct`. See #7539. Merges r123819-wpcom. * Infinite Scroll: add translation function to credit line. Merges #2537 Fixes #2528 https://[private link] Merges r132540-wpcom. * Infinite Scroll: fix IS when content includes Curly Quotes (and other non-UTF8 chars) Using wp_json_encode instead of json_encode allowing us to replace invalid chars with HTML entities. Merges #1447 Fixes #1446 props jtsternberg https://[private link] Merges r132541-wpcom. * Infinite Scroll: add check on ob_end_clean for cases where output_buffering is disabled Merges #2545 Props drrobotnik https://[private link] Merges r132542-wpcom. * Infinite Scroll: check that search terms exist before matching against post title. Merges #2128 Fixes #2075 Props cainm https://[private link] Merges r132543-wpcom. * Infinite Scroll: Fatal error when calling protected method from WP_Query Since we already have wp_query() we can use its query_vars['search_terms'] property instead of calling parse_search_terms(). It gets populated on https://github.com/WordPress/WordPress/blob/4.3.1/wp-includes/query.php#L2075 with the same data. Merges #2827 Fixes #2255 Props osiux https://[private link] Merges r132544-wpcom. * Infinite Scroll: Hide infinite-scroll class if the option is disabled The Jetpack support page says that the infinite-scroll class should be used in a theme to hide the navigation links. However, even when disabled in the Reading page, the class is still visible and the CSS is applied just as if the scroll is enabled. This commit adds an option check before filtering the body_class classes. Merges #1208 Props mpeshev https://[private link] Merges r132546-wpcom. * Infinite Scroll: Don't clobber the posts_per_page option if provided Infinite Scroll currently clobbers any passed-in value for posts_per_page if the type is set to click. This commit changes the behavior to match the documentation: https://jetpack.me/support/infinite-scroll/ Merges #2808 Props codebykat https://[private link] Merges r132547-wpcom. * Infinite Scroll: document all filter and action hooks Merges #2852 https://[private link] Merges r132551-wpcom. * Infinite Scroll: favor user set settings over theme settings If user changed their posts_per_page option, use that in Infinite Scroll instead of the value set in theme's IS support declaration. Only true when IS is set to click. Related: r132547 Discussion: https://[private link]#comment-31306 Merges r132764-wpcom. * Infinite Scroll: Merge changes from Jetpack into wpcom Just removing some whitespace so the 2 files are exactly similar and do not trigger the build script anymore. Merges r132787-wpcom. * Infinite Scroll: Make sure the body class gets updated once we are done with IS even when we just click Merges r134572-wpcom. * Remove `target="_blank"` from internal link. Accidentally added by #3600, which was intended to add only to external links. * JSON API: Removes PHP notice when no taxonomy description provided Fixes #4424 * JSON API: Removes PHP notice when no term description provided Fixes: #5943 * Google Analytics: hook tracking code into wp_footer. (#6284) get_footer might not be compatible with every theme out there. * Google Analytics: add HTML comment before the script output. (#6288) * Sync: Return expected response on Jetpack side * GA: Update inactive description to match calypso (#6291) * lodash: import specific function (#6295) * Change Infinite Scroll Google Analytics option label (#6239) * Sync: Fixes an issue where sync_wait_time was immediately overwritten in sync sender (#6281) * Documentation: reorganize current docs and create new ones. (#5985) * Documentation: reorganize current docs and create new ones. - Make contributing less frightening and easier for all potential contributors. - Make our guidelines and requirements clearer. - Surface all data in our contributing guide. - Offer options to contribute to everyone, even if it's not via code. - Outline our release management process, and approach to code reviews and Pull Requests. * Documentation: fix typos, headings, wrong links. * Add PHPCS and ESLint to the development environment documentation. * Docs: include some information about PHP Unit Testing. Fixes #6236 * Docs: add "Development" section. @see #5985 (comment) * update languages (#6302) * Bump version to 4.7-alpha (#6301) * Fix: Use the site_icon id instead of the url (#6303) When $image_url is set to a photon image we are not able to deremine the $image_id. Instead we should use the option that stores the ID instead. This fixes the issue when we show the default image instead of the site icon as the open graph main image. When photon is enabled and the site icon is set. * Add unit test for Publicize (#6018) * Add the accessible-focus library from dops-components to enable keyboard focus styles (#6300) * Use shorter WooCommerce Services MC stat slug. * Track WooCommerce services install as a module activation, not a WPCOM tools event. * Track WooCommerce Services JITM click and activation separately. * Update printThis to v1.9.0 (#6263) * Update to printThis v1.9 Additional options, including: * base tag * preservation of form values * doctype * canvas (experimental) * Additional cleanup * Added jshint * Date update Update date for `wp_enqueue_script` for printThis to prevent caching issues * Add filter for WordPress Posts widget content * Replace esc_html_e with esc_html__. Change the initialization of . * Don't call site_url() twice Instead of calling the function twice, which is a waste, assign the value to a variable and use that value to check whether we're on a tld-less domain or not. * Upgrades yarn lock file and fixes builds for master branch. (#6309) * Fixing a problem with local import. * Running yarn upgrade. * Added the print this library to jshint ignore. * Added a new generated RTL CSS file. * Changelog: update for release (#6280) * Changelog: add release post link. * Improved the changelog for readability and understanding * some minor adjustments were made to wording and to eliminate errors * Fix typos * update SSO changelog entry verbiage * changelogs edits per sdquirk * Adds vscode dir to ignore Visual Studio Code can store per-project settings in a .vscode folder; this updates .gitignore to ignore that, since it shouldn't be checked in. * Follow Widget: load translation files using wpcom language codes. (#5941) * Follow Widget: load translation files using wpcom language codes. Related: #2698 The widget previously used the site's language code to populate the `data-lang` parameter. that parameter is used to grab language files from WordPress.com, and should consequently use a language code that's available on WordPress.com. We consequently use the data available in locales.php to use the `slug` language code instead of `wp_locale` for each language. * Follow Widget / Notes: avoid calling get_locale() twice. It's been called before on the file. @see #5941 (review) * Remove Jetpack_Network::wp_get_sites in favor of core's wp_get_sites (#3405) * Removes Jetpack_Network::wp_get_sites and uses core's wp_get_sites instead. Changes usages of returned array since the one in Jetpack returned an array of objects and the one in core returns an array of arrays. Call to wp_get_sites has offset set to 1 to dismiss the first site since Function in Jetpack excluded the first site as well. * Make strings available for translation. * Use get_sites() instead of deprecated wp_get_sites() * Escape URLs in network admin, even though they are presumed safe * Network: introduce get_current_blog_id() when discarding the main site from sites table * Holiday Snow: remove settings outside of Holiday Snow period (#6298) * Only show holiday snow option 1 week before, through to 1 week after holiday snow period. Always show holiday snow option if custom rules have been set for holiday snow period. * Disable holiday snow option on Jan 4; not Jan 11 * Don't use Initial_State to fetch holiday snow option visibility * Media Summary: improve performance with single page load caching (#5938) * improve Jetpack_Media_Summary performance by caching result for a single pageload * spacing * no need to md5, also set cache to private * Remove unnecessary error_log (#6318) * Improve translatability of plurals and texts with variables (#6307) * Make gettext call plural aware * Add translator comments and convert plural strings * Slideshow: add filter to customize speed of the Slideshow. Suggested in https://wordpress.org/support/topic/gallery-slideshow-settings-editable-somehow * Slideshow: bump js version to bust cache. * Slideshow: use the timeout param instead of speed @see http://jquery.malsup.com/cycle/options.html * Slideshow shortcode: update version number in docblock. * Replace text labels with x and + icons. * Fix bug where wrong xmlrpc url was being sent to Jetpack Debugger (#6321) * Ignore GET parameters when checking an image's original file url matches container href (#6296) * Add email field to Contact Info Widget (#6275) * Add email field to Contact Info Widget * Replace admin email with sample email. Validate email and remove link if it is not * Display nothing when the email check fails * Remove email default value * Remove PHP error on uninstall, by making sure that jetpack includes all the required files (#6320) * Add image caching to jetpack_og_get_image() (#6297) * Add image caching to jetpack_og_get_image() This adds a transient to store the value of the $image_id to "speed up" the function to fix #6017 * Added missing semicolons at EOL on a couple lines * Adding some whitespace per coding standards * upgrade yarn.lock

nabsul and others added 8 commits January 23, 2017 16:59

Start implementing class for installing WooCommerce Hosted Services

3e7ed4a

Add WooCommerce Services JITM for US and Canada-based merchants.

54f0f33

Remove check for testing class.

2be7eea

Remove “Hello World” notice.

014ba86

Hook up “Install WooCommerce Services” button.

e318a8d

Add WooCommerce-specific JITM emblem.

d863d11

Add a nonce to the WC Services install link, use “safe” redirect meth…

752dfdc

…od after install.

Show an error message if the WooCommerce Services install fails, chan…

f7c1aa2

…ge entry hook to `admin_init`.

jeffstieler added [Status] In Progress [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it labels Jan 26, 2017

jeherve added the [Status] Needs Review This PR is ready for review. label Jan 26, 2017

jeherve added this to the Not Currently Planned milestone Jan 26, 2017

jeherve added the [Pri] Normal label Jan 26, 2017

jeherve assigned jeffstieler Jan 26, 2017

Don’t open a new tab when installing WooCommerce Services from JITM.

9da1e76

georgestephanis requested changes Jan 27, 2017

View reviewed changes

jeffstieler added 8 commits January 27, 2017 11:06

Remove usage of filter_input() in favor of the $_GET super global.

f5328c4

Ensure that JITMs are only shown to users who can install plugins.

e581bfd

Add a truthy return to WC Services installation method.

c383dac

Prune unnecessary variable usage.

fdf129d

No need to explicitly call Jetpack::do_stats().

55dc750

Use the singleton pattern to allow for easier unit testing. Move hook…

caff508

… attachment to constructor.

Use a switch statement for WC Services JITM country detection, allowi…

68e5e75

…ng for easier addition of future target markets.

Explicitly base the post-install redirect on the referring URL.

8c09a93

georgestephanis reviewed Jan 27, 2017

View reviewed changes

jeffstieler added 2 commits January 31, 2017 13:04

Update WooCommerce Services plugin slug for install and activation.

8594906

Add PHPDoc comments to WooCommerce Services installer class.

6a84c14

jeffstieler removed the [Status] In Progress label Jan 31, 2017

georgestephanis reviewed Jan 31, 2017

View reviewed changes

dereksmart self-requested a review January 31, 2017 22:32

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 Jan 31, 2017

dereksmart modified the milestones: 2/17 - February, Not Currently Planned Jan 31, 2017

dereksmart merged commit 0fe1b5f into master Jan 31, 2017

dereksmart deleted the woo/services-funnel branch January 31, 2017 22:34

jeherve added [Status] Needs Changelog and removed [Status] Needs Changelog labels Feb 1, 2017

jeherve added a commit that referenced this pull request Feb 1, 2017

Changelog: add #6183

00fd37f

kraftbj removed the [Status] Ready to Merge Go ahead, you can push that green button! label Oct 13, 2020

matticbot added the Touches WP.com Files label Oct 13, 2020

	/**
	* If there are any stats that need to be pushed, but haven't been, push them now.
	*/
	function __destruct() {
	if ( ! empty( $this->stats ) ) {
	$this->do_stats( 'server_side' );
	}
	}

WooCommerce Services JITM #6183

WooCommerce Services JITM #6183

Conversation

jeffstieler commented Jan 26, 2017 • edited Loading

Changes proposed in this Pull Request:

Testing instructions:

beaulebens commented Jan 27, 2017

jeffstieler commented Jan 27, 2017

georgestephanis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

georgestephanis Jan 27, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

georgestephanis commented Jan 27, 2017

georgestephanis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeffstieler commented Jan 31, 2017

georgestephanis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dereksmart commented Jan 31, 2017

jeffstieler commented Jan 26, 2017 •

edited

Loading

georgestephanis Jan 27, 2017 •

edited

Loading