Skip to content
This repository was archived by the owner on Jul 6, 2025. It is now read-only.

Conversation

vedanshujain
Copy link
Contributor

This PR has a couple of changes in response to https://core.trac.wordpress.org/changeset/48306

  1. Type mixed is now changed to appropriate scalar type with addition of null. So final type is now array( {scalar}, 'null' ).
  2. [High Impact] For settings API, the allowed type is now changed to array, string, null, and if an array, then allowed item type is string or null. This means that only one of nesting is now allowed for WooCommerce settings via API. Would appreciate the feedback here on if this is backward compatible or not. Looks like we have to mandatorily define the child properties if the native property is array or object. This means that just saying that type is object is not allowed, we have to define all the properties of children (and their children, etc if any).
  3. Added a method to convert from type => date-time to type => string, format => date-time.

@vedanshujain vedanshujain added the status: needs review PR that needs review. [auto] label Aug 5, 2020
@vedanshujain
Copy link
Contributor Author

note that code standard failures are unrelated to changes in this PR. I will address them in different PR directly.

@vedanshujain
Copy link
Contributor Author

Combined test results with core for different WP version in the merge/src PR : https://travis-ci.org/github/woocommerce/woocommerce/builds/715231616

Copy link

@jonathansadowski jonathansadowski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. I did a search for some cases of the nesting you were referring to in point number 2. I think I understand the problem re: backwards compatibility that you're pointing out, but do you have a more specific example?

@vedanshujain
Copy link
Contributor Author

@jonathansadowski so an example is, we will not be able to support a setting option where value is of type object. Whereas previously with mixed type we would have as there is no validation basically.

That said, we currently only define text, email, number, color, password, textarea, select, multiselect, radio, image_width and checkbox. setting types and all of these can be supported by the new type that I added. Only multiselect here needs array support rest of these can be supported by the string. (I went and explored all the settings from wp-json/wc/v3/settings/ endpoint to verify.)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: needs review PR that needs review. [auto]
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants