getkirby / kirby

Kirby's core application folder
https://getkirby.com
Other
1.32k stars 168 forks source link

Fields in mixed case in virtual pages can't be searched #4142

Closed dgsiegel closed 2 years ago

dgsiegel commented 2 years ago

Description

Suppose you have a virtual pages setup like described on this page with these fields:

[...]
$pages[] = [
                'slug'     => Str::slug($review->display_title),
                'num'      => $key+1,
                'template' => 'review',
                'model'    => 'review',
                'content'  => [
                    'title'    => $review->display_title,
                    'headline' => $review->headline,
                    'linkText' => $review->link->suggested_link_text,
                    'FooBar'   => $review->foobar,
                ]
            ];
[...]

If you now run a search for any field with uppercase letters (linkText or FooBar), e.g.

$site->search("foobar", "FooBar");

or even

$site->search("foobar", [
    [ 'fields' => ['FooBar'], ]
  ]);

it does not deliver any results. The only way to search for these fields is to leave the search fields empty.

The reason being config/components.php:215 which states $fields = array_map('strtolower', $options['fields']);, ie. that converts that field to foobar which then of course does not exist in the $keys array (still containing FooBar). The following array intersection doesn't yield any results of course.

Interestingly this does not happen with regular pages, where you can use any case in your field names.

Expected behavior
Virtual page data fields/keys should be lowercased either in seach component or should be normalized before.

Your setup

Kirby Version
3.6.2

afbora commented 2 years ago

Summary

If the contents have a content file, the text data handler lowercases the field keys while still reading the content (ucfirst while saving). However, this problem occurs because virtual pages do not have a content file.

https://github.com/getkirby/kirby/blob/3.6.2/src/Data/Txt.php#L118

Possible solution

We can always lowercased the field keys in the search component:

-   $keys = array_keys($data);
+   $keys = array_map('strtolower', array_keys($data));

https://github.com/getkirby/kirby/blob/3.6.2/config/components.php#L204

But remember, this is repetition for data with content file. I'm not sure if it will cause a performance issue.

afbora commented 2 years ago

What do you think about the solution? @lukasbestle @distantnative @bastianallgeier

lukasbestle commented 2 years ago

https://github.com/getkirby/kirby/pull/2945 (which we plan for 3.7) would convert all fields to lowercase field names in the Content constructor. I'm pretty sure this would also cover virtual pages.

bastianallgeier commented 2 years ago