Add better support for not saving empty values

wp-papi · Aug 6, 2016 · 844e8a8 · 844e8a8
1 parent aee7736
commit 844e8a8
Show file tree

Hide file tree

Showing 4 changed files with 86 additions and 34 deletions.
diff --git a/src/core/class-papi-core-property.php b/src/core/class-papi-core-property.php
@@ -283,10 +283,9 @@ public static function factory( $type ) {
 	 * @return mixed
 	 */
 	public function format_value( $value, $slug, $post_id ) {
-		return papi_maybe_json_decode(
-			maybe_unserialize( $value ),
-			$this->convert_type === 'array'
-		);
+		$value = maybe_unserialize( $value );
+
+		return papi_maybe_json_decode( $value, $this->convert_type === 'array' );
 	}
 
 	/**
@@ -590,10 +589,13 @@ public function import_settings() {
 	 * @return mixed
 	 */
 	public function import_value( $value, $slug, $post_id ) {
-		return papi_maybe_json_decode(
-			maybe_unserialize( $value ),
-			$this->convert_type === 'array'
-		);
+		if ( ! ( $value = $this->prepare_value( $value ) ) ) {
+			return;
+		}
+
+		$value = maybe_unserialize( $value );
+
+		return papi_maybe_json_decode( $value, $this->convert_type === 'array' );
 	}
 
 	/**
@@ -606,10 +608,9 @@ public function import_value( $value, $slug, $post_id ) {
 	 * @return mixed
 	 */
 	public function load_value( $value, $slug, $post_id ) {
-		return papi_maybe_json_decode(
-			maybe_unserialize( $value ),
-			$this->convert_type === 'array'
-		);
+		$value = maybe_unserialize( $value );
+
+		return papi_maybe_json_decode( $value, $this->convert_type === 'array' );
 	}
 
 	/**
@@ -628,22 +629,30 @@ public function match_slug( $slug ) {
 	}
 
 	/**
-	 * Prepare property value.
+	 * Prepare value before database.
 	 *
 	 * @param  mixed $value
 	 *
 	 * @return mixed
 	 */
 	protected function prepare_value( $value ) {
 		if ( papi_is_empty( $value ) ) {
-			return $this->default_value;
+			return;
 		}
 
-		if ( $this->convert_type === 'string' ) {
-			$value = papi_convert_to_string( $value );
+		$value = papi_santize_data( $value );
+
+		if ( is_array( $value ) ) {
+			$value = array_filter( $value, function ( $val ) {
+				return ! papi_is_empty( $val );
+			} );
+
+			if ( array_keys( $value ) !== range( 0, count( $value ) - 1 ) ) {
+				$value = array_values( $value );
+			}
 		}
 
-		return papi_santize_data( $value );
+		return $value;
 	}
 
 	/**
@@ -861,6 +870,10 @@ private function setup_options_settings( $options ) {
 	 * @return mixed
 	 */
 	public function update_value( $value, $slug, $post_id ) {
+		if ( ! ( $value = $this->prepare_value( $value ) ) ) {
+			return;
+		}
+
 		return papi_maybe_json_encode( $value );
 	}
 }
diff --git a/src/properties/class-papi-property-file.php b/src/properties/class-papi-property-file.php
@@ -286,21 +286,6 @@ protected function setup_filters() {
 		add_action( 'wp_get_attachment_metadata', [$this, 'wp_get_attachment_metadata'], 10, 2 );
 	}
 
-	/**
-	 * Update value before it's saved to the database.
-	 *
-	 * @param  mixed  $value
-	 * @param  string $slug
-	 * @param  int    $post_id
-	 *
-	 * @return mixed
-	 */
-	public function update_value( $value, $slug, $post_id ) {
-		$value = is_array( $value ) ? array_values( array_filter( $value ) ) : $value;
-
-		return parent::update_value( $value, $slug, $post_id );
-	}
-
 	/**
 	 * Get attachment metadata.
 	 *

diff --git a/src/properties/class-papi-property.php b/src/properties/class-papi-property.php
@@ -25,7 +25,15 @@ public function get_value() {
 			}
 		}
 
-		return $this->prepare_value( $value );
+		if ( papi_is_empty( $value ) ) {
+			return $this->default_value;
+		}
+
+		if ( $this->convert_type === 'string' ) {
+			$value = papi_convert_to_string( $value );
+		}
+
+		return papi_santize_data( $value );
 	}
 
 	/**

diff --git a/tests/cases/core/class-papi-core-property-test.php b/tests/cases/core/class-papi-core-property-test.php
@@ -554,7 +554,7 @@ public function test_html_name_array() {
 		$this->assertSame( 'papi_name[0]', $property->html_name( $sub_property, 0 ) );
 	}
 
-	public function test_property_import_value() {
+	public function test_import_value() {
 		$property = Papi_Core_Property::create( [
 			'type' => 'number',
 			'slug' => 'age'
@@ -563,6 +563,26 @@ public function test_property_import_value() {
 		$this->assertSame( 'test', $property->import_value( 'test', '', 0 ) );
 	}
 
+	public function test_import_value_empty() {
+		$property = Papi_Core_Property::create( [
+			'type'  => 'string',
+			'title' => 'Name'
+		] );
+
+		$value = $property->import_value( [null], '', 0 );
+		$this->assertNull( $value );
+	}
+
+	public function test_import_value_with_empty_values() {
+		$property = Papi_Core_Property::create( [
+			'type'  => 'string',
+			'title' => 'Name'
+		] );
+
+		$value = $property->import_value( ['', 75, 87], '', 0 );
+		$this->assertSame( [75, 87], $value );
+	}
+
 	public function test_load_value() {
 		$property = Papi_Core_Property::create( [
 			'type'  => 'string',
@@ -714,6 +734,32 @@ public function test_update_value() {
 		$this->assertSame( 'Fredrik', $actual );
 	}
 
+	public function test_update_value_empty() {
+		$property = Papi_Core_Property::create( [
+			'type'  => 'string',
+			'title' => 'Name'
+		] );
+
+		$value = $property->update_value( [null], '', 0 );
+		$this->assertNull( $value );
+
+		$value1 = $property->load_value( $value, '', 0 );
+		$this->assertNull( $value1 );
+	}
+
+	public function test_update_value_with_empty_values() {
+		$property = Papi_Core_Property::create( [
+			'type'  => 'string',
+			'title' => 'Name'
+		] );
+
+		$value = $property->update_value( ['', 75, 87], '', 0 );
+		$this->assertTrue( is_string( $value ) );
+
+		$value1 = $property->load_value( $value, '', 0 );
+		$this->assertSame( [75, 87], $value1 );
+	}
+
 	public function test_value_serialize() {
 		$property = Papi_Core_Property::create( [
 			'type'  => 'string',