fix #1158, show on rest api only the fields of the post type #1238
Conversation
|
Thanks for the PR! Just a general word of recommendation regarding your PR summary. It is much more likely the maintainers will be able to review and test if you provide more robust testing instructions. E.g. if you could provide the CPT setup you are using, the CMB box configuration, and the exact routes where you would see X or Y, that will go a long way towards us being able to quickly review and test. |
|
Seems that the bug in tests is that on https://github.com/CMB2/CMB2/blob/develop/tests/test-cmb-rest.php#L227 is creating a new REst_request that is not a string as suggesting by the comments in the code |
|
The CMB example code is in the ticket that show the issue :-) |
I think maybe you're misunderstanding. I am not saying we won't be able to figure out how to test this. Of course we can figure out how to test it. I'm saying the neater/nicer package you can give in your PR (even if you have to copy/paste from another issue) will go a long way towards getting the PR reviewed. I'm sure you're a busy guy too, and I very much appreciate the contribution, but when I see a PR with a description like this (where the description assumes context, where testing instructions are basically "do X", where the "Checklist" items are completely ignored/unchecked), it moves this PR to the bottom of my todo list, as I know it'll require extra hunting, and based on the PR description, that I won't get much help from the provider of the code, who does not appear to care much about the health of the project as much as caring that their piece of code gets added to deal with their one-off problem. Maybe that's not you, but when a PR is submitted in this way, this is what it indicates. As this project (and many others I maintain) are used and contributed to quite often, the more help I can get from the contributor, the more the workload is evened out among the many instead of the few. |
|
Yes, sorry I was having connectivity issues and studying the tests issue while pushing that patch on a job project -.-' I improved a bit the description and stuff in the ticket. Can you help me on understanding the problem with the tests? |
|
I can look at/fix the tests when I test this. Do me a favor and update the PR description to note that the tests will need to be fixed, so I don't forget. :) |
| if ( ! empty( self::$type_boxes[ $main_object_type ] ) ) { | ||
| foreach ( self::$type_boxes[ $main_object_type ] as $cmb_id ) { | ||
| $rest_box = self::$boxes[ $cmb_id ]; | ||
| if( in_array( $object_type, $rest_box->cmb->meta_box['object_types'] ) ) { |
There was a problem hiding this comment.
we are expecting a string to check if it is in the array but tests are using an object https://github.com/CMB2/CMB2/blob/develop/tests/test-cmb-rest.php#L227
|
There are conflicts so I am not sure if this issues is fixed |
* upstream/develop: (93 commits) Ensure to enqueue wp-color-picker Add props for CMB2#1331 Update "develop" bootstrap classname Proposal of CODE OF CONDUCT File Update version/copyright in scripts/stylesheets Fix issues with some tests to get them all passing `CMB2::is_box_type()` now also checks for taxonomies if box is registered to "term" object type. Prepare for 2.7.0 release Add all missing props/udpates to changelog bump tested-to value Rename CMB2_hookup to CMB2_Hookup Fixes CMB2#1328 Validate composer.json Tested Up To confirmations. Update issue templates Add license file to meet Community standards Add changelog for the register_rest_field_cb param Abstract some aspects of the getting/setting of box rest fields to make custom handling easier Add ability to short-circuit the register_rest_field with a callback. Allows registering callback with a different rest slug Oops, send the fallback value to the box_types method Add props for CMB2#1238 Fixes CMB2#1238 by merging with changes, and add CMB2::is_box_type method for checking. Fixes CMB2#1158 Add props for CMB2#1314 minify minify Re-add padding to textareas for readability Fix alpha color picker sample color styling clarify coment limit to 5.3 branch update block editor radio fix initial 5.3 prep work Add new "cmb2_display_class_{$fieldtype}" filter and field "display_class" parameter to allow specifying the class to use to display the field (in admin columns, etc) Fix another place where taxonomy_select_hierarchical was missed Add props for CMB2#1296 & CMB2#1297 Add props for CMB2#1300 Add props for CMB2#1307 Update npm dependencis. Fixes CMB2#1308 Remove grunt-contrib-compress and grunt-combine-media-queries to cut down on npm dependencies taxonomy_select_hierarchical is added in order to save correctly in to database Fix tests broke by introduction of character counter field property Fix issue introduced in 7cb3e00 where field was updated not not returned, breaking things If field has supporting data (e.g. file), add supporting data to field data (i.e supporting field id/value) Add changelog and props for CMB2#1300 Update .min file for CMB2#1300 Add comments to document changes in CMB2#1300 Include .CodeMirror class in sortable exclusions Run the field-specific actions for all fields (including fields in groups). Fixes CMB2#1157 Escaping Improvements to File Base Escaping improvements to File Fields Add changelog and props for CMB2#1276 Refactor character-counter integration ...
2 participants
Description
Show on rest api only the fields of the specific post type instead right now that show all the registered fields in all the various metabox.
Basically the problem is that now when you define different metaboxes on various post types and open a rest api page and see the fields you can find all of them (also of other post types) as empty.
Checking the code the problem was that there is an array filled with all of them without a check if it is the right place to show them.
There are issues with the tests right now and need to be fixed before this patch is ready to be approved.
Motivation and Context
Fixes #1158.
Testing procedure
Create 2 metabox to 2 different post types and you will see only the fields of the post type thaty ou are looking on rest api (check in the ticket for an example of the code)
Checklist: