https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Entity%21...

The documentation here says that "The size of the conditional is the size of its conditional array minus one, because one element is the conjunction." After getting some unexpected results though I stepped through the code and found that the conjunction is not in the conditions array that the count function is going through (please see screenshot).

As such to get the expected results we had to use the conditions() function to get the array of conditions and pass them through count() ourselves.

CommentFileSizeAuthor
#18 2961285-18-entity-query-condition-count-faulty.patch2.53 KBpavlosdan
#7 2961285-6-entity-query-condition-count.patch2.19 KBpavlosdan
#5 2961285-5-entity-query-condition-count.patch2.28 KBpavlosdan
#2 2961285-2-entity-query-condition-count.patch2.28 KBpavlosdan
Screen Shot 2018-04-16 at 9.39.09 AM.png60.96 KBpavlosdan
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

pavlosdan created an issue. See original summary.

pavlosdan’s picture

pavlosdan CreditAttribution: pavlosdan commented
FileSize
2961285-2-entity-query-condition-count.patch2.28 KB

Please find a patch with a test for this.

pavlosdan’s picture

pavlosdan CreditAttribution: pavlosdan commented
Status: Active » Needs review

Updating status.

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
CreditAttribution: borisson_ as a volunteer commented
Status: Needs review » Needs work

There is unneeded whitespace added in this patch that shouldn't be added.

pavlosdan’s picture

pavlosdan CreditAttribution: pavlosdan as a volunteer commented
Status: Needs work » Needs review
FileSize
2961285-5-entity-query-condition-count.patch2.28 KB

Updated patch to remove unneeded whitespace.

Status: Needs review » Needs work

The last submitted patch, 5: 2961285-5-entity-query-condition-count.patch, failed testing. View results

pavlosdan’s picture

pavlosdan CreditAttribution: pavlosdan as a volunteer commented
Status: Needs work » Needs review
FileSize
2961285-6-entity-query-condition-count.patch2.19 KB

Adjusting the test.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

pavlosdan’s picture

pavlosdan CreditAttribution: pavlosdan as a volunteer commented
FileSize
2961285-9-entity-query-condition-count-just-test.patch1.09 KB

Attaching patch with just the test. This should fail.

Status: Needs review » Needs work

The last submitted patch, 9: 2961285-9-entity-query-condition-count-just-test.patch, failed testing. View results

pavlosdan’s picture

pavlosdan CreditAttribution: pavlosdan as a volunteer commented
Version: 8.7.x-dev » 8.6.x-dev
Status: Needs work » Needs review

The test failed as expected. Setting back to needs review for the patch in #7

hchonov’s picture

hchonov
Primary language Bulgarian
Location 🇪🇺🇩🇪🇧🇬
CreditAttribution: hchonov as a volunteer commented

I've took a look and I guess this is just a copy paste left over, as for normal DB queries this is still the case, but entity queries deal with the conjunction differently.

@pavlosdan, could you please re-upload the patch from #7 and only add a test for "OR" condition groups? It is clear that those are broken as well, but with only couple of more lines we get a complete coverage. Beside that this is ready to go for me.

hchonov’s picture

hchonov
Primary language Bulgarian
Location 🇪🇺🇩🇪🇧🇬
CreditAttribution: hchonov as a volunteer commented
Title: Entity query condition count seems off » Entity query condition count is faulty
pavlosdan’s picture

pavlosdan CreditAttribution: pavlosdan as a volunteer commented
FileSize
2961285-14-entity-query-condition-count-is-faulty.patch2.84 KB

Thanks @hchonov. Adjusting the tests to test the count of AND condition groups and OR condition groups.

voleger’s picture

voleger
he/his
Primary language Ukrainian
Location Ukraine, Rivne
CreditAttribution: voleger as a volunteer and at Drupal Ukraine Community for Drupal Ukraine Community, Lemberg Solutions commented
Title: Entity query condition count is faulty » Entity query condition count seems off
Version: 8.6.x-dev » 8.7.x-dev
Status: Needs review » Reviewed & tested by the community

Looks good

voleger’s picture

voleger
he/his
Primary language Ukrainian
Location Ukraine, Rivne
CreditAttribution: voleger as a volunteer and at Drupal Ukraine Community for Drupal Ukraine Community, Lemberg Solutions commented
Title: Entity query condition count seems off » Entity query condition count is faulty
hchonov’s picture

hchonov
Primary language Bulgarian
Location 🇪🇺🇩🇪🇧🇬
CreditAttribution: hchonov as a volunteer commented
Status: Reviewed & tested by the community » Needs work

I am sorry, but we don't really need two test methods for this. Could you please put them in the same test method? I am sorry if my previous comment misled you, I should've explicitly mention this.

pavlosdan’s picture

pavlosdan CreditAttribution: pavlosdan as a volunteer commented
FileSize
2961285-18-entity-query-condition-count-faulty.patch2.53 KB

Apologies for misunderstanding. Merged the 2 methods :)

pavlosdan’s picture

pavlosdan CreditAttribution: pavlosdan as a volunteer commented
Status: Needs work » Needs review
hchonov’s picture

hchonov
Primary language Bulgarian
Location 🇪🇺🇩🇪🇧🇬
CreditAttribution: hchonov as a volunteer commented
Status: Needs review » Reviewed & tested by the community

Thank you.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed a4208fd3c8 to 8.7.x and 3f42b03bcf to 8.6.x. Thanks!

Backported to 8.6.x as a non-disruptive bugfix.

Checking changed files...
PHPCS: core/lib/Drupal/Core/Entity/Query/ConditionFundamentals.php passed
PHPCS: core/lib/Drupal/Core/Entity/Query/ConditionInterface.php passed
git pre-commit check failed: file core/tests/Drupal/KernelTests/Core/Entity/EntityQueryTest.php should be 644 not 755

FILE: ...al/core/tests/Drupal/KernelTests/Core/Entity/EntityQueryTest.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 575 | ERROR | [x] Line indented incorrectly; expected 4 spaces,
     |       |     found 5
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------

A couple of coding standard issues fixed on commit.

  • alexpott committed a4208fd on 8.7.x 
    Issue #2961285 by pavlosdan, hchonov, borisson_: Entity query condition...

  • alexpott committed 3f42b03 on 8.6.x 
    Issue #2961285 by pavlosdan, hchonov, borisson_: Entity query condition...

Status: Fixed » Closed (fixed)

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