Skip to content

Commit

Permalink
default empty sort to remove warning (#76)
Browse files Browse the repository at this point in the history
  • Loading branch information
Idokah authored Nov 22, 2021
1 parent d8c2543 commit 25320c2
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 23 deletions.
21 changes: 13 additions & 8 deletions packages/external-db-airtable/lib/airtable_data_provider.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
const { minifyAndFixDates, DEFAULT_MAX_RECORDS } = require('./airtable_utils')
const { minifyAndFixDates, DEFAULT_MAX_RECORDS, EMPTY_SORT } = require('./airtable_utils')

class DataProvider {
constructor(base, filterParser) {
Expand Down Expand Up @@ -30,12 +30,12 @@ class DataProvider {
const createExpr = this.bulkCreateExpr(items)
const inserted = await this.base(collectionName)
.create(createExpr)
return inserted.length;
return inserted.length
}

async update(collectionName, items) {
const updated = await Promise.all( items.map(async item => await this.updateSingle(collectionName, item)) )
return updated.length;
return updated.length
}

async delete(collectionName, itemIds) {
Expand All @@ -48,7 +48,7 @@ class DataProvider {
async truncate(collectionName) {
await this.base(collectionName)
.select()
.eachPage(async (records, fetchNextPage) => {
.eachPage(async(records, fetchNextPage) => {
await this.base(collectionName)
.destroy(records.map(record => record.id))
fetchNextPage()
Expand All @@ -58,20 +58,25 @@ class DataProvider {

async query({ collectionName, filterByFormula, limitExpr, sortExpr, idsOnly, skip}) {
const resultsByPages = []

const limit = limitExpr?.maxRecords ? limitExpr : { maxRecords: DEFAULT_MAX_RECORDS }
limit.maxRecords += skip

const sort = EMPTY_SORT
if (sortExpr && sortExpr.sort) Object.assign (sort, sortExpr)

await this.base(collectionName)
.select({ ...filterByFormula, ...limitExpr, ...sortExpr })
.select({ ...filterByFormula, ...limitExpr, ...sort })
.eachPage((records, fetchNextPage) => {
const recordsToReturn = idsOnly ? records.map(record=>record.id) : records
resultsByPages.push(recordsToReturn)
fetchNextPage()
})
return resultsByPages.flat().slice(skip); //TODO: find other solution for skip! at least don't load everything to memory.
return resultsByPages.flat().slice(skip) //TODO: find other solution for skip! at least don't load everything to memory.
}


filterToFilterByFormula(filter){
filterToFilterByFormula(filter) {
const { filterExpr } = this.filterParser.transform(filter)
return { filterByFormula: filterExpr || '' }
}
Expand All @@ -88,7 +93,7 @@ class DataProvider {
return updated.id
}

bulkCreateExpr = (items) => {
bulkCreateExpr(items) {
return items.reduce((pV, cV) => {
pV.push({ fields: { ...cV } })
return pV
Expand Down
4 changes: 3 additions & 1 deletion packages/external-db-airtable/lib/airtable_utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,6 @@ const minifyAndFixDates = record => {
}
const DEFAULT_MAX_RECORDS = 100

module.exports = { minifyAndFixDates, DEFAULT_MAX_RECORDS }
const EMPTY_SORT = { sort: [] }

module.exports = { minifyAndFixDates, DEFAULT_MAX_RECORDS, EMPTY_SORT }
5 changes: 3 additions & 2 deletions packages/external-db-airtable/lib/sql_filter_transformer.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
const { InvalidQuery } = require('velo-external-db-commons').errors
const { isObject } = require('velo-external-db-commons')
const { EMPTY_SORT } = require ('./airtable_utils')

class FilterParser {
constructor() {
Expand Down Expand Up @@ -123,12 +124,12 @@ class FilterParser {

orderBy(sort) {
if (!Array.isArray(sort) || !sort.every(isObject)) {
return []
return EMPTY_SORT
}

const results= sort.flatMap(this.parseSort)
if (results.length === 0) {
return []
return EMPTY_SORT
}
return {
sort: results
Expand Down
25 changes: 13 additions & 12 deletions packages/external-db-airtable/lib/sql_filter_transformer.spec.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
const FilterParser = require('./sql_filter_transformer')
const { EMPTY_SORT } = require ('./airtable_utils')
const { Uninitialized, gen } = require('test-commons')
const { InvalidQuery } = require('velo-external-db-commons').errors
const each = require('jest-each').default
Expand All @@ -11,18 +12,18 @@ describe('Sql Parser', () => {

// todo: should we even check for valid input or should we let the validation library to handle this ?
test('handles undefined sort', () => {
expect( env.filterParser.orderBy('') ).toEqual([])
expect( env.filterParser.orderBy(' ') ).toEqual([])
expect( env.filterParser.orderBy(undefined) ).toEqual([])
expect( env.filterParser.orderBy(null) ).toEqual([])
expect( env.filterParser.orderBy({invalid: 'object'}) ).toEqual([])
expect( env.filterParser.orderBy(555) ).toEqual([])
expect( env.filterParser.orderBy([5555]) ).toEqual([])
expect( env.filterParser.orderBy(['sdfsdf']) ).toEqual([])
expect( env.filterParser.orderBy([null]) ).toEqual([])
expect( env.filterParser.orderBy([undefined]) ).toEqual([])
expect( env.filterParser.orderBy([{invalid: 'object'}]) ).toEqual([])
expect( env.filterParser.orderBy([]) ).toEqual([])
expect( env.filterParser.orderBy('') ).toEqual(EMPTY_SORT)
expect( env.filterParser.orderBy(' ') ).toEqual(EMPTY_SORT)
expect( env.filterParser.orderBy(undefined) ).toEqual(EMPTY_SORT)
expect( env.filterParser.orderBy(null) ).toEqual(EMPTY_SORT)
expect( env.filterParser.orderBy({invalid: 'object'}) ).toEqual(EMPTY_SORT)
expect( env.filterParser.orderBy(555) ).toEqual(EMPTY_SORT)
expect( env.filterParser.orderBy([5555]) ).toEqual(EMPTY_SORT)
expect( env.filterParser.orderBy(['sdfsdf']) ).toEqual(EMPTY_SORT)
expect( env.filterParser.orderBy([null]) ).toEqual(EMPTY_SORT)
expect( env.filterParser.orderBy([undefined]) ).toEqual(EMPTY_SORT)
expect( env.filterParser.orderBy([{invalid: 'object'}]) ).toEqual(EMPTY_SORT)
expect( env.filterParser.orderBy([]) ).toEqual(EMPTY_SORT)
})

test('process single sort expression invalid sort will return empty result', () => {
Expand Down

0 comments on commit 25320c2

Please sign in to comment.