Fix retrieving autosaves when using a custom rest_namespace

[tomjn]

What?

When specifying rest_namespace in a custom post type, a hardcoded wp/v2 in getAutosaves results in a 404

So instead, use the rest namespace from the type, not just the base

Why?

Because bugs are bad and CPT's with rest_namespace deserve autosaves too!

How?

By not hardcoding /wp/v2

Testing Instructions

1. Register a CPT named test
2. In the registration, use rest_namespace and set it to something such as testing/v99
3. Save permalinks
4. Try to edit a post and there should not be a 404 for wp/v2/test?autosaves.... in the browser error console

[TimothyBJacobs]

Thanks for the PR @tomjn!

The test failures do look related. I'm guessing that the post types response is mocked somewhere and that mock is missing a rest_namespace field definition?

[adamsilverstein]

Good catch @tomjn - your code probably just needs a fallback if namespace is undefined to get tests passing. I did notice we have some other non-test instances of wp/v2 being hard coded (for example post trashing also has it, see https://github.com/WordPress/gutenberg/search?q=wp%2Fv2). it would be good to:

1. fix all instances of hard-coded name spaces
2. add tests with post with custom namespaces to validate endpoints are constructed as expected

[tomjn]

I'll be honest, I wrote this in the github UI then took a flight so not surprised it failed a check or two, so this is my first chance to respond.

I do remember seeing the namespace referenced like this elsewhere:

const namespace = postType?.rest_namespace ?? 'wp/v2';

[tomjn]

@adamsilverstein @TimothyBJacobs I've addressed the linter issue, took me a bit more time than expected to get the dependencies/npm setup locally, I'm unsure on the best way to implement ?? while destructing. I thought about something like this:

	const type = await resolveSelect.getPostType( postType );
	const base = type.rest_base;
	const namespace = type?.rest_namespace ?? 'wp/v2';
	const autosaves = await apiFetch( {
		path: `/${ namespace }/${ base }/${ postId }/autosaves?context=edit`,
	} );

I expect there'll be pushback over the naming of type though, suggestions?

[tomjn]

Updating the test mock data and adding new tests is beyond my bandwidth/knowledge for the project at the moment though, so I'll need help or someone else to pick it up

[TimothyBJacobs]

You should be able to assign a default while destructuring.

  const { rest_namespace: restNamespace = 'wp/v2' } = resolveSelect.getPostType( postType );

[tomjn]

🥳 thanks @TimothyBJacobs that has CI passing!

[adamsilverstein]

Looks good! Would be nice to add tests to validate and fix any other hard coded namespaces in a future PR.

[adamsilverstein]

Nice work @tomjn - approved!

I'd love to see a follow up adding the tests and fixing any other hard coded namespace instances. Could you please create an issue to track this (even if you don't have time to work on the PR)?

[hgodinho]

Is there a way to disable the autosave feature until this PR is merged?
The first call is taking more than 4 seconds and delaying the full loading of the page...

[tomjn]

@adamsilverstein I no longer have the bandwidth to work on this, I don't have the capacity to push it forward, if you or someone else can create an issue for it I'd appreciate it, this was mostly me trying to help out someone on stack exchange and doesn't impact me personally. I also see there are now conflicts resulting from formatting changes made elsewhere

[spacedmonkey]

This looks good to go to me!

[spacedmonkey]

Related: #41950

[Mamaduka]

Hi, @tomjn

It looks like static analysis checks are failing. Do you mind rebasing this branch and running the linter on the changed file?

[tomjn]

I don’t have the time to work on this, this will need a new owner to champion it and push it forward

[Mamaduka]

Thanks for the quick reply, @tomjn. Happy to take over to PR and see this merged.

[spacedmonkey]

@Mamaduka I think we are good to merge now.

[Mamaduka]

Thanks for the fix, @spacedmonkey!