Problem/Motivation

In \Drupal\jsonapi\Query\Sort, the property $fields is defined as a string, when it is an array. This causes IDEs such as VSCode to flag iteration of the array as an error when it is not.

Steps to reproduce

In an IDE, write something like

$fields = [
  ['path' => 'changed', 'direction' => 'DESC'], 
  ['path' => 'title', 'direction' => 'ASC'],
];
$sort = new \Drupal\jsonapi\Query\Sort($fields);
foreach ($sort->fields() as $field) {
  echo $field['path'];
}

The IDE will complain "Expected type 'iterable|object'. Found 'string'."

Proposed resolution

Define the $fields property as @var array instead of string.

Issue fork drupal-3453068

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

BenStallings created an issue. See original summary.

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

kalpanajaiswal opened merge request !8325

kalpanajaiswal’s picture

kalpanajaiswal commented

Created a MR and now $fields type is an array.
MR: https://git.drupalcode.org/project/drupal/-/merge_requests/8325

  /**
   * The fields on which to sort.
   *
   * @var array
   */
  protected $fields;

  /**
   * Constructs a new Sort object.
   *
   * Takes an array of sort fields. Example:
   *   [
   *     [
   *       'path' => 'changed',
   *       'direction' => 'DESC',
   *     ],
   *     [
   *       'path' => 'title',
   *       'direction' => 'ASC',
   *       'langcode' => 'en-US',
   *     ],
   *   ]
   *
   * @param array $fields
   *   The entity query sort fields.
   */
  public function __construct(array $fields) {
    $this->fields = $fields;
  }
kalpanajaiswal’s picture

kalpanajaiswal commented
Status: Active » Needs review
smustgrave’s picture

smustgrave commented
Priority: Normal » Minor
Status: Needs review » Needs work
Issue tags: +Novice

Seems straightforward but MR should be against 11.x

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

prabha1997 opened merge request !8367

kalpanajaiswal’s picture

kalpanajaiswal commented

MR https://git.drupalcode.org/project/drupal/-/merge_requests/8325 has been updated against 11.x

kalpanajaiswal’s picture

kalpanajaiswal commented
Status: Needs work » Needs review

ankitv18 changed the visibility of the branch 3453068-property-type-update to hidden.

ankitv18 changed the visibility of the branch 3453068-sortfields-property-is to hidden.

ankitv18’s picture

ankitv18 commented
Version: 10.4.x-dev » 11.x-dev
Status: Needs review » Reviewed & tested by the community

MR!8367 change looking as expected as per proposed solution.

bbrala’s picture

bbrala
Location Netherlands
commented

Yes, i think this is fine like this since it is only as docblock.

  • longwave committed 35848a5e on 10.3.x 
    Issue #3453068 by kalpanajaiswal, BenStallings: Sort::fields property is...
longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Reviewed & tested by the community » Fixed

Crediting @BenStallings for the original report and @kalpanajaiswal for the fix.

Backported down to 10.3.x as a docs-only fix.

Committed and pushed 9f6143c129 to 11.x and 6f54f45dc2 to 11.0.x and d5deca47b1 to 10.4.x and 35848a5ec5 to 10.3.x. Thanks!

  • longwave committed d5deca47 on 10.4.x 
    Issue #3453068 by kalpanajaiswal, BenStallings: Sort::fields property is...

  • longwave committed 6f54f45d on 11.0.x 
    Issue #3453068 by kalpanajaiswal, BenStallings: Sort::fields property is...

  • longwave committed 9f6143c1 on 11.x 
    Issue #3453068 by kalpanajaiswal, BenStallings: Sort::fields property is...
longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Issue tags: +DevDaysBurgas2024

longwave closed merge request !8367

Status: Fixed » Closed (fixed)

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