part: fix, but changed some test cases which could break in production,

#42

src/format_request.js

@@ -4,7 +4,6 @@ const DareError = require('./utils/error');
 const fieldReducer = require('./utils/field_reducer');
 const groupbyReducer = require('./utils/groupby_reducer');
 const orderbyReducer = require('./utils/orderby_reducer');
-const orderbyUnwrap = require('./utils/orderby_unwrap');
 const checkKey = require('./utils/validate_field');
 const checkTableAlias = require('./utils/validate_alias');
 const formatDateTime = require('./utils/format_datetime');
@@ -167,10 +166,8 @@ async function format_specs(options) {
 	// Orderby
 	// If the content is ordered
 	if (options.orderby) {
-
 		// Reduce
-		options.orderby = toArray(options.orderby).reduce(orderbyReducer(options.field_alias_path, joined), []);
-
+		options.orderby = toArray(options.orderby).reduce(orderbyReducer(options.field_alias_path || `${options.alias }.`, joined), []);
 	}
 
 	// Update the joined tables
@@ -441,25 +438,3 @@ function toArray(a) {
 	}
 	return a;
 }
-
-
-// Find a field alias which matches the orderby definition
-function replaceAlias(entry, fields) {
-
-	// If this isn't a string the error will get handled elsewhere anyway
-	if (typeof entry === 'string') {
-
-		// Split into field_definition and directions
-		const {field} = orderbyUnwrap(entry);
-
-		// Loop through the local fields and match content which contains a matching reference key
-		for (const _field of fields) {
-			if (typeof _field === 'object' && _field.hasOwnProperty(field)) {
-				return _field[field];
-			}
-		}
-	}
-
-	// There is no alias for this, just return.
-	return entry;
-}

src/get.js

@@ -60,7 +60,7 @@ function buildQuery(opts) {
 		sql_joins,
 		sql_join_values,
 		sql_filter,
-		sql_groupby,
+		groupby,
 		orderby,
 		sql_values,
 	} = this.traverse(opts, is_subquery);
@@ -77,6 +77,15 @@ function buildQuery(opts) {
 				item.label = '_count';
 				item.agg = true;
 			});
+
+		// Find the special _group column...
+		fields
+			.filter(item => item.expression === `${sql_alias}._group`)
+			.forEach(item => {
+				// Pick the first_groupby statement
+				item.expression = groupby[0].expression;
+				item.label = '_group';
+			});
 	}
 
 	// Conditions
@@ -96,14 +105,14 @@ function buildQuery(opts) {
 	// Groupby
 	// If the content is grouped
 	// Ensure that the parent has opts.groupby when we're joining tables
-	if (!is_subquery && !sql_groupby.length && has_many_join) {
+	if (!is_subquery && !groupby.length && has_many_join) {
 
 		// Are all the fields aggregates?
 		const all_aggs = fields.every(item => item.agg);
 
 		if (!all_aggs) {
 			// Determine whether there are non?
-			sql_groupby.push(`${opts.sql_alias}.id`);
+			groupby.push({expression: `${opts.sql_alias}.id`});
 		}
 	}
 
@@ -129,38 +138,10 @@ function buildQuery(opts) {
 	}
 
 	// Clean up sql_orderby
-	let sql_orderby = [];
-
-	if (orderby && orderby.length) {
+	const sql_orderby = aliasOrderAndGroupFields(orderby, fields);
 
-		console.log('ORDERBY haystack', orderby, fields);
-
-		sql_orderby = orderby.map(({expression, label, direction}) => {
-
-			// _count, etc...
-			// Is the value a shortcut to a labelled field?
-			// fields.find(_field => {
-			// 	if (_field.label && _field.label === expression) {
-			// 		return entry;
-			// 	}
-			// });
-
-			for (const field of fields) {
-
-				// Does the expression belong to something in the fields?
-				if (label && (field.label === label)) {
-					return [`\`${field.label}\``, direction].join(' ');
-				}
-				if (field.label && field.expression === expression) {
-					return [`\`${field.label}\``, direction].join(' ');
-				}
-			}
-
-			return [expression, direction].join(' ');
-		});
-
-		console.log(sql_orderby);
-	}
+	// Clean up sql_orderby
+	const sql_groupby = aliasOrderAndGroupFields(groupby, fields);
 
 
 	// Put it all together
@@ -197,7 +178,7 @@ function traverse(item, is_subquery) {
 	const sql_join_values = [];
 
 	// SQL GroupBy
-	const sql_groupby = [];
+	const groupby = [];
 
 	// SQL GroupBy
 	const orderby = [];
@@ -208,7 +189,7 @@ function traverse(item, is_subquery) {
 	const resp = {
 		sql_filter,
 		sql_values,
-		sql_groupby,
+		groupby,
 		orderby,
 		fields,
 		list,
@@ -399,15 +380,7 @@ function traverse(item, is_subquery) {
 	if (item.groupby) {
 
 		// Either an empty groupby
-		sql_groupby.push(...item.groupby.map(field => field_format(field, null, sql_alias).expression));
-
-		// Find the special _group column...
-		fields
-			.filter(item => item.expression === `${sql_alias}._group`)
-			.forEach(item => {
-				item.expression = sql_groupby[0];
-				item.label = '_group';
-			});
+		groupby.push(...item.groupby.map(field => field_format(field, null, sql_alias, item.field_alias_path)));
 	}
 
 	// Orderby
@@ -477,4 +450,36 @@ function formCondition(tbl_alias, field, condition) {
 	return condition.includes('??') ? condition.replace(/\?\?/g, field_definition) : `${field_definition} ${condition}`;
 }
 
+function aliasOrderAndGroupFields(arr, fields) {
+
+	if (arr && arr.length) {
+
+		return arr.map(({expression, label, direction, original}) => {
+
+			// _count, etc...
+			// Is the value a shortcut to a labelled field?
+			// fields.find(_field => {
+			// 	if (_field.label && _field.label === expression) {
+			// 		return entry;
+			// 	}
+			// });
+
+			for (const field of fields) {
+
+				// Does the expression belong to something in the fields?
+				if (field.label && (field.label === label)) {
+					expression = `\`${field.label}\``;
+					break;
+				}
+				if (field.label && field.label === original) {
+					expression = `\`${field.label}\``;
+					break;
+				}
+			}
+
+			return [expression, direction].filter(v => !!v).join(' ');
+		});
+	}
 
+	return [];
+}

src/utils/field_format.js

@@ -1,8 +1,8 @@
 const unwrap_expression = require('./unwrap_field');
 
-module.exports = function field_format(expression, label, table_prefix, label_prefix) {
+module.exports = function field_format(original, label, table_prefix, label_prefix) {
 
-	const m = unwrap_expression(expression);
+	const m = unwrap_expression(original);
 
 	const {field, prefix, suffix} = m;
 
@@ -27,7 +27,7 @@ module.exports = function field_format(expression, label, table_prefix, label_pr
 	label = label || undefined;
 
 	// Expression
-	expression = `${prefix || ''}${table_prefix}.${name}${suffix || ''}`;
+	const expression = `${prefix || ''}${table_prefix}.${name}${suffix || ''}`;
 
 	// aggregate function flag
 	let agg = false;
@@ -36,5 +36,10 @@ module.exports = function field_format(expression, label, table_prefix, label_pr
 		agg = true;
 	}
 
-	return {expression, label, agg};
+	return {
+		original,
+		expression,
+		label,
+		agg
+	};
 };

test/specs/field_format.js

@@ -68,7 +68,8 @@ describe('utils/field_format', () => {
 			expect(actual).to.eql({
 				expression: expected[0],
 				label: expected[1] || undefined,
-				agg: expected[2] || false
+				agg: expected[2] || false,
+				original: input[0]
 			});
 		});
 	});

test/specs/format_request.js

@@ -315,13 +315,13 @@ describe('format_request', () => {
 		describe('should accept', () => {
 
 			[
-				'table.field',
-				'table.field ASC',
-				'DATE(table.created_time)',
-				'DATE(table.created_time) DESC',
-				'DATE(table.created_time) DESC, name ASC',
+				'field',
+				'field ASC',
+				'DATE(created_time)',
+				'DATE(created_time) DESC',
+				'DATE(created_time) DESC, name ASC',
 				['name ASC'],
-				['DATE(table.created_time) DESC', 'name ASC'],
+				['DATE(created_time) DESC', 'name ASC'],
 			].forEach(orderby => {
 
 				it(`valid: ${ orderby } (${ typeof orderby })`, async() => {

test/specs/get-sort-group.js

@@ -32,7 +32,7 @@ const limit = 5;
 					SELECT a.email, b.name AS 'name'
 					FROM users_email a
 					LEFT JOIN users b ON(b.id = a.user_id)
-					${SQL_EXPR} b.name
+					${SQL_EXPR} \`name\`
 					LIMIT 5
 				`;
 
@@ -59,7 +59,7 @@ const limit = 5;
 					FROM users_email a
 					LEFT JOIN users b ON(b.id = a.user_id)
 					LEFT JOIN country c ON(c.id = b.country_id)
-					${SQL_EXPR} \`users.country.date\`${DESC}, c.name${ASC}
+					${SQL_EXPR} \`users.country.date\`${DESC}, \`CountryName\`${ASC}
 					LIMIT 5
 				`;
 

test/specs/get.js

@@ -182,7 +182,7 @@ describe('get', () => {
 			};
 
 			return dare
-				.get('test', basic_fields, {id: 1}, {orderby: 'test.id'});
+				.get('test', basic_fields, {id: 1}, {orderby: 'id'});
 
 
 		});