Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Social: Refactor feature option storing #34113

Merged
merged 26 commits into from
Dec 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
f37dd1a
Add the new settings object + tests
gmjuhasz Nov 14, 2023
d00d07e
Update initial states + previous usages
gmjuhasz Nov 14, 2023
012eb2c
Update UI + store functions
gmjuhasz Nov 14, 2023
b36d720
changelog
gmjuhasz Nov 14, 2023
5896416
Fix versions
gmjuhasz Nov 14, 2023
b6952a2
Add control for fetching both options together
gmjuhasz Nov 15, 2023
b5de327
Social: Remove deprecated feature files (#34114)
gmjuhasz Nov 20, 2023
2b51295
Social: Refresh store on Publicize module toggle via 1 call (#34142)
gmjuhasz Nov 20, 2023
8fbd642
Merge branch 'trunk' into refactor/social-register-feature-settings
gmjuhasz Nov 20, 2023
aa4ee16
Rename structure
gmjuhasz Nov 20, 2023
659e0a1
Merge branch 'trunk' into refactor/social-register-feature-settings
gmjuhasz Nov 27, 2023
77e27ee
Fixup versions
gmjuhasz Nov 27, 2023
e809f08
Fix template issue, and make sure it's migrated
gmjuhasz Nov 28, 2023
b9ac65c
Merge branch 'trunk' into refactor/social-register-feature-settings
gmjuhasz Nov 28, 2023
9c0234d
Merge branch 'trunk' into refactor/social-register-feature-settings
gmjuhasz Nov 28, 2023
6bec608
fix tests
gmjuhasz Nov 28, 2023
d4b52c6
Test fix
gmjuhasz Nov 28, 2023
484f96f
Merge branch 'trunk' into refactor/social-register-feature-settings
gmjuhasz Nov 30, 2023
0ba898a
Fix auto-conversion migration logic + add test
gmjuhasz Nov 30, 2023
f91feb3
Add auto-conversion sync option
gmjuhasz Nov 30, 2023
a89e7b5
changelog
gmjuhasz Nov 30, 2023
d7949bf
Merge branch 'trunk' into refactor/social-register-feature-settings
gmjuhasz Dec 5, 2023
b7c2568
Merge branch 'trunk' into refactor/social-register-feature-settings
gmjuhasz Dec 7, 2023
9b6020a
Merge branch 'trunk' into refactor/social-register-feature-settings
gmjuhasz Dec 7, 2023
c169499
Update versions
gmjuhasz Dec 7, 2023
df78925
Fix versions
gmjuhasz Dec 7, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: major
Type: changed

Social: Refactored storing of feature options to use core functions
1 change: 1 addition & 0 deletions projects/js-packages/publicize-components/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export { default as PublicizePanel } from './src/components/panel';
export { default as ReviewPrompt } from './src/components/review-prompt';
export { default as PostPublishReviewPrompt } from './src/components/post-publish-review-prompt';
export { default as PostPublishOneClickSharing } from './src/components/post-publish-one-click-sharing';
export { default as RefreshJetpackSocialSettingsWrapper } from './src/components/refresh-jetpack-social-settings';

export { default as useSocialMediaConnections } from './src/hooks/use-social-media-connections';
export { default as useSocialMediaMessage } from './src/hooks/use-social-media-message';
Expand Down
2 changes: 1 addition & 1 deletion projects/js-packages/publicize-components/package.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"private": true,
"name": "@automattic/jetpack-publicize-components",
"version": "0.41.9",
"version": "0.42.0-alpha",
"description": "A library of JS components required by the Publicize editor plugin",
"homepage": "https://github.com/Automattic/jetpack/tree/HEAD/projects/js-packages/publicize-components/#readme",
"bugs": {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { ToggleControl } from '@automattic/jetpack-components';
import { useSelect, useDispatch } from '@wordpress/data';
import { useCallback } from '@wordpress/element';
import { useEffect } from '@wordpress/element';
import React from 'react';
import { SOCIAL_STORE_ID } from '../../../social-store';
import { SocialStoreSelectors } from '../../../types/types';
Expand All @@ -16,11 +15,6 @@ type AutoConversionToggleProps = {
* The class name to add to the toggle.
*/
toggleClass?: string;

/**
* Whether or not to refresh the settings.
*/
shouldRefresh?: boolean;
};

/**
Expand All @@ -30,16 +24,9 @@ type AutoConversionToggleProps = {
* @returns {React.ReactElement} - JSX.Element
*/
const AutoConversionToggle: React.FC< AutoConversionToggleProps > = ( {
shouldRefresh = false,
toggleClass,
children,
} ) => {
const refreshSettings = useDispatch( SOCIAL_STORE_ID ).refreshAutoConversionSettings;

useEffect( () => {
shouldRefresh && refreshSettings();
}, [ shouldRefresh, refreshSettings ] );

const { isEnabled, isUpdating } = useSelect( select => {
const store = select( SOCIAL_STORE_ID ) as SocialStoreSelectors;
return {
Expand All @@ -52,7 +39,7 @@ const AutoConversionToggle: React.FC< AutoConversionToggleProps > = ( {

const toggleStatus = useCallback( () => {
const newOption = {
image: ! isEnabled,
enabled: ! isEnabled,
};
updateOptions( newOption );
}, [ isEnabled, updateOptions ] );
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import { useDispatch } from '@wordpress/data';
import { SOCIAL_STORE_ID } from '../../social-store';

/**
* HOC that refreshes all of the Jetpack Social settings in the store, to be used in class components.
*
* @param {object} props - The component props.
* @param {boolean} props.shouldRefresh - Whether or not to refresh the settings.
* @param {object} props.children - The children to render.
* @returns { object } The refreshJetpackSocialSettings function.
*/
export default function RefreshJetpackSocialSettingsWrapper( { shouldRefresh, children } ) {
const refreshOptions = useDispatch( SOCIAL_STORE_ID ).refreshJetpackSocialSettings;

if ( shouldRefresh ) {
refreshOptions();
}

return children;
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ const TemplatePickerButton: React.FC = () => {

useEffect( () => {
if ( currentTemplate ) {
const newOption = { defaults: { template: currentTemplate } };
const newOption = { template: currentTemplate };
updateOptions( newOption );
}
}, [ currentTemplate, updateOptions ] );
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { ToggleControl } from '@automattic/jetpack-components';
import { useSelect, useDispatch } from '@wordpress/data';
import { useCallback } from '@wordpress/element';
import { useEffect } from '@wordpress/element';
import React from 'react';
import { SOCIAL_STORE_ID } from '../../../social-store';
import { SocialStoreSelectors } from '../../../types/types';
Expand All @@ -16,11 +15,6 @@ type SocialImageGeneratorToggleProps = {
* The class name to add to the toggle.
*/
toggleClass?: string;

/**
* Whether or not to refresh the settings.
*/
shouldRefresh?: boolean;
};

/**
Expand All @@ -32,14 +26,7 @@ type SocialImageGeneratorToggleProps = {
const SocialImageGeneratorToggle: React.FC< SocialImageGeneratorToggleProps > = ( {
toggleClass,
children,
shouldRefresh = false,
} ) => {
const refreshSettings = useDispatch( SOCIAL_STORE_ID ).refreshSocialImageGeneratorSettings;

useEffect( () => {
shouldRefresh && refreshSettings();
}, [ refreshSettings, shouldRefresh ] );

const { isEnabled, isUpdating } = useSelect( select => {
const store = select( SOCIAL_STORE_ID ) as SocialStoreSelectors;
return {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export function* updateAutoConversionSettings( settings ) {
yield setAutoConversionSettings( settings );
yield updateAutoConversionSettingsControl( settings );
const updatedSettings = yield fetchAutoConversionSettings();
yield setAutoConversionSettings( updatedSettings );
yield setAutoConversionSettings( updatedSettings.jetpack_social_autoconvert_images );
return true;
} catch ( e ) {
const oldSettings = select( SOCIAL_STORE_ID ).getAutoConversionSettings();
Expand All @@ -41,7 +41,7 @@ export function* refreshAutoConversionSettings() {
try {
yield setUpdatingAutoConversionSettings();
const updatedSettings = yield fetchAutoConversionSettings();
yield setAutoConversionSettings( updatedSettings );
yield setAutoConversionSettings( updatedSettings.jetpack_social_autoconvert_images );
return true;
} catch ( e ) {
return false;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
import autoConversionSettingActions from './auto-conversion-settings';
import * as connectionData from './connection-data';
import siteSettingActions from './jetpack-settings';
import jetpackSocialSettings from './jetpack-social-settings';
import socialImageGeneratorSettingActions from './social-image-generator-settings';

const actions = {
...siteSettingActions,
...socialImageGeneratorSettingActions,
...autoConversionSettingActions,
...jetpackSocialSettings,
...connectionData,
};

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import { fetchJetpackSocialSettings } from '../controls';
import {
setAutoConversionSettings,
setUpdatingAutoConversionSettings,
setUpdatingAutoConversionSettingsDone,
} from './auto-conversion-settings';
import {
setSocialImageGeneratorSettings,
setUpdatingSocialImageGeneratorSettings,
setUpdatingSocialImageGeneratorSettingsDone,
} from './social-image-generator-settings';

/**
* Yield actions to refresh all of the Jetpack Social registered settings.
*
* @yields {object} - an action object.
* @returns {object} - an action object.
*/
export function* refreshJetpackSocialSettings() {
try {
yield setUpdatingAutoConversionSettings();
yield setUpdatingSocialImageGeneratorSettings();
const updatedSettings = yield fetchJetpackSocialSettings();
yield setAutoConversionSettings( updatedSettings.jetpack_social_autoconvert_images );
yield setSocialImageGeneratorSettings(
updatedSettings.jetpack_social_image_generator_settings
);
return true;
} catch ( e ) {
return false;
} finally {
yield setUpdatingAutoConversionSettingsDone();
yield setUpdatingSocialImageGeneratorSettingsDone();
}
Comment on lines +26 to +34
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TIL that finally block is executed before the return statement inside try/catch.

}

export default {
refreshJetpackSocialSettings,
};
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ export function* updateSocialImageGeneratorSettings( settings ) {
yield setSocialImageGeneratorSettings( settings );
yield updateSocialImageGeneratorSettingsControl( settings );
const updatedSettings = yield fetchSocialImageGeneratorSettings();
yield setSocialImageGeneratorSettings( updatedSettings );
yield setSocialImageGeneratorSettings(
updatedSettings.jetpack_social_image_generator_settings
);
return true;
} catch ( e ) {
const oldSettings = select( SOCIAL_STORE_ID ).getSocialImageGeneratorSettings();
Expand Down Expand Up @@ -69,7 +71,9 @@ export function* refreshSocialImageGeneratorSettings() {
try {
yield setUpdatingSocialImageGeneratorSettings();
const updatedSettings = yield fetchSocialImageGeneratorSettings();
yield setSocialImageGeneratorSettings( updatedSettings );
yield setSocialImageGeneratorSettings(
updatedSettings.jetpack_social_image_generator_settings
);
return true;
} catch ( e ) {
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ export const UPDATE_SOCIAL_IMAGE_GENERATOR_SETTINGS = 'UPDATE_SOCIAL_IMAGE_GENER

export const FETCH_AUTO_CONVERSION_SETTINGS = 'FETCH_AUTO_CONVERSION_SETTINGS';
export const UPDATE_AUTO_CONVERSION_SETTINGS = 'UPDATE_AUTO_CONVERSION_SETTINGS';
export const FETCH_JETPACK_SOCIAL_SETTINGS = 'FETCH_JETPACK_SOCIAL_SETTINGS';

/**
* fetchJetpackSettings action
Expand Down Expand Up @@ -67,6 +68,17 @@ export const fetchAutoConversionSettings = () => {
};
};

/**
* fetchJetpackSocialSettings action
*
* @returns {object} - an action object.
*/
export const fetchJetpackSocialSettings = () => {
return {
type: FETCH_JETPACK_SOCIAL_SETTINGS,
};
};

/**
* updateAutoConversionSettings action
*
Expand All @@ -92,23 +104,36 @@ export default {
} );
},
[ FETCH_SOCIAL_IMAGE_GENERATOR_SETTINGS ]: function () {
return apiFetch( { path: '/jetpack/v4/social-image-generator/settings' } );
return apiFetch( {
path: '/wp/v2/settings?_fields=jetpack_social_image_generator_settings',
} );
},
[ UPDATE_SOCIAL_IMAGE_GENERATOR_SETTINGS ]: function ( action ) {
return apiFetch( {
path: '/jetpack/v4/social-image-generator/settings',
path: '/wp/v2/settings',
method: 'POST',
data: action.settings,
data: {
jetpack_social_image_generator_settings: action.settings,
},
} );
},
[ FETCH_AUTO_CONVERSION_SETTINGS ]: function () {
return apiFetch( { path: '/jetpack/v4/auto-conversion/settings' } );
return apiFetch( {
path: '/wp/v2/settings?_fields=jetpack_social_autoconvert_images',
} );
},
[ UPDATE_AUTO_CONVERSION_SETTINGS ]: function ( action ) {
return apiFetch( {
path: '/jetpack/v4/auto-conversion/settings',
path: '/wp/v2/settings',
method: 'POST',
data: action.settings,
Comment on lines 112 to -111
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need multiple API calls? Can we make a single API call to get all the settings at once?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can add the settings fields separated by comma

/wp/v2/settings?_fields=jetpack_social_image_generator_settings,jetpack_social_autoconvert_images

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, to fetch the settings we can just have one call.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I'll do that.

Copy link
Contributor Author

@gmjuhasz gmjuhasz Nov 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This turned out to be a bigger change as I had to remove the old refreshes introduced in #33969, so I added the store functions in b6952a2 and created another PR on this branch for the others #34142

data: {
jetpack_social_autoconvert_images: action.settings,
},
} );
},
[ FETCH_JETPACK_SOCIAL_SETTINGS ]: function () {
return apiFetch( {
path: '/wp/v2/settings?_fields=jetpack_social_autoconvert_images,jetpack_social_image_generator_settings',
} );
},
};
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export function* getSocialImageGeneratorSettings() {
try {
const settings = yield fetchSocialImageGeneratorSettings();
if ( settings ) {
return setSocialImageGeneratorSettings( settings );
return setSocialImageGeneratorSettings( settings.jetpack_social_image_generator_settings );
}
} catch ( e ) {
// TODO: Add proper error handling here
Expand All @@ -55,7 +55,7 @@ export function* getAutoConversionSettings() {
try {
const settings = yield fetchAutoConversionSettings();
if ( settings ) {
return setAutoConversionSettings( settings );
return setAutoConversionSettings( settings.jetpack_social_autoconvert_images );
}
} catch ( e ) {
// TODO: Add proper error handling here
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
const autoConversionSettingsSelectors = {
getAutoConversionSettings: state => state.autoConversionSettings,
isAutoConversionAvailable: state => state.autoConversionSettings.available,
isAutoConversionEnabled: state => state.autoConversionSettings.image,
isAutoConversionEnabled: state => state.autoConversionSettings.enabled,
isAutoConversionSettingsUpdating: state => state.autoConversionSettings.isUpdating,
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@ const socialImageGeneratorSettingsSelectors = {
isSocialImageGeneratorAvailable: state => state.socialImageGeneratorSettings.available,
isSocialImageGeneratorEnabled: state => state.socialImageGeneratorSettings.enabled,
isUpdatingSocialImageGeneratorSettings: state => state.socialImageGeneratorSettings.isUpdating,
getSocialImageGeneratorDefaultTemplate: state =>
state.socialImageGeneratorSettings.defaultTemplate,
getSocialImageGeneratorDefaultTemplate: state => state.socialImageGeneratorSettings.template,
};

export default socialImageGeneratorSettingsSelectors;
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: major
Type: changed

Social: Refactored storing of feature options to use core functions
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: minor
Type: removed

Social: Removed deprecated files because of refactore
2 changes: 1 addition & 1 deletion projects/packages/publicize/composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@
"link-template": "https://github.com/Automattic/jetpack-publicize/compare/v${old}...v${new}"
},
"branch-alias": {
"dev-trunk": "0.37.x-dev"
"dev-trunk": "0.38.x-dev"
}
},
"config": {
Expand Down
2 changes: 1 addition & 1 deletion projects/packages/publicize/package.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"private": true,
"name": "@automattic/jetpack-publicize",
"version": "0.37.2",
"version": "0.38.0-alpha",
"description": "Publicize makes it easy to share your site’s posts on several social media networks automatically when you publish a new post.",
"homepage": "https://github.com/Automattic/jetpack/tree/HEAD/projects/packages/publicize/#readme",
"bugs": {
Expand Down
Loading
Loading