Test regression for settings update corrupting some wp_options

packages/block-library/src/site-logo/edit.js

@@ -255,45 +255,33 @@ export default function LogoEdit( {
 	const [ logoUrl, setLogoUrl ] = useState();
 	const [ error, setError ] = useState();
 	const ref = useRef();
-	const { mediaItemData, siteLogo, url, stylesheet } = useSelect(
-		( select ) => {
-			const siteSettings = select( coreStore ).getEditedEntityRecord(
-				'root',
-				'site'
-			);
-
-			const themeModOptionName = `theme_mods_${ siteSettings.stylesheet }`;
-
-			siteSettings[ themeModOptionName ] =
-				siteSettings[ themeModOptionName ] || {};
-			const mediaItem = siteSettings[ themeModOptionName ].custom_logo
-				? select( coreStore ).getEntityRecord(
-						'root',
-						'media',
-						siteSettings[ themeModOptionName ].custom_logo
-				  )
-				: null;
-			return {
-				mediaItemData: mediaItem && {
-					url: mediaItem.source_url,
-					alt: mediaItem.alt_text,
-				},
-				siteLogo: siteSettings[ themeModOptionName ].custom_logo,
-				url: siteSettings.url,
-				stylesheet: siteSettings.stylesheet,
-			};
-		},
-		[]
-	);
+	const { mediaItemData, siteLogo, url } = useSelect( ( select ) => {
+		const siteSettings = select( coreStore ).getEditedEntityRecord(
+			'root',
+			'site'
+		);
+		const mediaItem = siteSettings.site_logo
+			? select( coreStore ).getEntityRecord(
+					'root',
+					'media',
+					siteSettings.site_logo
+			  )
+			: null;
+		return {
+			mediaItemData: mediaItem && {
+				url: mediaItem.source_url,
+				alt: mediaItem.alt_text,
+			},
+			siteLogo: siteSettings.site_logo,
+			url: siteSettings.url,
+		};
+	}, [] );
 
 	const { editEntityRecord } = useDispatch( coreStore );
-	const setLogo = ( newValue ) => {
-		const settingsVal = {};
-		settingsVal[ `theme_mods_${ stylesheet }` ] = {
-			custom_logo: newValue,
-		};
-		editEntityRecord( 'root', 'site', undefined, settingsVal );
-	};
+	const setLogo = ( newValue ) =>
+		editEntityRecord( 'root', 'site', undefined, {
+			site_logo: newValue,
+		} );
 
 	let alt = null;
 	if ( mediaItemData ) {

packages/block-library/src/site-logo/index.php

@@ -25,6 +25,8 @@ function render_block_core_site_logo( $attributes ) {
 
 	$custom_logo = get_custom_logo();
 
+	remove_filter( 'wp_get_attachment_image_src', $adjust_width_height_filter );
+
 	if ( empty( $custom_logo ) ) {
 		return ''; // Return early if no custom logo is set, avoiding extraneous wrapper div.
 	}
@@ -56,10 +58,27 @@ function render_block_core_site_logo( $attributes ) {
 
 	$wrapper_attributes = get_block_wrapper_attributes( array( 'class' => implode( ' ', $classnames ) ) );
 	$html               = sprintf( '<div %s>%s</div>', $wrapper_attributes, $custom_logo );
-	remove_filter( 'wp_get_attachment_image_src', $adjust_width_height_filter );
 	return $html;
 }
 
+/**
+ * Register a core site setting for a site logo
+ */
+function register_block_core_site_logo_setting() {
+	register_setting(
+		'general',
+		'site_logo',
+		array(
+			'show_in_rest' => array(
+				'name' => 'site_logo',
+			),
+			'type'         => 'integer',
+			'description'  => __( 'Site logo.' ),
+		)
+	);
+}
+
+add_action( 'rest_api_init', 'register_block_core_site_logo_setting', 10 );
 
 /**
  * Registers the `core/site-logo` block on the server.
@@ -72,4 +91,74 @@ function register_block_core_site_logo() {
 		)
 	);
 }
+
 add_action( 'init', 'register_block_core_site_logo' );
+
+/**
+ * Overrides the custom logo with a site logo, if the option is set.
+ *
+ * @param string $custom_logo The custom logo set by a theme.
+ *
+ * @return string The site logo if set.
+ */
+function _override_custom_logo_theme_mod( $custom_logo ) {
+	$site_logo = get_option( 'site_logo' );
+	return false === $site_logo ? $custom_logo : $site_logo;
+}
+
+add_filter( 'theme_mod_custom_logo', '_override_custom_logo_theme_mod' );
+
+/**
+ * Updates the site_logo option when the custom_logo theme-mod gets updated.
+ *
+ * This function is hooked on "update_option_theme_mods_$theme" and not
+ * "pre_set_theme_mod_custom_logo" because by hooking in `update_option`
+ * the function accounts for remove_theme_mod() as well.
+ *
+ * @param mixed $old_value The old option value.
+ * @param mixed $value     The new option value.
+ */
+function _sync_custom_logo_to_site_logo( $old_value, $value ) {
+	// Delete the option when the custom logo does not exist or was removed.
+	// This step ensures the option stays in sync.
+	if ( empty( $value['custom_logo'] ) ) {
+		delete_option( 'site_logo' );
+	} else {
+		remove_action( 'update_option_site_logo', '_sync_site_logo_to_custom_logo' );
+		update_option( 'site_logo', $value['custom_logo'] );
+		add_action( 'update_option_site_logo', '_sync_site_logo_to_custom_logo', 10, 2 );
+	}
+}
+
+/**
+ * Hooks `_sync_custom_logo_to_site_logo` in `update_option_theme_mods_$theme`.
+ *
+ * Runs on `setup_theme` to account for dynamically-switched themes in the Customizer.
+ */
+function _sync_custom_logo_to_site_logo_on_setup_theme() {
+	$theme = get_option( 'stylesheet' );
+	add_action( "update_option_theme_mods_$theme", '_sync_custom_logo_to_site_logo', 10, 2 );
+}
+add_action( 'setup_theme', '_sync_custom_logo_to_site_logo_on_setup_theme', 11 );
+
+/**
+ * Updates the custom_logo theme-mod when the site_logo option gets updated.
+ *
+ * @param mixed $old_value The old option value.
+ * @param mixed $value     The new option value.
+ *
+ * @return void
+ */
+function _sync_site_logo_to_custom_logo( $old_value, $value ) {
+	// Delete the option when the custom logo does not exist or was removed.
+	// This step ensures the option stays in sync.
+	if ( empty( $value ) ) {
+		remove_theme_mod( 'custom_logo' );
+	} else {
+		remove_filter( 'pre_set_theme_mod_custom_logo', '_sync_custom_logo_to_site_logo' );
+		set_theme_mod( 'custom_logo', $value );
+		add_filter( 'pre_set_theme_mod_custom_logo', '_sync_custom_logo_to_site_logo' );
+	}
+}
+
+add_action( 'update_option_site_logo', '_sync_site_logo_to_custom_logo', 10, 2 );

packages/e2e-tests/specs/misc/settings.test.js

@@ -0,0 +1,43 @@
+/**
+ * WordPress dependencies
+ */
+import { visitAdminPage } from '@wordpress/e2e-test-utils';
+/**
+ * Internal dependencies
+ */
+
+async function getOptionsValues( selector ) {
+	await visitAdminPage( 'options.php' );
+	return page.evaluate( ( theSelector ) => {
+		const inputs = Array.from( document.querySelectorAll( theSelector ) );
+		return inputs.reduce( ( memo, input ) => {
+			memo[ input.id ] = input.value;
+			return memo;
+		}, {} );
+	}, selector );
+}
+
+describe( 'Settings', () => {
+	test( 'Regression: updating a specific option will only change its value and will not corrupt others', async () => {
+		// We won't select the option that we updated and will also remove some _transient options that seem to change at
+		// every update (?)
+		const optionsInputsSelector =
+			'form#all-options table.form-table input:not([id*="_transient"]):not([id="blogdescription"])';
+		const optionsBefore = await getOptionsValues( optionsInputsSelector );
+
+		await visitAdminPage( 'options-general.php' );
+		await page.type(
+			'input#blogdescription',
+			'Just another Gutenberg site'
+		);
+		await page.click( 'input#submit' );
+
+		const optionsAfter = await getOptionsValues( optionsInputsSelector );
+
+		Object.entries( optionsBefore ).forEach( ( optionBefore ) => {
+			const [ id ] = optionBefore;
+			const optionAfter = [ id, optionsAfter[ id ] ];
+			expect( optionAfter ).toStrictEqual( optionBefore );
+		} );
+	} );
+} );