fix(model): read the sort direction in an order string whatever its case

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`

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;
 	}
 

vendor/wheels/model/sql.cfm

@@ -238,6 +238,21 @@
 		return arguments.sql;
 	}
 
+	/**
+	 * Internal function. One `order` item as `<item> ASC` or `<item> 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, " "));

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);
+			});
+
+		});
+
+	}
+
+}