Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Anonymous’s picture

Status: Active » Needs review
FileSize
2.92 KB

Patch for making code work with taxonomy fields, and a PHP 5.4 notice/error

Status: Needs review » Needs work

The last submitted patch, ctools.patch, failed testing.

andypost’s picture

Marked as duplicate #1435426: Notices thrown when using visibility rules on hidden fields

Also related issue #1520036: Notices thrown when taxonomy used for selection criteria

Both issues are depends on assumption that field contains only one storage column named 'value' but this wrong.
I think both issues should take a same approach for storing config and checking values in loop

mikeytown2’s picture

Status: Needs work » Needs review
FileSize
754 bytes

This fix worked for us. Doesn't address the taxonomy issue, but it should pass the testbot.

neilhanvey’s picture

I'm having the same error but no joy with the patch.

I'd created some content types and was trying to set the selection rule on a panel as follows

Panels > Manage Pages > Edit Node Template > Add Variant with Selection Rules checked > Selecting Node type of my custom content. My selection rules on the summary page just say

This panel will be selected if value equals summary equals format equals

The main error i get is:

Notice: Undefined offset: 0 in ctools_entity_field_value_ctools_access_summary() (line 223 of /Library/WebServer/Documents/drupal/sites/all/modules/ctools/plugins/access/entity_field_value.inc).
Notice: Undefined offset: 1 in ctools_entity_field_value_ctools_access_summary() (line 223 of /Library/WebServer/Documents/drupal/sites/all/modules/ctools/plugins/access/entity_field_value.inc).
Notice: Undefined offset: 2 in ctools_entity_field_value_ctools_access_summary() (line 223 of /Library/WebServer/Documents/drupal/sites/all/modules/ctools/plugins/access/entity_field_value.inc).
Notice: Undefined offset: 0 in ctools_entity_field_value_ctools_access_summary() (line 223 of /Library/WebServer/Documents/drupal/sites/all/modules/ctools/plugins/access/entity_field_value.inc).
Notice: Undefined offset: 1 in ctools_entity_field_value_ctools_access_summary() (line 223 of /Library/WebServer/Documents/drupal/sites/all/modules/ctools/plugins/access/entity_field_value.inc).
Notice: Undefined offset: 2 in ctools_entity_field_value_ctools_access_summary() (line 223 of /Library/WebServer/Documents/drupal/sites/all/modules/ctools/plugins/access/entity_field_value.inc).

Anonymous’s picture

Using Chaos Tools version 7.x-1.1, cannot set selection rules based on taxonomy field.

Notice: Undefined offset: 0 in ctools_entity_field_value_ctools_access_summary() (line 220 of /sites/all/modules/ctools/plugins/access/entity_field_value.inc).

  foreach ($value_keys as $key => $value) {
    $values[$value] = $elements[$key]['#markup'];
  }

Here's some debug info:

print_r($value_keys);

/*
Array
(
    [0] => @tid_value
)
*/
print_r($values);

/*
Array
(
    [@tid_value] => 7
)
*/
print_r($elements);

/*
Array
(
)
*/
Dave Reid’s picture

If we're manually setting $display['type'] = 'list_default'; then we *must* always call list_field_formatter_view(). I'm trying to scratch my head why this is even running code through the list formatter by default. Why not just output the raw values in the summary??

Dave Reid’s picture

Personally I think until we're able to figure out how to get this to work showing field 'nice' values, then it should just stay simple and output the raw field values. Revised patch converts the summary to output the raw field values.

Dave Reid’s picture

Revised that doesn't fatal error. With the attached patch for taxonomy term fields I get a summary of 'tid equals 4, 5'

Anonymous’s picture

After patch applied I get summary "tid equals ", and a warning in the next step:

Warning: implode() [function.implode]: Invalid arguments passed in ctools_entity_field_value_ctools_access_summary() (in line 205 of /sites/all/modules/ctools/plugins/access/entity_field_value.inc).

  foreach ($field['columns'] as $column => $attributes) {
    $string .= " @{$column} equals @{$column}_value";
    $keys['@' . $column] = $column;
    // Display the field values raw separated by commas.
    $values["@{$column}_value"] = implode(', ', $conf[$field_name . '_' . $column]); // line 205
   }

$conf variable

Array
(
    [field_program] => Array
        (
            [und] => Array
                (
                    [0] => Array
                        (
                            [tid] => 7
                        )

                )

        )

    [field_program_tid] => 7
)

$field_name . '_' . $column evaluated value
field_program_tid

Passing a scalar value into implode() while array is expected. This could be made to work if checked for scalar value I suppose:

$value=!is_array($conf[$field_name . '_' . $column]) ? array($conf[$field_name . '_' . $column]) : $conf[$field_name . '_' . $column];
$values["@{$column}_value"] = implode(', ', $value);
Anonymous’s picture

Custom node layout is still not showing, in addition, your patch throws number of notices now:

• Notice: Undefined index: taxonomy_term in zgrp_process_page() (line 58 from /themes/zgrp/template.php).
• Notice: Trying to get property of non-object in zgrp_process_page() (line 58 from /themes/zgrp/template.php).
• Notice: Undefined index: value in _ctools_entity_field_value_ctools_access_get_conf_field_values() (line 177 from /sites/all/modules/ctools/plugins/access/entity_field_value.inc).
• Notice: Undefined index: value in ctools_entity_field_value_ctools_access_check() (line 160 from /sites/all/modules/ctools/plugins/access/entity_field_value.inc).
• Notice: Undefined index: value in _ctools_entity_field_value_ctools_access_get_conf_field_values() (line 177 from /sites/all/modules/ctools/plugins/access/entity_field_value.inc).
• Notice: Undefined index: value in ctools_entity_field_value_ctools_access_check() (line 160 from /sites/all/modules/ctools/plugins/access/entity_field_value.inc).

movin360’s picture

Anyone can fix this ? Please help. Need this in my project right now.
Cheers

movin360’s picture

Priority: Normal » Major
Status: Needs review » Needs work

The more selection rules you add to the node template the list of error messages becomes bigger.

inno81’s picture

I applied the patch in #9 and got the same output as #10.
Still experiencing the problem.

andypost’s picture

FileSize
55.61 KB
26.7 KB

This issue should be fixed after #1601062: Field access plugin ignoring selected value
Because currently ctools_entity_field_value_ctools_access_settings_submit() wrongly saves data for field and totally broken for compound fields.

The only question here is how to store values without breaking current config values, suppose hook_update_n() does not solve this problem.

The value that used to display summary stored in [field_name]_[column_name] see attached screenshot
ctools_access.jpg

For compound fields (link module for example) see second screenshot
ctools_access2.jpg

blackandcode’s picture

This is very big bug I don't believe that maintenaners allow this kind of bugs

blackandcode’s picture

Also perhaps this bug with tools produced another bug in views new feature. Groupde Filters.

http://drupal.org/node/1778378

drewish’s picture

Title: ctools_entity_field_value_ctools_access_summary completely broken » entity_field_value is completely broken

It's not just the summary the whole thing seems to throw warnings and not really work.

drupalerocant’s picture

I have the same issue with taxonomy fields, I have the Notices, although the visibility rule works correctly.
I get 18 Notices like this one, when I am in the content tab, in pages->edit node_view in every variant I created:

Notice: Undefined offset: 0 en ctools_entity_field_value_ctools_access_summary() (línea 232 de /home/virtualmin/**********/public_html/nuevaweb/sites/all/modules/ctools/plugins/access/entity_field_value.inc).

andypost’s picture

Also we could fix #1601062: Field access plugin ignoring selected value first
Because this Really completely broken

gerhardbaumeister’s picture

Hello,
i run into this problem by upgrading from 7.x-1.1 to 7.x-1.2, so upgrading crashes my whole site.
In 7.x-1.1 all works fine for months.
I would change the version of this thread from 7.x-1.x-dev to 7.x-1.2.

Adon Irani’s picture

Status: Needs work » Needs review
Jorrit’s picture

The error "Undefined index: module in ctools_entity_field_value_ctools_access_summary" in the current stable version is caused by a condition on a field that is marked as "Hidden" in the node type display settings:

The error is caused because of the following lines in entity_field_value.inc:

  $display         = field_get_display($instance, $view_mode, $entity);
  $display['type'] = 'list_default';
  $function        = $display['module'] . '_field_formatter_view';

When the field is hidden in the display settings, $display['module'] does not exist. The code should check for that.

andypost’s picture

Here's a right access check - we should not have False positives on NULLs

andypost’s picture

Next steps:
1) Re-make settings form, probably we need incorporate #1601062-2: Field access plugin ignoring selected value
2) Provide upgrade old settings
3) Fix admin summary

kaizerking’s picture

#24 isn't working

andypost’s picture

@kaizerking what's not working? please explain your case!

kaizerking’s picture

Here is the issue where I am getting notices, I have applied patch #24 and also the profile2 ctools patch

notices on page manager

ayalon’s picture

#24 did not work for me. I still have these notice error messages

Notice: Undefined index: module in ctools_entity_field_value_ctools_access_summary() Line 225 in sites\all\modules\ctools\plugins\access\entity_field_value.inc).

mh86’s picture

Notice from #29 happens if the field is hidden on the entity and thus doesn't use a formatter.
Attached an updated patch that fixes this notice.

ayalon’s picture

#30 works! Great! Thanks a lot!

firfin’s picture

I suspect I have a related problem. See #1876640: errors and not working selection rule when using negative check on a 'link'field for details. Might be a duplicate, not sure if the cause is exactly the same.
The patch from #30 doesn't solve the problem for me. It least not completely, still get errors in the backend but not on evaluation. So It probably has another (or secondary) cause.

firfin’s picture

Combining the patch from #30 with the patch from #1780280: Notice on node view page variant seems to remove all notices for me. The selection rules I have tested so far seem to be working also.

Updated patch attached.

firfin’s picture

Marked #1876640: errors and not working selection rule when using negative check on a 'link'field as a duplicate of this issue.
It really is a duplicate. Patch from #33 fixes it completely.

andypost’s picture

Status: Needs review » Reviewed & tested by the community

Great to see access-summary fixed! Now it's ready to be commited

PS: Marked as duplicate #1780280: Notice on node view page variant as this hunk included

merlinofchaos’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed.

andypost’s picture

The issue needs follow up to fix taxonomy fields when they have autocomplete/multiple values

maximpodorov’s picture

This patch solves the problem of saving and displaying values entered in the entity field form. Numbers, term references, entity references are tested (both singular and multiple (fixed) values). The remaining issue is multiple (unlimited) value form with 'Add more' button.

maximpodorov’s picture

Status: Fixed » Needs review
andypost’s picture

+++ b/plugins/access/entity_field_value.incundefined
@@ -103,8 +103,13 @@ function ctools_entity_field_value_ctools_access_settings($form, &$form_state, $
+        if (is_numeric($delta)) {

@@ -131,8 +136,33 @@ function ctools_entity_field_value_ctools_access_settings_submit($form, &$form_s
+  if (!is_array($items)) {

@@ -224,15 +266,58 @@ function ctools_entity_field_value_ctools_access_summary($conf, $context, $plugi
+      if (is_array($elements)) {

Suppose if we carefully massage the values in onSubmit so $conf values could be plain array $delta => $value and no matter how many values are stored.
The only thing to care a Compound fields.

Take care that some widgets accept "multiple values" but some not. This leads to the confusion we have.
Probably better check the field cardinality.

+++ b/plugins/access/entity_field_value.incundefined
@@ -224,15 +266,58 @@ function ctools_entity_field_value_ctools_access_summary($conf, $context, $plugi
+    // Choose simple formatter for well known cases.
+    switch ($display['module']) {
+      case 'list':
+        $display['type'] = 'list_default';
+        break;
+
+      case 'taxonomy':
+        $display['type'] = 'taxonomy_term_reference_plain';
+        break;
+
+      case 'entityreference':
+        $display['type'] = 'entityreference_label';
+        break;
+
+      default :
+        // Use field instance formatter setting.
+        break;

Awesome! The way of tokens/substitution does not help here...

+++ b/plugins/access/entity_field_value.incundefined
@@ -224,15 +266,58 @@ function ctools_entity_field_value_ctools_access_summary($conf, $context, $plugi
+      if (count($field['columns']) > 1) {
+        // Add some sort of handling for compound fields

Needs @todo here

maximpodorov’s picture

A small change in this patch: do not output unprintable garbage generated by field formatter.

andypost’s picture

+++ b/plugins/access/entity_field_value.incundefined
@@ -103,8 +103,13 @@ function ctools_entity_field_value_ctools_access_settings($form, &$form_state, $
+        if (is_numeric($delta)) {
+          if (is_array($conf_value)) {
+            $form['settings'][$field_name][LANGUAGE_NONE][$delta]['value']['#default_value'] = $conf_value['value'];
+          }
+          else {
+            $form['settings'][$field_name][LANGUAGE_NONE]['#default_value'][] = $conf_value;

This part is primary required

+++ b/plugins/access/entity_field_value.incundefined
@@ -131,8 +136,35 @@ function ctools_entity_field_value_ctools_access_settings_submit($form, &$form_s
+    $form_state['input']['settings'][$field_name][$langcode] =
+      _ctools_entity_field_value_filter_items($field, $form_state['input']['settings'][$field_name][$langcode]);
     $form_state['values']['settings'][$column] = $form_state['input']['settings'][$field_name][$langcode];
   }
+  if (isset($form_state['input']['settings'][$field['field_name']][$langcode])) {
+    $form_state['input']['settings'][$field['field_name']][$langcode] =
+      _ctools_entity_field_value_filter_items($field, $form_state['input']['settings'][$field['field_name']][$langcode]);
+    $form_state['values']['settings'][$field['field_name']][$langcode] = $form_state['input']['settings'][$field['field_name']][$langcode];

This part needs to be simplified. No reason to bother 'input' here

+++ b/plugins/access/entity_field_value.incundefined
@@ -131,8 +136,35 @@ function ctools_entity_field_value_ctools_access_settings_submit($form, &$form_s
+function _ctools_entity_field_value_filter_items($field, $items) {

awesome function but needs php-doc about @params and @return

+++ b/plugins/access/entity_field_value.incundefined
@@ -224,15 +269,60 @@ function ctools_entity_field_value_ctools_access_summary($conf, $context, $plugi
+      if (count($field['columns']) > 1) {
+        // Add some sort of handling for compound fields
+      }
+      else {
+        $keyed_elements["@{$column}_value"] = $elements;
+      }

Please change condition to exclude empty statement

maximpodorov’s picture

The next try. Autocomplete widgets still don't work. Compound fields (with multiple sql columns) seem to work. The full plugin file is attached also.

mrfelton’s picture

If a field does not have a value stored at all yet for a particular entity, checking against the '- None -' value (_none) fails.

mrfelton’s picture

I'm not convinced about this approach. But, I have taken the follow patch from #43, and added to it some provisions to handle the case where no value is saved for a field.

EclipseGc’s picture

Probably best to provide interdiffs for reviewers.

maximpodorov’s picture

Comparing with '_none' or other magic constants is not the right way. I'll provide the improved patch soon (with solves autocomplete fields issue also). BTW, the real fun is permitting to configure the field comparison against file field or field collection field. :)

andypost’s picture

EclipseGc++
Having interdiffs is much better way to get proper reviews!!!

Suppose it's clear that there's no way to save BC for broken data but after #33 we have in 7.x-dev there's a probability that some sites have working config. So the follow-up code that introduces support for "Compound fields" is really great and better to have it commited before next ctools' stable release.

@mrfelton please explain the difference in your approach. The issue is really needed and annoying same time...

+++ b/plugins/access/entity_field_value.incundefined
@@ -103,8 +103,13 @@ function ctools_entity_field_value_ctools_access_settings($form, &$form_state, $
+            $form['settings'][$field_name][LANGUAGE_NONE][$delta]['value']['#default_value'] = $conf_value['value'];
...
+            $form['settings'][$field_name][LANGUAGE_NONE]['#default_value'][] = $conf_value;

@@ -211,7 +244,19 @@ function ctools_entity_field_value_ctools_access_summary($conf, $context, $plugi
+              $entity->{$field_name}[LANGUAGE_NONE][] = array($column => $conf_value);
...
+              $entity->{$field_name}[LANGUAGE_NONE][] = array($column => $conf_value['value']);
...
+          $entity->{$field_name}[LANGUAGE_NONE][] = array($column => $conf[$conf_key]);

@@ -224,15 +269,60 @@ function ctools_entity_field_value_ctools_access_summary($conf, $context, $plugi
+      field_default_prepare_view($entity_type, $entity_group, $field, $instance_group, LANGUAGE_NONE, $item_group, $display);
+      $elements = $function($entity_type, $entity, $field, $instance, LANGUAGE_NONE, $item_group[0], $display);

Another funny part for localization...

+++ b/plugins/access/entity_field_value.incundefined
@@ -131,8 +136,35 @@ function ctools_entity_field_value_ctools_access_settings_submit($form, &$form_s
+      _ctools_entity_field_value_filter_items($field, $form_state['input']['settings'][$field_name][$langcode]);

using $form_state['values'] are solves the problem

merlinofchaos’s picture

I didn't want to derail the patch that's in here (though if people like what I've done it will derail things anyway). However, I was pretty pressed for time and I just wanted to make this work. So without further ado, I posted a patch here: #1955954: entity_field_value.inc is mostly useless.

mcfilms’s picture

Okay I'm seeking advice. I was having the same errors as above. I tried the patch in #45 and I still had errors. Then I followed the thread in #49 and applied that patch. The errors continued.

So I did two things that I thought would help. I installed the current dev version and used the patch in #45 and I uninstalled the relationship module (because it uses entity fields).

Now I simply get an error:

Error
Error message

Notice: Trying to get property of non-object in ctools_entity_field_value_ctools_access_check() (line 207 of /home/dte/public_html/sites/all/modules/ctools/plugins/access/entity_field_value.inc).
EntityMalformedException: Missing bundle property on entity of type node. in entity_extract_ids() (line 7663 of /home/dte/public_html/includes/common.inc).

The website encountered an unexpected error. Please try again later.

So what would your next move be here? I suppose I could roll back a few days, but then I would loose the Views I built.

Any ideas?

maximpodorov’s picture

Could you try #43?

mcfilms’s picture

@maximpodorov -- Sorry, late last night I made the call to just roll back the database, delete the Relationship module and rebuild my Views. However if you want I can email you the broken database.

I wonder if it would make sense to have a "known issues" notice on the cTools project page.

With a full version 1.3 release and no mention of any trouble, "module monkeys" (okay, site builders) like me can land them selves in some devastatingly hot water.

Of course CTools is needed now for Views, but it would nice nice to see a "Known Issue" warning stating that there is a current bug with evaluating conditional fields that causes problems with the Relationship module and autocomplete Taxonomy fields. See (this thread).

Had I read that I would have tread a bit more carefully.

maximpodorov’s picture

Here is the revised patch. It contains the fix of the problem described in #50. Other small improvements are there. Please test and review.

DamienMcKenna’s picture

DamienMcKenna’s picture

DamienMcKenna’s picture

Status: Needs review » Needs work

This needs some work:

  1. The condition's description on the Selection Rules page just displays the value, rather than e.g. "Only when set to '[value]'.".
  2. Because of the above, the Panels summary page will show e.g. This panel will be selected if Node being viewed is type "Page", and No. instead of e.g. This panel will be selected if Node being viewed is type "Page", and [field] is set to "[value]".
kenorb’s picture

maximpodorov’s picture

Status: Needs work » Needs review
FileSize
13.49 KB

This patch implements the detailed information output, as requested in #56. Warnings and errors for non-existing data are fixed.

jlapp’s picture

I haven't tested other aspects of the patch, but #58 fixed using list values as visibility rules for me.

kalabro’s picture

I confirm that #58fixes problems with taxonomy fields.

andypost’s picture

this one is more tightly integrated into core so patch from #1955954: entity_field_value.inc is mostly useless better to commit as field.inc and this one should stay as entity_field.inc

bryancasler’s picture

#58 worked for me and applied cleanly to the current stable and dev releases.

mikeker’s picture

+++ b/plugins/access/entity_field_value.inc
@@ -128,17 +132,83 @@ function ctools_entity_field_value_ctools_access_settings_submit($form, &$form_s
+  if (empty($context->data)) {
+    return FALSE;
+  }

Shouldn't this be:

if (!isset($context->data)) {

because a (string) "0" or empty array could still be a valid context. See #1292098: Check if entity exists and loadable before inject it as a context, specifically this patch.

maximpodorov’s picture

Empty array means no values to check. So the result will be FALSE in any case.

mikeker’s picture

Empty array means no values to check. So the result will be FALSE in any case.

Agreed about the empty array.

But $context->data can be an array or a string depending on whether it comes from a settings form or an argument.

jhedstrom’s picture

I don't fully understand the comment in #61 but the patch in #58 resolves an issue I was having with OG/entityreference field values as visibility rules (#2154851: Using ctools entity_field_value selection rule with an OG field doesn't save selected value).

DamienMcKenna’s picture

@jhedstrom: What andypost22 meant was that, because there are two different issues dealing with two different field issues, this one should be left as entity_field.inc while the one from #1955954 should be renamed to field.inc.

japerry’s picture

Issue summary: View changes
Status: Needs review » Needs work
FileSize
90.63 KB
123.5 KB

I think it still needs work. I get these errors with all over ctools patches applied from the sandbox for 1.4:

and

Not sure what to do here... =/

maximpodorov’s picture

@japerry
Do you mean current dev ctools with #58 patch applied produces these screens?

maximpodorov’s picture

I tested your sandbox, but after installing it, I can't edit access rules at all. I applied my patch #58 and saw the correct screen with inputs from the modal shown.

maximpodorov’s picture

The patch is rerolled for the current dev.

maximpodorov’s picture

FileSize
13.47 KB
maximpodorov’s picture

Status: Needs work » Needs review
jlapp’s picture

Status: Needs review » Reviewed & tested by the community

I tested the re-rolled patch from #72 with the latest CTools dev branch and it worked fine for me. I tested selection rules with different field types (text, list, term reference, and entity reference) and didn't get the warnings listed in #68.

magicmyth’s picture

I've tested the patch against HEAD as well now and can confirm it fixes the issues for me.

FYI using "patch" will not work against latest dev but if you clone git HEAD you can cleanly apply it with:
git apply -v -p1 ctools-entity_field_value-1630820-72.patch 2>&1

Exploratus’s picture

#72 Worked for me.

dgwebcreative’s picture

I've applied this patch #72 to Chaos tools 7.x-1.4. In my case I have a "List (integer)" type field with unlimited values attached to a Account entity. I've attempted to use a panels visibility rule to filter by this list and it doesn't appear to be working. No further information in the log.

kristiaanvandeneynde’s picture

RTBC for me as well

Kojo Unsui’s picture

Issue tags: +panels

I had Notice : Undefined index: #parents in field_default_form() (line 49... when trying to set a visibility rule to a pane, for any kind of field (config Commons with D7.24).

#72 fixed the issue for me. Thanks.

geresy’s picture

The patch fixed the warnings about "htmlspecialchars() expects parameter 1" in the selection rules of panels but now I get

Warning: array_flip(): Can only flip STRING and INTEGER values! in DrupalDefaultEntityController->load() (line 173 of public_html/includes/entity.inc).

maximpodorov’s picture

OK, I'll look at this new problem.

maximpodorov’s picture

@geresy, could you try to provide more information? For example, turn on backtrace showing for pages with errors (using Devel module) and provide the trace of illegal ID values.

geresy’s picture

The problem went away after I removed and then added again some of the selection rules of panels that were causing problems. Looks fine now.

japerry’s picture

I think this is working as well, but I'm going to keep the patch in my ctools repo for a while to get more testing out of it. I'm worried that uncomplete test suites could lead to us missing something.

maximpodorov’s picture

I want to add functionality of https://drupal.org/node/1188466#comment-8787523 to this patch.

kristiaanvandeneynde’s picture

Totally unrelated. This issue is about fixing something that's broken, the other is about adding new functionality.

maximpodorov’s picture

This is not a completely new functionality. You can treat this as "field has empty value" case.

kristiaanvandeneynde’s picture

People in this issue are looking to fix the buggy behavior of entity_field_value. By trying to slip in extra code, you're only delaying this fix. Adding extra functionality requires its own issue.

I never said it was completely new, but it is still totally unrelated to this issue.

maximpodorov’s picture

If so, I ask Jakob to accept this patch ASAP. :)

pfrenssen’s picture

In #1837650: Allow referencing a specific revision ID @maximpodorov raised concerns that adding revisioning information to Entity Reference fields would break this plugin. There was discussion there that is relevant to this issue.

In short, this plugin does not take into account the fact that modules can change the internal structure of their fields in updates. If a field is changed during an update, the previously saved configuration would not match the current configuration and the plugin would no longer be able to determine whether the field values match the expected data.

Another point is that the plugin relies on a private function _field_sql_storage_columnname() which is taken from the default SQL storage backend. This storage backend is used for most sites, but it would break if another backend is used (e.g. MongoDB).

I had a look on how the field module handles this. The field values are populated inside field_attach_load() which populates the fields on an entity. The main idea is to first determine the storage engine of the field, and then call hook_field_storage_load() on the storage engine (eg. field_sql_storage_field_storage_load() or mongodb_field_storage_field_storage_load()).

@maximpodorov asked:

Do you mean field value equality is equality of all required properties of the field?

From looking at both these storage backend implementations it indeed looks like the field values consist of all the column of a field. So yes, field value equality is equality of all properties/columns.

To solve the potential changes to the way a field value is structured I guess we should check if the field column keys match the keys that were previously saved. There are no hooks to act on database field changes so we need to monitor for changes ourselves. If any change is detected we should inform the site administrator to look at the configuration. I'm not familiar with this plugin but I guess resaving would usually do the trick. I guess you can compare this to how Views handles changes in field storage. It will mark the handlers as "broken", and the site maintainer would need to go in and resave the views and make any necessary adjustments.

pfrenssen’s picture

BTW I'm not suggesting that these things should be addressed in this issue. In line with the remarks of @kristiaanvandeneynde above we should probably tackle this in two separate tickets, one to address the storage backend issue, and the other for the changing fields issue.

maximpodorov’s picture

@pfrenssen, you avoid the main question: is it a way to set (per field instance) the field properties (columns) which are required and the properties which are additional (not relevant for field comparison)? Revision ID of entity reference fields can be treated as required in some cases and additional in other cases. If some module will add a text property (like description) to entity reference field, it's definitely an additional property.

pfrenssen’s picture

I'm not really clear on what you're asking, field comparison in what context? And what do you mean with required properties? There is no concept of marking columns as required or not on field level in D7 AFAIK, apart from setting the columns to NOT NULL in the database, but that is not reliable because not all field storage engines may support this. I might be wrong on this by the way.

Some fields mark some properties as required for their own use case. For example an Address field only has the country as a required input, but whether or not any of these properties are important for comparison depends on the context in which you are comparing. You might be looking for the full address (all columns are relevant), or only for addresses within a country or city (not all columns are relevant).

  • merlinofchaos committed a6e29d7 on 8.x-2.x
    Issue #1630820 by andypost, davereid, firfin, several others: Fix entity...
japerry’s picture

Status: Reviewed & tested by the community » Fixed

So I'm a little worried about #77 and #80 for updatability, but I cannot reproduce. Because this is such an issue for other reasons I'm thinking its an okay idea to commit this and tag 1.6-RC1 for people to test. We can revert it if its causing major issues or apply more patches to try to solve the edge case issues people find.

  • japerry committed 2313cde on 7.x-1.x authored by maximpodorov
    Issue #1630820 by maximpodorov, andypost, Dave Reid, japerry, mikeytown2...

Status: Fixed » Closed (fixed)

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

osopolar’s picture