Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Example use: making sure that group_audience field from OG is not empty before creating a relationship based on it.
Another example (assuming link tokens work): making sure a link field is not empty before using it as the path in a 301 redirection
This doesn't require anything that isn't in core, so it seems like it would be appropriate to have in ctools proper.
I'll be attaching a proposed patch in a moment.
Comment | File | Size | Author |
---|---|---|---|
#70 | panels-ctools-reference.png | 39.3 KB | philsward |
#64 | interdiff-1188466-42-64.txt | 3.35 KB | bdone |
#64 | ctools-n1188466-64.patch | 5.49 KB | bdone |
#42 | interdiff-ctools-n1188466-31-42.txt | 433 bytes | marcvangend |
#42 | ctools-n1188466-42.patch | 7.18 KB | marcvangend |
Comments
Comment #1
jastraat CreditAttribution: jastraat commentedThe most common use cases I believe would be to confirm that a field is NOT empty, so I've actually made the access rule 'is not empty' so that if a context doesn't exist and it evaluates to 'FALSE' it's as though the field IS empty.
I'm attaching a patch, but I'm not sure the format is correct for creating a new file, so I'll attach that new file as well.
Comment #2
andypostMarked as duplicate #1813992: entity_field_value doesn't handle conditions with empty values on references to the node.
trailing whitespace
Comment #3
andypost$conf['field'] should be replaced with $info
not needed
Comment #4
maximpodorov CreditAttribution: maximpodorov commentedReady to use module (for the testing purposes).
Comment #5
maximpodorov CreditAttribution: maximpodorov commentedThe updated patch to review.
Comment #6
maximpodorov CreditAttribution: maximpodorov commentedSpaces are deleted. :)
Comment #7
maximpodorov CreditAttribution: maximpodorov commentedSpaces are deleted. :)
Comment #8
andypost!isset($conf['field']) is not needed
empty($conf['field']) is enough here
space before deleted should be removed
Comment #9
maximpodorov CreditAttribution: maximpodorov commentedThe patch with the fixes of issues above.
Comment #10
andypostfield_info_fields() already returns none-deleted fields.
Also I'd like to see field_info_field_map() instead field_info_fields() here
needs space after if
Should be changed to
if (!empty($conf['field']) && field_info_field($conf['field']) {
// identifier...
Comment #11
maximpodorov CreditAttribution: maximpodorov commentedHere is a completely different approach to this task (based on entity_field_value.inc).
Comment #12
maximpodorov CreditAttribution: maximpodorov commentedIt's better to add this functionality to #1630820: entity_field_value is completely broken patch, since this approach creates tons of new sub-plugins (about 2000 in my case).
Comment #13
kopeboy CreditAttribution: kopeboy commentedCan we get this going please?
I really need this functionality and I am sure many other site builders would benefit!
Comment #15
maximpodorov CreditAttribution: maximpodorov commented#1630820: entity_field_value is completely broken is fixed, so it's possible to use it as a base for solving this issue. I modifield entity_field_value plugin to support entering empty values for field configuration, so "Field is empty" condition is possible in that plugin using the proposed patch.
Comment #16
maximpodorov CreditAttribution: maximpodorov commenteddpm() call is removed.
The uncertain point is multilingual fields. We have to decide what to do with them.
Comment #17
kristiaanvandeneynde@maximpodorov What would be the issue with multilingual fields? I might be able to update the patch.
Comment #18
maximpodorov CreditAttribution: maximpodorov commentedThe very concept of "field has a value" and "field has the value" must be defined for multilingual fields. Maybe the condition configuration form must be able to define multiple pairs of language + value.
Comment #19
kristiaanvandeneyndeDoesn't it just suffice to check in the current language? Something field_get_items() does already?
Comment #20
maximpodorov CreditAttribution: maximpodorov commentedYou have two stages: configuration time, check time.
Comment #21
kristiaanvandeneyndeOk, so you're saying someone should be able to configure the rule as:
Which would show all enabled languages as options, perhaps with an "All languages" field at the top, much like Pathauto does for path patterns.
If that's the case, I very much like the idea. But wouldn't that be a nice thing to try in a follow-up patch? That way, the fix for empty fields can go in already.
Comment #22
maximpodorov CreditAttribution: maximpodorov commentedWe have to decide what "empty field" means: "completely empty", "empty in the current language", or "empty in the current and default language".
Comment #23
kristiaanvandeneyndeIsn't the general expectation that fields are always used in the current language? So "empty" would mean "empty in the language it's being viewed in". In that case, the current patch seems sufficient.
If you want to expand the configuration screen to allow language-specific options like those I suggested in #21, it might be best to open a new issue for that.
Comment #24
osopolarPatch in #16 works for me.
1+ for committing this and creating a follow up patch to discuss the problem for multilingual fields.
Comment #25
annya CreditAttribution: annya commentedPatch in #16 works for me too.
Comment #26
maximpodorov CreditAttribution: maximpodorov commentedThese issues must be resolved in order to make field value based selection rules work reliable:
#2501751: Visibility rules aren't saved for unlimited multiple value fields
#2504465: Visibility rules aren't saved for unlimited multiple value fields
Comment #27
partdigital CreditAttribution: partdigital commentedPatch in #16 works for me as well.
Comment #28
crmn CreditAttribution: crmn commentedi got a fatal error with the patch from #16 - i had an old visibility rule depending on a field from a non-existent content type. as the bundle was not available field_info_instance returned NULL and later on _field_filter_items is called with an empty parameter.
i added a query to check if the bundle extists.
Comment #29
kristiaanvandeneyndeWhile it's not bad to check for something's existence, having visibility rules that point to nothing lingering around does kind of make you expect stuff will crash somewhere down the line :)
Comment #30
amaisano CreditAttribution: amaisano commentedThe patch in #28 above is missing a semi-colon on line 290/291, and causes an error after applied. Adding the semi-colon back in corrects this issue.
Comment #31
DamienMcKennaThis fixes the error in the last patch.
Comment #32
DamienMcKennaThis seems pretty safe.
Comment #33
maximpodorov CreditAttribution: maximpodorov commentedWhat about multilingual issues (see #18)?
Comment #34
Anonymous (not verified) CreditAttribution: Anonymous commentedWe've been using the patch in #31 for some time.
Comment #35
Anonymous (not verified) CreditAttribution: Anonymous commentedComment #36
B-Prod CreditAttribution: B-Prod commentedThanks, the patch works fine!
Comment #37
DamienMcKennaThis wasn't added to 7.x-1.10, bumping it to 7.x-1.11.
Comment #38
kepford CreditAttribution: kepford at Mediacurrent commentedThis doesn't appear to work with image fields. If I use an image field in my selection rules and leave the image empty the variant is active no matter what the state of the image field on a node. Additionally, the Reverse (NOT) checkbox doesn't work. If I check it and save the rule it doesn't actually save that value. All other field types I have tried work.
Comment #39
kepford CreditAttribution: kepford at Mediacurrent commentedSame story for Paragraphs field.
Comment #40
DamienMcKennaGuess it needs some additional logic to handle these field types.
Comment #41
maximpodorov CreditAttribution: maximpodorov commentedMaybe comment #26 can be helpful.
Comment #42
marcvangendHere's an improved (IMHO) version of the patch from #31.
The Path Breadcrumbs module uses ctools access plugins to decide if a breadcrumb pattern should be activated on a given page. Compared to Panels, Path Breadcrumbs seems to store its configuration in a slightly different way, because the $conf array that gets passed to
ctools_entity_field_value_ctools_access_check()
looks like this for example:In the example above, $field_name was configured as "field_section". As you can see, the $conf[$field_name] doesn't contain actual field values, but it does exist. That's why the if-statement from the original patch didn't work as expected:
My patch changes that to:
This should be more tolerant towards different modules and their configuration. I can't think of a situation where this would have an adverse effect.
Comment #43
Anonymous (not verified) CreditAttribution: Anonymous commentedDoes #42 address #38?
Comment #44
marcvangend@vilepickle Possibly, but I dont know, I didn't test. Perhaps you can try it and report back?
Comment #45
DamienMcKennaThis wasn't added to 1.11.
Comment #46
RogerRogers CreditAttribution: RogerRogers commentedHighly recommend this module: https://www.drupal.org/project/panels_substitutions_access
It solves this problem and so many others. Get it. Use it. Love it.
Comment #47
MichelleTrying to test #42 but I am not seeing any option added to selection rules for a field being empty. I've looked through the patch and can't see where it's being added. If someone could add repro steps for this, I can give another go at testing.
Comment #48
marcvangend@Michelle If I recall correctly, there is no new option. Instead, you should be able to use the existing Entity: Field Value option which should, thanks to the patch, also allow entering an empty value.
Comment #49
rivimeyThis patch, and related 1955954, has a long history... is it really ready for prime time? I would worry about two aspects before it was committed:
- there are apparently known cases both patches don't work. If true, perhaps we can avoid additional coding with suitable docs and help messages to indicate where the use-case boundaries are? It would be counter-productive to half-fix these issues.
- more for 1955954 than this, but possibly people are relying on current behaviour in either case, and this change could break sites. Is that very likely, not likely, impossible? If likely, it would seem to me that creating a new thing with the not-broken code and leaving the broken code alone would be the right path, with suitable "deprecated" messages in the help and code comments.
Also, what is the relevance of #46? Is panels_substitutions_access a reimplementation of this code, or something else? If reimplementation, perhaps we can simply deprecate this file entirely and redirect to the apparently-working module.
Finally, ctools as a whole needs many more tests, and given all the comments here, that this file definitely needs its share of tests.
Comment #50
MichelleBased on marcvangend's answer in #48, I tested the patch in #42.
- When using the selection criteria with the "body" field, it functions as expected. If I leave the field blank in the popup, the selection rule becomes "Content article: Body Field - Body is empty". If I check the "Reverse" box, it becomes "Content article: Body Field - NOT (Body is empty)". When I add or remove body content, the proper variant shows up to match.
- When using an image field, however, I still see the same issues noted in #38, that the "Reverse" checkbox has no effect and that the selection rule doesn't work.
Comment #51
philsward CreditAttribution: philsward commentedPlease push to get
#2501751: Visibility rules aren't saved for unlimited multiple value fields
#2504465: Visibility rules aren't saved for unlimited multiple value fields
committed so we can focus on THIS issue.
I just tested #42 and it did exactly what I needed which was to NOT show an empty value. Both of the visibility patches MUST be installed first in order for this patch to work (for panels)
Can we at the very least, get #42 committed and deal with the residual issues in a new one?
I was unable to re-produce Michelle's issue against an image field. I tested it with a visibility rule that is tied to an unlimited multivalue entity reference with an empty/null value, set to NOT. It saved it and worked as expected. Let's move to RTBC!! :D
@Michelle, maybe your caches weren't cleared? Maybe the page wasn't refreshed after a cache clear?
Comment #52
philsward CreditAttribution: philsward commentedBumping to 1.13. LET'S DO THIS!!
Comment #53
MichelleThat was almost 6 month ago. Don't remember. :) If you can't repro the problem then don't worry about it.
Comment #54
philsward CreditAttribution: philsward commentedLast call, anyone have any issues with marking this as RTBC?
Comment #55
philsward CreditAttribution: philsward commentedComment #56
joelpittetThis does still need tests as there are many points that could break existing sites that we want to mitigate before committing this.
Also #38 and #39 haven't been addressed. And last but code cleanup should be removed so that it doesn't distract from the active parts of this feature request.
Comment #57
DamienMcKennaThis didn't get into 1.13, maybe it will get into 1.14.
We need to try getting this to RTBC.
Comment #58
philsward CreditAttribution: philsward commented[start @joelpittet]
Mind elaborating on that statement a bit?
I want to say both #38 & #39 work fine too. It's been a while since I tested images, but I did comment in #51 that images worked just fine for me (with both patches installed).
I'm pretty confident paragraphs are working too. I use the snot out of paragraphs and don't recall running into this issue, but... I'm not sure if I've ever actually used this scenario with paragraphs to say one way or the other...
[end @joelpittet]
My vote is to get this committed and move #38 & #39 out into separate issues "if" they are actually a problem. This patch has been sitting for entirely too long (7 Years) to continue being ignored and it's a very annoying bug to deal with when you actually meet the conditions. Disappointed it's being blocked yet again, but I suppose code cleanup should be removed so that it doesn't distract from the active parts of this feature request makes enough sense to warrant it.
Comment #59
philsward CreditAttribution: philsward commentedConfirmation that this patch still applies without error on ctools-1.13
Comment #60
joelpittetThe current patch doesn't apply.
@philsward Sorry for being vague but when you change an existing function to operate different from previous you can potentially break a great many sites that are using it how it currently works. Considering this module is installed on about 1million or so sites changes can have a big impact no matter how correct they are so we must be careful.
The test coverage is hopefully to show it works as intended but also hopefully shows that it doesn't break existing functionality, and that future changes don't unfix this.
Comment #61
maximpodorov CreditAttribution: maximpodorov commentedThat's why it can be better to have a separate plugin for this (see #11, but #12 is still important).
Comment #62
philsward CreditAttribution: philsward commented@joelpittet
I should have clarified that Patch #42 applies cleanly in my environment on 1.13. I can't speak for the other patches.
Comment #63
philsward CreditAttribution: philsward commentedPatch #42 does NOT apply cleanly against 1.14
Applying anyway gives the following lines to focus on:
It appears that the items that do succeed, seem to work? No idea what the residual side-effects will be just yet...
Maybe with the fact that it works, a new patch can be setup to exclude the failed hunks and only leave in the successful ones? I'll report back if I end up with any weird problems.
Comment #64
bdone CreditAttribution: bdone as a volunteer and at Red Hat commentedhere's re-roll of #42 that (currently) applies cleanly to 1.14.
Comment #65
osopolarThanks for the re-roll. Seems that it just removes some fixes on coding standards which got applied meanwhile and shouldn't have be part of this issue anyway.
Still needs tests, see #56.
Comment #66
philsward CreditAttribution: philsward commentedThanks @bdone!
Tested #64 against 1.14 and it works as advertised with no patch errors.
Comment #67
DamienMcKennaComment #68
philsward CreditAttribution: philsward commentedTested #64 against 1.15 and it works as advertised with no patch errors.
Can we finally get this into the next stable release please? #64 has patched cleanly for the last 2 stable releases. I've been using the various patches on this issue for the last 2 years on multiple sites and have yet to run across an ill-fated effect from it.
Regarding the images problem, last time I loosely checked, it was working fine but I personally think that should be opened as a separate issue if it's still broken so we can get this one fixed.
Regarding #56, what needs testing in order to push this through?
Comment #69
cbfannin CreditAttribution: cbfannin commented#64 is working on 1.15 for me. Have been using this patch for a couple years now as well.
Comment #70
philsward CreditAttribution: philsward commentedRedacted original comment for a lack of evidence and later belief that it was environment related.
Comment #71
AnybodyConfirming that #64 fixes the problems with empty field value checks! This is still needed and works. RTBC +1
If this will never get fixed, you may want to have a look at https://www.drupal.org/project/panels/issues/2503509#comment-11966412 for a custom module which adds that check.
Comment #72
AlfTheCat CreditAttribution: AlfTheCat commentedBrilliant, thanks so much! Applies cleanly against latest dev. Using this for a selection rule on panel variants in page manager. Selecting a field based selection rule, leaving the value empty and then checking the negate/NOT box successfully triggers the panels variant when the specified field value is not empty.
Hope this patch makes it into the next release :)
Comment #73
shubham.prakash CreditAttribution: shubham.prakash at OpenSense Labs commentedComment #74
NWOM CreditAttribution: NWOM commented#64 works great! However as mentioned in #56, tests are still needed before it can be implemented.
Comment #75
Anybody#64 does not provide a solution for file / image fields. It shows the field even if the selected file / image field of the entity is empty.
https://www.drupal.org/project/panels_substitutions_access solves this problem as workaround.
Comment #76
philsward CreditAttribution: philsward commentedPatch #64 applies cleanly against 1.19
We should get this issue committed and open a new issue to address files/images. This has been open for a decade now and the non-file field fix has been working for years.
A half working fix is better than it being completely broken.
Comment #77
rivimeyI agree with @philsward, especially if a new issue is added indicating that 'is empty' doesn't apply to files, and possibly add a note in the rule description itself.
Just re-run the patch on php7.4/mysql and it patches and runs, great!
So, all that's needed now is for some tests.
Comment #78
philsward CreditAttribution: philsward commentedUpdates cleanly against v1.21 on PHP 7.4
How do we run the tests?