Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Meta boxes: server side validation with add_settings_error isn't possible #3964

Closed
ghost opened this issue Dec 12, 2017 · 10 comments
Closed
Assignees
Labels
Backwards Compatibility Issues or PRs that impact backwards compatability [Feature] Meta Boxes A draggable box shown on the post editing screen Needs Technical Feedback Needs testing from a developer perspective. [Priority] Low Used to indicate that the issue at hand isn't a top priority to address and can be handled later [Type] Developer Documentation Documentation for developers [Type] Plugin Interoperability Incompatibilities between a specific plugin and the block editor. Close with workaround notes.

Comments

@ghost
Copy link

ghost commented Dec 12, 2017

Issue Overview

A lot of existing meta boxes, especially in enterprise environments, rely on server side validation / sanitation, including meaningful feedback to the user.

This fails in Gutenberg currently.

Steps to Reproduce (for bugs)

  1. Setup validation as in https://tommcfarlin.com/post-meta-data-error-messages/ , this requires a CPT 'Location' and a meta box with a single text field.

  2. Create a new location, fill in data and save it.

  3. API Call will fail with an error "Call to undefined function add_settings_error()", which is used in the example

  4. Replace add_settings_error() with a simple string to continue testing.
    https://gist.github.com/wsydney76/0dd717fd061908d8037d29ce12569b86

  5. Enter a value and save.

  6. Then leave the text field blank and save. "Post saved" indicates ok, no error messages shown, the field is still blank.

  7. Reload. The old field value is still present.

  8. Enter content like 'with tags and save. Tags will be stripped server side, but that is not reflected in the editor. Only a reload shows the altered content.

Gutenberg 1.9

Expected Behavior

The user has to get information about the data being rejected or changed server side. After a "successful" update the content in the editor must not differ from the content in the database.

Current Behavior

If it works at all, the users workflow is broken, potentially resulting in invalid data.

Possible Solution

Documentation is needed:

  • Which kind of implementations of server side validation breaks in Gutenberg?
  • How can it be changed to work in Gutenberg?
@youknowriad youknowriad added the [Feature] Meta Boxes A draggable box shown on the post editing screen label Dec 13, 2017
@danielbachhuber danielbachhuber added Backwards Compatibility Issues or PRs that impact backwards compatability [Type] Plugin Interoperability Incompatibilities between a specific plugin and the block editor. Close with workaround notes. labels Mar 30, 2018
@danielbachhuber
Copy link
Member

Related #5590

API Call will fail with an error "Call to undefined function add_settings_error()", which is used in the example

@wsydney76 Can you clarify what you mean by this?

@ghost
Copy link
Author

ghost commented Mar 30, 2018

@danielbachhuber Sure, as far as i remember the error occurred when saving the post and with it the meta boxes, so the term "API Call" may be incorrect for the meta boxes part.

The call to add_settings_error is in part 4 of https://tommcfarlin.com/post-meta-data-error-messages/

@danielbachhuber
Copy link
Member

Created #5975 to define a formal notices API.

@danielbachhuber
Copy link
Member

In thinking through this further, I'm not sure it will be possible to support this formally in Gutenberg.

We could potentially detect calls to add_settings_error() in the POST request to save meta boxes. However, there's no formal API between calls to add_settings_error() and their display, so I'm not sure what Gutenberg would do if it detected these calls. Reloading the meta box part of the page would be insufficient, because admin_notices is in the header of the page.

With this being said, someone could produce a plugin implementation that replicates the validation behavior, and we could refer to that as a recommended compatibility shim. I'm not sure Gutenberg could do something like this correctly by default, but a workflow where the user has to install and test the plugin would also verify the validation works for them as expected.

@mcsf
Copy link
Contributor

mcsf commented Aug 30, 2018

Came here via #4063. @danielbachhuber, I noticed you self-assigned this one. Out of your curiosity, what's your plan here?

@danielbachhuber
Copy link
Member

Out of your curiosity, what's your plan here?

Document and close, most likely.

@mtias mtias removed this from the Merge: Back Compat milestone Oct 3, 2018
@mtias mtias added the [Type] Developer Documentation Documentation for developers label Oct 3, 2018
@mtias mtias added this to the WordPress 5.0 milestone Oct 3, 2018
@danielbachhuber
Copy link
Member

@chrisvanpatten Can you document this in https://github.com/danielbachhuber/gutenberg-migration-guide (when you have a chance) and then we can close?

@mtias mtias added the [Priority] Low Used to indicate that the issue at hand isn't a top priority to address and can be handled later label Nov 1, 2018
@chrisvanpatten
Copy link
Contributor

Also relates to discussion about the Notices API in Slack today.

It'd be nice to see this addressed but until then I've put a note in #11251 and I'm going to rename the ticket for clarity.

@chrisvanpatten chrisvanpatten changed the title Meta boxes: server side validation fails Meta boxes: server side validation with add_settings_error fails Nov 6, 2018
@chrisvanpatten chrisvanpatten changed the title Meta boxes: server side validation with add_settings_error fails Meta boxes: server side validation with add_settings_error isn't possible Nov 6, 2018
@youknowriad youknowriad removed this from the WordPress 5.0.x Follow Ups milestone Jan 9, 2019
@ryanwelcher
Copy link
Contributor

Is this issue still relevant?

@ryanwelcher ryanwelcher added the Needs Technical Feedback Needs testing from a developer perspective. label Feb 7, 2023
@ndiego
Copy link
Member

ndiego commented May 25, 2023

This issue was reviewed in today's Documentation Issue Scrub. Given the inactivity of this issue in the last couple of years, I am going to close. However, if anyone feels like this should be reopened, please don't hesitate to do so. Thanks!

@ndiego ndiego closed this as completed May 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backwards Compatibility Issues or PRs that impact backwards compatability [Feature] Meta Boxes A draggable box shown on the post editing screen Needs Technical Feedback Needs testing from a developer perspective. [Priority] Low Used to indicate that the issue at hand isn't a top priority to address and can be handled later [Type] Developer Documentation Documentation for developers [Type] Plugin Interoperability Incompatibilities between a specific plugin and the block editor. Close with workaround notes.
Projects
None yet
Development

No branches or pull requests

7 participants