Problem/Motivation

Running \Drupal\Tests\language\Kernel\Views\FilterLanguageTest() on PHP 8.0 fails because it does not return the ***LANGUAGE_language_content*** language.

Steps to reproduce

See https://www.drupal.org/pift-ci-job/1855374

OR change \Drupal\views\Plugin\views\PluginBase::listLanguages() from

if (isset($type['name']) && !isset($list[$id]) && in_array($id, $current_values)) {

to

if (isset($type['name']) && !isset($list[$id]) && in_array($id, $current_values, TRUE)) {

Proposed resolution

Comparison of strings with numbers is different https://wiki.php.net/rfc/string_to_number_comparison

Ensure we make the comparison we expect to in \Drupal\views\Plugin\views\PluginBase::listLanguages()

@todo Discuss the impacts of making this change.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#11 3177546-2-11.patch1.75 KBalexpott
#11 3177546-2-11-will-fail.patch630 bytesalexpott
#9 3177546-9.patch2.4 KBalexpott
#8 3177546-8-will-fail.patch958 bytesalexpott
#2 3177546-2.patch1.46 KBalexpott
#2 3177546-2-will-fail.patch908 bytesalexpott

Comments

alexpott created an issue. See original summary.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Active » Needs review
StatusFileSize
new 3177546-2-will-fail.patch908 bytes
new 3177546-2.patch1.46 KB
andypost’s picture

andypost
he/him
Primary language Russian
commented

As I see only \Drupal\views\Plugin\views\filter\LanguageFilter::getValueOptions() using to pass 2nd argument to this method, probably it's set of configured values in filter, and its tests pass both

The last submitted patch, 2: 3177546-2-will-fail.patch, failed testing. View results

andypost’s picture

andypost
he/him
Primary language Russian
commented 
+++ b/core/modules/views/src/Plugin/views/PluginBase.php
@@ -603,7 +603,7 @@ protected function listLanguages($flags = LanguageInterface::STATE_ALL, array $c
-          if (isset($type['name']) && !isset($list[$id]) && in_array($id, $current_values)) {
+          if (isset($type['name']) && !isset($list[$id]) && in_array($id, $current_values, TRUE)) {

quick debug shows that $current_values are 

array (
  0 => 0,
)

atm ***LANGUAGE_language_content*** is ID when in_array($id, $current_values) !== in_array($id, $current_values, TRUE)

andypost’s picture

andypost
he/him
Primary language Russian
commented
Issue summary: View changes

Added to summary https://wiki.php.net/rfc/string_to_number_comparison

Because in_array() looking for string in number indexed array

https://3v4l.org/D3n5g

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs review » Reviewed & tested by the community

As https://3v4l.org/D3n5g shows that old compare was wrong

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs review
StatusFileSize
new 3177546-8-will-fail.patch958 bytes

@andypost asked a good question in slack - why is $current_values in \Drupal\views\Plugin\views\PluginBase::listLanguages() equal to [0 => 0].

Well we get the same error if we make the following change. And then $current_values is equal to [0 => 'en'] and then [0 => 'xx-lolspeak'] in the test which makes way more sense.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
StatusFileSize
new 3177546-9.patch2.4 KB

Here's #8 combined with #2 which I think is the correct fix.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

Or in the test should we change from

      $view->displayHandlers->get('default')->overrideOption('filters', [
        'langcode' => [
          'id' => 'langcode',
          'table' => 'views_test_data',
          'field' => 'langcode',
          'value' => [$langcode],
        ],
      ]);

to

      $view->displayHandlers->get('default')->overrideOption('filters', [
        'langcode' => [
          'id' => 'langcode',
          'table' => 'views_test_data',
          'field' => 'langcode',
          'value' => [$langcode => $langcode],
        ],
      ]);

... I dunno.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
StatusFileSize
new 3177546-2-11-will-fail.patch630 bytes
new 3177546-2-11.patch1.75 KB

The more I look at it the more I'm convinced we're setting the value wrong in this test. Let's do that instead of #9.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

Yeah if I create a filter on a language field the config looks like this:

        langcode:
          id: langcode
          table: node_field_data
          field: langcode
          relationship: none
          group_type: group
          admin_label: ''
          operator: in
          value:
            '***LANGUAGE_site_default***': '***LANGUAGE_site_default***'
            '***LANGUAGE_language_interface***': '***LANGUAGE_language_interface***'
            en: en

So I think this test has been setting the value wrong all this time.

andypost’s picture

andypost
he/him
Primary language Russian
commented
Related issues: +#2420737: Differences in dynamic language names are confusing in views, content, etc.

Git history shows that in related array_keys() been added - #2420737: Differences in dynamic language names are confusing in views, content, etc.

andypost’s picture

andypost
he/him
Primary language Russian
commented

Looks usage in \Drupal\views\Plugin\views\display\DisplayPluginBase::buildRenderingLanguageOptions() is not affected as values are passed properly

The last submitted patch, 8: 3177546-8-will-fail.patch, failed testing. View results

The last submitted patch, 11: 3177546-2-11-will-fail.patch, failed testing. View results

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs review » Reviewed & tested by the community

Back to rtbc!

  • catch committed f113289 on 9.2.x 
    Issue #3177546 by alexpott: \Drupal\views\Plugin\views\PluginBase::...

  • catch committed ac75616 on 9.1.x 
    Issue #3177546 by alexpott: \Drupal\views\Plugin\views\PluginBase::...
catch’s picture

catch
he/him
Primary language English
commented
Version: 9.2.x-dev » 9.1.x-dev
Status: Reviewed & tested by the community » Fixed

Committed f113289 and pushed to 9.2.x. Thanks! Cherry-picked to 9.1.x too.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.