Fixes and new features

[a-barbieri]

Fixed

1. Requesting menu items by menu ID
2. Requesting menu items by menu slug

Add

1. Ability to get locations
2. New route to properly fetch menu items by location slug menus/v1/locations/your-slug
3. Add error message which to be more helpful

[NHollmann]

I'm not a contributor to this project but I've just seen your PR and got some thoughts on it.

First of all, you have changed the indentation of the codebase. This makes the comparison of your changes against the base really hard. 😕

I really like custom errors on wrong ids, but just adding an error field to the response is somewhat messy. I would rather use the appropriate WP_Error response. With that the error would be more consistently with other REST APIs in WP.

Also, it is really hard to understand how the routes will change. What benefit does the locations route provide? How does it change the behavior of the menus route?

For me it looks like this PR would break compatibility. I've not tested it, so correct me if I'm wrong, but: The wp_api_v2_locations_get_menu_data method replaces the wp_api_v2_menus_get_menu_data in its functionality. But it does not initialize the $menu variable with the get_term call (see this). I think this will result in missing fields in the response.

@a-barbieri Maybe you could change the indentation so the comparison of changes would be easier? @thebatclaudio what is your opinion on this?

[a-barbieri]

Indentation

First of all, you have changed the indentation of the codebase. This makes the comparison of your changes against the base really hard. 😕

Really sorry. I guess my editor has done it without me noticing it.

---

WP_Error

I really like custom errors on wrong ids, but just adding an error field to the response is somewhat messy. I would rather use the appropriate WP_Error response. With that the error would be more consistently with other REST APIs in WP.

This is definitely right. Will work on this.

---

Locations route benefit

What benefit does the locations route provide? How does it change the behavior of the menus route?

Menus have their own slug which is different from the location slug. Unless you created a location or you haven't edited the theme you don't know what the locations slugs are. In the current release v0.5 the README says an ambiguous thing:

Get menus data from slug:

GET /menus/v1/menus/{slug}

But the slug isn't actually the menu slug, but the location one. From the endpoint GET /menus/v1/menus you don't get the location slug, so I thought a GET /menus/v1/locations would be useful to get them.

Once I did that a natural consequence has been to create a GET /menus/v1/locations/{slug} where slug is the location slug, not the menu one and then fix the above ambiguity allowing menu slug to be passed to GET /menus/v1/locations/{slug}.

Ultimately once you have GET /menus/v1/locations/{slug} and GET /menus/v1/menus/{slug} a good approach would be to deprecate the possibility to use GET /menus/v1/menus/{slug}with a location slug.

Depending on your administration organisation you will either use menus or locations, not both. For example I don't use locations at all because I prefer not to edit the default theme and hardcoding stuff in it. Therefore I use Appearance > Customise > Menus where I can place as many menus I want.

---

get_term()

For me it looks like this PR would break compatibility. I've not tested it, so correct me if I'm wrong, but: The wp_api_v2_locations_get_menu_data method replaces the wp_api_v2_menus_get_menu_data in its functionality. But it does not initialize the $menu variable with the get_term call (see this). I think this will result in missing fields in the response. You are right. Need fixing this.

---

[a-barbieri]

Ok, @NHollmann I think I fixed everything.

WP_Error

If menu or location hasn't been found an error gives you an explicit 404 error message.

Let's say location 3 doesn't exists, this is the error:

{
    "code": "not_found",
    "message": "No location has been found with this id or slug: `3`. Please ensure you passed an existing location ID or location slug.",
    "data": {
        "status": 404
    }
}

---

get_term()

Any request for specific menu or location now returns the menu info as you requested.

---

Locations route benefit

I've enhanced the locations route which now gives you info about the location and whether a menu is assigned to it or not.

Let's say menu-1 location has primary-it menu, while footer location is empty.

{
    "menu-1": {
        "slug": "menu-1",
        "menu": {
            "term_id": 11,
            "name": "Primary IT",
            "slug": "primary-it",
            "term_group": 0,
            "term_taxonomy_id": 11,
            "taxonomy": "nav_menu",
            "description": "",
            "parent": 0,
            "count": 4,
            "filter": "raw"
        }
    },
    "footer": {
        "slug": "footer",
        "menu": {
            "errors": {
                "invalid_term": [
                    "Empty Term."
                ]
            },
            "error_data": []
        }
    }
}

---

[a-barbieri]

It'd be good to update the README as well. I might do it once the PR is confirmed.

[NHollmann]

Now this really looks good!

I hope @thebatclaudio reviews your changes in a few days.

[thebatclaudio]

Hi guys, thank you for your contributions! I will update the plugin with your contributions ASAP

[thebatclaudio]

Hi guys, do you want to be mentioned as contributors? (I mean here: https://wordpress.org/plugins/wp-rest-api-v2-menus/)

[a-barbieri]

Sure

[thebatclaudio]

Ok, you have to tell me your wordpress.org username

[a-barbieri]

alebarbieri 🙋🏻‍♂️

Il giorno 8 apr 2019, alle ore 17:03, Claudio La Barbera notifications@github.com ha scritto:

Ok, you have to tell me your wordpress.org username

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/thebatclaudio/wp-rest-api-v2-menus/pull/12#issuecomment-480870355, or mute the thread https://github.com/notifications/unsubscribe-auth/ABdSlOpO_7mVd-7m1McGkQtRq0-NBoexks5ve1pCgaJpZM4cAcQ9.