Log individual values of settings that are string arrays

Automattic · May 27, 2019 · 944c500 · 944c500
1 parent 6e6a2d9
commit 944c500
Show file tree

Hide file tree

Showing 2 changed files with 122 additions and 3 deletions.
diff --git a/includes/class-sensei-settings.php b/includes/class-sensei-settings.php
@@ -726,15 +726,25 @@ public function log_settings_update( $old_value, $value ) {
 		// Find changed fields.
 		$changed = [];
 		foreach ( $this->fields as $field => $field_config ) {
+			// Handle absent fields.
 			$old_field_value = isset( $old_value[ $field ] ) ? $old_value[ $field ] : '';
 			$new_field_value = isset( $value[ $field ] ) ? $value[ $field ] : '';
+
 			if ( $new_field_value !== $old_field_value ) {
+				// Create an array for this section of settings if needed.
 				$section = $field_config['section'];
 				if ( ! isset( $changed[ $section ] ) ) {
-					$changed[ $section ] = [ $field ];
-				} else {
-					array_push( $changed[ $section ], $field );
+					$changed[ $section ] = [];
 				}
+
+				// Get changed setting values to be logged. In most cases, this
+				// will be an array containing only the name of the field.
+				$changed_values = $this->get_changed_setting_values(
+					$field,
+					$new_field_value,
+					$old_field_value
+				);
+				$changed[ $section ] = array_merge( $changed[ $section ], $changed_values );
 			}
 		}
 
@@ -749,6 +759,48 @@ public function log_settings_update( $old_value, $value ) {
 			);
 		}
 	}
+
+	/**
+	 * Get an array of setting values which were changed. In most cases, this
+	 * will simply be the name of the setting. However, if the setting is an
+	 * array of strings, then this will return an array of the string values
+	 * that were changed. These values returned will be of the form
+	 * "field_name[value_name]".
+	 *
+	 * @since 2.1.0
+	 *
+	 * @param string $field     The name of the setting field.
+	 * @param array  $new_array The new value.
+	 * @param array  $old_array The old value.
+	 *
+	 * @return array The array of strings representing the field that was
+	 *               changed, or an array containing the field name.
+	 */
+	private function get_changed_setting_values( $field, $new_value, $old_value ) {
+		// If the old and new values are not arrays, return the field name.
+		if ( ! is_array( $new_value ) || ! is_array( $old_value ) ) {
+			return [ $field ];
+		}
+
+		// Now, make sure they are both string arrays.
+		foreach ( array_merge( $new_value, $old_value ) as $value ) {
+			if ( ! is_string( $value ) ) {
+				return [ $field ];
+			}
+		}
+
+		// We have two string arrays. Return the difference in their values.
+		$added   = array_diff( $new_value, $old_value );
+		$removed = array_diff( $old_value, $new_value );
+		$diff    = array_merge( $added, $removed );
+
+		return array_map(
+			function( $value ) use ( $field ) {
+				return $field . '[' . $value . ']';
+			},
+			$diff
+		);
+	}
 } // End Class
 
 /**

diff --git a/tests/unit-tests/test-class-sensei-settings.php b/tests/unit-tests/test-class-sensei-settings.php
@@ -81,6 +81,73 @@ public function testOnlyLogSettingsOnUserUpdate() {
 		$this->assertCount( 0, $events );
 	}
 
+	/**
+	 * Test logging of array settings.
+	 *
+	 * @covers Sensei_Settings::log_settings_update
+	 */
+	public function testLogArraySettings() {
+		$settings = Sensei()->settings;
+
+		// Set "email_teachers" setting.
+		$settings->set( 'email_teachers', [ 'teacher-started-course', 'teacher-completed-course' ] );
+
+		// Get current settings.
+		$new = $settings->get_settings();
+
+		// Change the "email_teachers" setting.
+		$new['email_teachers'] = [ 'teacher-started-course', 'teacher-completed-lesson' ];
+
+		// Trigger update with new setting values.
+		$this->simulateSettingsRequest();
+		$old = $settings->get_settings();
+		$settings->log_settings_update( $old, $new );
+
+		// Ensure array setting change was logged.
+		$events = Sensei_Test_Events::get_logged_events();
+		$this->assertCount( 1, $events );
+
+		$changed = explode( ',', $events[0]['url_args']['settings'] );
+		sort( $changed );
+		$this->assertEquals(
+			[
+				'email_teachers[teacher-completed-course]',
+				'email_teachers[teacher-completed-lesson]',
+			],
+			$changed
+		);
+	}
+
+	/**
+	 * Test logging of non-string array settings.
+	 *
+	 * @covers Sensei_Settings::log_settings_update
+	 */
+	public function testLogNonStringArraySettings() {
+		$settings = Sensei()->settings;
+
+		// Set an array setting with non-string values.
+		$settings->set( 'email_learners', [ 'a string', [ 'a sub-array' ] ] );
+
+		// Get current settings.
+		$new = $settings->get_settings();
+
+		// Change the array setting.
+		$new['email_learners'] = [ 'a different string', [ 'a different sub-array' ] ];
+
+		// Trigger update with new setting values.
+		$this->simulateSettingsRequest();
+		$old = $settings->get_settings();
+		$settings->log_settings_update( $old, $new );
+
+		// Ensure array setting change was logged without values.
+		$events = Sensei_Test_Events::get_logged_events();
+		$this->assertCount( 1, $events );
+		$changed = explode( ',', $events[0]['url_args']['settings'] );
+		sort( $changed );
+		$this->assertEquals( [ 'email_learners' ], $changed );
+	}
+
 	/**
 	 * Simulate Sensei settings update request.
 	 */