Skip to content

Update the list of post meta keys to sync for WooCommerce#8601

Merged
zinigor merged 1 commit intomasterfrom
fix/add-missing-woocommerce-postmeta-keys
Jan 26, 2018
Merged

Update the list of post meta keys to sync for WooCommerce#8601
zinigor merged 1 commit intomasterfrom
fix/add-missing-woocommerce-postmeta-keys

Conversation

@westi
Copy link
Copy Markdown
Contributor

@westi westi commented Jan 24, 2018

This includes all the ones defined as used by WooCommerce and to include all the Refund related keys too.

Related: Automattic/wp-calypso#21243 D9461-code

This also needs server side fixes to test too - D9552-code

…all the ones defined as used by WooCommerce and to include all the Refund related keys too.
@westi westi added this to the 5.8 milestone Jan 24, 2018
@westi westi self-assigned this Jan 24, 2018
@westi westi requested a review from nabsul January 24, 2018 02:47
@westi westi requested a review from a team as a code owner January 24, 2018 02:47
@jeherve jeherve added Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Status] Needs Review This PR is ready for review. labels Jan 24, 2018
@westi westi requested a review from psealock January 25, 2018 14:52
@westi westi added Bug When a feature is broken and / or not performing as intended and removed Enhancement Changes to an existing feature — removing, adding, or changing parts of it labels Jan 25, 2018
@westi
Copy link
Copy Markdown
Contributor Author

westi commented Jan 25, 2018

This is a Bug more than an Enhancement because without the synced data we can't correctly calculate refunds.

private static $wc_post_meta_whitelist = array(
//woocommerce products
'_stock_status',
// https://github.com/woocommerce/woocommerce/blob/8ed6e7436ff87c2153ed30edd83c1ab8abbdd3e9/includes/data-stores/class-wc-product-data-store-cpt.php#L21
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.

Instead of manually keeping this in sync with Jetpack_Sync_Module_WooCommerce::$internal_meta_keys, I think it would be better to use the jetpack_sync_post_meta_whitelist hook.

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.

I agree that this would be a good enhancement to add to WooCommerce, but I don't think it should block merging this change.

Copy link
Copy Markdown
Contributor

@nabsul nabsul left a comment

Choose a reason for hiding this comment

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

I double checked that no pre-existing entries in the whitelist were accidentally removed. Since this is just a change to an array list, that's probably sufficient "testing" for this PR :-)

@zinigor zinigor 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 26, 2018
@zinigor zinigor merged commit 5fbe2f5 into master Jan 26, 2018
@zinigor zinigor deleted the fix/add-missing-woocommerce-postmeta-keys branch January 26, 2018 11:57
@zinigor zinigor removed the [Status] Ready to Merge Go ahead, you can push that green button! label Jan 26, 2018
jeherve added a commit that referenced this pull request Jan 29, 2018
zinigor pushed a commit that referenced this pull request Jan 30, 2018
* Changelog 5.8: create base for changelog.

* Update 5.8 release post link

* fix 5.8 release date

* Updates to plugin description

* Changelog: add #8499

* Changelog: add #8506

* Changelog: add #8509

* Changelog: add #8516

* Changelog: add #8517

* Changelog: add #8523

* Changelog: add #8547

* Changelog: add #8496

* Changelog: add #8584

* Changelog: add #8595

* Changelog: add #8445

* Changelog: add #8431

* Changelog: add #8284

* Changelog: add #8270

* Changelog: add #8124

* Changelog: add #8581

* Changelog: add #8463

* Changelog: add #8568 (#8646)

* Updates to testing list and changelog

* Changelog: add #8443

* Changelog: add #8459

* Changelog: add #8469

* Changelog: add #8464

* Changelog: add #8478 and #8479

* Changelog: add #8483

* Changelog: add #8488

* Changelog: add #8513

* Changelog: add #8555

* Changelog: add #8565

* Changelog: add #8601

* Changelog: add #8612

* Changelog: add first pass at Search items.

* Changelog: add more info to help test Search.

* Changelog: add #8144

* Changelog: add #8313

* Changelog: add #8419

* Changelog: add #8465

* Changelog: add #8515

* Changelog: add #8587

* Changelog: add #8591

* Changelog: add #8659

* Changelog: add #8661

* Changelog: add #8671

* Changelog: add 5.7.1 to archived changelog too.

* Reverted changes to readme, removed entry about backups.
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 [Package] Sync

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants