From e7f129a6c80dcb620e9dab6850ecd65cd6d0485a Mon Sep 17 00:00:00 2001 From: Peter Amiri Date: Mon, 5 Oct 2026 07:55:34 -0700 Subject: [PATCH] fix(model): read the sort direction in an order string whatever its case $orderByClause() checked for " ASC" / " DESC" with case-sensitive Find(), so a lowercase direction was missed and " ASC" was appended: order="id desc" sorted ascending, and a "table.column desc" item became "table.column desc ASC" and threw Wheels.InvalidOrderClause. The query builder's orderBy("id", "desc") reached the same code. Each order item now goes through $normalizeOrderItem(): whitespace runs (spaces, tabs) collapse to one space, a trailing asc/desc is read in any case and emitted in upper case, and an item without one sorts ascending. The query builder stores the direction in upper case too. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Peter Amiri --- changelog.d/order-direction-case.fixed.md | 1 + vendor/wheels/model/query/QueryBuilder.cfc | 2 +- vendor/wheels/model/sql.cfm | 20 ++++-- .../specs/model/orderDirectionCaseSpec.cfc | 64 +++++++++++++++++++ 4 files changed, 82 insertions(+), 5 deletions(-) create mode 100644 changelog.d/order-direction-case.fixed.md create mode 100644 vendor/wheels/tests/specs/model/orderDirectionCaseSpec.cfc diff --git a/changelog.d/order-direction-case.fixed.md b/changelog.d/order-direction-case.fixed.md new file mode 100644 index 0000000000..b7b8125b88 --- /dev/null +++ b/changelog.d/order-direction-case.fixed.md @@ -0,0 +1 @@ +- A lowercase or mixed-case sort direction in an `order` string is now honoured: `order="createdAt desc"` (or `Desc`, or with extra spaces or a tab before it) sorts descending in `findAll()`, paginated finders, `table.column` items and the query builder's `orderBy()`. It used to sort ascending, and a `table.column desc` item threw `Wheels.InvalidOrderClause` diff --git a/vendor/wheels/model/query/QueryBuilder.cfc b/vendor/wheels/model/query/QueryBuilder.cfc index 6af8285ca7..8c8b294280 100644 --- a/vendor/wheels/model/query/QueryBuilder.cfc +++ b/vendor/wheels/model/query/QueryBuilder.cfc @@ -168,7 +168,7 @@ component output="false" { public any function orderBy(required string property, string direction = "ASC") { $validatePropertyName(arguments.property); $validateDirection(arguments.direction); - ArrayAppend(variables.orderClauses, "#arguments.property# #arguments.direction#"); + ArrayAppend(variables.orderClauses, "#arguments.property# #UCase(arguments.direction)#"); return this; } diff --git a/vendor/wheels/model/sql.cfm b/vendor/wheels/model/sql.cfm index 7312e4e603..f1b13b91eb 100644 --- a/vendor/wheels/model/sql.cfm +++ b/vendor/wheels/model/sql.cfm @@ -238,6 +238,21 @@ return arguments.sql; } + /** + * Internal function. One `order` item as ` ASC` or ` DESC`: whitespace runs (spaces, + * tabs) become single spaces, and a trailing direction is read whatever its case and emitted in + * upper case. An item with no direction sorts ascending. A lowercase direction used to be + * missed, so `order="id desc"` sorted ascending. + */ + public string function $normalizeOrderItem(required string item) { + local.text = Trim(ReReplace(arguments.item, "\s+", " ", "all")); + local.last = ListLast(local.text, " "); + if (ListLen(local.text, " ") > 1 && ListFindNoCase("ASC,DESC", local.last)) { + return Left(local.text, Len(local.text) - Len(local.last) - 1) & " " & UCase(local.last); + } + return local.text & " ASC"; + } + /** * Internal function. */ @@ -258,10 +273,7 @@ local.orderArray = ListToArray(arguments.order); local.iEnd = ArrayLen(local.orderArray); for (local.i = 1; local.i <= local.iEnd; local.i++) { - local.iItem = Trim(local.orderArray[local.i]); - if (!Find(" ASC", local.iItem) && !Find(" DESC", local.iItem)) { - local.iItem &= " ASC"; - } + local.iItem = $normalizeOrderItem(local.orderArray[local.i]); if (Find("(", local.iItem)) { // Reject raw SQL expressions — calculated properties should be referenced by name local.property = Trim(SpanExcluding(local.iItem, " ")); diff --git a/vendor/wheels/tests/specs/model/orderDirectionCaseSpec.cfc b/vendor/wheels/tests/specs/model/orderDirectionCaseSpec.cfc new file mode 100644 index 0000000000..0065e27449 --- /dev/null +++ b/vendor/wheels/tests/specs/model/orderDirectionCaseSpec.cfc @@ -0,0 +1,64 @@ +/** + * The sort direction in an `order` string is read whatever its case or the whitespace before it: + * `desc`, `Desc` and `DESC` all sort descending, through findAll(), paginated finders, dot-notation + * items and the query builder's orderBy(). A lowercase direction used to sort ascending. + */ +component extends="wheels.WheelsTest" { + + function beforeAll() { + variables.g = application.wo; + var ids = variables.g.model("author").findAll(select = "id", order = "id ASC", returnAs = "query"); + variables.minId = ids.id[1]; + variables.maxId = ids.id[ids.recordCount]; + } + + function firstId(required query result) { + return arguments.result.id[1]; + } + + function run() { + + describe("The direction in an order string", () => { + + it("sorts descending for desc in any case", () => { + for (var dir in ["DESC", "desc", "Desc", "dEsC"]) { + expect(firstId(variables.g.model("author").findAll(order = "id #dir#", maxRows = 1))).toBe(variables.maxId, "order=""id #dir#"""); + } + }); + + it("sorts ascending for asc in any case, and when no direction is given", () => { + for (var dir in ["ASC", "asc", "Asc", ""]) { + expect(firstId(variables.g.model("author").findAll(order = Trim("id #dir#"), maxRows = 1))).toBe(variables.minId, "order=""id #dir#"""); + } + }); + + it("reads the direction after repeated spaces or a tab", () => { + expect(firstId(variables.g.model("author").findAll(order = "id desc", maxRows = 1))).toBe(variables.maxId); + expect(firstId(variables.g.model("author").findAll(order = "id#Chr(9)#desc", maxRows = 1))).toBe(variables.maxId); + expect(firstId(variables.g.model("author").findAll(order = " id desc ", maxRows = 1))).toBe(variables.maxId); + }); + + it("reads each item's direction in a multi-column order", () => { + var rows = variables.g.model("author").findAll(order = "lastName asc, id desc", returnAs = "query"); + var expected = variables.g.model("author").findAll(order = "lastName ASC, id DESC", returnAs = "query"); + expect(ValueList(rows.id)).toBe(ValueList(expected.id)); + }); + + it("reads a lowercase direction on a table.column item", () => { + var tableName = variables.g.model("author").tableName(); + expect(firstId(variables.g.model("author").findAll(order = "#tableName#.id desc", maxRows = 1))).toBe(variables.maxId); + }); + + it("reads a lowercase direction in a paginated finder", () => { + expect(firstId(variables.g.model("author").findAll(order = "id desc", page = 1, perPage = 2))).toBe(variables.maxId); + }); + + it("reads a lowercase direction passed to the query builder's orderBy()", () => { + expect(firstId(variables.g.model("author").orderBy("id", "desc").limit(1).get())).toBe(variables.maxId); + }); + + }); + + } + +}