Problem/Motivation

In \Drupal\views\Plugin\views\query\Sql both ::addField, ::addOrderBy has $table typed as string. But the documentation specifies that you're allowed to pass NULL as well.

   * @param string $table
   *   The table this field is part of. If a formula, enter NULL.
   *   If you want to orderby random use "rand" as table and nothing else.

Steps to reproduce

N/A

Proposed resolution

Change the type to string|null

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3554551

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

marcusml created an issue. See original summary.

marcusml opened merge request !13588

marcusml’s picture

marcusml commented
Status: Active » Needs review
marcusml’s picture

marcusml commented
Assigned: marcusml » Unassigned
smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

Think if we are also making this change should update the variables too.

marcusml’s picture

marcusml commented
Status: Needs work » Needs review

@smustgrave If I understood you correctly I've done what you asked. 😄

Thanks for the quick response!

marcusml’s picture

marcusml commented
Status: Needs review » Needs work

Changing status due to the test failures.

smustgrave’s picture

smustgrave commented

Sorry lets revert those changes as that will be a bigger lift for BC concern. Sorry for the noise can mark once reverted.

marcusml’s picture

marcusml commented
Status: Needs work » Needs review

No worries! Reverted and pushed.

smustgrave’s picture

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

Thanks for baring with me. Adding the typehints would be a whole dance.

catch closed merge request !13588

  • catch committed 49cf3f23 on 11.3.x 
    task: #3554551 Missing nullable type in \Drupal\views\Plugin\views\query...

  • catch committed d5fc0345 on 11.x 
    task: #3554551 Missing nullable type in \Drupal\views\Plugin\views\query...
catch’s picture

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

Committed/pushed to 11.x and cherry-picked to 11.3.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.

Status: Fixed » Closed (fixed)

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