There are 3 orderBy methods available for EntityFieldQuery. All support ordering ASC or DESC (ascending or descending), much like the orderBy method of SelectQuery. However, quite unlike SelectQuery it doesn't default to ASC but requires you to enter a direction. This is silly. I have spoken with chx and bojanz and both agree that it is silly and adding a default of ASC to those methods would harm nothing but improve DX.

Let's do that. :-)

CommentFileSizeAuthor
#19 903698.patch1.06 KBbojanz
#16 entityfieldquery-OrderByASC.patch1.05 KBjpstrikesback
#6 903698.patch1.06 KBbuzzybron
#3 903698.patch0 bytesbuzzybron
#1 entityfieldquery-OrderByASC.patch1.05 KBjpstrikesback

Comments

jpstrikesback’s picture

jpstrikesback commented
StatusFileSize
new entityfieldquery-OrderByASC.patch1.05 KB

How's this?

jpstrikesback’s picture

jpstrikesback commented
Status: Active » Needs review
buzzybron’s picture

buzzybron commented
Version: 7.x-dev » 7.0-alpha6
Category: bug » task
StatusFileSize
new 903698.patch0 bytes

Did the patch. I guess it is the same as jpstrikesback.

Virgin patch. Please let me know if anything is wrong at all. Thank you!

buzzybron’s picture

buzzybron commented
Version: 7.0-alpha6 » 7.x-dev

Pardon, this should be correct file. Please ignore previous empty patch file.

bojanz’s picture

bojanz commented
Version: 7.x-dev » 7.0-alpha6
Status: Needs review » Reviewed & tested by the community

#1 is green, and matches the EntityFieldQuery behavior with the SelectQuery behavior.

RTBC.

buzzybron’s picture

buzzybron commented
Version: 7.0-alpha6 » 7.x-dev
Status: Reviewed & tested by the community » Needs review
StatusFileSize
new 903698.patch1.06 KB
bojanz’s picture

bojanz commented
Status: Needs review » Reviewed & tested by the community

So, #1.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 903698.patch, failed testing.

bojanz’s picture

bojanz commented
Status: Needs work » Reviewed & tested by the community
sun’s picture

sun
Primary language German
Location Karlsruhe
commented

#6: 903698.patch queued for re-testing.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Reviewed & tested by the community » Needs work 
+++ b/includes/entity.inc
@@ -708,7 +708,7 @@ class EntityFieldQuery {
+  public function entityOrderBy($name, $direction = "ASC") {

@@ -730,7 +730,7 @@ class EntityFieldQuery {
+  public function fieldOrderBy($field, $column, $direction = "ASC") {

@@ -763,7 +763,7 @@ class EntityFieldQuery {
+  public function propertyOrderBy($column, $direction = "ASC") {

Should use single quotes.

Powered by Dreditor.

bojanz’s picture

bojanz commented
Status: Needs work » Reviewed & tested by the community

Once again, #1 is RTBC. And it has single quotes (sun, you're looking at the wrong patch).

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 903698.patch, failed testing.

bojanz’s picture

bojanz commented
Status: Needs work » Reviewed & tested by the community

I really want every patch after #1 to disappear.

bojanz’s picture

bojanz commented
Issue tags: +Quick fix

Calling #1 trivial would be an overstatement.

jpstrikesback’s picture

jpstrikesback commented
StatusFileSize
new entityfieldquery-OrderByASC.patch1.05 KB

here it is again, same as the patch in #1 :P

jpstrikesback’s picture

jpstrikesback commented
Status: Reviewed & tested by the community » Needs review
jpstrikesback’s picture

jpstrikesback commented
Status: Needs review » Reviewed & tested by the community
bojanz’s picture

bojanz commented
StatusFileSize
new 903698.patch1.06 KB

No longer applies, here's a reroll.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

This seems like a straight-forward oversight in the API, and doesn't break anything.

Committed to HEAD. Thanks!

Status: Fixed » Closed (fixed)
Issue tags: -Quick fix

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

Issue tags: +Quick fix

Restoring issue tags, see #2125755: System messages removed all issue tags during D7 upgrade.