jetpack raw option: Add a way to bypass it #7875

enejb · 2017-09-27T21:10:30Z

In some cases it might be better if we bypassed that option to get the raw info.

Changes proposed in this Pull Request:

Allows other devs to define constants and filters that lets them define which options should be allows to be set directly.

Testing instructions:

Do the tests pass?

Proposed changelog entry for your changes:

lezama · 2017-09-28T00:47:36Z

class.jetpack-options.php

+		 * @param array $disabled_raw_options has the key set of the option that you want to disable
+		 */
+		$disabled_raw_options = apply_filters( 'jetpack_disabled_raw_option', array() );
+		return isset( $disabled_raw_options[$name] );


missing spaces around $name

lezama · 2017-09-28T00:48:42Z

class.jetpack-options.php

+	 */
+	static function by_pass_raw_option( $name ) {
+
+		if ( Jetpack_Constants::get_constant( 'JETPACK_DISABLE_RAW_OPTION' ) ) {


should this be JETPACK_DISABLE_RAW_OPTIONS?

lezama · 2017-09-28T00:50:31Z

class.jetpack-options.php

+		 *
+		 * @param array $disabled_raw_options has the key set of the option that you want to disable
+		 */
+		$disabled_raw_options = apply_filters( 'jetpack_disabled_raw_option', array() );


jetpack_disabled_raw_option => jetpack_disabled_raw_options

lezama · 2017-09-28T00:51:22Z

class.jetpack-options.php

+			return true;
+		}
+		/**
+		 * Allows us to disable the some raw options.


Allows us to disable the some raw options. => Allows to disable particular raw options.

lezama · 2017-09-28T00:52:26Z

class.jetpack-options.php

+		 * @since 5.5.0
+		 *
+		 *
+		 * @param array $disabled_raw_options has the key set of the option that you want to disable


I think this needs better English, cc @roccotripaldi could you help us here?

An array of option names that you can selectively blacklist from being managed via direct database queries.

lezama · 2017-09-28T00:52:42Z

class.jetpack-options.php

@@ -404,4 +414,27 @@ static function get_raw_option( $name, $default = null ) {
 		return $value;
 	}

+	/**
+	 * This function lets us by pass certain options to be gotten via the raw sql calls.


I think this needs better English, cc @roccotripaldi could you help us here?

"This function checks for a constant that, if present, will disable direct DB queries Jetpack uses to manage certain options and force Jetpack to always use Options API instead.

Options can be selectively managed via a blacklist by filtering option names via the jetpack_disabled_raw_option filter."

lezama · 2017-09-28T00:56:05Z

class.jetpack-options.php

@@ -337,6 +337,9 @@ private static function delete_grouped_option( $group, $names ) {
 	 * @return bool Is the option deleted?
 	 */
 	static function delete_raw_option( $name ) {
+		if ( self::by_pass_raw_option( $name ) ) {


allover we need to change by_pass to bypass

lezama · 2017-09-28T00:57:46Z

Great idea @enejb!

Just left some minor comments around language, code looks great to me!

5.3 tests where failing but seemed unrelated to this PR, I just clicked the button to run them again.

In some cases it might be better if we bypassed that option to get the raw info.

zinigor · 2017-09-28T13:20:55Z

Rebased the PR to get around the problem with Twitter shortcode tests, Travis should pass now.

roccotripaldi · 2017-09-28T14:17:29Z

class.jetpack-options.php

@@ -404,4 +414,27 @@ static function get_raw_option( $name, $default = null ) {
 		return $value;
 	}

+	/**
+	 * This function lets us by pass certain options to be gotten via the raw sql calls.


"This function checks for a constant that, if present, will disable direct DB queries Jetpack uses to manage certain options and force Jetpack to always use Options API instead.

Options can be selectively managed via a blacklist by filtering option names via the jetpack_disabled_raw_option filter."

roccotripaldi · 2017-09-28T14:19:02Z

class.jetpack-options.php

+		 * @since 5.5.0
+		 *
+		 *
+		 * @param array $disabled_raw_options has the key set of the option that you want to disable


An array of option names that you can selectively blacklist from being managed via direct database queries.

improved english and monor fixes

enejb · 2017-09-29T04:58:44Z

Updated with the suggested fixes. Thanks for the review @lezama and @roccotripaldi !

lezama

Thanks 🙇

enejb added [Package] Sync [Status] Needs Review To request a review from Crew. Label will be renamed soon. labels Sep 27, 2017

enejb added this to the 5.4 milestone Sep 27, 2017

enejb self-assigned this Sep 27, 2017

enejb requested review from lezama, zinigor, ebinnion and roccotripaldi September 27, 2017 21:10

enejb requested a review from a team as a code owner September 27, 2017 21:10

matticbot added the Touches WP.com Files label Sep 27, 2017

lezama reviewed Sep 28, 2017

View reviewed changes

jeherve added [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! and removed [Status] Needs Review To request a review from Crew. Label will be renamed soon. labels Sep 28, 2017

jetpack raw option: Add a way to bypass it

cd074ea

In some cases it might be better if we bypassed that option to get the raw info.

zinigor force-pushed the fix/add-constant-to-disable-raw-options branch from cdd7016 to cd074ea Compare September 28, 2017 13:20

roccotripaldi requested changes Sep 28, 2017

View reviewed changes

Fixes as suggested in the PR

7a5b835

improved english and monor fixes

enejb added [Status] Needs Review To request a review from Crew. Label will be renamed soon. and removed [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! labels Sep 29, 2017

lezama approved these changes Sep 29, 2017

View reviewed changes

roccotripaldi approved these changes Sep 29, 2017

View reviewed changes

enejb merged commit 9904c57 into master Oct 2, 2017

enejb deleted the fix/add-constant-to-disable-raw-options branch October 2, 2017 19:12

matticbot removed the [Status] Needs Review To request a review from Crew. Label will be renamed soon. label Oct 2, 2017

lezama mentioned this pull request Oct 26, 2017

Sync: use the API to update wp_options instead of accessing the database directly #8048

Closed

jeherve mentioned this pull request Oct 26, 2017

Can not connect Jetpack ProjectNami/projectnami#237

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

jetpack raw option: Add a way to bypass it #7875

jetpack raw option: Add a way to bypass it #7875

enejb commented Sep 27, 2017

lezama Sep 28, 2017

lezama Sep 28, 2017

lezama Sep 28, 2017

lezama Sep 28, 2017

lezama Sep 28, 2017

roccotripaldi Sep 28, 2017

lezama Sep 28, 2017

roccotripaldi Sep 28, 2017

lezama Sep 28, 2017

lezama commented Sep 28, 2017

zinigor commented Sep 28, 2017

roccotripaldi Sep 28, 2017

roccotripaldi Sep 28, 2017

enejb commented Sep 29, 2017

lezama left a comment

jetpack raw option: Add a way to bypass it #7875

jetpack raw option: Add a way to bypass it #7875

Conversation

enejb commented Sep 27, 2017

Changes proposed in this Pull Request:

Testing instructions:

Proposed changelog entry for your changes:

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

lezama commented Sep 28, 2017

zinigor commented Sep 28, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

enejb commented Sep 29, 2017

lezama left a comment

Choose a reason for hiding this comment