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

Allow for optional/empty money field. #577

Merged
merged 1 commit into from
May 31, 2016

Conversation

jrfnl
Copy link
Contributor

@jrfnl jrfnl commented Jan 31, 2016

The number formatting of fields of the text_money type caused a money field to always be saved.
If the field was empty, it would be saved as '0.00'.

This fix allows for the proper (non-)saving of empty money fields.

@jrfnl jrfnl force-pushed the feature/fix-allow-empty-money branch from dfafe1d to dc9d625 Compare March 1, 2016 00:58
The number formatting of field of the text_money type caused a money field to *always* be saved. If the field was empty, it would be saved as '0.00'.

This fix allows for the proper (non-)saving of empty money fields.
@jrfnl jrfnl force-pushed the feature/fix-allow-empty-money branch from dc9d625 to 2167d89 Compare April 22, 2016 11:48
@monecchi
Copy link

This issue seems not to be solved yet for the cmb2-rest-api branch, I'm still getting money field being saved even if it is empty causing it to have a '0.00' value.

@jrfnl
Copy link
Contributor Author

jrfnl commented May 31, 2016

Did you test with this branch or with the trunk / master branch ? This PR (unfortunately) has not been merged in yet.

@jtsternberg jtsternberg merged commit 2e0317b into CMB2:trunk May 31, 2016
@jtsternberg
Copy link
Member

Just saw this. Thanks!

@jrfnl jrfnl deleted the feature/fix-allow-empty-money branch June 1, 2016 00:11
@monecchi
Copy link

monecchi commented Jun 1, 2016

Hey, @jrfnl. I did actually tested with this branch, which by the way seems not to expose CMB2 custom fields to the WP Rest API either. Maybe I am misunderstanding the way the CMB2 branches work. I've forked the cmb2-rest-api branch and updated my custom post type and cmb2 fields in order to expose it to the WP Rest API v2, as long as I thought it would have all the goods from the master branch + the WP API support.

@jrfnl
Copy link
Contributor Author

jrfnl commented Jun 2, 2016

I've not worked with the cmb2-rest-api branch myself yet. Maybe @jtsternberg would be the better person to ask in this case.

The only thing I can say about it, is that by the looks of it the rest-api branch is a little behind the trunk branch (which now has is fix) and trunk would need to be merged in for the rest-api branch to also benefit from this fix.

@monecchi
Copy link

monecchi commented Jun 2, 2016

@jrfnl oh, I got it, thanks for the input. So I think it's a matter of waiting for those branches to get merged.

The fact is the cmb2-rest-api branch is not working as it should yet, it does not expose my fields to the WP API, so I'm thinking about going back to the master or the trunk branch for the meantime.

If at least I could get the API functionality at my disposal, then the saved '0.00' value for the empty text_money field is a thing I could live with.

@jtsternberg
Copy link
Member

The cmb2-rest-api branch is still in-progress, and as such, documentation is sparse. The fields showing in the REST API is an opt-in feature, not opt-out, so you wouldn't see anything by default. Just search for 'show_in_rest' in the example-functions.php file of that branch. Also, the rest api branch is up-to-date with trunk now.

@monecchi
Copy link

monecchi commented Jun 9, 2016

Hey @jsternberg. Thanks for pointing out the file, I've already followed all the necessary steps to make it work but no success. I'll try the trunk branch to see if it keeps up with all my cmb2 plugins while solves the WP Rest API not exposing the fields issue.

@jsternberg
Copy link

@jtsternberg I think the above message was meant for you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants