Skip to content

Display settings 'label' defined by the 'register_setting' method #59243

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.

Sign up for GitHub

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

Merged
Mamaduka merged 11 commits into from
Mar 20, 2024
Merged

Display settings 'label' defined by the 'register_setting' method #59243

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
f875c12
Try: Add 'label' argument to the 'register_setting' method
Mamaduka Feb 21, 2024
cffa889
Use title keywork
Mamaduka Feb 21, 2024
bf0b9ff
Fix route override
Mamaduka Feb 21, 2024
30ba923
Use just filter
Mamaduka Feb 21, 2024
579dce7
Update getOrLoadEntitiesConfig
Mamaduka Feb 21, 2024
96ee0ea
Temp update selectors
Mamaduka Feb 27, 2024
0c1a177
Finalize first iteration
Mamaduka Feb 27, 2024
3f2d0f1
Fix unit test
Mamaduka Feb 27, 2024
3ad2c56
Feedback
Mamaduka Mar 14, 2024
f95e7af
Restore site entity shortcuts
Mamaduka Mar 18, 2024
fb77e6b
Update 'getOrLoadEntitiesConfig' doc-block
Mamaduka Mar 18, 2024
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
50 changes: 50 additions & 0 deletions lib/compat/wordpress-6.6/option.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
<?php
/**
* Options API changes for the Gutenberg plugin.
*
* @package gutenberg
*/

/**
* Updates arguments for default settings available in WordPress.
*
* Note: During the core sync, the updates will be made to `register_initial_settings`.
*/
function gutenberg_update_initial_settings( $args, $defaults, $option_group, $option_name ) {
$settings_label_map = array(
'blogname' => __( 'Title' ),
'blogdescription' => __( 'Tagline' ),
'site_logo' => __( 'Logo' ),
'site_icon' => __( 'Icon' ),
'show_on_front' => __( 'Show on front' ),
'page_on_front' => __( 'Page on front' ),
'posts_per_page' => __( 'Maximum posts per page' ),
'default_comment_status' => __( 'Allow comments on new posts' ),
);

if ( isset( $settings_label_map[ $option_name ] ) ) {
$args['label'] = $settings_label_map[ $option_name ];
}

// Don't update schema when label isn't provided.
if ( ! isset( $args['label'] ) ) {
return $args;
}

$schema = array( 'title' => $args['label'] );
if ( ! is_array( $args['show_in_rest'] ) ) {
$args['show_in_rest'] = array(
'schema' => $schema,
);
return $args;
}

if ( ! empty( $args['show_in_rest']['schema'] ) ) {
$args['show_in_rest']['schema'] = array_merge( $args['show_in_rest']['schema'], $schema );
} else {
$args['show_in_rest']['schema'] = $schema;
}

return $args;
}
add_filter( 'register_setting_args', 'gutenberg_update_initial_settings', 10, 4 );
1 change: 1 addition & 0 deletions lib/load.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ function gutenberg_is_experiment_enabled( $name ) {

// WordPress 6.6 compat.
require __DIR__ . '/compat/wordpress-6.6/block-bindings/pattern-overrides.php';
require __DIR__ . '/compat/wordpress-6.6/option.php';

// Experimental features.
require __DIR__ . '/experimental/block-editor-settings-mobile.php';
Expand Down
10 changes: 5 additions & 5 deletions packages/core-data/src/actions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ export const deleteEntityRecord =
{ __unstableFetch = apiFetch, throwOnError = false } = {}
) =>
async ( { dispatch } ) => {
const configs = await dispatch( getOrLoadEntitiesConfig( kind ) );
const configs = await dispatch( getOrLoadEntitiesConfig( kind, name ) );
const entityConfig = configs.find(
( config ) => config.kind === kind && config.name === name
);
Expand Down Expand Up @@ -503,7 +503,7 @@ export const saveEntityRecord =
} = {}
) =>
async ( { select, resolveSelect, dispatch } ) => {
const configs = await dispatch( getOrLoadEntitiesConfig( kind ) );
const configs = await dispatch( getOrLoadEntitiesConfig( kind, name ) );
const entityConfig = configs.find(
( config ) => config.kind === kind && config.name === name
);
Expand Down Expand Up @@ -780,7 +780,7 @@ export const saveEditedEntityRecord =
if ( ! select.hasEditsForEntityRecord( kind, name, recordId ) ) {
return;
}
const configs = await dispatch( getOrLoadEntitiesConfig( kind ) );
const configs = await dispatch( getOrLoadEntitiesConfig( kind, name ) );
const entityConfig = configs.find(
( config ) => config.kind === kind && config.name === name
);
Expand Down Expand Up @@ -824,7 +824,7 @@ export const __experimentalSaveSpecifiedEntityEdits =
setNestedValue( editsToSave, item, getNestedValue( edits, item ) );
}

const configs = await dispatch( getOrLoadEntitiesConfig( kind ) );
const configs = await dispatch( getOrLoadEntitiesConfig( kind, name ) );
const entityConfig = configs.find(
( config ) => config.kind === kind && config.name === name
);
Expand Down Expand Up @@ -948,7 +948,7 @@ export function receiveDefaultTemplateId( query, templateId ) {
export const receiveRevisions =
( kind, name, recordKey, records, query, invalidateCache = false, meta ) =>
async ( { dispatch } ) => {
const configs = await dispatch( getOrLoadEntitiesConfig( kind ) );
const configs = await dispatch( getOrLoadEntitiesConfig( kind, name ) );
const entityConfig = configs.find(
( config ) => config.kind === kind && config.name === name
);
Expand Down
108 changes: 71 additions & 37 deletions packages/core-data/src/entities.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,36 +61,6 @@ export const rootEntitiesConfig = [
syncObjectType: 'root/base',
getSyncObjectId: () => 'index',
},
{
label: __( 'Site' ),
name: 'site',
kind: 'root',
baseURL: '/wp/v2/settings',
// The entity doesn't support selecting multiple records.
// The property is maintained for backward compatibility.
plural: 'sites',
getTitle: ( record ) => {
return record?.title ?? __( 'Site Title' );
},
syncConfig: {
fetch: async () => {
return apiFetch( { path: '/wp/v2/settings' } );
},
applyChangesToDoc: ( doc, changes ) => {
const document = doc.getMap( 'document' );
Object.entries( changes ).forEach( ( [ key, value ] ) => {
if ( document.get( key ) !== value ) {
document.set( key, value );
}
} );
},
fromCRDTDoc: ( doc ) => {
return doc.getMap( 'document' ).toJSON();
},
},
syncObjectType: 'root/site',
getSyncObjectId: () => 'index',
},
{
label: __( 'Post Type' ),
name: 'postType',
Expand Down Expand Up @@ -253,6 +223,12 @@ export const rootEntitiesConfig = [
export const additionalEntityConfigLoaders = [
{ kind: 'postType', loadEntities: loadPostTypeEntities },
{ kind: 'taxonomy', loadEntities: loadTaxonomyEntities },
{
kind: 'root',
name: 'site',
plural: 'sites',
loadEntities: loadSiteEntity,
},
];

/**
Expand Down Expand Up @@ -409,6 +385,56 @@ async function loadTaxonomyEntities() {
} );
}

/**
* Returns the Site entity.
*
* @return {Promise} Entity promise
*/
async function loadSiteEntity() {
const entity = {
label: __( 'Site' ),
name: 'site',
kind: 'root',
baseURL: '/wp/v2/settings',
syncConfig: {
fetch: async () => {
return apiFetch( { path: '/wp/v2/settings' } );
},
applyChangesToDoc: ( doc, changes ) => {
const document = doc.getMap( 'document' );
Object.entries( changes ).forEach( ( [ key, value ] ) => {
if ( document.get( key ) !== value ) {
document.set( key, value );
}
} );
},
fromCRDTDoc: ( doc ) => {
return doc.getMap( 'document' ).toJSON();
},
},
syncObjectType: 'root/site',
getSyncObjectId: () => 'index',
meta: {},
};

const site = await apiFetch( {
path: entity.baseURL,
method: 'OPTIONS',
} );

const labels = {};
Object.entries( site?.schema?.properties ?? {} ).forEach(
( [ key, value ] ) => {
// Ignore properties `title` and `type` keys.
if ( typeof value === 'object' && value.title ) {
labels[ key ] = value.title;
}
}
);

return [ { ...entity, meta: { labels } } ];
}

/**
* Returns the entity's getter method name given its kind and name or plural name.
*
Expand Down Expand Up @@ -443,17 +469,21 @@ function registerSyncConfigs( configs ) {
}

/**
* Loads the kind entities into the store.
* Loads the entities into the store.
*
* @param {string} kind Kind
* Note: The `name` argument is used for `root` entities requiring additional server data.
*
* @param {string} kind Kind
* @param {string} name Name
* @return {(thunkArgs: object) => Promise<Array>} Entities
*/
export const getOrLoadEntitiesConfig =
( kind ) =>
( kind, name ) =>
Copy link
Contributor

@youknowriad youknowriad Mar 17, 2024

Choose a reason for hiding this comment

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

Why does this function need a "name" now. I believe the purpose of this function is to load the entities of a given "kind", so "name" is not needed?

Copy link
Member Author

@Mamaduka Mamaduka Mar 17, 2024

Choose a reason for hiding this comment

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

The root is a standard kind, and execution never reaches additional loaders. I'm using the name to narrow things down.

Copy link
Contributor

@youknowriad youknowriad Mar 17, 2024

Choose a reason for hiding this comment

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

I admin that I'm not following. There's something I'm obviously missing.

Copy link
Member Author

@Mamaduka Mamaduka Mar 17, 2024

Choose a reason for hiding this comment

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

  • Every entity in rootEntitiesConfig has a kind root (as the const name suggests).
  • The select.getEntitiesConfig( kind ) will always return the list of configs for kind === root
  • The condition in the code below is met, and configs objects are returned.
  • The getOrLoadEntitiesConfig never reaches the addition entity config loading logic to load the site entity.

gutenberg/packages/core-data/src/entities.js

Lines 459 to 468 in ec86a82

let configs = select.getEntitiesConfig( kind );
if ( configs && configs.length !== 0 ) {
if ( window.__experimentalEnableSync ) {
if ( process.env.IS_GUTENBERG_PLUGIN ) {
registerSyncConfigs( configs );
}
}
return configs;
}

Copy link
Contributor

@youknowriad youknowriad Mar 17, 2024

Choose a reason for hiding this comment

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

Can we instead update the condition to check whether a key exist in additionalEntityConfigLoaders and maybe have some "meta" state to indicate that we're loading or we've already loaded (maybe we can even reuse the metadata state that exist in all stores)

Copy link
Contributor

@youknowriad youknowriad Mar 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I've been thinking more, basically what we want is a resolver for. getEntityConfig( kind, name ). In other words, I'm ok with passing the name but we should change the description of the function.

Copy link
Member Author

@Mamaduka Mamaduka Mar 17, 2024

Choose a reason for hiding this comment

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

Sure, I'm happy to give it a try. However, we might encounter a similar issue without using the entity name when additionalEntityConfigLoaders contains more than one entity of the same kind.

Copy link
Contributor

@youknowriad youknowriad Mar 17, 2024

Choose a reason for hiding this comment

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

Let's keep your changes but just clarify things in the description. I'm not entirely satisfied with how we're dealing with all of that, but it's not clear to me what's the best path forward is yet:

  • We have static entities (kind, name)
  • We have dynamic entities: For these entities, for some the kind is enough to resolve their config, for others we need both the kind and name.

It seems there's potentially a better way/API to "register" these entities.

async ( { select, dispatch } ) => {
let configs = select.getEntitiesConfig( kind );
if ( configs && configs.length !== 0 ) {
const hasConfig = !! select.getEntityConfig( kind, name );

if ( configs?.length > 0 && hasConfig ) {
if ( window.__experimentalEnableSync ) {
if ( process.env.IS_GUTENBERG_PLUGIN ) {
registerSyncConfigs( configs );
Expand All @@ -463,9 +493,13 @@ export const getOrLoadEntitiesConfig =
return configs;
}

const loader = additionalEntityConfigLoaders.find(
( l ) => l.kind === kind
);
const loader = additionalEntityConfigLoaders.find( ( l ) => {
if ( ! name || ! l.name ) {
return l.kind === kind;
}

return l.kind === kind && l.name === name;
} );
if ( ! loader ) {
return [];
}
Expand Down
16 changes: 12 additions & 4 deletions packages/core-data/src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,24 @@ import * as privateSelectors from './private-selectors';
import * as actions from './actions';
import * as resolvers from './resolvers';
import createLocksActions from './locks/actions';
import { rootEntitiesConfig, getMethodName } from './entities';
import {
rootEntitiesConfig,
additionalEntityConfigLoaders,
getMethodName,
} from './entities';
import { STORE_NAME } from './name';
import { unlock } from './private-apis';

// The entity selectors/resolvers and actions are shortcuts to their generic equivalents
// (getEntityRecord, getEntityRecords, updateEntityRecord, updateEntityRecords)
// Instead of getEntityRecord, the consumer could use more user-friendly named selector: getPostType, getTaxonomy...
// The "kind" and the "name" of the entity are combined to generate these shortcuts.
const entitiesConfig = [
...rootEntitiesConfig,
...additionalEntityConfigLoaders.filter( ( config ) => !! config.name ),
];

const entitySelectors = rootEntitiesConfig.reduce( ( result, entity ) => {
const entitySelectors = entitiesConfig.reduce( ( result, entity ) => {
const { kind, name, plural } = entity;
result[ getMethodName( kind, name ) ] = ( state, key, query ) =>
selectors.getEntityRecord( state, kind, name, key, query );
Expand All @@ -33,7 +41,7 @@ const entitySelectors = rootEntitiesConfig.reduce( ( result, entity ) => {
return result;
}, {} );

const entityResolvers = rootEntitiesConfig.reduce( ( result, entity ) => {
const entityResolvers = entitiesConfig.reduce( ( result, entity ) => {
const { kind, name, plural } = entity;
result[ getMethodName( kind, name ) ] = ( key, query ) =>
resolvers.getEntityRecord( kind, name, key, query );
Expand All @@ -48,7 +56,7 @@ const entityResolvers = rootEntitiesConfig.reduce( ( result, entity ) => {
return result;
}, {} );

const entityActions = rootEntitiesConfig.reduce( ( result, entity ) => {
const entityActions = entitiesConfig.reduce( ( result, entity ) => {
const { kind, name } = entity;
result[ getMethodName( kind, name, 'save' ) ] = ( record, options ) =>
actions.saveEntityRecord( kind, name, record, options );
Expand Down
10 changes: 5 additions & 5 deletions packages/core-data/src/resolvers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ export const getCurrentUser =
export const getEntityRecord =
( kind, name, key = '', query ) =>
async ( { select, dispatch } ) => {
const configs = await dispatch( getOrLoadEntitiesConfig( kind ) );
const configs = await dispatch( getOrLoadEntitiesConfig( kind, name ) );
const entityConfig = configs.find(
( config ) => config.name === name && config.kind === kind
);
Expand Down Expand Up @@ -194,7 +194,7 @@ export const getEditedEntityRecord = forwardResolver( 'getEntityRecord' );
export const getEntityRecords =
( kind, name, query = {} ) =>
async ( { dispatch } ) => {
const configs = await dispatch( getOrLoadEntitiesConfig( kind ) );
const configs = await dispatch( getOrLoadEntitiesConfig( kind, name ) );
const entityConfig = configs.find(
( config ) => config.name === name && config.kind === kind
);
Expand Down Expand Up @@ -429,7 +429,7 @@ export const canUser =
export const canUserEditEntityRecord =
( kind, name, recordId ) =>
async ( { dispatch } ) => {
const configs = await dispatch( getOrLoadEntitiesConfig( kind ) );
const configs = await dispatch( getOrLoadEntitiesConfig( kind, name ) );
const entityConfig = configs.find(
( config ) => config.name === name && config.kind === kind
);
Expand Down Expand Up @@ -726,7 +726,7 @@ export const getDefaultTemplateId =
export const getRevisions =
( kind, name, recordKey, query = {} ) =>
async ( { dispatch } ) => {
const configs = await dispatch( getOrLoadEntitiesConfig( kind ) );
const configs = await dispatch( getOrLoadEntitiesConfig( kind, name ) );
const entityConfig = configs.find(
( config ) => config.name === name && config.kind === kind
);
Expand Down Expand Up @@ -851,7 +851,7 @@ getRevisions.shouldInvalidate = ( action, kind, name, recordKey ) =>
export const getRevision =
( kind, name, recordKey, revisionKey, query ) =>
async ( { dispatch } ) => {
const configs = await dispatch( getOrLoadEntitiesConfig( kind ) );
const configs = await dispatch( getOrLoadEntitiesConfig( kind, name ) );
const entityConfig = configs.find(
( config ) => config.name === name && config.kind === kind
);
Expand Down
21 changes: 18 additions & 3 deletions packages/core-data/src/test/entities.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,18 +52,29 @@ describe( 'getKindEntities', () => {
const dispatch = jest.fn();
const select = {
getEntitiesConfig: jest.fn( () => entities ),
getEntityConfig: jest.fn( () => ( {
kind: 'postType',
name: 'post',
} ) ),
};
const entities = [ { kind: 'postType' } ];
await getOrLoadEntitiesConfig( 'postType' )( { dispatch, select } );
await getOrLoadEntitiesConfig(
'postType',
'post'
)( { dispatch, select } );
expect( dispatch ).not.toHaveBeenCalled();
} );

it( 'shouldn’t do anything if there no defined kind config', async () => {
const dispatch = jest.fn();
const select = {
getEntitiesConfig: jest.fn( () => [] ),
getEntityConfig: jest.fn( () => undefined ),
};
await getOrLoadEntitiesConfig( 'unknownKind' )( { dispatch, select } );
await getOrLoadEntitiesConfig(
'unknownKind',
undefined
)( { dispatch, select } );
expect( dispatch ).not.toHaveBeenCalled();
} );

Expand All @@ -82,10 +93,14 @@ describe( 'getKindEntities', () => {
const dispatch = jest.fn();
const select = {
getEntitiesConfig: jest.fn( () => [] ),
getEntityConfig: jest.fn( () => undefined ),
};
triggerFetch.mockImplementation( () => fetchedEntities );

await getOrLoadEntitiesConfig( 'postType' )( { dispatch, select } );
await getOrLoadEntitiesConfig(
'postType',
'post'
)( { dispatch, select } );
expect( dispatch ).toHaveBeenCalledTimes( 1 );
expect( dispatch.mock.calls[ 0 ][ 0 ].type ).toBe( 'ADD_ENTITIES' );
expect( dispatch.mock.calls[ 0 ][ 0 ].entities.length ).toBe( 1 );
Expand Down
Loading