docs for EntityFieldQuery::fieldCondition are really sparse

A patch would be a great way to explain the changes you are suggesting.

This is not applicable to Drupal 8 anymore with the refactoring of EntityFieldQuery in #1801726: EntityFieldQuery v2.

Here's the change record: https://www.drupal.org/node/1827278.

The equivalent can now be found in core/lib/Drupal/Core/Entity/Query/QueryInterface.php.

There is an issue asking for docs cleanup of the D8 version here: #1827476: Docs cleanup for EFQ v2.

Setting version to 7.x.

Here's a first patch that just copies the docs from addFieldCondition to fieldCondition.

I'm not sure how the comparison operators work and should be described.

#4 is still relevant in D8 though, I think it makes most sense to make that a separate issue that can be backported to D7.

fixing the amsterdam sprint tag to amsterdam2014

fixing the amsterdam sprint tag to amsterdam2014

Due to #7, I think we should leave this as Drupal 8, fix the docs there, and then backport and add the other D7 stuff.

The quoted issue #1827476: Docs cleanup for EFQ v2 is really only covering the exists() method right now. Nonetheless, it looks like the Drupal 8 docs for this method have been very much cleaned up.

I therefore think this is fixed in 8.0.x and now needs backporting, but marking as Needs Review so someone else can confirm.

Agreed, the Drupal 8 docs appear to be in good shape now. So yes let's move this to Drupal 7.

1. +++ b/includes/entity.inc
   @@ -688,14 +688,36 @@
   +   *   The column that should hold the value to be matched, defined in the
   +   *   hook_field_schema() of this field. If this is omitted then the query
   +   *   will find only entities that have data in this field, using the
   +   *   entity and property conditions if there are any.
   

   This documentation is very confusing to me, as to what happens if $column is omitted... and I think we should fix it on addFieldCondition too (copied from there).

   So. Looking at the code and delving deep... I believe what it is trying to say is that if you omit $column, then all of the other parameters you pass to this function are ignored, except $field, and this call to addFieldCondition() or fieldCondition() will just be adding a condition that says that the field has a value, rather than testing the value itself.

   Can we reword these two @param docs so this is clearer?

2. +++ b/includes/entity.inc
   @@ -688,14 +688,36 @@
       *   The operator to be used to test the given value.
   +   *   Possible values:
   

   The rest of the comments in this review are small "nitpick" things that need to be fixed.

   For this spot... This should be wrapped better. Documentation lines should go out to as close to 80 characters as possible without going over.

3. +++ b/includes/entity.inc
   @@ -688,14 +688,36 @@
   +   *   two columns, 'color' and 'shape', and for entity id 1, there are two
   

   id should be ID

4. +++ b/includes/entity.inc
   @@ -688,14 +688,36 @@
   +   *   corresponding to 'red circle', however if you pass 'red' and 'circle' as
   

   punctuation:

   ... circle'; however, if ...

Patch to fix the points in #15
Thanks @jhodgdon

Looking very good! Just a couple of very small things to fix:

1. +++ b/includes/entity.inc	2016-03-11 14:34:01.420062332 -0300
   @@ -688,14 +688,36 @@
   +   *   two columns, 'color' and 'shape', and for entity ID 1, there are two ¶
   

   nitpick: There is an extra space at the end of this line.

   You should be able to set up your code editor to either remove or highlight end-of-line spaces.

2. +++ b/includes/entity.inc	2016-03-11 14:34:01.420062332 -0300
   @@ -791,7 +815,7 @@
   -   *   two columns, 'color' and 'shape', and for entity id 1, there are two
   +   *   two columns, 'color' and 'shape', and for ID 1, there are two
   

   The word "entity" got taken out of this line by mistake I think?

Hi jhodgdon,
I have recreated a patch for #19.
Thanks

@Girish-jerk please update with interdiff.

New patch with interdiff.

The test failure is unrelated to the patch, so ignore that. The patch looks good. Thanks!

Marked for commit, thanks!

Looks a lot better, thanks!

+++ b/includes/entity.inc
@@ -688,14 +688,36 @@ class EntityFieldQuery {
+   *   conditions, it will appear in the  results - by default queries will run

To be fixed on commit: double space, and s/-/-- (seems we don't do — in docs ;)

Committed to 7.x - thanks!

I fixed #27 on commit, but also fixed it in the original location (since this part of the comment was just copied from the original on addFieldCondition()):

diff --git a/includes/entity.inc b/includes/entity.inc
index ed6070f..e80ce3b 100644
--- a/includes/entity.inc
+++ b/includes/entity.inc
@@ -714,7 +714,7 @@ class EntityFieldQuery {
    *   two columns, 'color' and 'shape', and for entity ID 1, there are two
    *   values: red/square and blue/circle. Entity ID 1 does not have values
    *   corresponding to 'red circle'; however if you pass 'red' and 'circle' as
-   *   conditions, it will appear in the  results - by default queries will run
+   *   conditions, it will appear in the results -- by default queries will run
    *   against any combination of deltas. By passing the conditions with the
    *   same $delta_group it will ensure that only values attached to the same
    *   delta are matched, and entity 1 would then be excluded from the results.
@@ -818,7 +818,7 @@ class EntityFieldQuery {
    *   two columns, 'color' and 'shape', and for entity ID 1, there are two
    *   values: red/square and blue/circle. Entity ID 1 does not have values
    *   corresponding to 'red circle', however if you pass 'red' and 'circle' as
-   *   conditions, it will appear in the  results - by default queries will run
+   *   conditions, it will appear in the results -- by default queries will run
    *   against any combination of deltas. By passing the conditions with the
    *   same $delta_group it will ensure that only values attached to the same
    *   delta are matched, and entity 1 would then be excluded from the results.