Menus endpoints don't allow for menu item reordering

[tellthemachines]

Is your feature request related to a problem? Please describe.

For the experiment to use the navigation block in nav-menus.php, we need the block to have identical functionality to the nav-menus.php page. Part of this is the ability to reorder menu items.

To save edits to a menu in the navigation block, we're currently using the __experimental/menu-items API endpoint to save each menu item individually. But this code prevents us from saving any changes to a menu item position, so we can't use this endpoint to reorder menu items.

Describe the solution you'd like

A possible solution would be an endpoint that allowed us to save the menu as a whole for each edit, or to PATCH it, instead of saving items one by one.

Describe alternatives you've considered

Not sure if removing the check for potential duplicates qualifies as an alternative solution 😅

[spacedmonkey]

The menus endpoint does support the reordering of menu items. However, at the moment, there are check to see you do not place a menu item in the place of another, so 2 menu items can not be 5 for example. Meaning if you want to reorder a whole menu, you would have set the order in a clever way. So move everything back a order number first. So 9 moved to 10 etc, until 5 is free. This isn't great.

The REST API doesn't currently support multiple updates in one request, which it may have to do. Thoughts @TimothyBJacobs

[TimothyBJacobs]

Yeah this is what I was referring to in the ergonomic issues WP-API/menus-endpoints#28. To me the solution isn't making multiple updates in one request, because even if we allowed that, unless the updates were processed simultaneously on the server as well, you'd still run into the issue of orders stepping on each other.

I think menu ordering needs to be thought of as a distinct RPC-like action.

[adamziel]

There is also this discussion about showing save/error notifications on save:

#21344

I don't think we're able to provide a consistently good experience without an atomic navigation save endpoint that either stores all the changes at once or doesn't change anything at all. The worst possible scenario here is that 50% of the requests succeeds, and the other 50% does not not - which means the live site now is broken and it's quite hard to communicate how to get it fixed.

To me the solution isn't making multiple updates in one request, because even if we allowed that, unless the updates were processed simultaneously on the server as well, you'd still run into the issue of orders stepping on each other.

I am imagining the following scenario:

1. An API endpoint accepts the entire navigation as a parameter
2. It starts a transaction and performs all the inserts / deletes / updates required. It also knows how to handle reordering.
3. If anything fails, it rolls back the entire transaction and returns a failure
4. If everything works, it commits the transaction and returns success

cc @spacedmonkey @TimothyBJacobs @talldan @tellthemachines

[noisysocks]

Agree with @adamziel.

Could we collapse the menu-item resource into the menu resource?

GET /wp/v2/menus/5

{
	"id": 5,
	"slug": "distracting-sites",
	"description": "Distracting sites",
	...
	"items": [
		{
			"id": 125,
			"type": "custom",
			"url": "http://twitter.com",
			"title": { "rendered": "Twitter" }
			...
			"items": []
		},
		{
			"id": 126,
			"type": "custom",
			"url": "http://reddit.com",
			"title": { "rendered": "Reddit" }
			...
			"items": []
		}
	]
}

I can't think of a use case where we'd want to fetch a single menu item without also fetching its siblings, and, in WP Admin, the existing menu screens and the proposed Navigation screen all take a click Save to save everything at once approach. I know that menu items are stored as posts but this could be an implementation detail.

[spacedmonkey]

To be clear what I meant, a would add an new endpoint that is specially designed for reordering.

POST /wp/v2/menu-items/<menu_id>/ordering
{
    "menu-items": {
          <menu_id>: <order>,
          <menu_id>: <order>,
    } 
}

This would make the menu order in one request.
Updating of menu items other values, can and should be done as different requests. It is only menu items that require knowledge of each other to stop colliding with each others orders.

[adamziel]

@spacedmonkey I'm thinking about a single API endpoint to save everything. Otherwise how would you handle partial success if we use multiple requests to save changes?

[spacedmonkey]

I'm thinking about a single API endpoint to save everything. Otherwise how would you handle partial success if we use multiple requests to save changes?

Yes, I understand that.

Menu items are storied as posts in WP. Each menu item has be updated in it's own request. I don't understand the issue here.
If you change a menu, you would fire off one request to change the order (if the order changed at all) and a request to each menu item to update the name or other value you changed. Because you are making a requset per item, you can easily find see fails and retry to save. Otherwise, we would need to develop a system of transactioning and error handling we do not currently have.

[adamziel]

Menu items are storied as posts in WP. Each menu item has be updated in it's own request. I don't understand the issue here.

@spacedmonkey let's imagine the following scenario:

I have a website with a navigation such as:

Now, I want to refresh it a little bit and achieve the following result:

I go either to nav-menus.php, remove some undergraduate degrees, add graduate degrees and a bunch of submenu items, then I click save.

Because you are making a requset per item, you can easily find see fails and retry to save.

Let's imagine some of my requests worked and some other did not. There are multiple possible causes - a Gutenberg/WordPress bug, a network issue, a database availability problem. If that's a temporary network problem, we can just retry. If for whatever reason a request cannot be processed properly by the backend, retrying won't help. Ditto for more persistent network failure (like flaky internet that just disconnected mid-save).

Now - what's stored in the database is neither what I started with, nor what I wanted to end up with. It's some interim state, and now it's live on my website. How do I know this happened? How should I fix it? I don't think it's just a corner case that we could dismiss.

Otherwise, we would need to develop a system of transactioning and error handling we do not currently have.

The endpoint shouldn't be overly complex. I'm not sure about the frontend part - I'd need to take a deeper dive in core/data and such. But I'm happy to volunteer and prepare a proof of concept that we could discuss further.

[talldan]

Otherwise, we would need to develop a system of transactioning and error handling we do not currently have.

I can see that the difficulty here is the way the menu items are represented as posts, but I think the alternative being proposed is to move this transactional and error handling logic to the client side, which makes less sense to me as the client side code is further from the source of truth and less able to reason about the state of data.

[adamziel]

Just surfacing my early explorations: #22148

[TimothyBJacobs]

I think there are three issues here that need separating.

1. Reordering a menu isn't straightforward with the current endpoints.
2. Needing to make separate HTTP requests to each menu item isn't great performance wise and can lead to an odd user experience if some items validate, and some don't.
3. What if the actual saving fails. The menu is now in a consistent state.

---

1. I think this needs to be a separate endpoint and I think what @spacedmonkey proposed makes sense.
2. I think this would be solved by formally introducing batch requests to the REST API. And validate all item requests before starting any updates.
3. This is much more tricky, WordPress doesn't support transactions. With actions rolling back would get really difficult. What'd probably be easier would be to update any menu item that was previously updated to its previous values. This could be done generically in the batch endpoint.

Though, I think we should consider how practical of a problem it is. From what I can tell of the customizer, it essentially does 2 where each menu item is validated. Then if validation is successful, does the actual saving. If the saving fails, this is sent back in the response, but as far as I can tell, not shown in the UI anywhere.

Error Response

{
    "success": true,
    "data": {
        "setting_validities": {
            "nav_menu_item[1176]": true
        },
        "changeset_status": "publish",
        "next_changeset_uuid": "a30aa304-493c-479d-8c01-c75d39a2fb4e",
        "nav_menu_item_updates": [
            {
                "post_id": 1176,
                "previous_post_id": null,
                "error": "test_error",
                "status": "error"
            }
        ]
    }
}

Given that, I don't think it is too likely for menu items to pass validation, but then fail when actually persisting the data.

Maybe @westonruter could shed some light on the customizer menus implementation and how often the persistence of menu items actually fails.

So from my perspective, we should 1) add a new ordering endpoint, 2) add batch processing to the REST API. For now, we could skip rolling back, but if it turned out to be a common issue we could work to support it.

[westonruter]

Though, I think we should consider how practical of a problem it is. From what I can tell of the customizer, it essentially does 2 where each menu item is validated.

Correct. Only when all settings are validated should they actually be persisted to the DB.

Then if validation is successful, does the actual saving. If the saving fails, this is sent back in the response, but as far as I can tell, not shown in the UI anywhere.

How are you causing an error? From looking at the code, it doesn't look like the error status was implemented.

[TimothyBJacobs]

Thanks for clarifying @westonruter!

I returned a new WP_Error at the beginning of wp_update_nav_menu_item.

[draganescu]

👋 @TimothyBJacobs what does "formally introduce" mean?

[TimothyBJacobs]

@draganescu Introduce as a first-class feature. In other words, not tied so closely to the menus endpoint.

[adamziel]

Reordering menus is now functional and this issue can be closed. For now we're using the customizer endpoint, but the endgame there is using batch/bulk REST api endpoints.

[TimothyBJacobs]

See #25096.