🇺🇸United States @lwalley

Looks like PanelizerEntityDefault::panelizer_access() calls og_user_access() which will always return TRUE if user has 'administer group' permission.

The result of the og access check is always appended to panelizer access regardless of the outcome of hook_panelizer_access(), and only one of the access array items needs to be TRUE to grant access.

So it seems anyone with 'administer group' permission will always completely bypass panelizer permissions for an entity that is a group or a member of a group. If that is the case, it is maybe a bit unexpected. Perhaps special consideration is needed for users with 'administer group' access, one option could be to ignore admin permission e.g.:

Instead of:

$og_access = og_user_access($this->entity_type, $entity_id, "administer panelizer og_group $op");

Pass in TRUE for $ignore_admin:

$og_access = og_user_access($this->entity_type, $entity_id, "administer panelizer og_group $op", NULL, FALSE, TRUE);

References:

https://git.drupalcode.org/project/panelizer/-/blob/7.x-3.x/plugins/enti...
https://git.drupalcode.org/project/og/-/blob/7.x-2.x/og.module?ref_type=...

lwalley → created an issue.

I updated the pull request to account for the field not being set yet on the entity on node/add. Fixes Warning: Undefined property: stdClass::$field_coordinates in geocoder_field_attach_presave() (line 308 of geocoder.widget.inc)..

I also spent a bit of time looking at the manual override to make sure the logic made sense, it seems to, but it also assumes overrides are actually working for multi-value fields which they don't seem to be. My best guess is that since geocoder module is declaring custom behavior for multiple values that it needs to actually handle cardinality > 1 when rendering the hidden geofield for overrides:

The geocoder_field_widget_info() function declares 'multiple values' => FIELD_BEHAVIOR_CUSTOM. However,
geocoder_field_widget_form() directly calls geofield_field_widget_form() but doesn’t handle deltas. Compare this to e.g., media_field_widget_form() where it checks cardinality and handles adding field elements for each delta.

Supporting multi-value overrides is probably out of scope for this issue of keeping geocoded values in sync with address fields. So I haven't made any attempts to fix it. Separate issue should probably be opened if someone confirms it's actually broken, and not just something I couldn't get to work with my configuration.

I have only tested this patch with geocoding from multi-value address fields (both geofield and address field set to cardinality 10). More testing is needed for other scenarios, as indicated in the previous issue that modified this logic: #3002517: Geofield remains empty when I choose "Geocode from another field" → .

There are other issues open that attempt to adjust this function and might also be accidentally solved or further broken with my pull-request, e.g.: 🐛 Empty geocoding results do not respect the override checkbox state. Needs review .

Alternative patch added in [3074598].

The pull request is a rewrite of the logic in geocoder_field_attach_presave() on 7.x-1 that decides which field delta items to update.

I have only tested it when geocoding from a multi-value addressfield without any overrides. It needs testing with other types of fields, as indicated in the previous issue that modified this logic: #3002517: Geofield remains empty when I choose "Geocode from another field" → .

I based the rewrite on the existing logic and some assumptions:

• $geocoded_value = geocoder_widget_get_field_value($entity_type, $instance, $entity); is always the source of truth for field items regardless of the source field.
• If $entity->geocoder_overridden[$field_name][$langcode][$delta] is not empty, then we should ignore the newly geocoded value and instead use the existing field item as is if it exists, if it doesn't then use the geocoded value (made should throw an error as that sounds like a logic issue).

lwalley → made their first commit to this issue’s fork.

This still appears to be an issue in D7, see comment #983072-2: views_content_plugin_display_ctools_context->render() doesn't include all files and results in blank panes → .

This still appears to be an issue in D7. It is only noticeable if there is a views-view.tpl.php template in the theme, and if the view in use does not have any exposed filters or similar that independently loads "views/theme/theme.inc".

The issue, as described in the original post, is that "views_content_plugin_display_ctools_context::render()" only includes the template path and file, it does not include the other includes. When overriding the template in the theme, the template path and file are to the theme template, not to "views/theme/theme.inc" so "template_preprocess_views_view()" is not available and never called:

      // We want to process the view like we're theming it, but not actually
      // use the template part. Therefore we run through all the preprocess
      // functions which will populate the variables array.
      $hooks = theme_get_registry();
      $info = $hooks[$this->definition['theme']];
      if (!empty($info['file'])) {
        @include_once './' . $info['path'] . '/' . $info['file'];
      }
      $this->variables = array('view' => &$this->view);

      if (isset($info['preprocess functions']) && is_array($info['preprocess functions'])) {
        foreach ($info['preprocess functions'] as $preprocess_function) {
          if (function_exists($preprocess_function)) {
            $preprocess_function($this->variables, $this->definition['theme']);
          }
        }
      }
    }

In D7, also including everything in "$info['includes']", solves the issue e.g. something like:

+++ b/views_content/plugins/views/views_content_plugin_display_ctools_context.inc
@@ -80,6 +80,11 @@ class views_content_plugin_display_ctools_context extends views_plugin_display {
       if (!empty($info['file'])) {
         @include_once './' . $info['path'] . '/' . $info['file'];
       }
+      if (!empty($info['includes'])) {
+        foreach ($info['includes'] as $include) {
+            @include_once './' . $include;
+        }
+      }
       $this->variables = array('view' => &$this->view);

       if (isset($info['preprocess functions']) && is_array($info['preprocess functions'])) {

Since this issue is closed, I won't add a patch, but just wanted to make a note in case anyone else is running into this.

See #3401366-5: ArgumentCountError: Too few arguments to function esi_panels__esi_pane_prepare() if request made to esi/%esi_component without additional arguments → for patch that alters menu callback `esi_handle_component()` to return 404 if render callback returns FALSE.

Updated merge request to account for cases when arguments are invalid and we are unable to find a pane. Combine with 🐛 ArgumentCountError: Too few arguments to function esi_panels__esi_pane_prepare() if request made to esi/%esi_component without additional arguments Needs review patch to return 404 in menu callback when render returns FALSE.

Added commit to allow render callbacks to return FALSE to trigger a 404 in the menu callback. Might help with #2609714: esi_block__esi_block_prepare should return esi_fast_404 not FALSE when no esi → .

For usage example for panels pane see #2299903: Doesn't work with exported panels? → .

Patch assumes all components require additional arguments. In our case we are only using panels_pane component which requires additional arguments. If there is a component type that does not require additional arguments then the patch would need to be adjusted to either check the component type or ensure callbacks such as esi_panels__esi_pane_prepare() can handle no arguments.

lwalley → created an issue.

lwalley → created an issue.

lwalley → created an issue.

I can confirm with patch from #57 I am able to export and take ownership of "file_type" and "file_display" components, with default hooks in place i.e. without needing file_entity patch in #2192391-83: Default file entities are not exportable by features (Sibling Issue) → or media patch #2104193-113: Default file entities are not exportable by features (Media File Entity Overridden) → .

We are exclusively using drush to manage features so I have not tested UI.

One quirk to note is that if there is an alter hook that appends data (e.g. hook_file_default_types_alter() is used to append a mime type to the array), it will be added again on each feature export so you'll end up with duplicate entries. I've noticed this before in features so I don't think it is caused by the patch in #57 just something to watch out for.

Another minor weirdness is when running drush fda -y it will sometimes list our custom feature with media exports in it but just says it's in the default state; the expected behavior is that if there is no diff the custom feature would not be listed at all.

I have not had time to uncover the cause of either if these minor issues, and do not consider them release blockers.

@joseph.olstad I work with @xlin and we have switched to using the features patch. It seems to be working. With patch from features #2446485-57: Proper way to define module defaults. → I am able to export and take ownership of "file_type" and "file_display" components, with default hooks in place i.e. without needing patch in #83 or media patch #2104193-113: Default file entities are not exportable by features (Media File Entity Overridden) → .

One quirk to note is that if there is an alter hook that appends data (e.g. hook_file_default_types_alter() is used to append a mime type to the array), it will be added again on each feature export so you'll end up with duplicate entries. I've noticed this before in features so I don't think it is caused by the patch in #2446485-57: Proper way to define module defaults. → just something to watch out for.