From f4041ed68c6d24afe93325757be54906a3bf1ff7 Mon Sep 17 00:00:00 2001 From: pepe Date: Wed, 10 Nov 2021 19:33:37 -0500 Subject: [PATCH] fix: A bug fix Description: Store the fields that have been ordered by with (and throw if you try to orderBy twice in the same field) instead of just checking if any fields have been ordered by. An orderBy function cannot be called more than once in the same query expression, except on different fields. Fixing issue https://github.com/wovalle/fireorm/issues/266 --- src/BaseFirestoreRepository.spec.ts | 18 +++++++++++++++++- src/QueryBuilder.ts | 19 +++++++++++++++++-- 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/src/BaseFirestoreRepository.spec.ts b/src/BaseFirestoreRepository.spec.ts index 97a40b7..0c9c51f 100644 --- a/src/BaseFirestoreRepository.spec.ts +++ b/src/BaseFirestoreRepository.spec.ts @@ -126,7 +126,16 @@ describe('BaseFirestoreRepository', () => { albumsSubColl.orderByAscending('releaseDate').orderByDescending('releaseDate'); }).toThrow(); }); + + it('must succeed when orderBy* function is called more than once in the same expression with different fields', async () => { + const pt = await bandRepository.findById('porcupine-tree'); + const albumsSubColl = pt.albums; + expect(() => { + albumsSubColl.orderByAscending('releaseDate').orderByDescending('name'); + }).toBeTruthy(); + }); }); + }); describe('orderByDescending', () => { it('must order repository objects', async () => { @@ -164,8 +173,15 @@ describe('BaseFirestoreRepository', () => { albumsSubColl.orderByAscending('releaseDate').orderByDescending('releaseDate'); }).toThrow(); }); + + it('must succeed when orderBy* function is called more than once in the same expression with different fields', async () => { + const pt = await bandRepository.findById('porcupine-tree'); + const albumsSubColl = pt.albums; + expect(() => { + albumsSubColl.orderByAscending('releaseDate').orderByDescending('name'); + }).toBeTruthy(); + }); }); - }); describe('findById', () => { it('must find by id', async () => { diff --git a/src/QueryBuilder.ts b/src/QueryBuilder.ts index 5ea60c8..73fea9a 100644 --- a/src/QueryBuilder.ts +++ b/src/QueryBuilder.ts @@ -17,6 +17,7 @@ export class QueryBuilder implements IQueryBuilder { protected limitVal: number; protected orderByObj: IOrderByParams; protected customQueryFunction?: ICustomQuery; + protected orderByFields: Set = new Set(); constructor(protected executor: IQueryExecutor) {} @@ -144,12 +145,19 @@ export class QueryBuilder implements IQueryBuilder { } orderByAscending(prop: IWherePropParam) { - if (this.orderByObj) { + const fieldProp: string = typeof prop == 'string' ? prop : ''; + const alreadyOrderedByField = this.orderByFields.has(fieldProp); + + if (this.orderByObj && alreadyOrderedByField) { throw new Error( 'An orderBy function cannot be called more than once in the same query expression' ); } + if (!alreadyOrderedByField && fieldProp) { + this.orderByFields.add(fieldProp); + } + this.orderByObj = { fieldPath: this.extractWhereParam(prop), directionStr: 'asc', @@ -159,12 +167,19 @@ export class QueryBuilder implements IQueryBuilder { } orderByDescending(prop: IWherePropParam) { - if (this.orderByObj) { + const fieldProp: string = typeof prop == 'string' ? prop : ''; + const alreadyOrderedByField = this.orderByFields.has(fieldProp); + + if (this.orderByObj && alreadyOrderedByField) { throw new Error( 'An orderBy function cannot be called more than once in the same query expression' ); } + if (!alreadyOrderedByField && fieldProp) { + this.orderByFields.add(fieldProp); + } + this.orderByObj = { fieldPath: this.extractWhereParam(prop), directionStr: 'desc',