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.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jastraat’s picture

FileSize
2.95 KB
3.42 KB

The 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.

andypost’s picture

Status: Active » Needs review

Marked as duplicate #1813992: entity_field_value doesn't handle conditions with empty values on references to the node.

+++ b/plugins/access/field_not_empty.incundefined
@@ -0,0 +1,107 @@
+function ctools_field_not_empty_ctools_access_settings($form, &$form_state, $conf) {    ¶
...
+  ¶
...
+    ¶
...
+  ¶
...
+  ¶
...
+  ¶
...
+  ¶

trailing whitespace

andypost’s picture

Status: Needs review » Needs work
+++ b/plugins/access/field_not_empty.incundefined
@@ -0,0 +1,107 @@
+    $info = field_info_field($conf['field']);
+    $emptyfunc = $info['module'] . '_field_is_empty';
+    if(function_exists($emptyfunc)) {
+      foreach ($items as $item) {
+        //If any item is not empty, return TRUE
+        if(!$emptyfunc($item, $conf['field'])) {

$conf['field'] should be replaced with $info

+++ b/plugins/access/field_not_empty.incundefined
@@ -0,0 +1,107 @@
+      return FALSE;

not needed

maximpodorov’s picture

Status: Needs work » Active
FileSize
1.63 KB

Ready to use module (for the testing purposes).

maximpodorov’s picture

The updated patch to review.

maximpodorov’s picture

Status: Active » Needs review
FileSize
3.13 KB

Spaces are deleted. :)

maximpodorov’s picture

Spaces are deleted. :)

andypost’s picture

+++ b/plugins/access/field_not_empty.incundefined
@@ -0,0 +1,106 @@
+  if (empty($context) || !isset($conf['field']) || empty($conf['field']) || empty($context->data)) {

!isset($conf['field']) is not needed
empty($conf['field']) is enough here

+++ b/plugins/access/field_not_empty.incundefined
@@ -0,0 +1,106 @@
+    return t('Missing/ deleted field "@field"', array('@field' => $field));

space before deleted should be removed

maximpodorov’s picture

The patch with the fixes of issues above.

andypost’s picture

Status: Needs review » Needs work
+++ b/plugins/access/field_not_empty.incundefined
@@ -0,0 +1,106 @@
+  foreach ($fields = field_info_fields() as $name => $field) {
+    if ($field['active'] && !$field['deleted']) {

field_info_fields() already returns none-deleted fields.
Also I'd like to see field_info_field_map() instead field_info_fields() here

+++ b/plugins/access/field_not_empty.incundefined
@@ -0,0 +1,106 @@
+  if(!isset($entity->$conf['field'])) {
...
+  if(empty($entity->$conf['field'])) {
...
+    if(function_exists($emptyfunc)) {
...
+        if(!$emptyfunc($item, $info)) {

needs space after if

+++ b/plugins/access/field_not_empty.incundefined
@@ -0,0 +1,106 @@
+  if (!isset($conf['field'])) {
+    $conf['field'] = '';
+  }
+  $fields = array();
+
+  // Only include active fields in the list.
+  foreach (field_info_fields() as $name => $field) {
+    if ($field['active'] && !$field['deleted']) {
+      $fields[$name] = $name;
+    }
+  }
+
+  $field = $conf['field'];
+
+  if (!empty($field) && !empty($fields[$field])) {
+    return t('@identifier @field is not empty', array('@field' => $field, '@identifier' => $context->identifier));
+  }
+  else {
+    return t('Missing/deleted field "@field"', array('@field' => $field));

Should be changed to

if (!empty($conf['field']) && field_info_field($conf['field']) {
// identifier...

maximpodorov’s picture

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

Here is a completely different approach to this task (based on entity_field_value.inc).

maximpodorov’s picture

It'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).

kopeboy’s picture

Can we get this going please?
I really need this functionality and I am sure many other site builders would benefit!

maximpodorov’s picture

#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.

maximpodorov’s picture

FileSize
6.52 KB

dpm() call is removed.
The uncertain point is multilingual fields. We have to decide what to do with them.

kristiaanvandeneynde’s picture

@maximpodorov What would be the issue with multilingual fields? I might be able to update the patch.

maximpodorov’s picture

The 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.

kristiaanvandeneynde’s picture

Doesn't it just suffice to check in the current language? Something field_get_items() does already?

maximpodorov’s picture

You have two stages: configuration time, check time.

kristiaanvandeneynde’s picture

Ok, so you're saying someone should be able to configure the rule as:

  • "Field has value (EN): Cat"
  • "Field has value (NL): Kat"
  • "Field has value (FR): Chat"
  • "Field has value (DE): Katze"

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.

maximpodorov’s picture

We have to decide what "empty field" means: "completely empty", "empty in the current language", or "empty in the current and default language".

kristiaanvandeneynde’s picture

Isn'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.

osopolar’s picture

Patch in #16 works for me.

1+ for committing this and creating a follow up patch to discuss the problem for multilingual fields.

annya’s picture

Patch in #16 works for me too.

maximpodorov’s picture

partdigital’s picture

Patch in #16 works for me as well.

crmn’s picture

i 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.

kristiaanvandeneynde’s picture

While 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 :)

amaisano’s picture

The 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.

DamienMcKenna’s picture

FileSize
7.14 KB

This fixes the error in the last patch.

DamienMcKenna’s picture

This seems pretty safe.

maximpodorov’s picture

What about multilingual issues (see #18)?

Anonymous’s picture

We've been using the patch in #31 for some time.

Anonymous’s picture

Status: Needs review » Reviewed & tested by the community
B-Prod’s picture

Thanks, the patch works fine!

DamienMcKenna’s picture

This wasn't added to 7.x-1.10, bumping it to 7.x-1.11.

kepford’s picture

This 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.

kepford’s picture

Same story for Paragraphs field.

DamienMcKenna’s picture

Status: Reviewed & tested by the community » Needs work

Guess it needs some additional logic to handle these field types.

maximpodorov’s picture

Maybe comment #26 can be helpful.

marcvangend’s picture

Here'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:

$conf = array (
  'field_section' => array (
    'und' => array (),
  ),
  'field_section_tid' => array (),
)

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:

  // Field is empty.
  if (!isset($conf[$field_name])) {
    // Return TRUE if field is empty and the configuration is missing.
    return TRUE;
  }

My patch changes that to:

  // Field is empty.
  if (empty($conf[$field_name]) || empty($conf[$field_name][$langcode])) {
    // Return TRUE if field is empty and the configuration is missing.
    return TRUE;
  }

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.

Anonymous’s picture

Does #42 address #38?

marcvangend’s picture

@vilepickle Possibly, but I dont know, I didn't test. Perhaps you can try it and report back?

DamienMcKenna’s picture

This wasn't added to 1.11.

RogerRogers’s picture

Highly 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.

Michelle’s picture

Trying 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.

marcvangend’s picture

@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.

rivimey’s picture

Issue tags: +Needs tests

This 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.

Michelle’s picture

Based 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.

philsward’s picture

Please 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?

philsward’s picture

Bumping to 1.13. LET'S DO THIS!!

Michelle’s picture

That was almost 6 month ago. Don't remember. :) If you can't repro the problem then don't worry about it.

philsward’s picture

Last call, anyone have any issues with marking this as RTBC?

philsward’s picture

Status: Needs work » Reviewed & tested by the community
joelpittet’s picture

Status: Reviewed & tested by the community » Needs work

This 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.

DamienMcKenna’s picture

This didn't get into 1.13, maybe it will get into 1.14.

We need to try getting this to RTBC.

philsward’s picture

[start @joelpittet]

there are many points that could break existing sites

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.

philsward’s picture

Confirmation that this patch still applies without error on ctools-1.13

joelpittet’s picture

The 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.

maximpodorov’s picture

That's why it can be better to have a separate plugin for this (see #11, but #12 is still important).

philsward’s picture

@joelpittet

I should have clarified that Patch #42 applies cleanly in my environment on 1.13. I can't speak for the other patches.

philsward’s picture

Patch #42 does NOT apply cleanly against 1.14

patch -p1 < ctools-n1188466-42.patch
patching file plugins/access/entity_field_value.inc
Reversed (or previously applied) patch detected!

Applying anyway gives the following lines to focus on:

Hunk #1 FAILED at 68.
Hunk #2 FAILED at 84.
Hunk #4 succeeded at 162 (offset 1 line).
Hunk #5 FAILED at 176.
Hunk #6 succeeded at 204 (offset 1 line).
Hunk #7 succeeded at 243 (offset 1 line).
Hunk #8 succeeded at 271 (offset 1 line).
Hunk #9 succeeded at 308 (offset 1 line).
Hunk #10 succeeded at 344 (offset 1 line).
Hunk #11 FAILED at 387.
Hunk #12 FAILED at 415.

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.

bdone’s picture

Status: Needs work » Needs review
FileSize
5.49 KB
3.35 KB

here's re-roll of #42 that (currently) applies cleanly to 1.14.

osopolar’s picture

Status: Needs review » Needs work

Thanks 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.

philsward’s picture

Thanks @bdone!

Tested #64 against 1.14 and it works as advertised with no patch errors.

DamienMcKenna’s picture

philsward’s picture

Tested #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?

cbfannin’s picture

#64 is working on 1.15 for me. Have been using this patch for a couple years now as well.

philsward’s picture

FileSize
39.3 KB

Redacted original comment for a lack of evidence and later belief that it was environment related.

Anybody’s picture

Confirming 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.

AlfTheCat’s picture

Brilliant, 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 :)

shubham.prakash’s picture

Status: Needs work » Needs review
NWOM’s picture

Status: Needs review » Needs work

#64 works great! However as mentioned in #56, tests are still needed before it can be implemented.

Anybody’s picture

#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.

philsward’s picture

Patch #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.

rivimey’s picture

I 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.

philsward’s picture

Updates cleanly against v1.21 on PHP 7.4

How do we run the tests?