Code review changes

xwp · Dec 10, 2016 · 202bd64 · 202bd64
1 parent d94b1e1
commit 202bd64
Show file tree

Hide file tree

Showing 8 changed files with 85 additions and 50 deletions.
diff --git a/css/customize-snapshots.css b/css/customize-snapshots.css
@@ -9,6 +9,7 @@
 #snapshot-expand-button {
 	display: block;
 }
+
 #snapshot-expand-button:hover,
 #snapshot-expand-button:focus,
 #snapshot-expand-button:active {
@@ -72,8 +73,8 @@
 	top: 46px;
 	position: absolute;
 	width: 100%;
-	box-shadow: 0 5px 0 0 rgba(0,0,0,0.05);
-	padding:10px;
+	box-shadow: 0 5px 0 0 rgba(0, 0, 0, 0.05);
+	padding: 10px;
 	box-sizing: border-box;
 }
 
@@ -89,6 +90,7 @@
 	margin-bottom: 0.5em;
 	margin-top: 0;
 }
+
 #customize-snapshot .timezone-info {
 	margin-top: 0.5em;
 	font-size: smaller;
@@ -169,13 +171,13 @@
 	min-width: 4em;
 }
 
-.snapshot-schedule-control input[type="number"]{
-	width:100%;
+.snapshot-schedule-control input[type="number"] {
+	width: 100%;
 }
 
 .snapshot-schedule-control .time-special-char {
-	padding-left:2px;
-	padding-right:2px;
+	padding-left: 2px;
+	padding-right: 2px;
 }
 
 .snapshot-schedule-control select {
@@ -203,22 +205,22 @@
 	z-index: 500100 !important;
 }
 
-.snapshot-control input[type="text"]{
+.snapshot-control input[type="text"] {
 	width: 100%;
 	line-height: 18px;
 	margin: 0;
 }
 
-#customize-snapshot .snapshot-schedule-title h3{
+#customize-snapshot .snapshot-schedule-title h3 {
 	font-size: 16px;
 }
 
-#customize-snapshot .snapshot-controls{
+#customize-snapshot .snapshot-controls {
 	list-style: none;
 	margin: 0;
 }
 
-.snapshot-future-date-notification.notice{
+.snapshot-future-date-notification .notice {
 	margin-bottom: 5px;
 	border-top: 1px solid #eee;
 	padding: 5px;
@@ -227,15 +229,15 @@
 /*Status Button*/
 /** @todo Need to add snapshot specific class for all ui classes. **/
 
-#snapshot-status-button-wrapper{
+#snapshot-status-button-wrapper {
 	float: right;
 	position: relative;
 	margin-top: 9px;
-	height:28px;
+	height: 28px;
 }
 
-#snapshot-status-button-wrapper .button-primary{
-	margin-top:0;
+#snapshot-status-button-wrapper .button-primary {
+	margin-top: 0;
 	-webkit-touch-callout: none; /* iOS Safari */
 	-webkit-user-select: none; /* Chrome/Safari/Opera */
 	-moz-user-select: none; /* Firefox */
@@ -251,6 +253,7 @@
 	left: 0;
 	display: none;
 }
+
 .ui-selectmenu-menu .ui-menu {
 	overflow: auto;
 	margin: 0;
@@ -259,15 +262,15 @@
 	background: #FFFFFF;
 	border-top: 0;
 	border-radius: 0 0 5px 5px;
-	box-shadow: 0 1px 4px 0 rgba( 33,38,34,.12 ), 0 2px 11px 0 rgba( 30,36,37,.14 )
+	box-shadow: 0 1px 4px 0 rgba(33, 38, 34, .12), 0 2px 11px 0 rgba(30, 36, 37, .14)
 }
 
-.ui-selectmenu-menu .ui-menu li{
+.ui-selectmenu-menu .ui-menu li {
 	padding: 5px 10px 5px 5px;
 	margin: 0;
 }
 
-.ui-selectmenu-menu .ui-menu .ui-state-focus{
+.ui-selectmenu-menu .ui-menu .ui-state-focus {
 	background: #0073aa;
 	color: #FFFFFF;
 	cursor: pointer;
@@ -282,6 +285,7 @@
 	height: auto;
 	border: 0;
 }
+
 .ui-selectmenu-open {
 	display: block;
 }
@@ -290,36 +294,36 @@
 	text-indent: 0;
 	float: right;
 	border-radius: 0 3px 3px 0;
-	padding-left:0;
-	padding-right:0;
+	padding-left: 0;
+	padding-right: 0;
 	-webkit-box-shadow: none;
 	-moz-box-shadow: none;
 	box-shadow: none;
 	font-size: 20px;
 	width: 20px;
 }
 
-#snapshot-status-button-wrapper .ui-selectmenu-button span.dashicons:before{
+#snapshot-status-button-wrapper .ui-selectmenu-button span.dashicons:before {
 	display: inline-block;
 	margin-left: -3px;
 }
 
-@media screen and ( max-width: 640px ){
+@media screen and ( max-width: 640px ) {
 	#snapshot-status-button-wrapper .ui-selectmenu-button span.dashicons {
-		font-size:17px;
+		font-size: 17px;
 	}
 }
 
 .ui-selectmenu-button span.ui-selectmenu-text {
 	border-radius: 3px 0 0 3px;
-	opacity:0;
+	opacity: 0;
 }
 
-.ui-selectmenu-menu.ui-front{
+.ui-selectmenu-menu.ui-front {
 	z-index: 999999;
 }
 
-.snapshot-status-button-overlay.button{
+.snapshot-status-button-overlay.button {
 	width: calc( 100% - 20px );
 	height: 28px;
 	position: absolute;
@@ -330,7 +334,7 @@
 	box-shadow: none;
 }
 
-#customize-header-actions .ui-selectmenu-button{
+#customize-header-actions .ui-selectmenu-button {
 	display: inline-block;
 	outline-width: 2px;
 	outline-offset: -2px;

diff --git a/instance.php b/instance.php
@@ -63,10 +63,11 @@ function current_snapshot_uuid() {
  * @return bool is compat.
  */
 function is_back_compat() {
-	// Fix in case version contains 'src' for example 4.7-src in that case php version compare fails to compare correctly.
 	$wp_version = get_bloginfo( 'version' );
-	if ( $pos = strpos( $wp_version, 'src' ) ) {
-		$wp_version = substr( $wp_version, 0, $pos - 1 );
+
+	// Fix in case version contains extra string for example 4.7-src in that case php version_compare fails.
+	if ( $pos = strpos( $wp_version, '-' ) ) {
+		$wp_version = substr( $wp_version, 0, $pos );
 	}
 	return version_compare( $wp_version, '4.7', '<' );
 }
diff --git a/js/customize-migrate.js b/js/customize-migrate.js
@@ -5,6 +5,12 @@
 		doingAjax: false,
 		postMigrationCount: 20
 	};
+
+	/**
+	 * Initialize js.
+	 *
+	 * @return {void}
+	 */
 	component.init = function() {
 		$( function() {
 			component.el = $( '#customize-snapshot-migration' );
@@ -13,6 +19,12 @@
 			component.spinner.css( 'margin', '0' );
 		} );
 	};
+
+	/**
+	 * Bind migrate click event.
+	 *
+	 * @return {void}
+	 */
 	component.bindClick = function() {
 		component.el.click( function() {
 			if ( component.doingAjax ) {
@@ -23,13 +35,24 @@
 			component.migrate( component.el.data( 'nonce' ), component.postMigrationCount );
 		} );
 	};
+
+	/**
+	 * Initiate migrate ajax request.
+	 *
+	 * @param {String} nonce Nonce.
+	 * @param {Number} limit Limit for migrate posts.
+	 *
+	 * @return {void}
+	 */
 	component.migrate = function( nonce, limit ) {
-		var requestData, request;
-		requestData = {
-			nonce: nonce,
-			limit: limit
-		};
+		var request,
+			requestData = {
+				nonce: nonce,
+				limit: limit
+			};
+
 		request = wp.ajax.post( 'customize_snapshot_migration', requestData );
+
 		request.done( function( data ) {
 			var outerDiv = $( 'div.customize-snapshot-migration' ), delay = 100, newLimit;
 			if ( data.remaining_posts ) {
@@ -41,6 +64,7 @@
 				component.doingAjax = false;
 			}
 		} );
+
 		request.fail( function() {
 			component.spinner.css( 'visibility', 'initial' );
 			component.doingAjax = false;
@@ -49,5 +73,7 @@
 			}
 		} );
 	};
+
 	component.init();
+
 })( jQuery );
diff --git a/php/class-customize-snapshot-command.php b/php/class-customize-snapshot-command.php
@@ -33,7 +33,7 @@ class Customize_Snapshot_Command {
 	 */
 	public function migrate( $arg, $assoc_args ) {
 		unset( $arg );
-		$migrate_obj = new Migrate();
+		$migrate_obj = new Migrate( get_plugin_instance() );
 		if ( $migrate_obj->compat ) {
 			\WP_CLI::error( __( 'You\'re using older WordPress version please upgrade 4.7 or above to migrate.', 'customize-snapshots' ) );
 			return;

diff --git a/php/class-migrate.php b/php/class-migrate.php
@@ -19,16 +19,18 @@ class Migrate {
 	/**
 	 * Plugin instance.
 	 *
-	 * @var bool
+	 * @var Plugin
 	 */
-	public $compat;
+	public $plugin;
 
 	/**
 	 * Migrate constructor.
+	 *
+	 * @param Plugin $plugin plugin.
 	 */
-	public function __construct() {
-		$this->compat = is_back_compat();
-		if ( ! $this->compat && is_admin() && is_super_admin() ) {
+	public function __construct( Plugin $plugin ) {
+		$this->plugin = $plugin;
+		if ( ! $plugin->compat && is_admin() && is_super_admin() ) {
 			$this->maybe_migrate();
 		}
 	}

diff --git a/php/class-plugin.php b/php/class-plugin.php
@@ -62,7 +62,7 @@ public function __construct() {
 	 * @action init
 	 */
 	public function init_migration() {
-		$this->migrate = new Migrate();
+		$this->migrate = new Migrate( $this );
 		if ( defined( 'WP_CLI' ) && WP_CLI ) {
 			require_once( __DIR__ . '/class-customize-snapshot-command.php' );
 		}

diff --git a/tests/php/test-class-migrate.php b/tests/php/test-class-migrate.php
@@ -78,15 +78,15 @@ function test_construct() {
 		     ->method( 'maybe_migrate' );
 		$reflected_class = new \ReflectionClass( $class_name );
 		$constructor = $reflected_class->getConstructor();
-		$constructor->invoke( $mock );
+		$constructor->invoke( $mock, $this->plugin );
 		set_current_screen( 'index' );
-		$constructor->invoke( $mock );
+		$constructor->invoke( $mock, $this->plugin );
 		$user_id = $this->factory()->user->create( array( 'role' => 'administrator' ) );
 		if ( is_multisite() ) {
 			grant_super_admin( $user_id );
 		}
 		wp_set_current_user( $user_id );
-		$constructor->invoke( $mock );
+		$constructor->invoke( $mock, $this->plugin );
 	}
 
 	/**
@@ -95,7 +95,7 @@ function test_construct() {
 	 * @see Migrate::is_migrated()
 	 */
 	function test_is_migrated() {
-		$migrate = new Migrate();
+		$migrate = new Migrate( $this->plugin );
 		update_option( Migrate::KEY, 0 );
 		$this->assertFalse( $migrate->is_migrated() );
 		update_option( Migrate::KEY, 1 );
@@ -108,7 +108,7 @@ function test_is_migrated() {
 	 * @see Migrate::maybe_migrate()
 	 */
 	function test_maybe_migrate() {
-		$migrate = new Migrate();
+		$migrate = new Migrate( $this->plugin );
 		$migrate->maybe_migrate();
 		$this->assertEquals( 10, has_action( 'admin_notices', array( $migrate, 'show_migration_notice' ) ) );
 		$this->assertEquals( 10, has_action( 'admin_enqueue_scripts', array( $migrate, 'enqueue_script' ) ) );
@@ -121,7 +121,7 @@ function test_maybe_migrate() {
 	 * @see Migrate::show_migration_notice()
 	 */
 	function test_show_migration_notice() {
-		$migrate = new Migrate();
+		$migrate = new Migrate( $this->plugin );
 		ob_start();
 		$migrate->show_migration_notice();
 		$data = ob_get_clean();
@@ -144,13 +144,14 @@ function test_changeset_migrate() {
 			'status' => 'draft',
 			'data' => array(),
 		) );
-		$migrate = new Migrate();
+		$migrate = new Migrate( $this->plugin );
 		$posts_count = $migrate->changeset_migrate( - 1, true );
 		$this->assertEquals( $post_id, array_shift( $posts_count ) );
 
 		$migrate_obj = $this->getMockBuilder( 'CustomizeSnapshots\Migrate' )
 		                    ->setMethods( array( 'migrate_post' ) )
-		                    ->getMock();
+							->setConstructorArgs( array( $this->plugin ) )
+							->getMock();
 		$migrate_obj->expects( $this->once() )
 		            ->method( 'migrate_post' )
 		            ->will( $this->returnValue( null ) );
@@ -182,7 +183,7 @@ function test_migrate_post() {
 		$wp_customize->add_setting( 'foo', array( 'default' => 'foo_default' ) );
 		$this->action_customize_register_for_dynamic_settings();
 
-		$migrate = new Migrate();
+		$migrate = new Migrate( $this->plugin );
 
 		$has_kses = ( false !== has_filter( 'content_save_pre', 'wp_filter_post_kses' ) );
 		if ( $has_kses ) {

diff --git a/tests/php/test-class-snapshot-ajax.php b/tests/php/test-class-snapshot-ajax.php
@@ -39,6 +39,7 @@ function test_handle_migrate_changeset_request() {
 		delete_option( Migrate::KEY );
 		$migrate_obj = $this->getMockBuilder( 'CustomizeSnapshots\Migrate' )
 		                    ->setMethods( array( 'changeset_migrate' ) )
+							->setConstructorArgs( array( $this->plugin ) )
 		                    ->getMock();
 		$migrate_obj->expects( $this->once() )
 		            ->method( 'changeset_migrate' )