Allow autosaving to be disabled on a per post type basis

[Frank Klein]

Autosaving should be a post type feature, so that individual post types can opt out. Disabling this feature should remove both the server-side and the client-side saving.

Adds autosave support by default if the post type has editor support, to maintain backward compatibility.

Support can be removed by explicitly calling remove_post_type_support().

Avoid unnecessarily registering autosave REST controllers.

Trac ticket: https://core.trac.wordpress.org/ticket/41172

This gets automatically added to the post type REST API right? Can we confirm that with a test or something? I'm thinking that it's a useful information for the editor to know. I know we've been hardcoding some checks recently about whether to enable or disable autosaves.

This gets automatically added to the post type REST API right? Can we confirm that with a test or something? I'm thinking that it's a useful information for the editor to know. I know we've been hardcoding some checks recently about whether to enable or disable autosaves.

There is a supports field in the /wp/v2/types endpoint(s) that can be used to check if autosaves are supported.

This could be used in the isEditedPostAutosaveable selector here:

​https://github.com/WordPress/gutenberg/blob/8d94c3bd7af977d998466b56bd773f9b19de8d03/packages/editor/src/store/selectors.js#L600-L611

No need for a hardcoded wp_template check anymore

[hellofromTonya]

The Font Library #59166 is brand new in 6.5. It's currently ​using a hacky way to workaround this requested need. With this need and movement with a new patch, moving this into 6.5.

I understand why this needs to be opt-out for back-compat purposes.

What do folks think of allowing developers to opt out by setting supports to false?

register_post_type(
        'no-autosave',
        [
                'public' => true,
                'show_in_rest' => true,
                'supports' => [
                        'title',
                        'editor',
                        'autosaves' => false,
                ],
        ]
);

At present supports['thing'] = false would actually enable support of the thing, so this would be a technical BC break. However, I think the intent of the code is clear so it may be something that can be introduced relatively safely.

I'm wondering if we are overthinking this by adding a new supports item.

How about allowing: register_post_type( 'cpt', [ 'autosave_rest_controller_class' => null ] );?

The patch could then become:

public function get_autosave_rest_controller() {
        if ( ! $this->show_in_rest ) {
                return null;
        }

        if ( null === $this->autosave_rest_controller_class ) {
                return null;
        }
        // ...

Regrettably, setting autosave_rest_controller_class => false actually means: enable the controller and use the default. But null is a new value that can indicate the controller should not be enabled.

It would still need a dev note for plugins outside the repository.

I think the current approach would need some additional changes for add|remove_post_types_support to ensure that adding autosaves only works for posts with the editor and removing support for the editor would also need to remove support for autosaves. Similar to ​this PR for footnotes.

null is better than stdClass, so that would be a step in the right direction. However it doesn‘t solve the discoverability issue for the editor, see ​https://github.com/WordPress/wordpress-develop/pull/6035#issuecomment-1929260835

Any thoughts on how to do that? Maybe with a _links entry in the API.

But then it would be weird to use the existance of a class to check for feature support, when post type supports is designed for this.

Any thoughts on how to do that? Maybe with a _links entry in the API.

A non-embedable links entry makes sense. Otherwise could it check the schema, either at ​http://xu-a8c-woo.local/wp-json/wp/v2 or higher up the tree if it's already loaded?

I don't think Gutenberg looks at the schemas.

@youknowriad Would checking the post type's _links property in isEditedPostAutosaveable in work? i.e. if there's no autosave endpoint mentioned, then it means it's not supported.

In Gutenberg, we don't look either at links or schemas so far in core-data selectors and I personally don't like adding "special cases" for some entities to the code base to retrieve information about a given entity. I see "links" as "schemas" as technical informations about the REST API implementation but not something a component user API should provide.

From a data information architecture, I think this is being part of the "supports" key of the endpoint makes more sense. If I'm fetching a post type through an endpoint, I want to know what it supports and what it doesn't. It seems weird to have to check "links" or "schema" to know what a post type support when we actually have a supports property for said entity.

That said, obviously we can adapt.

That is certainly true, yeah. I don't think there's anything cleaner than using supports.

In the ongoing discussion about adding autosave support to the post type REST API in WordPress, there seems to be a debate regarding the most appropriate method for indicating autosave support. While some suggest utilizing the supports field in the endpoint, others propose alternative approaches like inspecting the _links property or relying on schema checks. What are the advantages and disadvantages of each approach, and how might the chosen method impact the usability and maintainability of the WordPress codebase?
<a href="​https://dinetuatuwhoo.tk"> </a><a href="​https://dickblenag.tk"> </a><a href="​https://ceoplasthonnilanwho.tk"> </a><a href="​https://apimslufli.tk"> </a><a href="​https://enycuvceled.tk"> </a><a href="​https://mowuwyjonofo.tk"> </a>

In the ongoing discussion about adding autosave support to the post type REST API in WordPress, there seems to be a debate regarding the most appropriate method for indicating autosave support. While some suggest utilizing the supports field in the endpoint, others propose alternative approaches like inspecting the _links property or relying on schema checks. What are the advantages and disadvantages of each approach, and how might the chosen method impact the usability and maintainability of the WordPress codebase?
<a href="​https://dinetuatuwhoo.tk"> </a><a href="​https://dickblenag.tk"> </a><a href="​https://ceoplasthonnilanwho.tk"> </a><a href="​https://apimslufli.tk"> </a><a href="​https://enycuvceled.tk"> </a><a href="​https://mowuwyjonofo.tk"> </a>

Just pinging people again to see whether this is something we'd like to commit in 6.5. Otherwise this would be punted.

Thanks for the ping @swissspidy.

I don't think there's anything cleaner than using supports.

I too am not seeing a more clean solution than what's being proposed.

​I'm not a fan of having only this one support be an opt-out, but I do understand why it has to be. That said, I like @peterwilsoncc's thinking:

At present supportsthing? = false would actually enable support of the thing, so this would be a technical BC break. However, I think the intent of the code is clear so it may be something that can be introduced relatively safely.

but it seems out-of-scope for this specific ticket. Why? It enhances the supports to allow configuring which ones should not be supported, whereas this ticket is about only autosave. IMO this kind of enhancement needs it own ticket and could happen in the next major.

Back to this specific PR.

The Font Library has an immediate need for this change to avoid shipping the hack. That said, the hack is internal and could be replaced in the next cycle once a full solution / enhancement is ready.

Are there any other immediate needs within the editor?

If no, I'm lean towards punting. Then in the next cycle, add the autosave support plus enhance supports to accept false (as Peter suggested).

I'm inclined to punt this for now.

While using stdClass is not ideal, it works for now and I think it would be rushing things to try and get this solved in the time we (ie, those of us tracking this ticket) have between now and the beta release.

If using stdClass going to lock WP Core in to supporting something in to the future then let's try to figure it out.

Alright, punting for now 👍

Should WordPress consider implementing a more robust solution for enabling or disabling autosaving on a per post type basis, possibly by enhancing the 'supports' parameter to accept 'false' values, as suggested by some contributors? How might such a feature enhancement align with the platform's backward compatibility and development roadmap?
#!html<a href='https://rubyofsiamthai.com'></a><a href='https://singha88.com'></a><a href='https://dgbet.org'></a><a href='https://avkuy.com'></a><a href='https://zimprofiles.com'></a><a href='https://cakecartsdisposable.com'></a><a href='https://igame.news'></a><a href='https://casinowire.io'></a><a href='https://igamingfeed.org'></a><a href='https://www.forca.nu'></a><a href='https://marcusoskarsson.se'></a><a href='https://Svensktderby.se'></a><a href='https://spelnytt.nu'></a><a href='https://cardleague.org'></a>

Should WordPress consider implementing a more robust solution for enabling or disabling autosaving on a per post type basis, possibly by enhancing the 'supports' parameter to accept 'false' values, as suggested by some contributors? How might such a feature enhancement align with the platform's backward compatibility and development roadmap?
#!html<a href='https://rubyofsiamthai.com'></a><a href='https://singha88.com'></a><a href='https://dgbet.org'></a><a href='https://avkuy.com'></a><a href='https://zimprofiles.com'></a><a href='https://cakecartsdisposable.com'></a><a href='https://igame.news'></a><a href='https://casinowire.io'></a><a href='https://igamingfeed.org'></a><a href='https://www.forca.nu'></a><a href='https://marcusoskarsson.se'></a><a href='https://Svensktderby.se'></a><a href='https://spelnytt.nu'></a><a href='https://cardleague.org'></a>

@peterwilsoncc WDYT about picking this up again for 6.6? I believe it's ready.

The back compat code is small and required, and the new autosave support allows for a better discoverability in Gutenberg, which is a nice win.

[swissspidy]

Note that this blocks #60131 a bit

[rajinsharwar]

I have tested the patch with the below code.

<?php
/**
 * Register a custom post type called "book".
 *
 * @see get_post_type_labels() for label keys.
 */
function wpdocs_codex_book_init() {
        $labels = array(
                'name'                  => _x( 'Books', 'Post type general name', 'textdomain' ),
                'singular_name'         => _x( 'Book', 'Post type singular name', 'textdomain' ),
                'menu_name'             => _x( 'Books', 'Admin Menu text', 'textdomain' ),
                'name_admin_bar'        => _x( 'Book', 'Add New on Toolbar', 'textdomain' ),
                'add_new'               => __( 'Add New', 'textdomain' ),
                'add_new_item'          => __( 'Add New Book', 'textdomain' ),
                'new_item'              => __( 'New Book', 'textdomain' ),
                'edit_item'             => __( 'Edit Book', 'textdomain' ),
                'view_item'             => __( 'View Book', 'textdomain' ),
                'all_items'             => __( 'All Books', 'textdomain' ),
                'search_items'          => __( 'Search Books', 'textdomain' ),
                'parent_item_colon'     => __( 'Parent Books:', 'textdomain' ),
                'not_found'             => __( 'No books found.', 'textdomain' ),
                'not_found_in_trash'    => __( 'No books found in Trash.', 'textdomain' ),
        );

        $args = array(
                'labels'             => $labels,
                'public'             => true,
                'publicly_queryable' => true,
                'show_ui'            => true,
                'show_in_menu'       => true,
                'query_var'          => true,
                'rewrite'            => array( 'slug' => 'book' ),
                'capability_type'    => 'post',
                'has_archive'        => true,
                'hierarchical'       => false,
                'menu_position'      => null,
                'show_in_rest'       => true,
                'supports'           => array( 'title' ),
        );

        register_post_type( 'book', $args );
}

add_action( 'init', 'wpdocs_codex_book_init' );

✅ With Editor Support, AutoSave is turned on by default.

✅ With NO Editor Support, AutoSave is turned NOT turned on by default.

[swissspidy]

In 58201:

Posts, Post Types: Autosave: Allow disabling autosave support per post type.

Not all post types support autosaving. By making autosave a post type feature, support can be more granularly handled without any workarounds or hardcoded allowlists. wp_font_family and wp_font_face are examples of built-in post types which do not support autosave.

For backward compatibility reasons, adding editor support implies autosave support, so one would need to explicitly use remove_post_type_support() to remove it again.

Props swissspidy, youknowriad, hellofromtonya, peterwilsoncc.
Fixes #41172.

Committed in https://core.trac.wordpress.org/changeset/58201

[spacedmonkey]

I wonder recommend making attachments as not supporting autosaves as well. The existing work around that checks for post type === attachment can be removed. @swissspidy

[swissspidy]

In 58211:

Posts, Post Types: Remove now obsolete hardcoded attachment check for autosave support.

Follow-up to [58201].

Props swissspidy, spacedmonkey.
See #41172.

[swissspidy]

In 58216:

Build/Test Tools: Update REST API fixtures after [58211].

See #41172.

[peterwilsoncc]

I've added the needs-dev-note keyword. It might be possible to include a paragraph or two in a general/grouped dev note rather than a full note dedicated to this change.

These two post types were missing while working on the original ticket.

Gutenberg issue: ​https://github.com/WordPress/gutenberg/pull/64733.
Trac ticket: https://core.trac.wordpress.org/ticket/41172

@swissspidy, I used the old Trac ticket as a reference. Should I create a new one?

It looks like an attempt was made to make the autosaves endpoint work for Templates and Template Parts (https://core.trac.wordpress.org/ticket/56922). These PHP unit tests are failing now.

How do we want to proceed here? Should we disable the PHP unit tests until these post types fully support autosaving?

Note: The autosaving is disabled on the client side - ​https://github.com/WordPress/gutenberg/pull/64733.

@swissspidy​[[Image(chrome-extension://hgomfjikakokcbkjlfgodhklifiplmpg/images/wp-logo.png)]], I used the old Trac ticket as a reference. Should I create a new one?

Yes. New milestone/release = new ticket. The old ticket is already fixed on a closed milestone and must not be reopened/amended for changes in a new milestone.

How do we want to proceed here? Should we disable the PHP unit tests until these post types fully support autosaving?

I suppose we have to make up our minds here :-)

Just so I understand correctly:

[56819] added proper autosave support for wp_template and wp_template_part because previously the endpoints were sort of broken. (Commit message says: _"Consequently, when the post types template and template part were introduced in [52062], it led to the registration of REST API endpoints for autosaves and revisions with invalid URL structures."_)

But the editor never actually supported autosaves for wp_template anyway...? 🤔 Is that because it's not desired, or because it was previously broken?

If the former, we should update the core code here accordingly. If the latter, we should update Gutenberg, because it apparently seems to be supported by core.

So, which one is it?

Based on the latest comments on 56922 and new ticket, it seems that autosaves aren't fully working for Templates and Template Parts.

cc @anton-vlasenko, @ironprogrammer

That one is labelled as an enhancement though, just updating the regex and adding some hardening.

The template parts and autosave endpoint still generate a warning. One I'm trying to fix in Gutenberg PR.

In any case, let's see what others think about this.

That one is labelled as an enhancement though, just updating the regex and adding some hardening.

Correct. This is an enchancement and not a blocker.

The template parts and autosave endpoint still generate a warning.

I'm looking into the issue.

I've fixed the issue with the autosaves endpoint.
The revisions endpoint is likely fixed as well, but I would like to write more unit tests to ensure it is fully resolved and to prevent it from occurring again.
The work is being done here: ​draft PR. The estimated time of completion is sometime tomorrow.

@Mamaduka, if you don't mind, I'm going to create a new Trac issue for this bug, as https://core.trac.wordpress.org/ticket/41172 doesn't seem to be an exact fit.

Thank you, @anton-vlasenko!

Core ticket: https://core.trac.wordpress.org/ticket/61970
GH PR: ​https://github.com/WordPress/wordpress-develop/pull/7272

The PR is ready for review. The error occurs because the Editor is trying to fetch autosaves for a file-based template. Since file-only templates do not have any entities in the database, it is impossible to fetch autosaves (or revisions) for them.

My PR addresses this issue by changing the REST response code from 403 to 400 when the controller attempts to get autosaves (revisions) for a file-only template (template part) and also fixes the fatal PHP error.
Additionally, it adds more unit tests to cover the wp_template_part post type.

I would appreciate your review and approval, @Mamaduka and @swissspidy.

This PR resolves the fatal error issue, but it will not enable autosaves automatically.
Should the Editor first check if the template has a database entity, and if not, create one? This seems to be the way forward ...