October: Getting the default value for a settings model input field, when field is empty

Created on 5 Jun 2016  路  9Comments  路  Source: octobercms/october

When getting a value from a SettingsModel via get($key, $default) shouldn't this return the default value specified either in the fields.yaml or in the get() call instead of empty - if the field is empty?

I am especially referencing to this method

/**
 * Get a single setting value, or return a default value
 */
public function getSettingsValue($key, $default = null)
{
    if (array_key_exists($key, $this->fieldValues)) {
        return $this->fieldValues[$key];
    }

    return $default;
}

As I interpreted the default values: if the fieldValue[$key] is empty or null, return the default value - but that is obviously not what the method does.

Do you get my point or am I just thinking wrongly?

Question

Most helpful comment

I think I understand. The default value controls the form field, when the form renders as a user interface (when the <input /> is displayed). It does not control the PHP API (via the ::get call), so this is expected.

All 9 comments

array_key_existsmeans if the key exists in the array. It does not check the value which can be null, empty, 0...

@gabsource Yes, I know, but I was wondering if I have a _default_ value set, why it is not returned? Wouldn't it make sense to return the _default_ value if the actual value is empty, null, ...

@codedge Ok, sorry I misread your question. That's indeed an interesting question.

There are edges cases maybe when you are dealing with some variable types about what is considered _no value_.

  • string: null and empty string (trimmed or not ?)
  • other types: null only ?
/**
 * Get a single setting value, or return a default value
 */
public function getSettingsValue($key, $default = null)
{
    if (array_key_exists($key, $this->fieldValues) && $this->fieldValues[$key] !== null) {
        // @todo what about empty strings ?
        return $this->fieldValues[$key];
    }

    return $default;
}

@gabsource Right, that is my point. I just created a couple of settings fields in the backend settings section, defined some default: abc values in the fields.yaml and was wondering why an empty string was returned - even if the type of the field is number.

So, let's wrap it up. I suggest, if the field is empty (f. ex. when initially created), just return the _default_ value from the fields.yaml or the get('fieldname', '<defaultval>').

  • strings: empty string
  • number: empty string

As the fields.yaml can use any form field, it may not be limited to string and numeric there can be maybe bool, array, class... I think that every possible situations must be considered carefully before implementing something if the October team wants this to be changed.

Completely right. So what would be a next step? Asking @daftspunk and/or anybody else what he thinks about this?

Settings models have their default values applied using the initSettingsData method, for example:

public function initSettingsData()
{
    $this->require_activation = true;
    $this->activate_mode = self::ACTIVATE_AUTO;
    $this->use_throttle = true;
    $this->welcome_template = 'rainlab.user::mail.welcome';
    $this->login_attribute = self::LOGIN_EMAIL;
}

This will set the defaults on the model and supports dynamic values. This approach is used because not all values require a form field, they might be accessed purely via the code.

However there can be a problem when a new attribute / field is introduced. For example, if we wanted to add allow_registration to the settings model and set it to true by default. It would not be possible to set it on the existing data using initSettingsData because it is only called at the time of birth.

public function initSettingsData()
{
    // [...]
    // It is too late for me now
    $this->allow_registration = true;
}

The workaround for it at the moment is to use the afterFetch method, something like this:

function afterFetch()
{
    if ($this->allow_registration === null) {
        $this->allow_registration = true;
    }
}

Or alternatively

SettingsModel::get('allow_registration', true);

This is sort of a known issue at the moment, is this the problem you are experiencing that prompted this issue? The ThemeData model works around this by using the form field default values and merging them in when the value is missing from the dataset, although this is a completely different usage case.

I think that is not the problem that prompted this issue.

To reproduce:

  • fields.yaml
test_color:
        label: Color
        span: left
        tab: General
        required: 1
        type: text
        default: '#F2F2F2'
  • Settings model of a package
    /**
     * @var array
     */
    public $implement = ['System.Behaviors.SettingsModel'];

    /**
     * @var string
     */
    public $settingsFields = 'fields.yaml';
  • Fetch the value in a component

In the init() method of a component I fetch the value but it returns null instead of the default from the fields.yaml which should be #F2F2F2

$this->color = Settings::get('test_color'); // returns null if the field is empty

For me it would be logical to get the default value (#F2F2F2) back that has been specified in the fields.yaml instead of null, if the fields is empty.

Do you get my point?

I think I understand. The default value controls the form field, when the form renders as a user interface (when the <input /> is displayed). It does not control the PHP API (via the ::get call), so this is expected.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

EbashuOnHolidays picture EbashuOnHolidays  路  3Comments

ChVuagniaux picture ChVuagniaux  路  3Comments

axomat picture axomat  路  3Comments

gergo85 picture gergo85  路  3Comments

lukaszbanas-extremecoding picture lukaszbanas-extremecoding  路  3Comments