Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
This function is completely broken. Try add a condition on any field, and on saving you will get errors and notices everywhere.
I tried to fix it, but code is very complex, and i dont want to break it much more! Hope somebody can fix it soon, as lots of cases are not handled (conditions on taxonomy terms, etc)
Comment | File | Size | Author |
---|---|---|---|
#72 | ctools-entity_field_value-1630820-72.patch | 13.47 KB | maximpodorov |
#58 | ctools-entity_field_value-1630820-58.patch | 13.49 KB | maximpodorov |
#53 | entity-field-value-1630820-53.patch | 12.83 KB | maximpodorov |
#50 | Screen shot 2013-05-14 at 5.26.15 PM.png | 54 KB | mcfilms |
#45 | entity-field-value-1630820-45.patch | 13.1 KB | mrfelton |
Comments
Comment #1
Anonymous (not verified) CreditAttribution: Anonymous commentedPatch for making code work with taxonomy fields, and a PHP 5.4 notice/error
Comment #3
andypostMarked 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
Comment #4
mikeytown2 CreditAttribution: mikeytown2 commentedThis fix worked for us. Doesn't address the taxonomy issue, but it should pass the testbot.
Comment #5
neilhanvey CreditAttribution: neilhanvey commentedI'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
The main error i get is:
Comment #6
Anonymous (not verified) CreditAttribution: Anonymous commentedUsing Chaos Tools version 7.x-1.1, cannot set selection rules based on taxonomy field.
Here's some debug info:
Comment #7
Dave ReidIf 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??Comment #8
Dave ReidPersonally 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.
Comment #9
Dave ReidRevised that doesn't fatal error. With the attached patch for taxonomy term fields I get a summary of 'tid equals 4, 5'
Comment #10
Anonymous (not verified) CreditAttribution: Anonymous commentedAfter patch applied I get summary "tid equals ", and a warning in the next step:
$conf variable
$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:
Comment #11
Anonymous (not verified) CreditAttribution: Anonymous commentedCustom node layout is still not showing, in addition, your patch throws number of notices now:
Comment #12
movin360 CreditAttribution: movin360 commentedAnyone can fix this ? Please help. Need this in my project right now.
Cheers
Comment #13
movin360 CreditAttribution: movin360 commentedThe more selection rules you add to the node template the list of error messages becomes bigger.
Comment #14
inno81 CreditAttribution: inno81 commentedI applied the patch in #9 and got the same output as #10.
Still experiencing the problem.
Comment #15
andypostThis 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
For compound fields (link module for example) see second screenshot
Comment #16
blackandcode CreditAttribution: blackandcode commentedThis is very big bug I don't believe that maintenaners allow this kind of bugs
Comment #17
blackandcode CreditAttribution: blackandcode commentedAlso perhaps this bug with tools produced another bug in views new feature. Groupde Filters.
http://drupal.org/node/1778378
Comment #18
drewish CreditAttribution: drewish commentedIt's not just the summary the whole thing seems to throw warnings and not really work.
Comment #19
drupalerocant CreditAttribution: drupalerocant commentedI 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:
Comment #20
andypostAlso we could fix #1601062: Field access plugin ignoring selected value first
Because this Really completely broken
Comment #21
gerhardbaumeister CreditAttribution: gerhardbaumeister commentedHello,
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.
Comment #22
Adon Irani CreditAttribution: Adon Irani commented#9: 1630820-field-value-access-summary-borked.patch queued for re-testing.
Comment #23
Jorrit CreditAttribution: Jorrit commentedThe 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:
When the field is hidden in the display settings,
$display['module']
does not exist. The code should check for that.Comment #24
andypostHere's a right access check - we should not have False positives on NULLs
Comment #25
andypostNext 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
Comment #26
kaizerking CreditAttribution: kaizerking commented#24 isn't working
Comment #27
andypost@kaizerking what's not working? please explain your case!
Comment #28
kaizerking CreditAttribution: kaizerking commentedHere is the issue where I am getting notices, I have applied patch #24 and also the profile2 ctools patch
notices on page manager
Comment #29
ayalon CreditAttribution: ayalon commented#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).
Comment #30
mh86 CreditAttribution: mh86 commentedNotice 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.
Comment #31
ayalon CreditAttribution: ayalon commented#30 works! Great! Thanks a lot!
Comment #32
firfin CreditAttribution: firfin commentedI 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.
Comment #33
firfin CreditAttribution: firfin commentedCombining 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.
Comment #34
firfin CreditAttribution: firfin commentedMarked #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.
Comment #35
andypostGreat 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
Comment #36
merlinofchaos CreditAttribution: merlinofchaos commentedCommitted and pushed.
Comment #37
andypostThe issue needs follow up to fix taxonomy fields when they have autocomplete/multiple values
Comment #38
maximpodorov CreditAttribution: maximpodorov commentedThis 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.
Comment #39
maximpodorov CreditAttribution: maximpodorov commentedComment #40
andypostSuppose 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.
Awesome! The way of tokens/substitution does not help here...
Needs @todo here
Comment #41
maximpodorov CreditAttribution: maximpodorov commentedA small change in this patch: do not output unprintable garbage generated by field formatter.
Comment #42
andypostThis part is primary required
This part needs to be simplified. No reason to bother 'input' here
awesome function but needs php-doc about @params and @return
Please change condition to exclude empty statement
Comment #43
maximpodorov CreditAttribution: maximpodorov commentedThe next try. Autocomplete widgets still don't work. Compound fields (with multiple sql columns) seem to work. The full plugin file is attached also.
Comment #44
mrfelton CreditAttribution: mrfelton commentedIf a field does not have a value stored at all yet for a particular entity, checking against the '- None -' value (_none) fails.
Comment #45
mrfelton CreditAttribution: mrfelton commentedI'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.
Comment #46
EclipseGc CreditAttribution: EclipseGc commentedProbably best to provide interdiffs for reviewers.
Comment #47
maximpodorov CreditAttribution: maximpodorov commentedComparing 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. :)
Comment #48
andypostEclipseGc++
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...
Another funny part for localization...
using $form_state['values'] are solves the problem
Comment #49
merlinofchaos CreditAttribution: merlinofchaos commentedI 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.
Comment #50
mcfilms CreditAttribution: mcfilms commentedOkay 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:
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?
Comment #51
maximpodorov CreditAttribution: maximpodorov commentedCould you try #43?
Comment #52
mcfilms CreditAttribution: mcfilms commented@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.
Comment #53
maximpodorov CreditAttribution: maximpodorov commentedHere is the revised patch. It contains the fix of the problem described in #50. Other small improvements are there. Please test and review.
Comment #54
DamienMcKennaMarked #2000440: Ctools access plugin (field) has wrong default value. as a duplicate.
Comment #55
DamienMcKennaMarked #1435458: Access plugin saves visibility rule to database but does not reflect changes in the UI as a duplicate.
Comment #56
DamienMcKennaThis needs some work:
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]".
Comment #57
kenorb CreditAttribution: kenorb commentedCould be related: #1435968: visibility rule broken
Comment #58
maximpodorov CreditAttribution: maximpodorov commentedThis patch implements the detailed information output, as requested in #56. Warnings and errors for non-existing data are fixed.
Comment #59
jlapp CreditAttribution: jlapp commentedI haven't tested other aspects of the patch, but #58 fixed using list values as visibility rules for me.
Comment #60
kalabroI confirm that #58fixes problems with taxonomy fields.
Comment #61
andypostthis 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 asentity_field.inc
Comment #62
bryancasler CreditAttribution: bryancasler commented#58 worked for me and applied cleanly to the current stable and dev releases.
Comment #63
mikeker CreditAttribution: mikeker commentedShouldn'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.
Comment #64
maximpodorov CreditAttribution: maximpodorov commentedEmpty array means no values to check. So the result will be FALSE in any case.
Comment #65
mikeker CreditAttribution: mikeker commentedAgreed 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.
Comment #66
jhedstromI 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).
Comment #67
DamienMcKenna@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.
Comment #68
japerryI 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... =/
Comment #69
maximpodorov CreditAttribution: maximpodorov commented@japerry
Do you mean current dev ctools with #58 patch applied produces these screens?
Comment #70
maximpodorov CreditAttribution: maximpodorov commentedI 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.
Comment #71
maximpodorov CreditAttribution: maximpodorov commentedThe patch is rerolled for the current dev.
Comment #72
maximpodorov CreditAttribution: maximpodorov commentedComment #73
maximpodorov CreditAttribution: maximpodorov commentedComment #74
jlapp CreditAttribution: jlapp commentedI 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.
Comment #75
magicmyth CreditAttribution: magicmyth commentedI'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
Comment #76
Exploratus CreditAttribution: Exploratus commented#72 Worked for me.
Comment #77
dgwebcreative CreditAttribution: dgwebcreative commentedI'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.
Comment #78
kristiaanvandeneyndeRTBC for me as well
Comment #79
Kojo Unsui CreditAttribution: Kojo Unsui commentedI 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.
Comment #80
geresy CreditAttribution: geresy commentedThe 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).
Comment #81
maximpodorov CreditAttribution: maximpodorov commentedOK, I'll look at this new problem.
Comment #82
maximpodorov CreditAttribution: maximpodorov commented@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.
Comment #83
geresy CreditAttribution: geresy commentedThe problem went away after I removed and then added again some of the selection rules of panels that were causing problems. Looks fine now.
Comment #84
japerryI 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.
Comment #85
maximpodorov CreditAttribution: maximpodorov commentedI want to add functionality of https://drupal.org/node/1188466#comment-8787523 to this patch.
Comment #86
kristiaanvandeneyndeTotally unrelated. This issue is about fixing something that's broken, the other is about adding new functionality.
Comment #87
maximpodorov CreditAttribution: maximpodorov commentedThis is not a completely new functionality. You can treat this as "field has empty value" case.
Comment #88
kristiaanvandeneyndePeople 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.
Comment #89
maximpodorov CreditAttribution: maximpodorov commentedIf so, I ask Jakob to accept this patch ASAP. :)
Comment #90
pfrenssenIn #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 callhook_field_storage_load()
on the storage engine (eg.field_sql_storage_field_storage_load()
ormongodb_field_storage_field_storage_load()
).@maximpodorov asked:
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.
Comment #91
pfrenssenBTW 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.
Comment #92
maximpodorov CreditAttribution: maximpodorov commented@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.
Comment #93
pfrenssenI'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).
Comment #95
japerrySo 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.
Comment #98
osopolarI created a new issue for the bug reported in #77: #2501751: Visibility rules aren't saved for unlimited multiple value fields.