Add has() method to ItemInterface

Created on 24 June 2024, 5 months ago
Updated 25 June 2024, 5 months ago

Problem/Motivation

In Feeds "update existing" - how to avoid wiping out all existing data? Active we want to check, in multiple places, if a source item has a value rather than checking if the value is null.

Currently the workaround was to call fromArray() and check if the array key was set.

This code would be cleaner if the item had a has() method that we can call to check if an item has a value for a given field.

Steps to reproduce

n/a

Proposed resolution

Introduce has() method to Drupal\feeds\Feeds\Item\ItemInterface and implement for classes that implement this interface.

Remaining tasks

  1. Agree on approach
  2. Decide what to do with Item classes that have predefined properties, such as "SyndicationItem".
  3. Implement
  4. Add test coverage (unit tests)

User interface changes

n/a

API changes

Change to interface

Data model changes

n/a

Feature request
Status

Active

Version

3.0

Component

Code

Created by

🇳🇿New Zealand ericgsmith

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @ericgsmith
  • 🇳🇱Netherlands megachriz

    I think that:

    • DynamicItem needs to check if the requested field exists on the $data array using array_key_exists();
    • OpmlItem, SitemapItem and SyndicationItem should return TRUE when the requested field is a defined property on the class. For example, SyndicationItem has a property called $author_name, so that would mean that $item->has('author_name'); is always TRUE. I think it would need to be done that way because I don't know how else you could distinguish NULL from "doesn't exist" for these items. I think that class properties that aren't set will just equal to NULL, or isn't this the case?
    • BaseItem could already implement has() that would check if the class property exists. However, DynamicItem should not return TRUE for $item->has('data'); (unless the data array contains an array key called "data").

    This would result into at least the following test cases:

    • SyndicationItem: $item->has('author_name'); should return TRUE.
    • SyndicationItem: $item->has('foo'); should return FALSE.
    • DynamicItem with fields 'foo' and 'bar' set: $item->has('data'); should return FALSE.
    • DynamicItem with fields 'foo' and 'bar' set: $item->has('foo'); should return TRUE.
    • DynamicItem with fields 'foo' and 'bar' set: $item->has('qux'); should return FALSE.
    • DynamicItem with field 'data' set: $item->has('data'); should return TRUE.

    Perhaps for OpmlItem and SitemapItem we need similar tests (almost the same) as for SyndicationItem.

  • 🇳🇱Netherlands megachriz

    Related issue that also affects the implementation of item classes: 🐛 Creation of dynamic property Drupal\feeds\Feeds\Item\SyndicationItem::$parent:fid is deprecated Needs work .

Production build 0.71.5 2024