Contrib.social

Invalid context for call to FieldDefinitionInterface->isDisplayConfigurable()

Open on Drupal.org →
Created on 28 June 2023, over 1 year ago
Updated 23 October 2023, about 1 year ago

Problem/Motivation

I'm using this module on a site where I am also using the Computed Field module. This caused a whitescreen when displaying my first computed field.

Steps to reproduce

Enable Field Permissions module. Create a Computed Field (there's a reverse entity reference plugin built-in -- if you create an entity reference to your bundle from somewhere, this will work)

Add your computed field to a display mode rendering the target entity.

Create content appropriate to make something display.

View the entity with the Computed field.

Proposed resolution

The problem is triggered by Computed Field's strict implementation of `FieldDefinitionInterface->isDisplayConfigurable()` in `src/Entity/ComputedField.php`. It uses a `match()` to decide on a response. The documentation for FieldDefinitionInterface states that only "view" or "form" are valid values for the $display_context variable, and the match() statement only accounts for these.

  /**
   * Returns whether the display for the field can be configured.
   *
   * @param string $display_context
   *   The display context. Either 'view' or 'form'.
   *
   * @return bool
   *   TRUE if the display for this field is configurable in the given context.
   *   If TRUE, the display options returned by getDisplayOptions() may be
   *   overridden via the respective entity display.
   *
   * @see \Drupal\Core\Entity\Display\EntityDisplayInterface
   */
  public function isDisplayConfigurable($display_context);

However, field_permissions alters the context argument before invoking `->isDisplayConfigurable($context)` by specifically replacing values of "view" with "display". That violates the specification for isDisplayConfigurable().

You can see the strange code here. The git history does not make it clear to me why this is being done.

I assume this works in most cases because other implementations of isDisplayConfigurable() are less picky, but I suspect this may also be producing a lot of php warnings.

Remaining tasks

I'll submit a patch as I believe it's a pretty simple change, but confirmation on this change from module experts would be helpful since it's clearly a very deliberate piece of code. I just can't figure out .

Related issue on Computed Field is #3369317 🐛 Incompatible with Field Permissions module Closed: works as designed

🐛 Bug report
Status

Fixed

Version

1.0

Component

Code

Created by

🇺🇸United States gcb

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

Comments & Activities

  • Issue created by @gcb
  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 8.0 & MySQL 5.7
    last update over 1 year ago
    48 pass
  • @gcb opened merge request.
  • Status changed to Needs review over 1 year ago
  • Comment over 1 year ago →
    🇺🇸United States gcb
  • Comment over 1 year ago →
    🇺🇸United States jhedstrom Portland, OR

    Wow, that's bizarre. I wonder how this was ever working since BaseFieldDefinition is just checking the definition array (which is presumably also just using view and edit):

      public function isDisplayConfigurable($display_context) {
    return $this->definition['display'][$display_context]['configurable'] ?? FALSE;
  }
  • Status changed to Needs work over 1 year ago
  • Comment over 1 year ago →
    🇺🇸United States mariacha1

    @jhedstrom I'm glad you're as confused as I am!

    Tests don't fail, and it does look like that's why this was added in the first place.

    My only note is that the "hook_jsonapi_entity_field_filter_access" hook below also checks against "display" and that should probably be "view" as well.

    Otherwise this feels pretty good to me.

  • Comment over 1 year ago →
    🇺🇸United States jhedstrom Portland, OR

    That code is from a very early port of the module to 8.x (commit 68bbb58).

    There's one more use where the module hard codes display to that method call in field_permissions_jsonapi_entity_field_filter_access that should be updated to view I guess.

    (cross post!)

  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 8.0 & MySQL 5.7
    last update over 1 year ago
    48 pass
  • Status changed to Needs review over 1 year ago
  • Comment over 1 year ago →
    🇺🇸United States gcb

    Wow, that's an unheard-of level of responsiveness in the contrib queue in my experience. Two maintainers so quick they overlap!

    Alright, I've added a fix for that line to my MR, let me know if there's more needed!

  • Comment over 1 year ago →
    System Message
  • Status changed to Fixed over 1 year ago
  • Comment over 1 year ago →
    🇺🇸United States mariacha1

    Cool, merging!

  • Comment over 1 year ago →
    System Message

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

  • Status changed to Fixed over 1 year ago
  • Comment over 1 year ago →
    🇬🇧United Kingdom joachim

    The fix is wrong:

      $context = ($operation == 'view') ? 'view' : 'edit';

    The docs in core say:

    > * The display context. Either 'view' or 'form'.

    'edit' is not a valid value.

  • Comment over 1 year ago →
    🇺🇸United States wxman

    @jhedstrom @mariacha1 This bug is still not working. I created a new issue at #3394215 to show it's still crashing Computed Fields.

  • @gcb opened merge request.
  • Comment about 1 year ago →
    🇺🇸United States gcb

    Hrm, please ignore the extra branches an noise I made here. The fix is already in for the form/edit mismatch on the 8.x-1.x branch https://git.drupalcode.org/project/field_permissions/-/commit/003b4f069a...

  • Comment about 1 year ago →
    System Message
  • Comment about 1 year ago →
    🇺🇸United States wxman

    @gcb I am running the 8.x-1.2 version now and computed field still is not working, even my cron fails showing an error caused by Field Permissions
    TypeError: Drupal\field_permissions\FieldPermissionsService::fieldGetPermissionType(): Argument #1 ($field) must be of type Drupal\field\FieldStorageConfigInterface, Drupal\computed_field\Field\FieldStorageDefinition given, called in /var/www/website/web/modules/contrib/field_permissions/src/FieldPermissionsService.php on line 163 in Drupal\field_permissions\FieldPermissionsService->fieldGetPermissionType() (line 138 of modules/contrib/field_permissions/src/FieldPermissionsService.php).

    I'm going to do a backup and install the DEV an see if that works. Also the link above: https://git.drupalcode.org/project/field_permissions/-/commit/003b4f069a... says the fix was NOT added to the branch, unless I'm reading it wrong.

  • Comment about 1 year ago →
    🇺🇸United States gcb

    Yes, as I said, it's on the dev branch. There hasn't been a release since it was merged, so you have to use the dev branch or treat the commit as a patch if you want it.

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024