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?
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_.
/**
* 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>').
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.yamltest_color:
label: Color
span: left
tab: General
required: 1
type: text
default: '#F2F2F2'
/**
* @var array
*/
public $implement = ['System.Behaviors.SettingsModel'];
/**
* @var string
*/
public $settingsFields = 'fields.yaml';
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.
Most helpful comment
I think I understand. The
defaultvalue 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::getcall), so this is expected.