Automattic · mdmoore · Sep 19, 2024 · Sep 4, 2024 · Sep 6, 2024 · Sep 9, 2024
diff --git a/changelog/update-9305-settings-disable-save-changes-button b/changelog/update-9305-settings-disable-save-changes-button
@@ -0,0 +1,4 @@
+Significance: minor
+Type: add
+
+Disable save changes button until a setting has changed.
diff --git a/client/components/csv-export-modal/test/index.test.tsx b/client/components/csv-export-modal/test/index.test.tsx
@@ -44,6 +44,7 @@ describe( 'RefundModal', () => {
 
 		mockUseSettings.mockReturnValue( {
 			isLoading: false,
+			isDirty: false,
 			isSaving: false,
 			saveSettings: ( a ) => a,
 		} );

diff --git a/client/data/settings/hooks.js b/client/data/settings/hooks.js
@@ -328,6 +328,7 @@ export const useSettings = () => {
 	const isSaving = useSelect( ( select ) =>
 		select( STORE_NAME ).isSavingSettings()
 	);
+	const isDirty = useSelect( ( select ) => select( STORE_NAME ).isDirty() );
 
 	const isLoading = useSelect( ( select ) => {
 		select( STORE_NAME ).getSettings();
@@ -342,6 +343,7 @@ export const useSettings = () => {
 		isLoading,
 		saveSettings,
 		isSaving,
+		isDirty,
 	};
 };
 

diff --git a/client/data/settings/reducer.js b/client/data/settings/reducer.js
@@ -6,6 +6,7 @@
 import ACTION_TYPES from './action-types';
 
 const defaultState = {
+	isDirty: false,
 	isSaving: false,
 	savingError: null,
 	data: {},
@@ -20,12 +21,14 @@ export const receiveSettings = (
 			return {
 				...state,
 				data: action.data,
+				isDirty: false,
 			};
 
 		case ACTION_TYPES.SET_SETTINGS_VALUES:
 			return {
 				...state,
 				savingError: null,
+				isDirty: true,
 				data: {
 					...state.data,
 					...action.payload,
@@ -35,13 +38,16 @@ export const receiveSettings = (
 		case ACTION_TYPES.SET_IS_SAVING_SETTINGS:
 			return {
 				...state,
+				isDirty:
+					action.isSaving || action.error ? state.isDirty : false,
 				isSaving: action.isSaving,
 				savingError: action.error,
 			};
 
 		case ACTION_TYPES.SET_SELECTED_PAYMENT_METHOD:
 			return {
 				...state,
+				isDirty: true,
 				data: {
 					...state.data,
 					enabled_payment_method_ids: state.data.enabled_payment_method_ids.concat(
@@ -53,6 +59,7 @@ export const receiveSettings = (
 		case ACTION_TYPES.SET_UNSELECTED_PAYMENT_METHOD:
 			return {
 				...state,
+				isDirty: true,
 				data: {
 					...state.data,
 					enabled_payment_method_ids: state.data.enabled_payment_method_ids.filter(

diff --git a/client/data/settings/selectors.js b/client/data/settings/selectors.js
@@ -49,6 +49,10 @@ export const isSavingSettings = ( state ) => {
 	return getSettingsState( state ).isSaving || false;
 };
 
+export const isDirty = ( state ) => {
+	return getSettingsState( state ).isDirty || false;
+};
+
 export const getAccountStatementDescriptor = ( state ) => {
 	return getSettings( state ).account_statement_descriptor || '';
 };

diff --git a/client/data/settings/test/hooks.js b/client/data/settings/test/hooks.js
@@ -177,6 +177,7 @@ describe( 'Settings hooks tests', () => {
 				hasFinishedResolution: jest.fn(),
 				isResolving: jest.fn(),
 				isSavingSettings: jest.fn(),
+				isDirty: jest.fn(),
 			};
 		} );
 

diff --git a/client/data/settings/test/reducer.js b/client/data/settings/test/reducer.js
@@ -28,6 +28,7 @@ describe( 'Settings reducer tests', () => {
 			isSaving: false,
 			data: {},
 			savingError: null,
+			isDirty: false,
 		} );
 	} );
 
@@ -60,6 +61,7 @@ describe( 'Settings reducer tests', () => {
 
 		test( 'leaves fields other than `data` unchanged', () => {
 			const oldState = {
+				isDirty: false,
 				foo: 'bar',
 				data: {
 					baz: 'quux',
@@ -74,6 +76,7 @@ describe( 'Settings reducer tests', () => {
 			const state = reducer( oldState, updateSettings( newSettings ) );
 
 			expect( state ).toEqual( {
+				isDirty: false,
 				foo: 'bar',
 				data: {
 					quuz: 'corge',
@@ -101,6 +104,7 @@ describe( 'Settings reducer tests', () => {
 
 		test( 'leaves other fields unchanged', () => {
 			const oldState = {
+				isDirty: false,
 				foo: 'bar',
 				isSaving: false,
 				savingError: {},
@@ -112,6 +116,7 @@ describe( 'Settings reducer tests', () => {
 			);
 
 			expect( state ).toEqual( {
+				isDirty: false,
 				foo: 'bar',
 				savingError: null,
 				isSaving: true,
@@ -137,6 +142,7 @@ describe( 'Settings reducer tests', () => {
 
 		test( 'leaves other fields unchanged', () => {
 			const oldState = {
+				isDirty: false,
 				foo: 'bar',
 				data: {
 					is_manual_capture_enabled: false,
@@ -151,6 +157,7 @@ describe( 'Settings reducer tests', () => {
 			);
 
 			expect( state ).toEqual( {
+				isDirty: true,
 				savingError: null,
 				foo: 'bar',
 				data: {
@@ -181,6 +188,7 @@ describe( 'Settings reducer tests', () => {
 
 		test( 'leaves other fields unchanged', () => {
 			const oldState = {
+				isDirty: false,
 				foo: 'bar',
 				data: {
 					account_statement_descriptor: 'Statement',
@@ -195,6 +203,7 @@ describe( 'Settings reducer tests', () => {
 			);
 
 			expect( state ).toEqual( {
+				isDirty: true,
 				foo: 'bar',
 				savingError: null,
 				data: {
@@ -224,6 +233,7 @@ describe( 'Settings reducer tests', () => {
 
 		test( 'leaves other fields unchanged', () => {
 			const oldState = {
+				isDirty: false,
 				foo: 'bar',
 				data: {
 					is_payment_request_enabled: false,
@@ -238,6 +248,7 @@ describe( 'Settings reducer tests', () => {
 			);
 
 			expect( state ).toEqual( {
+				isDirty: true,
 				foo: 'bar',
 				savingError: null,
 				data: {
@@ -271,6 +282,7 @@ describe( 'Settings reducer tests', () => {
 
 		test( 'leaves other fields unchanged', () => {
 			const oldState = {
+				isDirty: false,
 				foo: 'bar',
 				data: {
 					payment_request_enabled_locations: initPaymentRequestState,
@@ -285,6 +297,7 @@ describe( 'Settings reducer tests', () => {
 			);
 
 			expect( state ).toEqual( {
+				isDirty: true,
 				foo: 'bar',
 				data: {
 					payment_request_enabled_locations: enableAllpaymentRequestState,
@@ -350,6 +363,7 @@ describe( 'Settings reducer tests', () => {
 			'leaves other fields unchanged `%j`',
 			( setting ) => {
 				const oldState = {
+					isDirty: false,
 					foo: 'bar',
 					data: {
 						[ setting.stateKey ]: setting.settingValue,
@@ -364,6 +378,7 @@ describe( 'Settings reducer tests', () => {
 				);
 
 				expect( state ).toEqual( {
+					isDirty: true,
 					foo: 'bar',
 					savingError: null,
 					data: {
@@ -378,6 +393,7 @@ describe( 'Settings reducer tests', () => {
 	describe( 'SET_IS_WOOPAY_ENABLED', () => {
 		test( 'toggles `data.is_woopay_enabled`', () => {
 			const oldState = {
+				isDirty: true,
 				data: {
 					is_woopay_enabled: false,
 				},
@@ -391,6 +407,7 @@ describe( 'Settings reducer tests', () => {
 
 		test( 'leaves other fields unchanged', () => {
 			const oldState = {
+				isDirty: false,
 				foo: 'bar',
 				data: {
 					is_woopay_enabled: false,
@@ -402,6 +419,7 @@ describe( 'Settings reducer tests', () => {
 			const state = reducer( oldState, updateIsWooPayEnabled( true ) );
 
 			expect( state ).toEqual( {
+				isDirty: true,
 				foo: 'bar',
 				savingError: null,
 				data: {
@@ -430,6 +448,7 @@ describe( 'Settings reducer tests', () => {
 
 		test( 'leaves other fields unchanged', () => {
 			const oldState = {
+				isDirty: false,
 				foo: 'bar',
 				data: {
 					woopay_custom_message: '',
@@ -444,6 +463,7 @@ describe( 'Settings reducer tests', () => {
 			);
 
 			expect( state ).toEqual( {
+				isDirty: true,
 				foo: 'bar',
 				data: {
 					woopay_custom_message: 'test',
@@ -469,6 +489,7 @@ describe( 'Settings reducer tests', () => {
 
 		test( 'leaves other fields unchanged', () => {
 			const oldState = {
+				isDirty: false,
 				foo: 'bar',
 				data: {
 					woopay_store_logo: '',
@@ -480,6 +501,7 @@ describe( 'Settings reducer tests', () => {
 			const state = reducer( oldState, updateWooPayStoreLogo( 'test' ) );
 
 			expect( state ).toEqual( {
+				isDirty: true,
 				foo: 'bar',
 				data: {
 					woopay_store_logo: 'test',

diff --git a/client/disputes/test/index.tsx b/client/disputes/test/index.tsx
@@ -176,6 +176,7 @@ describe( 'Disputes list', () => {
 			isLoading: false,
 			isSaving: false,
 			saveSettings: ( a ) => a,
+			isDirty: false,
 		} );
 
 		global.wcpaySettings = {

diff --git a/client/settings/fraud-protection/advanced-settings/cards/order-items-threshold.tsx b/client/settings/fraud-protection/advanced-settings/cards/order-items-threshold.tsx
@@ -37,6 +37,7 @@ const OrderItemsThresholdCustomForm: React.FC< OrderItemsThresholdCustomFormProp
 		protectionSettingsUI,
 		setProtectionSettingsUI,
 		setProtectionSettingsChanged,
+		setIsDirty,
 	} = useContext( FraudPreventionSettingsContext );
 
 	const settingUI = useMemo(
@@ -97,7 +98,10 @@ const OrderItemsThresholdCustomForm: React.FC< OrderItemsThresholdCustomFormProp
 						placeholder={ '0' }
 						value={ minItemsCount }
 						type="number"
-						onChange={ setMinItemsCount }
+						onChange={ ( value ) => {
+							setMinItemsCount( value );
+							setIsDirty( true );
+						} }
 						onKeyDown={ ( e ) =>
 							/^[+-.,e]$/m.test( e.key ) && e.preventDefault()
 						}
@@ -121,7 +125,10 @@ const OrderItemsThresholdCustomForm: React.FC< OrderItemsThresholdCustomFormProp
 						placeholder={ '0' }
 						type="number"
 						value={ maxItemsCount }
-						onChange={ setMaxItemsCount }
+						onChange={ ( value ) => {
+							setMaxItemsCount( value );
+							setIsDirty( true );
+						} }
 						onKeyDown={ ( e ) =>
 							/^[+-.,e]$/m.test( e.key ) && e.preventDefault()
 						}

diff --git a/client/settings/fraud-protection/advanced-settings/cards/purchase-price-threshold.tsx b/client/settings/fraud-protection/advanced-settings/cards/purchase-price-threshold.tsx
@@ -56,6 +56,7 @@ const PurchasePriceThresholdCustomForm: React.FC< PurchasePriceThresholdCustomFo
 		protectionSettingsUI,
 		setProtectionSettingsUI,
 		setProtectionSettingsChanged,
+		setIsDirty,
 	} = useContext( FraudPreventionSettingsContext );
 
 	const settingUI = useMemo(
@@ -111,7 +112,10 @@ const PurchasePriceThresholdCustomForm: React.FC< PurchasePriceThresholdCustomFo
 						prefix={ currencySymbol }
 						placeholder={ '0.00' }
 						value={ minAmount.toString() }
-						onChange={ ( val ) => setMinAmount( Number( val ) ) }
+						onChange={ ( val ) => {
+							setMinAmount( Number( val ) );
+							setIsDirty( true );
+						} }
 						help={ __(
 							'Leave blank for no limit',
 							'woocommerce-payments'
@@ -130,7 +134,10 @@ const PurchasePriceThresholdCustomForm: React.FC< PurchasePriceThresholdCustomFo
 						prefix={ currencySymbol }
 						placeholder={ '0.00' }
 						value={ maxAmount.toString() }
-						onChange={ ( val ) => setMaxAmount( Number( val ) ) }
+						onChange={ ( val ) => {
+							setMaxAmount( Number( val ) );
+							setIsDirty( true );
+						} }
 						help={ __(
 							'Leave blank for no limit',
 							'woocommerce-payments'

diff --git a/client/settings/fraud-protection/advanced-settings/cards/test/address-mismatch.test.tsx b/client/settings/fraud-protection/advanced-settings/cards/test/address-mismatch.test.tsx
@@ -34,6 +34,7 @@ describe( 'Address mismatch card', () => {
 		setProtectionSettingsUI: setSettings,
 		protectionSettingsChanged: false,
 		setProtectionSettingsChanged: jest.fn(),
+		setIsDirty: jest.fn(),
 	};
 	test( 'renders correctly', () => {
 		settings.address_mismatch.enabled = false;

diff --git a/client/settings/fraud-protection/advanced-settings/cards/test/avs-mismatch.test.tsx b/client/settings/fraud-protection/advanced-settings/cards/test/avs-mismatch.test.tsx
@@ -42,6 +42,7 @@ describe( 'AVS mismatch card', () => {
 			setProtectionSettingsUI: setSettings,
 			protectionSettingsChanged: false,
 			setProtectionSettingsChanged: jest.fn(),
+			setIsDirty: jest.fn(),
 		};
 		const { container } = render(
 			<FraudPreventionSettingsContext.Provider value={ contextValue }>
@@ -70,6 +71,7 @@ describe( 'AVS mismatch card', () => {
 			setProtectionSettingsUI: setSettings,
 			protectionSettingsChanged: false,
 			setProtectionSettingsChanged: jest.fn(),
+			setIsDirty: jest.fn(),
 		};
 		const { container } = render(
 			<FraudPreventionSettingsContext.Provider value={ contextValue }>

diff --git a/client/settings/fraud-protection/advanced-settings/cards/test/cvc-verification.test.tsx b/client/settings/fraud-protection/advanced-settings/cards/test/cvc-verification.test.tsx
@@ -42,6 +42,7 @@ describe( 'CVC verification card', () => {
 			setProtectionSettingsUI: setSettings,
 			protectionSettingsChanged: false,
 			setProtectionSettingsChanged: jest.fn(),
+			setIsDirty: jest.fn(),
 		};
 		const { container } = render(
 			<FraudPreventionSettingsContext.Provider value={ contextValue }>
@@ -73,6 +74,7 @@ describe( 'CVC verification card', () => {
 			setProtectionSettingsUI: setSettings,
 			protectionSettingsChanged: false,
 			setProtectionSettingsChanged: jest.fn(),
+			setIsDirty: jest.fn(),
 		};
 		const { container } = render(
 			<FraudPreventionSettingsContext.Provider value={ contextValue }>

diff --git a/.../settings/fraud-protection/advanced-settings/cards/test/international-ip-address.test.tsx b/.../settings/fraud-protection/advanced-settings/cards/test/international-ip-address.test.tsx
@@ -43,6 +43,8 @@ describe( 'International IP address card', () => {
 		setProtectionSettingsUI: setSettings,
 		protectionSettingsChanged: false,
 		setProtectionSettingsChanged: jest.fn(),
+		isDirty: false,
+		setIsDirty: jest.fn(),
 	};
 	global.wcSettings = {
 		admin: {