Provide new XProfile field object with WPDB's insert_id

[Offereins]

For my plugin for #5192 I stumbled upon this missing logic during the creation of a new XProfile field. While this is ​done correctly for XProfile field groups, using wpdb->insert_id, the logic in BP_XProfile_Field::save() does not do so. It does not update the field object with the newly created field's id. This is unfortunate when you'd like to use the field's object in later hooks.

I suggest this easy fix by replacing:

if ( ! empty( $this->id ) ) {
  $field_id = $this->id;
} else {
  $field_id = $wpdb->insert_id;
}

// Other code. No other use of $field_id here.

return $field_id;

with

if ( empty( $this->id ) ) {
  $this->id = $wpdb->insert_id;
}

// Other code. No other use of $field_id here.

return $this->id;

in ​class-bp-xprofile-field.php.

PS. Excuse me for not providing a correct patch file at the moment.

[boonebgorges]

Thanks for the patch. Unfortunately, this change by itself won't work, as there are a number of other empty( $this->id ) checks in the method.

[boonebgorges]

In 9983:

Set the id property of a new BP_XProfile_Field object during the save() method.

Props Offereins.
Fixes #6545.

[Offereins]

Replying to boonebgorges:

Thanks for the patch. Unfortunately, this change by itself won't work, as there are a number of other empty( $this->id ) checks in the method.

While this is true, the second empty-check results in the same: it uses either $this->id or $wpdb->insert_id. This check would become redundant since it just means $parent_id = $this->id. Am I right? Less is more :)

[boonebgorges]

Oh right, in that case, I suppose it would. That said, it obfuscates the code a bit to be doing those checks when unneeded. (Setting $this->id also would've result in the child node tree being deleted, though it would've had no real effect.) In any case, fixed now :)

[Offereins]

Sorry to say, but the fix doesn't solve my issue. The 'xprofile_field_after_save' hook has already fired with the field object before your new code steps in. Setting $this->id should at least happen some 10 lines earlier: ​https://buddypress.trac.wordpress.org/browser/trunk/src/bp-xprofile/classes/class-bp-xprofile-field.php?rev=9983#L371

[boonebgorges]

Thanks, Offereins. Can you please look at 6545.diff​ to see if it will accomplish what you want?

[Offereins]

Looks good to me.

[boonebgorges]

In 9986:

Better setting of id property in BP_XProfile_Field::save().

This ensures that the property is set before the after_save hook is fired.

Props Offereins.
Fixes #6545.