Problem/Motivation

HandlerBase::breakString doesn't handle tokens, leading to type mismatches in calling code. For example when sending a token to the numeric argument: 

Warning: Undefined variable $operator in /var/www/html/web/core/modules/views/src/Plugin/views/HandlerBase.php on line 867
The website encountered an unexpected error. Try again later.

TypeError: Cannot assign null to property Drupal\views\Plugin\views\argument\ArgumentPluginBase::$operator of type string in Drupal\views\Plugin\views\argument\NumericArgument->title() (line 75 of core/modules/views/src/Plugin/views/argument/NumericArgument.php).

Steps to reproduce

  • Create a view with an argument and set it to allow multiple values
  • In the preview set a token style arguments input, i.e. [token:for:something]
  • Check the logs

Proposed resolution

Default the operator to a string so at least the type mismatches won't occur. We could consider setting it to a different default, but starting with a empty string feels safe.

Remaining tasks

Review
Discuss

User interface changes

None

Introduced terminology

None

API changes

None

Data model changes

None

Release notes snippet

Issue fork drupal-3546894

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

lendude created an issue. See original summary.

lendude’s picture

lendude
Primary language Dutch
Location Amsterdam
commented
Issue summary: View changes

lendude opened merge request !13247

lendude’s picture

lendude
Primary language Dutch
Location Amsterdam
commented
Status: Active » Needs review

This should fix the mismatches at least. Setting the default to either 'or' or 'and' feels like we would be assuming a little more than we are doing now, so an empty string feels safe.

perryjanssen’s picture

perryjanssen
he/him
Primary language English
commented

I was able to reproduce this bug and applying the patch fixes it!

lendude’s picture

lendude
Primary language Dutch
Location Amsterdam
commented
Assigned: lendude » Unassigned

smustgrave made their first commit to this issue’s fork.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs Review Queue Initiative

Rebased because something weird was going on with cspell. Seems to have been resolved

Ran the test-only

1) Drupal\Tests\views\Unit\Plugin\argument\NumericArgumentTitleTest::testTitleWithBreakPhraseAndTokenValue
TypeError: Cannot assign null to property Drupal\views\Plugin\views\argument\ArgumentPluginBase::$operator of type string
/builds/issue/drupal-3546894/core/modules/views/src/Plugin/views/argument/NumericArgument.php:75
/builds/issue/drupal-3546894/core/modules/views/tests/src/Unit/Plugin/argument/NumericArgumentTitleTest.php:50
ERRORS!
Tests: 3, Assertions: 2, Errors: 1.
Exiting with EXIT_CODE=2

Which shows the test for the scenario described here. Nice to see additional test coverage get added!

Manually testing following the steps I do see the warning and the fix does seem to address it.

LGTM!

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Version: 11.x-dev » 11.2.x-dev
Status: Reviewed & tested by the community » Fixed

Backported down to 11.2.x as an eligible bug fix. This likely affects 10.x as well but I don't think the test is backportable, not sure this is worth the effort of making that change.

Committed and pushed 62a4821009d to 11.x and 9d5cb3f3fbe to 11.3.x and 0f18475e22d to 11.2.x. Thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

longwave closed merge request !13247

  • longwave committed 0f18475e on 11.2.x 
    fix: #3546894 HandlerBase::breakString doesn't handle tokens, leading to...

  • longwave committed 9d5cb3f3 on 11.3.x 
    fix: #3546894 HandlerBase::breakString doesn't handle tokens, leading to...

  • longwave committed 62a48210 on 11.x 
    fix: #3546894 HandlerBase::breakString doesn't handle tokens, leading to...

Status: Fixed » Closed (fixed)

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