#5472 closed defect (bug) (fixed)
xprofile_insert_field -> type - 'option' not allowed?
Reported by: | shanebp | Owned by: | boonebgorges |
---|---|---|---|
Milestone: | 2.1 | Priority: | normal |
Severity: | normal | Version: | 1.9.2 |
Component: | Extended Profile | Keywords: | needs-unit-tests |
Cc: | shane@… |
Description
When using a script to populate xprofile fields, this works:
// in a loop xprofile_insert_field( array( 'field_group_id' => 1, 'parent_id' => $some_id, 'type' => 'selectbox', 'name' => $some_name, 'option_order' => $i ));
But option_order is not set in the db.
Perhaps because 'type' is selectbox so option_order is ignored in
class BP_XProfile_Field->save() ?
But if 'type' => 'option', nothing is inserted.
Perhaps because it fails here ?
// in function xprofile_insert_field() if ( !in_array( $type, (array) $bp->profile->field_types ) )
Populating xprofile selectboxes is a common chore.
Being able to set the 'option_order' is important.
I've been side-stepping it by using $wpdb->insert.
But I was going to do a plugin re djpaul's comment in this thread:
http://buddypress.org/support/topic/timestamps-in-members-timezone/#post-176014
So I probably should use the 'BP Approved' approach - which isn't possible unless I missing something.
Attachments (2)
Change History (27)
#2
@
11 years ago
- Component changed from Core to XProfile
imath - thanks for the pointer. Ton of work being done in 5220!
#3
@
11 years ago
Tested against BP 2.0 beta.
Same issues persist:
xprofile_insert_field -> type does not allow 'option'
xprofile_insert_field -> option_order is ignored
#4
@
11 years ago
- Milestone changed from Awaiting Review to 2.1
Thanks for the additional info, shanebp.
option_order is not set in the db.
Perhaps because 'type' is selectbox so option_order is ignored in
class BP_XProfile_Field->save() ?
Correct. It's saved only for children. I wouldn't classify this as a bug - the correct field type for your use case is not 'selectbox' but is instead 'option'.
But if 'type' => 'option', nothing is inserted.
Perhaps because it fails here ?
Yup, this diagnosis is correct. Either we should add 'option' to the permitted field types, or we should make a special exception in xprofile_insert_field() for 'option'. Moving to 2.1 to solve it one way or another. I'm also attaching a unit test that demonstrates the failure.
#5
@
10 years ago
- Keywords dev-feedback added
Considering BP_XProfile_Field->save()
looks in POST for option values, and that this approach is pretty much built around our current UI for adding/remove fields, rather than being more general and abstracted, I think adding option
as a permitted field type is the best way forward.
If agreed, this should be a quick change to make, since Boone's already written the unit test. :)
#6
@
10 years ago
Thanks, DJPaul. Only tricky bit is that we now define our field_types
whitelist by looking at the registered BP_XProfile_Field_Type
classes. I don't think it makes sense to have one of these for 'option', since it's not really a field type. 5472.01.patch seems like a reasonable place to make the manual modification to the whitelist. An alternative is to change the relevant check in xprofile_insert_field()
to
if ( !in_array( $type, (array) $bp->profile->field_types ) && 'option' !== $type )
This latter technique is more conservative, but it might also fail to address other edge cases for programatically creating fields. DJPaul, I'd like your feedback on this before going forward.
#7
@
10 years ago
The patch works for me with BP 2.0.1
Here's an insert Countries selectbox function:
https://gist.github.com/shanebp/119ffd879acc56c324ab
#8
@
10 years ago
Boone, I agree with your patch. Code looks like it will fallback and use the BP_XProfile_Field_Type_Placeholder
class type internally where we need to use a validation option, so I think it will be OK.
#9
@
10 years ago
- Owner set to boonebgorges
- Resolution set to fixed
- Status changed from new to closed
In 8518:
#10
@
10 years ago
- Resolution fixed deleted
- Status changed from closed to reopened
Found an odd issue...
When using xprofile_insert_field to create options, I guess we need to use 'field_order' instead of 'option_order' because
if ( $this->type_obj->supports_options ) {
will fail for field_types[] = 'option'; ... ?
This script works fine the first time
https://gist.github.com/shanebp/119ffd879acc56c324ab
But if you delete the Country field and then run the script again, the display order does not follow the field_order. On the second run, all the database fields are exactly the same, except for the 'id' field which is auto-incremented.
If I delete the Country field ( via page=bp-profile-setup ) and then manually set the auto-increment value for the bp_xprofile_fields table to the next unused id, and then run the script, the display order follows the field_order. I don't understand why that is.
Perhaps this request has opened a can of worms.
And it might be best to roll-back the change-set.
And for programatically inserting fields, use xprofile_insert_field to create the type and $wpdb->insert to create the options.
#11
@
10 years ago
- Keywords needs-unit-tests added; dev-feedback removed
Thanks for the feedback, shanebp.
And for programatically inserting fields, use xprofile_insert_field to create the type and $wpdb->insert to create the options.
No, this is unacceptable. The fact that it's not possible to do what you're asking is a failure of our API.
I'm afraid I don't really understand what you're describing with the autoincrement stuff. What we really need here is unit tests that demonstrate the failure, so we'll have something to build against.
#12
@
10 years ago
I don't really understand what you're describing with the autoincrement stuff.
Say the countries script inserts 200 countries.
And say the database row, for the field type = selectbox, has an id of 10.
Then the last countries option row id would be 210.
And the display order of the options follows the field_order.
Then delete the Country field, via page=bp-profile-setup, and run the countries script again.
The database row, for the field type = selectbox, now has an id of 211.
Then the last countries option row id would be 411.
But the display order of the options does not follow the field_order.
If I delete the Country field again, and manually change the database table's auto-increment value to 10 and run the script again, the the display order of the options will follow the field_order.
Not sure if this matters, but I notice in your field test, you are using this to create the parent:
$parent = $this->factory->xprofile_field->create
Instead of xprofile_insert_field
If you change your unit test to:
- use a loop to insert a handful of options and increment the field order
- delete the parent and all the options
- run the insert parent and options again
The test will not fail, but the display order will not ( afaik ) follow the field_order.
I sorry, but I'm not sure how to write a unit test that reveals if display order honors the field_order.
#13
@
10 years ago
A gut feeling says this issue is connected to having to use 'field_order' rather than 'option_order' when using a loop to add options to a Parent.
#14
@
10 years ago
Or maybe there is a cache that needs busting when programatically inserting fields ?
#15
@
10 years ago
A gut feeling says this issue is connected to having to use 'field_order' rather than 'option_order' when using a loop to add options to a Parent.
Yes. https://buddypress.trac.wordpress.org/browser/tags/2.0.1/bp-xprofile/bp-xprofile-classes.php#L729 Children are fetched by option_order
. field_order
has nothing to do with it.
if ( $this->type_obj->supports_options ) {
I guess you're referring to BP_XProfile_Field::save()
? I'm not sure how that would affect this particular issue.
I tested your script using option_order
instead of field_order
when inserting country names, and it appears to work fine. Have you tested this?
#16
@
10 years ago
I tested your script using option_order instead of field_order when inserting country names, and it appears to work fine. Have you tested this?
Yes. The option_order value for all rows is 0.
I believe due to BP_XProfile_Field::save() check re supports_options
But this works:
- set the Parent 'order_by' to 'custom'
- use array_reverse in the foreach options loop
- don't bother with field_order or option_order in the loop
Then the display order is preserved even if you delete the whole field and run the script again.
I've updated the gist:
https://gist.github.com/shanebp/119ffd879acc56c324ab
#17
@
10 years ago
I believe due to BP_XProfile_Field::save() check re supports_options
Can you please clarify what you mean by this? In the case of the Country field, the field type *does* support options. And in the case of the individual country 'option' fields, the logic in the supports_options
block is not relevant anyway. Or maybe I'm missing something? Can you please restate the exact steps that need to be taken to reproduce the bug that you're seeing, along with a restatement of what the bug is? (what you think you should be seeing, and exactly what you are seeing, and where - eg, URLs)
#18
@
10 years ago
Can you please clarify what you mean by this?
Sorry, but I'm just guessing as to why the option_order field is saved as 0 for every row when inserting options in a loop.
$i = 0; foreach ( $countries as $country ) { xprofile_insert_field( array( 'field_group_id' => 1, 'parent_id' => $country_list_id, 'type' => 'option', 'name' => $country, 'option_order' => $i )); $i++; }
Even if I hard-code 'option_order' to 123, it still gets inserted as 0.
#20
@
10 years ago
Sorry, but I'm just guessing as to why the option_order field is saved as 0 for every row when inserting options in a loop.
This is where I'm getting lost. When I put your code into bp-custom.php, and then visited ?page=bp-profile-setup, all options were created with the correct option order. Maybe I'm not attempting to reproduce the problem in the same way that you're doing it? Exact steps to reproduce would be very, very helpful.
btw - I'm not using trunk.
Just the 5472.01.patch on BP: 2.0.1
Can you try with trunk? We are trying to determine if there is a problem to fix for BP 2.1.
If it does turn out that there are discrepancies between what you're seeing and what I'm seeing when following the same steps, it's likely that there's something (a plugin of some sort?) on your installation that's causing issues. Again, exact steps to reproduce (including environment information) is of the essence here.
#21
@
10 years ago
Versions:
- WP: 3.9.1
- theme 2013
- BP: trunk
- PHP 5.4.28
- MYSQL: 5.5.37
No other plugins.
This function is the only thing in bp-custom.php:
function bp_add_custom_country_list() { if ( !xprofile_get_field_id_from_name('Country') && 'bp-profile-setup' == $_GET['page'] ) { $country_list_args = array( 'field_group_id' => 1, 'name' => 'Country', 'description' => 'Please select your country', 'can_delete' => false, 'field_order' => 2, 'is_required' => false, 'type' => 'selectbox', 'order_by' => 'default' ); $country_list_id = xprofile_insert_field( $country_list_args ); if ( $country_list_id ) { $countries = array( "United States", "Afghanistan", "Albania", "Algeria", "Andorra", "Angola" ); $i = 0; foreach ( $countries as $country ) { xprofile_insert_field( array( 'field_group_id' => 1, 'parent_id' => $country_list_id, 'type' => 'option', 'name' => $country, 'option_order' => $i )); $i++; } } } } add_action('bp_init', 'bp_add_custom_country_list');
- I navigate to wp-admin/users.php?page=bp-profile-setup
- the script runs
- for every row 'option_order' is zero ( the default value for that field)
- if I hard-code 'option_order' to 123, it still gets saved as zero
- for what it's worth: if I add 'field_order' => $i, the correct values are stored in the field_order column
#22
@
10 years ago
Here is a sql dump of the parent row and the option rows:
http://philopress.com/fuw_bp_xprofile_fields.sql
Hi shanebp
I know DJPaul is working on profile API, you should check #5220 and test the last patch he posted to see if the improvements introduced is matching your need.