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

Site Editor: Revert templates to their original theme-provided files #27208

Closed
wants to merge 9 commits into from
Closed
Show file tree
Hide file tree
Changes from 6 commits
Commits
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
14 changes: 14 additions & 0 deletions lib/templates.php
Original file line number Diff line number Diff line change
Expand Up @@ -374,6 +374,20 @@ function filter_rest_prepare_add_wp_theme_slug_and_file_based( $response ) {
$file_based = in_array( '_wp_file_based', $wp_theme_slugs, true );
$response->data['file_based'] = $file_based;

$response->data['original_file_exists'] = false;
$template_files = array_unique(
array_merge(
_gutenberg_get_template_paths( get_stylesheet_directory() . DIRECTORY_SEPARATOR . 'block-templates' ),
_gutenberg_get_template_paths( get_template_directory() . DIRECTORY_SEPARATOR . 'block-templates' )
)
);
foreach ( $template_files as $template_file ) {
if ( basename( $template_file, '.html' ) === $response->data['slug'] ) {
$response->data['original_file_exists'] = true;
break;
}
}

$theme_slug = array_values(
array_filter(
$wp_theme_slugs,
Expand Down
36 changes: 31 additions & 5 deletions packages/edit-site/src/components/template-details/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,33 @@ import { useDispatch, useSelect } from '@wordpress/data';
/**
* Internal dependencies
*/
import { isTemplateRevertable } from '../../utils';
import { MENU_TEMPLATES } from '../navigation-sidebar/navigation-panel/constants';

export default function TemplateDetails( { template, onClose } ) {
const { title, description } = useSelect(
( select ) =>
select( 'core/editor' ).__experimentalGetTemplateInfo( template ),
[]
const { title, description, currentTheme } = useSelect( ( select ) => {
const templateInfo = select(
'core/editor'
).__experimentalGetTemplateInfo( template );
return {
title: templateInfo?.title,
description: templateInfo?.description,
currentTheme: select( 'core' ).getCurrentTheme()?.stylesheet,
};
}, [] );
const { openNavigationPanelToMenu, revertTemplate } = useDispatch(
'core/edit-site'
);
const { openNavigationPanelToMenu } = useDispatch( 'core/edit-site' );

if ( ! template ) {
return null;
}

const revert = () => {
revertTemplate( template );
onClose();
};

const showTemplateInSidebar = () => {
onClose();
openNavigationPanelToMenu( MENU_TEMPLATES );
Expand Down Expand Up @@ -55,6 +68,19 @@ export default function TemplateDetails( { template, onClose } ) {
) }
</div>

{ isTemplateRevertable( template, currentTheme ) && (
<div className="edit-site-template-details">
<Button isLink onClick={ revert }>
{ __( 'Revert' ) }
</Button>
<Text variant="caption">
{ __(
'Reset this template to the theme supplied default'
) }
</Text>
</div>
) }

<Button
className="edit-site-template-details__show-all-button"
onClick={ showTemplateInSidebar }
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
.edit-site-template-details {
margin: $grid-unit-10 $grid-unit-20;
border-bottom: $border-width solid $gray-400;
padding: $grid-unit-10 $grid-unit-20;

&:last-of-type {
border-bottom: none;
}

p {
padding: $grid-unit-10 0;
Expand Down
82 changes: 82 additions & 0 deletions packages/edit-site/src/store/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@
*/
import { controls } from '@wordpress/data';
import { apiFetch } from '@wordpress/data-controls';
import { __ } from '@wordpress/i18n';

/**
* Internal dependencies
*/
import { findTemplate } from './controls';
import { isTemplateRevertable } from '../utils';

/**
* Returns an action object used to toggle a feature flag.
Expand Down Expand Up @@ -228,3 +230,83 @@ export function updateSettings( settings ) {
settings,
};
}

/**
* Reverts a template to its original theme-provided file.
*
* This works with the assumption that the template being reverted
* belongs to the current theme.
*
* @param {Object} template The template to revert.
*/
export function* revertTemplate( template ) {
const postType = yield controls.resolveSelect(
'core',
'getPostType',
'wp_template'
);

const currentTheme = yield controls.resolveSelect(
'core',
'getCurrentTheme'
);

if ( ! isTemplateRevertable( template, currentTheme ) ) {
yield controls.dispatch(
'core/notices',
'createErrorNotice',
__( 'This template is not revertable' ),
{ type: 'snackbar' }
);
return;
}

try {
yield apiFetch( {
path: `/wp/v2/${ postType.rest_base }/${ template.id }`,
method: 'DELETE',
} );
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
yield apiFetch( {
path: `/wp/v2/${ postType.rest_base }/${ template.id }`,
method: 'DELETE',
} );
yield controls.dispatch(
'core',
'deleteEntityRecord',
'postType',
'wp_template',
template.id
);

Copy link
Member

Choose a reason for hiding this comment

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

Using the built-in deleteEntityRecord should take care of invalidating the cache and doing a rerequest.

Copy link
Contributor Author

@Copons Copons Nov 25, 2020

Choose a reason for hiding this comment

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

Nice catch @david-szabo97, I didn't think of checking the sidebar. ✨


const fileTemplates = yield controls.resolveSelect(
'core',
'getEntityRecords',
'postType',
'wp_template',
{
slug: template.slug,
status: 'auto-draft',
per_page: 1,
theme: currentTheme?.stylesheet,
}
);

if ( ! fileTemplates.length ) {
yield controls.dispatch(
'core/notices',
'createErrorNotice',
__( 'This template is not revertable' ),
{ type: 'snackbar' }
);
return;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like we can create this notice in 2 places:

  1. Prior to this step when isTemplateRevertable comes back falsy.
  2. Here after we have already sent the apiFetch to delete the template and fail to find the auto-draft to replace it with.

Should we be checking that autodraft and conditionally sending this notice before sending the apiFetch to delete?

Copy link
Contributor

@Addison-Stavlo Addison-Stavlo Nov 24, 2020

Choose a reason for hiding this comment

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

Oh, Yeah I see why we have to do it this way and missed your recent note above:

Now, the case 2 is not really testable, and would cause many headaches if it happens for real. Given how the sync mechanism work, it shouldn't be possible to happen, but I've still decided to add an error notice there just in case.
Maybe the notice could change into "An unexpected error occurred. Please reload."

The unexpected error message may make more sense in this case. 🤔

Perhaps it would make sense to always create the auto-draft if the auto-draft doesn't exist. So the new auto-draft would be created as soon as the original one was customized and published, as opposed to right after it is deleted? Not a blocker for this PR, but a follow up if necessary.


yield setTemplate( fileTemplates[ 0 ].id );
yield controls.dispatch(
'core/notices',
'createSuccessNotice',
__( 'Template reverted' ),
{ type: 'snackbar' }
);
} catch ( error ) {
const errorMessage =
error.message && error.code !== 'unknown_error'
? error.message
: __( 'Template revert failed' );
yield controls.dispatch(
'core/notices',
'createErrorNotice',
errorMessage,
{ type: 'snackbar' }
);
}
}
1 change: 1 addition & 0 deletions packages/edit-site/src/utils/index.js
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
export { default as findTemplate } from './find-template';
export { default as isTemplateRevertable } from './is-template-revertable';
20 changes: 20 additions & 0 deletions packages/edit-site/src/utils/is-template-revertable.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/**
* Check if a template is revertable to its original theme-provided template file.
*
* @param {Object} template The template entity to check.
* @param {string} currentTheme The current theme slug (stylesheet).
* @return {boolean} Whether the template is revertable.
*/
export default function isTemplateRevertable( template, currentTheme ) {
if ( ! template || ! currentTheme ) {
return false;
}
return (
'auto-draft' !== template.status &&
/* eslint-disable camelcase */
template?.file_based &&
template?.original_file_exists &&
currentTheme === template?.wp_theme_slug
/* eslint-enable camelcase */
);
}