select field with structure query does not save passed value

[jaro-io]

description

i have a select field which queries a structure field from another page. this works fine. because we don’t have structure object uuids yet, it only saves the regular id (= index) of the structure object in the content txt file. but that’s fine for me.

now i want to manually update the select field value, without using the panel, but through code. but no matter what value i pass, it just doesn’t get saved.

the select field:

select:
    type: select
    options:
        type: query
        query: site.structure.toStructure
        text: '{{ item.name }}'

the update code:

$page->update([ 'select' => 0 ]);

if i set the field type to multiselect, text or anything else, the value gets saved in the content txt file perfectly fine. but as soon as i use select, it stops working.

if i switch the select field to the regular options with hard-coded values and then pass one of them, it does get saved as well. so i bet there is some internal validation which checks if the select option actually exists. but with dynamic queries it doesn’t work.

i don’t exactly know what the $validate attribute is for, but it is set to false by default. i thought having this off, would mean no validation is done on kirby’s side?

expected behavior

the passed value will be saved in the content.txt file. simple as that.

your setup

kirby 3.9

🙏🏻 ✨

[afbora]

@lukasbestle I've track down the issue and this is related about strict comparison. Option text and values return as string and the page tried to update with int.

array(2) {
  [0]=>
  array(5) {
    ["disabled"]=>
    bool(false)
    ["icon"]=>
    NULL
    ["info"]=>
    NULL
    ["text"]=>
    string(1) "1"
    ["value"]=>
    string(1) "0"
  }
  [1]=>
  array(5) {
    ["disabled"]=>
    bool(false)
    ["icon"]=>
    NULL
    ["info"]=>
    NULL
    ["text"]=>
    string(1) "1"
    ["value"]=>
    string(1) "1"
  }
}

@jaro-io I'm not sure that this is bug or not but you can use like => '0' instead => 0

$page->update([ 'select' => '0' ]);

[jaro-io]

thank you so much @afbora, passing a string actually did the trick. works on my side now. but shouldn’t it be possible to pass an int as well here? i mean all the other fields types accept it.. so it could be a little misleading?

[afbora]

As I said, I'm not sure about this is bug or not.

I think the reason of different behaviors are using different native functions. tags and multiselect fields uses array_intersect() function and that function have not strict mode. select field uses in_array with strict mode. So we need to opinions of other team members.

[lukasbestle]

@afbora You mean the strict in_array() in the sanitizeOption method of the options mixin, right? In this case we have the same bug in the radio and toggles fields as they also use the sanitizeOption method.

I'm wondering if there is a use case where a value for one of those fields should ever be something else than a string. At least in core usage, the value is stored in a content file, so any other type than string will be implicitly converted anyway. If this is correct and there would be nothing we would break, we could fix the bug by casting the value to string before the comparison. But in any case I do see it as a bug. A user should not have to deal with type conversions for field values.

@distantnative You have a better overview here, what do you think?

[afbora]

You mean the strict in_array() in the sanitizeOption method of the options mixin, right? In this case we have the same bug in the radio and toggles fields as they also use the sanitizeOption method.

Exactly! We have same bug for fields using options mixin.

[distantnative]

I think @lukasbestle is right. I can't imagine a case where we have a range of options to compare against and it would be not ok to cast the value to string before comparing it to the options.

But, @lukasbestle, do you see any advantage in casting value to a string over just removing the strict flag from in_array? https://github.com/getkirby/kirby/blob/066b0506a81fdd331edc54add601d907a61ebbb4/config/fields/mixins/options.php#LL39C4-L39C69

[lukasbestle]

@distantnative Using in_array() with mixed types feels like a hack to me. It's as if we used == for direct comparisons. PHP's (and JavaScript's) non-strict comparison magic is just too much of a mess IMO.

[jaro-io]

hey @distantnative 🌳

seems like this issue is not resolved yet. i’ve just manually integrated #5761 into my v4.0.0-beta.2 starterkit, but i can still reproduce the issue. when trying to save a numeric value (int 0) to my select field programatically via $page->update(), it is not being saved in the content file. when converting to a string, it does work.

additionally, in v4 it actually got even worse. because now the same thing is happening on the front-end, too. in v3.9.6 i am able to select & save a value in the panel. in v4.0.0-beta.2 this is not working anymore. nothing gets saved.

structure.mp4

the field setup in this example is just like before and very simple to reproduce:

1. create the following two fields
2. add an entry to the structure field
3. reload the page
4. select the newly added structure entry in the select field
5. save
6. reload the page
7. previously selected value is gone / not saved

# structure
structure:
    type: structure
    fields:
        title:
            type: text

# select
select:
    type: select
    options:
        type: query
        query: page.structure.toStructure
        text: '{{ structureItem.title }}'

[distantnative]

Wondering if your problem now is more related to #5702 or if it also occurs if you have integer values/keys not coming form structure fields.

[jaro-io]

@distantnative yup, that seems to be exactly what i am experiencing with v4. let me give it a try!

[distantnative]

@jaro-io can you please try if it is resolved if you replace kirby/src/Cms/Structure.php with Structure.php.zip ?

[jaro-io]

@distantnative yup, it does! 🤗

[distantnative]

🎉 thanks for testing

[jaro-io]

@distantnative did some more tests. and the latest issue #5702 related to structure ids in v4 is definitely fixed with the code you provided.

the original issue (first post in this thread) however is still open, even after integrating #5761. when trying to save a numeric value (int 0) to a select field programatically via $page->update(), it is not being saved in the content file. only when it’s converted to string before.

[lukasbestle]

@jaro-io Have you tested it with the v4/develop branch? This issue was assigned to the milestone 4.0-beta.3, so it's not in beta 2 already.

[afbora]

@jaro-io You can test v4/develop here: kirby.zip

[jaro-io]

@lukasbestle i manually added all the changes from #5761 to my v4.0-beta.2 build. and it did not work then. however, with @afbora’s zip, it’s all working fine now. sorry for the back and forth. seems to be all good now! 🪂✨