Skip to content

Commit

Permalink
fix: make pages consistent when using sort on non-unique attributes (#…
Browse files Browse the repository at this point in the history
…633)

* Fix sort order consistency by always adding the primaryKey as the last sort option

* Make sure to only include the PK once in the query in case the user explicitly sorts on the PK

* Fix import

* remove quote

* update tests

* Make sure first and last search for orderByRaw instead of orderBy

* Fix formatting

* Fix formatting

* Ignore prefer-template for uniq line

* Change two more instances of orderBy to orderByRaw
  • Loading branch information
nickschot authored and zacharygolba committed Jan 16, 2017
1 parent d1589ab commit 93aa5c4
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 14 deletions.
22 changes: 14 additions & 8 deletions src/packages/database/query/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import { camelize } from 'inflection';

import entries from '../../../utils/entries';
import uniq from '../../../utils/uniq';
import type Model from '../model';

import scopesFor from './utils/scopes-for';
Expand Down Expand Up @@ -162,12 +163,13 @@ class Query<+T: any> extends Promise {

if (columnName) {
this.snapshots = this.snapshots
.filter(([method]) => method !== 'orderBy')
.filter(([method]) => method !== 'orderByRaw')
.concat([
['orderBy', [
`${this.model.tableName}.${columnName}`,
direction
]]
// eslint-disable-next-line prefer-template
['orderByRaw', uniq([columnName, this.model.primaryKey])
.map(key => `${this.model.tableName}.${key}`)
.join(', ') + ` ${direction}`
]
]);
}
}
Expand Down Expand Up @@ -222,7 +224,9 @@ class Query<+T: any> extends Promise {

first(): this {
if (!this.shouldCount) {
const willSort = this.snapshots.some(([method]) => method === 'orderBy');
const willSort = this.snapshots.some(
([method]) => method === 'orderByRaw'
);

this.collection = false;

Expand All @@ -238,7 +242,9 @@ class Query<+T: any> extends Promise {

last(): this {
if (!this.shouldCount) {
const willSort = this.snapshots.some(([method]) => method === 'orderBy');
const willSort = this.snapshots.some(
([method]) => method === 'orderByRaw'
);

this.collection = false;

Expand Down Expand Up @@ -399,7 +405,7 @@ class Query<+T: any> extends Promise {
if (scopes.length) {
const keys = scopes.map(scope => {
if (scope === 'order') {
return 'orderBy';
return 'orderByRaw';
}

return scope;
Expand Down
12 changes: 6 additions & 6 deletions src/packages/database/test/query.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -335,15 +335,15 @@ describe('module "database/query"', () => {
const result = subject.order('id', 'DESC');

expect(result.snapshots).to.deep.equal([
['orderBy', ['posts.id', 'DESC']]
['orderByRaw', 'posts.id DESC']
]);
});

it('defaults sort direction to `ASC`', () => {
const result = subject.order('id');

expect(result.snapshots).to.deep.equal([
['orderBy', ['posts.id', 'ASC']]
['orderByRaw', 'posts.id ASC']
]);
});

Expand Down Expand Up @@ -440,7 +440,7 @@ describe('module "database/query"', () => {
const result = subject.first();

expect(result.snapshots).to.deep.equal([
['orderBy', ['posts.id', 'ASC']],
['orderByRaw', 'posts.id ASC'],
['limit', 1]
]);
});
Expand All @@ -455,7 +455,7 @@ describe('module "database/query"', () => {
const result = subject.order('createdAt', 'DESC').first();

expect(result.snapshots).to.deep.equal([
['orderBy', ['posts.created_at', 'DESC']],
['orderByRaw', 'posts.created_at, posts.id DESC'],
['limit', 1]
]);
});
Expand Down Expand Up @@ -494,7 +494,7 @@ describe('module "database/query"', () => {
const result = subject.last();

expect(result.snapshots).to.deep.equal([
['orderBy', ['posts.id', 'DESC']],
['orderByRaw', 'posts.id DESC'],
['limit', 1]
]);
});
Expand All @@ -509,7 +509,7 @@ describe('module "database/query"', () => {
const result = subject.order('createdAt', 'DESC').last();

expect(result.snapshots).to.deep.equal([
['orderBy', ['posts.created_at', 'DESC']],
['orderByRaw', 'posts.created_at, posts.id DESC'],
['limit', 1]
]);
});
Expand Down

0 comments on commit 93aa5c4

Please sign in to comment.