I have a field of the type List (text) that is in a weird state after a feature revert of its components. To reproduce:

  1. Add a field of the type List (text) to a content type
  2. Add these allowed values:
    0|Test 1
    test2|Test 2
  3. Set the default value to be Test 1
  4. Export the content type into a feature
  5. Change any setting to field instance
  6. Revert the feature
  7. Go to the node add form of the content type
  8. The default value is not correct anymore. All options of the field have the attribute selected="selected"

Having 0 as a allowed value might be something stupid to do, but since core allows me to do that, features should handle that properly, I guess...

Just re-saving the field fixes the issue. But since I have almost daily deployments, this is really annoying...

Comments

hefox’s picture

Could you provide what the exported field instance looks like?

luksak’s picture

Status: Active » Needs review

Here is a feature that is built using the instructions in the summary.

hefox’s picture

Status: Needs review » Active

needs review implies a patch.

also, you didn't add the feature?

luksak’s picture

StatusFileSize
new5.96 KB

Forgot the feature...

fmb’s picture

This is what Features outputs:

// Exported field_instance: 'MYINSTANCE'
  $field_instances['MYINSTANCE'] = array(
    'bundle' => 'MYBUNDLE',
    'default_value' => array(
      0 => array(
        'value' => 0,
      ),
    ),
  // ...

It should output 'value' => '0' instead of a numeric value (it then works if I manually edit the feature on the testing instance and revert it). I believe this is controlled by features_var_export(). Since is_numeric('0') returns TRUE, '0' is transformed into 0. As I do not understand the original intention, I cannot provide a patch; should we evaluate is_string before all the scalar types?

PawelR’s picture

I'm having similar issue caused by the same thing.
This is caused by this piece of code in features_var_export() function:

   else if (is_numeric($var)) {
    $floatval = floatval($var);
    if (is_string($var) && ((string) $floatval !== $var)) {
      // Do not convert a string to a number if the string
      // representation of that number is not identical to the
      // original value.
      $output = var_export($var, TRUE);
    }
    else {
      $output = $floatval;
    }

Which converts numeric strings to float for sake of this fixing issue #1721926

But this fix introduced new bug which is IMHO more critical than this weight override ping/pong.
If integer list field has allowed values like these:

0|Zero
1|One

Then options.module:options_field_widget_validate() fails on:

if ($element['#required'] && $element['#value'] == '_none')

Because when $element['#value'] is set to 0 not "0" it gets evaluated to FALSE.
Features don't respect original default value "0" returned from field_info_instance() and convert it to 0.
This get's inserted to the database field_config_instance 'data' column.

So when we add new integer list field with mentioned allowed values then default value written to the database is "0" .
But when we revert the feature then integer 0 is written to the database instead of sting "0" and when we select 'Zero' radio button in the form (which corresponds with value 0) then (given that this field is required) validation fails and despite selecting the radio button corresponding with 0|Zero field value Drupal complains that this field is required as if any radio button hasn't been selected at all.

I think it would be better to remove this code ASAP and find better way how to fix override ping/pong later.

PawelR’s picture

StatusFileSize
new870 bytes

Patch which fixes this issue

PawelR’s picture

Priority: Normal » Major
Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, 7: 2248129_dont_convert_numeric_strings.patch, failed testing.

mpotter’s picture

Priority: Major » Normal
shi99’s picture

Hello,

I am having the same issue. The code mentioned in #6 is what I also notice that is causing the export issue.
I'm not sure if the if condition is correct.

features.export.inc line 603

$floatval = floatval($var);
    if (is_string($var) && ((string) $floatval !== $var)) {
      // Do not convert a string to a number if the string
      // representation of that number is not identical to the
      // original value.
      $output = var_export($var, TRUE);
    }
    else {
      $output = $floatval;
    }

I my case $var is '0' so this condition will never validate for me since $var is a '0' string and the (string) floatval will also always be a '0' string and thus always equal.

Changing the code to the following resolves the issue for me

$floatval = floatval($var);
    if (is_string($var)) {
      // Do not convert a string to a number if the string
      // representation of that number is not identical to the
      // original value.
      $output = var_export($var, TRUE);
    }
    else {
      $output = $floatval;
    }

But I am not sure of the consequences of removing the code && ((string) $floatval !== $var)

Any input would be appreciated.

Thanks

mpotter’s picture

So, the real problem here is that configuration in D7 is not "typed". Features has no idea if a value should be a string or a numeric/float value. In fact, some modules even handle this inconsistently. Removing this code check would cause all numeric values to be exported as strings, which breaks a bunch of other stuff (such as weights).

There is no real solution to this and I'm not going to ping/pong back and forth. Some stuff will break without the numeric checking and some different stuff will break *with* the numeric checking. Until somebody comes up with a foolproof way to handle this I'm not going to change it and cause new problems on people's existing sites.

The real fix is to move to D8 where configuration has proper schema and type and is handled consistently in core. This is likely a "Wont Fix" issue, but leaving it open in case somebody has a brilliant idea that works for all cases.

shi99’s picture

StatusFileSize
new129.81 KB

Thanks for looking into this. I have done some more research and tests and it looks like the field default information provided by drupal is able to tell the difference between strings and integers.

When the features export field is run, the drupal core function field_info_instance() is called to get information about the field (that function is called within the features_field_instance_load() function). In the field information array that is returned, the default value type seems to be correctly identified as a string or integer.

In my test I created a few different types of fields.
I created two boolean fields:

  • Boolean Test: Single on/off checkbox
  • Boolean Test 2: Check boxes/radio buttons

The field "Boolean Test" had it's default value of type integer since it stored a numeric 0 or 1
The field "Boolean Test 2" had it's default value of type string since it stored a string "0", "1", or "NULL" (NULL is used for the N/A option)

I have attached an image showing the field details from field_info_instance() call.

I was thinking it would be best to have the feature field export to be consistent with the Drupal field details. So for the cases if Drupal identifies numbers as strings would be fine for the export since that is how Drupal sees them and would be consistent between both Drupal and Features.

Thanks for your time looking into this. I really appreciate it.

mpotter’s picture

But the problem remains. The features_export function that is responsible for this doesn't have any idea what it is exporting. It just gets passed a $var to export. It's great that fields have this type information, but general configuration in D7 doesn't have this (thus, why config schema was added to D8).

If somebody wants to submit a patch within the includes/features.field.inc to fix the export for fields based on the field type data, that would be something I could review and potentially commit. But all of the above suggestions for changing the generic features_var_export() cannot be committed because that function doesn't have any type information to work with (and can't since it's called for generic $var export)

shi99’s picture

Understood. Thanks for taking the time.
If I come up with anything I will update here.

alex.bukach’s picture

For those who faces this issue as well, I'm using the following workaround: hook_post_features_revert() and hook_post_features_rebuild() that restore string default value like this

$instance = field_info_instance($my_entity_type, $my_field_name, $my_bundle);
if ($instance['default_value'][0]['value'] === 0) {
  $instance['default_value'][0]['value'] = "0";
  field_update_instance($instance);
}