Validation

[spacedmonkey]

Improved validation of saving and displaying fields. Now correct types are checks and fields sanitized.

Much of this code is copied from wp-cli. See this.

There is more sanitize found in the customizer here.

Also improved all interger values defaults and allowed values.

This PR validation against the following use cases.

• If menus field submitted, then check if 1 menu is given, if more than 1 is given, error out as a menu item can't be in two menus.
• If item type is set to taxononmy, check if term exists.
• If item type is set to post_type, check if post exists.
• If item type is set to post_type_archive, check is post type exists.
• If item type is custom, valid that title and url are not empty.
• Check if menu id is a valid existing menu
• If menu exists and menu item is 0, set position to last position +1 putting nehew menu items to the end.
• If menu item position is set, check that position isn't already in use.
• If menu item parent is set, check if menu item exists and in same menu.
• Run title, title attr and description through filters.
• Validate url is valid url.
• Validate post status, publish and draft are only valid post statuses.

Other changes.

• Add defaults to params
• Add min and max to int fields.
• Add embed to context.
• sanitize_key on string fields.
• Change key for parent to parent over menu parent.
• Populate existing values of menu item on update.

Fixes #38

[TimothyBJacobs]

Thanks for adding more to the PR description! I've added some preliminary feedback.

[spacedmonkey]

I am going to completely honest here, some of the validation here, might be overkill or repeated. I think maybe some of it needs scaled back.

[kadamwhite]

I concur with @TimothyBJacobs' notes on cleaning up some of the validation, and presenting data in the format we'd want to work with it rather than in whatever legacy manner core stores values.

I noted some areas where I found the comments or formatting a bit confusing but these are not blockers for merge.

[spacedmonkey]

@kadamwhite All your feedback as been actioned.

[draganescu]

Hey @spacedmonkey this is great! It's a big-ish PR I'd say.

Should all the new things in prepare_item_for_database be covered by new tests?

Left minor comments and also noting that tests fail apparently only because of small style violations.

[draganescu]

I have tested and indeed it also fixes #38 .

[spacedmonkey]

Hey @spacedmonkey this is great! It's a big-ish PR I'd say.

Should all the new things in prepare_item_for_database be covered by new tests?

I know this PR is a got a little to large. It got a little out of hand. To stop it getting any bigger and become impossible to review, I have agree with the rest of the team to just get this merged and work on another PR with test. Hopefully this will make the code review process quicker and mean we don't get blocked like this again.

[draganescu]

Given that the remaining issues can be addressed in separate smaller PRs and that this PR works well I think we can merge this and be able to test working with menus via the api in the master of this plugin.

[TimothyBJacobs]

Going ahead and merging this. I'd really like to see tests for handling the negative menu item IDs and reordering.