Problem/Motivation

Although the class defines it as follows, it is not mandated by any of the interfaces it implements:

  /**
   * {@inheritdoc}
   */
  public function isRequired() {
    return FALSE;
  }

Found in #2407801: Views generic field handler does not work with base fields.

Proposed resolution

Remove it once #2407801: Views generic field handler does not work with base fields lands. If there are no further fails, that that should be easy.

Remaining tasks

Do it.

User interface changes

None.

API changes

None. This method is not expected to be exposed by Field storage configuration.

CommentFileSizeAuthor
#4 remove-isrequired-2411323-4.patch673 bytestadityar
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

yched’s picture

yched CreditAttribution: yched commented

That looks like the remnants of some past back and forth dance about definition structures.

#2390495: Support marking field storage definitions as required does intend to introduce support for isRequired() on the storage level at some point. But meanwhile, yeah the current situation is weird.

tadityar’s picture

tadityar CreditAttribution: tadityar commented
Status: Active » Postponed

Postponed until #2407801: Views generic field handler does not work with base fields lands~

tadityar’s picture

tadityar CreditAttribution: tadityar commented
Status: Postponed » Active

That one landed.

tadityar’s picture

tadityar CreditAttribution: tadityar commented
Status: Active » Needs review
FileSize
remove-isrequired-2411323-4.patch673 bytes
yched’s picture

yched CreditAttribution: yched commented
Status: Needs review » Reviewed & tested by the community

Thanks @tadityar.
#2390495: Support marking field storage definitions as required might reevaluate this, but for now let's be consistent - FieldStorage does not carry requiredness.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented

Ha, the reason I opened this followup to #2407801: Views generic field handler does not work with base fields was I was expecting this to blow up somewhere, but looks like the only place that used this was there. Cool. Agreed with RTBC.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Fixed

This issue is a normal bug fix, and doesn't include any disruptive changes, so it is allowed per https://www.drupal.org/core/beta-changes. Committed 82fad36 and pushed to 8.0.x. Thanks!

  • alexpott committed 82fad36 on 8.0.x 
    Issue #2411323 by tadityar: FieldStorageConfig::isRequired should not...

Status: Fixed » Closed (fixed)

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