October: RFE: Empty option for backend dropdown

[leoquijano]

Hi,

I'm building a backend form, like this:

  interval_type:
      label: Foo
      type: dropdown
      showSearch: false
      span: left

It would be useful to have an option to specify an empty option:

      emptyOption: Select a Foo

This is similar to what Laravel does, and to what the frontend does. Importantly, it should save NULL as the model value for that field, if the field is not specified. This is important for numeric dropdowns (such as an enum), since adding "" => "something" is not the same, and can bring a database error.

Is this feasible?

[mplodowski]

Try to use placeholder property.

[leoquijano]

Thanks, that works. Maybe the documentation could use an example in the dropdown section. Even though the placeholder property is mentioned in the general field options, it is not clear it can be used in the dropdown field, since HTML5 doesn't support placeholders for SELECTs.

[leoquijano]

There's an additional issue with the usage of the placeholder property. Typically, a lot of dropdowns like this have integer keys. For example, consider the following possible values for a "status" dropdown:

[ 
  1 => "Pending",
  2 => "Processed",
  3 => "Completed"
]

But if I pick the placeholder only, the SELECT will return an empty string ''. A database, like PostgreSQL, won't accept that for a numeric value:

"SQLSTATE[22P02]: Invalid text representation: 7 ERROR: invalid input syntax for integer: "" (SQL: insert into "plan_types" ("recurring", "code", "name", "description", "currency_code", "amount", "recurring_amount", "interval_type", "interval_count", "updated_at", "created_at") values (0, test_plan, Test Plan, Single $50 fee, USD, 500, , , 1, 2016-10-28 18:42:40, 2016-10-28 18:42:40) returning "code")" 

So I'm stuck when using the placeholder property. Note that FormController doesn't allow me an easy way to filter values, since the update_onSave and create_onSave methods are a bit convoluted.

It would be nice to make dropdowns default to NULL when the placeholder is selected. Something like this in modules/backend/widgets/Form.php line 976:

/*
 * Number fields should be converted to integers
 */
if ($field->type === 'number') {
    $value = !strlen(trim($value)) ? null : (float) $value;
}
/*
 * Empty dropdown fields should be converted to NULL
 */
elseif ($field->type === 'dropdown') {
    $value = !strlen(trim($value)) ? null : $value;
}

is this possible?

[Patroklo]

Way I see this problem, you are adding a placeholder into a dropdown. Usually the way that works in web pages is that, if you don't select anything in that object, it will throw a validation error, since those are there to make you select one option. Usually they have texts like "Select an option" or something like that.

To sum up: a placeholder it's a text that gives you a hint of the field, NOT a default value, and you should not use it as that.

For example, in the google sign up form, the month field has the "Month" text by default, and you have to manually add it.

BUT if you want to use a default value in a form field, you should use the default parameter:

For instance:

        fieldDerp:
            label: Foo
            type: dropdown
            showSearch: false
            span: left
            default: 0
            options:
              0: Default value!
              2: Cacafuti value!

It will show Default value! the first time you load that form. With this you will be able to use a "placeholderlike" field with an acceptable value for your database table.

You can extend this kind of behavior to relation dropdowns with a different approach. Since you can't add a default parameter into your yaml file because that value will probably be dynamic, you can add into your model class a method that will return that value by default when accessing that attribute:

For instance, we have a chapter attribute that it's a many on one relation:

        chapter:
            tab: nicarane.cms::lang.forms.tabs.strip.secondary
            type: relation
            label: nicarane.cms::lang.models.strip.chapter
            emptyOption: nicarane.cms::lang.models.strip.chapter_placeholder
            nameFrom: title

The best way I could find to add default values here it's overwriting the attribute get method with this:

     public function getChapterIdAttribute()
    {
        if (!array_key_exists('chapter_id', $this->attributes))
        {
            // here the code you need to load a default chapter id
        }

        return $this->attributes['chapter_id'];
    }

Note that I'm using directly the $this->attributes array because any other method, like calling getAttribute() will cause a stack overflow because they will call the method you are overriding.

With this code you'll have a default value in a relation.

tl:dr: don't ever use placeholder as a default value, use default instead, and if you use placeholders in dropdowns, check with validation that it's not selected before saving into the database.

[leoquijano]

There's probably a software design rule against modifying model objects for presentation / form validation reasons.

I mean, for example: if object A has a property "Foo", and that property is optional, then I should just set that property to NULL and that's it.

When I present the option to the user, I shouldn't be modifying object A's class to support something that the form object should support itself: setting a value to NULL. That increases coupling and it's bad design that increases maintenance headaches if I start using it all over the place.

So, no modifying of the get method for that. Especially not modifying it in a way that relies on an internal $this->attributes class to avoid a stack overflow (this only increases the coupling issue I mentioned in the previous paragraph, and it's brittle: what happens when we change the $this->attributes property?).

I agree that the name "placeholder" is usually for hints, not defaults. However, SELECTs don't have placeholders in HTML5. So, this is a Laravel Collective construction:

https://laravelcollective.com/docs/5.2/html#drop-down-lists

I don't mind it being called "placeholder". But if we want to rename it to emptyOption, that is ok for me as well. The bottom line is that there should be a way of defining what text to show when you haven't selected any of the options. And not selecting anything shouldn't require model attribute changes. Instead, it should pick NULL (not 0, since they're not the same, especially for numeric fields).

I fixed this particular case by doing something that is domain specific (this field is required if another field is present, and ignored if it's not; so I override the save method to clean any ignored fields before saving the model). But the general case should allow developers to set NULL fields to their optional values.

[Patroklo]

First of all, forms in october cms don't inherit anythin from LaravelCollective, they are done diferently, in the collective form library they don't use views or anything like that, html is handled inside php classes, while in october you can see in 'backendwidgetsformpartialsthe.htm` file that makes the drowdown widget, they don't have anything to do speaking of code.

But, and that's true, in this case, they have used a similar solution, that it's the most used one when working with the select html object, and its, since it doesn't have a placeholder per se, to add an additional row and make it selected by default if there's no defined value for this form object. example of that here and here

I see something that could be better in this case in october cms and it's that it would be better to make that option disabled so, in case the user doesn't change the select object, no data would be sent via post to the server, and a "X field it's required." would be sent. I think that would be the best approach here because it would simulate a whole lot better the placeholder behavior.

About the modification you are talking about in the models, I can't really understand where it is the bad practice. You have really two options. A placeholder, that kinda works like a placeholder (and it could be better implemented here) and a default value, that works like the example I have sent you before. No changes needed anywhere.

The method I posted you was a special case with an additional implementation in case you are working with relation in many to one or many to many and you want to define a dynamic default value, really nothing to do with your problem here, but wanted to add a more wide explanation in case someone sees this issue in the future. Also, using the attributes array it's really the only way of getting that value once you overwrite it with the getXAttribute, and its heavily used in all october code, so, in my oppinion, not a big deal, since you are only working with that attribute inside it's own getter.

So, right now, if you want to use a placeholder in a select field, as october cms is, you should add a validation that checks if that attribute is sent as empty via post, if so, throw a required field exception. That will simulate exactly the placeholder behavior if I'm not wrong.

If you want to add a default value, use the default option, it works, but not exactly in your case, keep reading.

BUT you can't send null via post or get or ajax. In your example, if you change the select field with, for example, an input text field, the exception would be the same, an incorrect value '' for that database field, because the concept of null doesn't exist in html, and what you are sending really it's an empty value. I don't decide if in this case it would be better or not to catch this data and decide to change it into a null in case the field it's an integer, but right now it's the default behavior and also it makes sense because it's the same error sql throws when trying to add the same data via query.

So, null with a html field... right now it doesn't work for database int values at least in text and select because it doesn't change the empty html value into php / database null value. It should work like this? I don't decide that and won't give my opinion here.

Tldring, I think the select's placeholder could be better implemented, but not in the way you are talking about, also, a couple options you can use. It's even possible to make something more, extending the default dropdown partial and adding disabled selected into the placeholder html option. I think that will give you exactly the behavior you need, because it would add a null value in your int field.

[leoquijano]

1. I actually think catching the data and changing "" to NULL before saving is a good option. But I've noticed that beforeUpdate() in the form object, is called before the model is populated! I mean something like this wouldn't work:

   public function formBeforeUpdate($model) {
    if(empty($model->foo)) {
      $model->foo = NULL;
    }
   }
   

   I can overwrite update_onSave (and I actually did, to envelop everything in a transaction), but I guess that could be a good, related improvement to make there. "Before save" should let me see the populated model before saving. Or maybe we need a new event.

2. Note that "disabled" is not the same as empty, semantically. I wouldn't go that route, and it wouldn't help much, since "disabled" means the user can't pick a choice, and here we're talking about optionally selecting the empty option.
3. AH yes, you can't send NULL via ajax. So we're stuck on how to interpret "". Note that the default behavior, at least when using PGSQL, was to send "" directly, instead of 0, and that caused a syntax error. So we end up either converting to NULL or converting to 0.
4. A larger solution may be to add sanitization rules, which would make more sense anyway. We could trim strings (trim to empty or trim to null), etc. For now, that manual validation is probably the best approach.

Thanks for clarifying the relation part. Yes, that is another thing entirely.

[Patroklo]

I don't know october internally enough to know the possibility of making a '' => null implementation, it could be a source of a lot of problems. Let's wait for @mplodowski to know that, but it would be the only solution in this case.

About emptyOption, it's exactly the same as placeholder in the dropdown field, so there's nothing to do here, probably october will check internally if the value it's empty and ignore it when working with it.

In your case, I think that you should not use 'null' as a valid value in your form, maybe '-1', '0' or something like that, it would be a whole lot simpler that way to handle, also, it doesn't make a lot of sense, as i have said before, handling null data in html, so it would be more natural and will require less changes.

I have checked the dropdown system in my installation and it's using select2, so adding a true placeholder shouldn't be very difficult, but will require lot of changes, because emptyOption depends on it and should be a selectable option in the object. So the changes here to make the placeholder functional should be:

1. Using the select2 placeholder behavior in octobercms, it's a must-to in my opinion (I have changed my october code and works just fine, but still have to check for problems)
2. If step 1 is implemented, change the way emptyOption works because it's now using placeholder.

Finally: the only thing shouldn't be acceptable it's using a placeholder as a value in any field 8D You can right now change the empty value into null in your model or use another one.

edit: looking at the insides of october, a change in select also affecs tags and some other objects, it's tricky to change this without knowing what you are doing, ¡and that's me!

[leoquijano]

I think for now changing empty to null in my form save method (not the model) is the best approach.

Using "-1", "0" or something like that, is a big no for me: for a numeric field 0 is a value, NULL is not. I know, programmers do this all the time... but I like to keep my field values as clean as possible.

Anyway, probably having sanitization in place (as a form step) is the best approach. I think Laravel doesn't have it either (I remember implementing it once), which I think may be an important omission. We need standardized & configurable ways of converting strings to actual values. And model objects should only contain actual values (true instead of "true", NULL instead of "", etc).

Thanks for the discussion.

[Patroklo]

Lol, it's pretty obvious I'm a total noob working with github, but there, changes for the dropdown widget, with luck they will be added.

[leoquijano]

Not sure if this allows a SELECT to default to the empty option (placeholder + allowClear + default ''?), but let's see.

[daftspunk]

I think you may need the Nullable trait, this will convert empty strings to NULL. See the documentation for more detail:

http://octobercms.com/docs/database/traits#nullable

[daftspunk]

Docs have been improved to mention emptyOption and placeholder config.

[leoquijano]

This is great. Thanks!

[webkonstantin]

Is it possible to make placeholder work with relation dropdowns as well?

[daftspunk]

@sw-double, this should work already for singular relations. Ensure you are using the build 380+

[zone42]

I have a problem with default selection for dropdowns. In my case i use dropdown for nullable boolean field.

is_busy:
    label: 'Is Busy'
    type: dropdown
    emptyOption: 'Unknown'
    options:
        0: 'No'
        1: 'Yes'

But, when rendering dropdown, options with 0 value get selected also when "null" has been passed (and that's an unwanted behaviour)

I'm not sure how this could affect all possible cases, but maybe it would be reasonable to modify the file _field_dropdown.htm?

row 25:
<?= $value == $field->value ? 'selected="selected"' : '' ?>
replace with
<?= ($value == $field->value && $field->value !== null) ? 'selected="selected"' : '' ?>

and maybe also add the following to emptyOption (row 18)
<?= ($field->value === null) ? 'selected="selected"' : '' ?>

Of course, i could substitute null value with something else in afterFetch() function and set it back to null in beforeSave(), but that's not an elegant solution.

What would be the best solution, if I nedded to use both the values: null and 0 together?

[daftspunk]

Please report a new issue and this is a complex case. Please submit a Pull Request to the test plugin for investigation that demonstrates the issue. Don't forget to include step by step instructions how how to replicate it using the test plugin.