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 custom templates to their original theme-provided files #28141

Merged
merged 37 commits into from
Apr 1, 2021
Merged
Show file tree
Hide file tree
Changes from 30 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
a9bc63f
Add original_file_exists property to templates and parts
Copons Jan 12, 2021
37ec486
Add a revertTemplate action (not fully working)
Copons Jan 12, 2021
edaeb81
Invalidate the cache and use correct template content during the reve…
Copons Jan 13, 2021
192aeab
Update revert button styles
Copons Jan 13, 2021
67d310d
Turn the revert button into a MenuItem
Copons Jan 13, 2021
1cc29eb
Adjust labels
jameskoster Jan 13, 2021
ff592f6
Handle revert on the endpoint
Copons Jan 15, 2021
abec6dd
Format
david-szabo97 Mar 4, 2021
0a306dd
Format
david-szabo97 Mar 4, 2021
1118c5e
Fix undo
david-szabo97 Mar 4, 2021
bd23825
Format
david-szabo97 Mar 5, 2021
108aff1
Fix undo
david-szabo97 Mar 8, 2021
a8c1cf4
Fix PHP unit tests
david-szabo97 Mar 8, 2021
b4b76b9
Fix undo
david-szabo97 Mar 9, 2021
b434762
Add undo to notice
david-szabo97 Mar 9, 2021
0d33138
Export wpDataSelect
david-szabo97 Mar 9, 2021
4653313
Add e2e tests
david-szabo97 Mar 9, 2021
36049c6
Format
david-szabo97 Mar 9, 2021
dbce5ba
Rename variables
david-szabo97 Mar 9, 2021
5bfd86b
Destructure siteEditor
david-szabo97 Mar 9, 2021
32aa328
Replace store string literals
david-szabo97 Mar 9, 2021
a5b32c3
Add save button checks
david-szabo97 Mar 9, 2021
d581dbc
Remove visible check
david-szabo97 Mar 9, 2021
8cddbc9
Add comments and remove unncessary properties
david-szabo97 Mar 9, 2021
feadc71
Revert without autosaving
david-szabo97 Mar 10, 2021
743b71d
Use getEntityRecord instead of apiFetch
david-szabo97 Mar 10, 2021
e66c425
Rename original_file_exists to has_theme_file
david-szabo97 Mar 24, 2021
a9ce620
Use edited entity template
david-szabo97 Mar 29, 2021
01bdfe1
Update ID format
david-szabo97 Mar 30, 2021
4a01921
Try apiFetch
david-szabo97 Mar 30, 2021
5a6cd9f
Use source instead of is_custom
david-szabo97 Mar 31, 2021
360cb85
Don't use hardcoded path
david-szabo97 Mar 31, 2021
653fc4b
Merge remote-tracking branch 'origin/trunk' into add/templates-revert…
david-szabo97 Mar 31, 2021
e646065
Format
david-szabo97 Mar 31, 2021
e5603d0
Merge remote-tracking branch 'origin/trunk' into add/templates-revert…
david-szabo97 Mar 31, 2021
fb73a6b
Use variable for border width
david-szabo97 Mar 31, 2021
2d8aa9e
Use variables
david-szabo97 Mar 31, 2021
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
69 changes: 46 additions & 23 deletions lib/full-site-editing/block-templates.php
Original file line number Diff line number Diff line change
Expand Up @@ -184,15 +184,16 @@ function _gutenberg_build_template_result_from_file( $template_file, $template_t
$template_content = file_get_contents( $template_file['path'] );
$theme = wp_get_theme()->get_stylesheet();

$template = new WP_Block_Template();
$template->id = $theme . '//' . $template_file['slug'];
$template->theme = $theme;
$template->content = _inject_theme_attribute_in_content( $template_content );
$template->slug = $template_file['slug'];
$template->is_custom = false;
$template->type = $template_type;
$template->title = $template_file['slug'];
$template->status = 'publish';
$template = new WP_Block_Template();
$template->id = $theme . '//' . $template_file['slug'];
$template->theme = $theme;
$template->content = _inject_theme_attribute_in_content( $template_content );
$template->slug = $template_file['slug'];
$template->is_custom = false;
$template->type = $template_type;
$template->title = $template_file['slug'];
$template->status = 'publish';
$template->has_theme_file = true;

if ( 'wp_template' === $template_type && isset( $default_template_types[ $template_file['slug'] ] ) ) {
$template->description = $default_template_types[ $template_file['slug'] ]['description'];
Expand Down Expand Up @@ -224,19 +225,22 @@ function _gutenberg_build_template_result_from_post( $post ) {
return new WP_Error( 'template_missing_theme', __( 'No theme is defined for this template.', 'gutenberg' ) );
}

$theme = $terms[0]->name;

$template = new WP_Block_Template();
$template->wp_id = $post->ID;
$template->id = $theme . '//' . $post->post_name;
$template->theme = $theme;
$template->content = $post->post_content;
$template->slug = $post->post_name;
$template->is_custom = true;
$template->type = $post->post_type;
$template->description = $post->post_excerpt;
$template->title = $post->post_title;
$template->status = $post->post_status;
$theme = $terms[0]->name;
$has_theme_file = wp_get_theme()->get_stylesheet() === $theme &&
null !== _gutenberg_get_template_file( $post->post_type, $post->post_name );

$template = new WP_Block_Template();
$template->wp_id = $post->ID;
$template->id = $theme . '//' . $post->post_name;
$template->theme = $theme;
$template->content = $post->post_content;
$template->slug = $post->post_name;
$template->is_custom = true;
$template->type = $post->post_type;
$template->description = $post->post_excerpt;
$template->title = $post->post_title;
$template->status = $post->post_status;
$template->has_theme_file = $has_theme_file;

if ( 'wp_template_part' === $post->post_type ) {
$type_terms = get_the_terms( $post, 'wp_template_part_area' );
Expand Down Expand Up @@ -331,7 +335,7 @@ function gutenberg_get_block_templates( $query = array(), $template_type = 'wp_t
/**
* Retrieves a single unified template object using its id.
*
* @param string $id Template unique identifier (example: theme|slug).
* @param string $id Template unique identifier (example: theme_slug//template_slug).
* @param array $template_type wp_template or wp_template_part.
*
* @return WP_Block_Template|null Template.
Expand Down Expand Up @@ -367,6 +371,25 @@ function gutenberg_get_block_template( $id, $template_type = 'wp_template' ) {
}
}

return gutenberg_get_block_file_template( $id, $template_type );
}

/**
* Retrieves a single unified template object using its id.
* Retrieves the file template.
*
* @param string $id Template unique identifier (example: theme_slug//template_slug).
* @param array $template_type wp_template or wp_template_part.
*
* @return WP_Block_Template|null File template.
*/
function gutenberg_get_block_file_template( $id, $template_type = 'wp_template' ) {
$parts = explode( '//', $id, 2 );
if ( count( $parts ) < 2 ) {
return null;
}
list( $theme, $slug ) = $parts;

if ( wp_get_theme()->get_stylesheet() === $theme ) {
$template_file = _gutenberg_get_template_file( $template_type, $slug );
if ( null !== $template_file ) {
Expand Down
7 changes: 7 additions & 0 deletions lib/full-site-editing/class-wp-block-template.php
Original file line number Diff line number Diff line change
Expand Up @@ -80,4 +80,11 @@ class WP_Block_Template {
* @var string
*/
public $status;

/**
* Whether a template is, or is based upon, an existing template file.
*
* @var boolean
*/
public $has_theme_file;
}
56 changes: 36 additions & 20 deletions lib/full-site-editing/class-wp-rest-templates-controller.php
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,11 @@ public function get_item_permissions_check( $request ) {
* @return WP_REST_Response|WP_Error
*/
public function get_item( $request ) {
$template = gutenberg_get_block_template( $request['id'], $this->post_type );
if ( isset( $request['is_custom'] ) && 'false' === $request['is_custom'] ) {
$template = gutenberg_get_block_file_template( $request['id'], $this->post_type );
Copons marked this conversation as resolved.
Show resolved Hide resolved
} else {
$template = gutenberg_get_block_template( $request['id'], $this->post_type );
}

if ( ! $template ) {
return new WP_Error( 'rest_template_not_found', __( 'No templates exist with that id.', 'gutenberg' ), array( 'status' => 404 ) );
Expand Down Expand Up @@ -199,6 +203,11 @@ public function update_item( $request ) {
return new WP_Error( 'rest_template_not_found', __( 'No templates exist with that id.', 'gutenberg' ), array( 'status' => 404 ) );
}

if ( isset( $request['reverted_file_content'] ) && isset( $request['content'] ) && $request['reverted_file_content'] === $request['content'] ) {
Copy link
Member

@david-szabo97 david-szabo97 Mar 11, 2021

Choose a reason for hiding this comment

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

I wonder if there is any way to get consistent markup out of $template->content? I wanted to compare the content with the file template's content but looks like the JS serializer gives a different markup from the PHP serializer. Thus when comparing the content from the request to the $template->content, they are different. I've even tried to reserialize both using serialize_blocks ( parse_blocks ( $content ) )

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not quite sure why we need to compare the template content. 🤔
Couldn't we just rely on isset( $request['is_custom'] ) && 'false' === $request['is_custom'] to determine that this is a revert call?

Copy link
Contributor

Choose a reason for hiding this comment

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

Couldn't we just rely on isset( $request['is_custom'] ) && 'false' === $request['is_custom'] to determine that this is a revert call?

I think we need a way to differentiate between 1 - retrieving the non-custom content and 2 - reverting to that. Reverting based on presence of $request['is_custom'] === false means we cannot access that non-custom content without reverting the post as well. 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe another request param like is_revert could get around needing to compare content? 🤔

Copy link
Member

Choose a reason for hiding this comment

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

It's related to saving a template for the first time. It would save as is_custom = false since we aren't updating its state on the client side. So we rely on the content. To get around this, we would need to set is_custom to true when we start making changes to the template.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Addison-Stavlo It's not super clear by the diff, but I was specifically talking about update_item; you can still get_item a non-custom item.
(Unless I've misunderstood your comment? 🤔 )

@david-szabo97 Apologies if these are dumb comments, but I really forgot everything I researched while working on this issue. 😓
When we save a template for the first time (and subsequent times as well, I think?), we only really send ID and content.

Screenshot 2021-03-31 at 10 57 35

$request['is_custom'] is undefined, and the isset( $request['is_custom'] ) && 'false' === $request['is_custom'] check will fail.

Copy link
Member

Choose a reason for hiding this comment

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

There is also an issue I encountered while trying to use the implementation you suggest. For some reason the selector that takes care of collecting the changed properties of the entity, doesn't return the is_custom property as changed. It only happens with boolean properties. Changing true/false to 'yes'/'no' gets around this problem.

According to our discussion, I renamed is_custom to source and it uses two string values: theme and custom.

Copons marked this conversation as resolved.
Show resolved Hide resolved
wp_delete_post( $template->wp_id, true );
return $this->prepare_item_for_response( gutenberg_get_block_file_template( $request['id'], $this->post_type ), $request );
}

$changes = $this->prepare_item_for_database( $request );

if ( $template->is_custom ) {
Expand Down Expand Up @@ -384,19 +393,20 @@ protected function prepare_item_for_database( $request ) {
*/
public function prepare_item_for_response( $template, $request ) { // phpcs:ignore VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedVariable
$result = array(
'id' => $template->id,
'theme' => $template->theme,
'content' => array( 'raw' => $template->content ),
'slug' => $template->slug,
'is_custom' => $template->is_custom,
'type' => $template->type,
'description' => $template->description,
'title' => array(
'id' => $template->id,
'theme' => $template->theme,
'content' => array( 'raw' => $template->content ),
'slug' => $template->slug,
'is_custom' => $template->is_custom,
'type' => $template->type,
'description' => $template->description,
'title' => array(
'raw' => $template->title,
'rendered' => $template->title,
),
'status' => $template->status,
'wp_id' => $template->wp_id,
'status' => $template->status,
'wp_id' => $template->wp_id,
'has_theme_file' => $template->has_theme_file,
);

if ( 'wp_template_part' === $template->type ) {
Expand Down Expand Up @@ -495,61 +505,67 @@ public function get_item_schema() {
'title' => $this->post_type,
'type' => 'object',
'properties' => array(
'id' => array(
'id' => array(
'description' => __( 'ID of template.', 'gutenberg' ),
'type' => 'string',
'context' => array( 'embed', 'view', 'edit' ),
'readonly' => true,
),
'slug' => array(
'slug' => array(
'description' => __( 'Unique slug identifying the template.', 'gutenberg' ),
'type' => 'string',
'context' => array( 'embed', 'view', 'edit' ),
'required' => true,
'minLength' => 1,
'pattern' => '[a-zA-Z_\-]+',
),
'theme' => array(
'theme' => array(
'description' => __( 'Theme identifier for the template.', 'gutenberg' ),
'type' => 'string',
'context' => array( 'embed', 'view', 'edit' ),
),
'is_custom' => array(
'is_custom' => array(
'description' => __( 'Whether the template is customized.', 'gutenberg' ),
'type' => 'bool',
'context' => array( 'embed', 'view', 'edit' ),
'readonly' => true,
),
'content' => array(
'content' => array(
'description' => __( 'Content of template.', 'gutenberg' ),
'type' => array( 'object', 'string' ),
'default' => '',
'context' => array( 'embed', 'view', 'edit' ),
),
'title' => array(
'title' => array(
'description' => __( 'Title of template.', 'gutenberg' ),
'type' => array( 'object', 'string' ),
'default' => '',
'context' => array( 'embed', 'view', 'edit' ),
),
'description' => array(
'description' => array(
'description' => __( 'Description of template.', 'gutenberg' ),
'type' => 'string',
'default' => '',
'context' => array( 'embed', 'view', 'edit' ),
),
'status' => array(
'status' => array(
'description' => __( 'Status of template.', 'gutenberg' ),
'type' => 'string',
'default' => 'publish',
'context' => array( 'embed', 'view', 'edit' ),
),
'wp_id' => array(
'wp_id' => array(
'description' => __( 'Post ID.', 'gutenberg' ),
'type' => 'integer',
'context' => array( 'embed', 'view', 'edit' ),
'readonly' => true,
),
'has_theme_file' => array(
'description' => __( 'Theme file exists.', 'gutenberg' ),
'type' => 'bool',
'context' => array( 'embed', 'view', 'edit' ),
'readonly' => true,
),
),
);

Expand Down
14 changes: 14 additions & 0 deletions packages/e2e-test-utils/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -713,6 +713,20 @@ _Parameters_
- _width_ `number`: Width of the window.
- _height_ `number`: Height of the window.

<a name="wpDataSelect" href="#wpDataSelect">#</a> **wpDataSelect**

Queries the WordPress data module.

_Parameters_

- _store_ `string`: Store to query e.g: core/editor, core/blocks...
- _selector_ `string`: Selector to exectute e.g: getBlocks.
- _parameters_ `...Object`: Parameters to pass to the selector.

_Returns_

- `Promise<?Object>`: Result of querying.


<!-- END TOKEN(Autogenerated API docs) -->

Expand Down
1 change: 1 addition & 0 deletions packages/e2e-test-utils/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,5 +77,6 @@ export { visitAdminPage } from './visit-admin-page';
export { waitForWindowDimensions } from './wait-for-window-dimensions';
export { showBlockToolbar } from './show-block-toolbar';
export { openPreviewPage } from './preview';
export { wpDataSelect } from './wp-data-select';
Addison-Stavlo marked this conversation as resolved.
Show resolved Hide resolved

export * from './mocks';
34 changes: 33 additions & 1 deletion packages/e2e-tests/experimental-features.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* WordPress dependencies
*/
import { addQueryArgs } from '@wordpress/url';
import { visitAdminPage } from '@wordpress/e2e-test-utils';
import { visitAdminPage, wpDataSelect } from '@wordpress/e2e-test-utils';

async function setExperimentalFeaturesState( features, enable ) {
const query = addQueryArgs( '', {
Expand Down Expand Up @@ -129,4 +129,36 @@ export const siteEditor = {

await elementToClick.click();
},

async getEditedPostContent() {
Copons marked this conversation as resolved.
Show resolved Hide resolved
const postId = await wpDataSelect(
'core/edit-site',
'getEditedPostId'
);
const postType = await wpDataSelect(
'core/edit-site',
'getEditedPostType'
);
const record = await wpDataSelect(
'core',
'getEditedEntityRecord',
'postType',
postType,
postId
);
if ( record ) {
if ( typeof record.content === 'function' ) {
return record.content( record );
} else if ( record.blocks ) {
return await page.evaluate(
( blocks ) =>
window.wp.blocks.__unstableSerializeAndClean( blocks ),
record.blocks
);
} else if ( record.content ) {
return record.content;
}
}
return '';
},
};
Loading