airtable-deno icon indicating copy to clipboard operation
airtable-deno copied to clipboard

A list of sort objects doesn't stringify correctly

Open kitcat-dev opened this issue 3 years ago • 4 comments

I suppose there is an error in the getRequestUrl function. In addition to the bug mentioned in the pull request #5, this function does not properly handle the sort array.

Input

const { records } = await airtable.select<Fields>({
  fields: ['Item', 'Done'],
  maxRecords: 100,
  view: 'Main View',
  sort: [
    {
      field: 'Item',
      direction: 'asc',
    },
    {
      field: 'Done',
      direction: 'desc',
    },
  ],
})

Expected request url

.../TableName/?fields=Item&fields=Done&maxRecords=100&view=Main%20View
&sort[0][field]=Item&sort[0][direction]=asc&sort[1][field]=Done&sort[1][direction]=desc

Actual result

.../TableName/?/fields=Item&fields=Done&maxRecords=100&view=Main%20View
&sort=%5Bobject%20Object%5D&sort=%5Bobject%20Object%5D
               ⚠️                        ⚠️

Description

As mentioned in the Airtable API, array of objects [ { field: 'Item', direction: 'desc' } ] should be stringified to: sort[0][field]=Item&sort[0][direction]=desc:

image

kitcat-dev avatar Apr 21 '21 18:04 kitcat-dev

@grikomsn Hey! Thanks for the awesome library. It's much more convenient than using the official Airtable library.

Can I add a pull request to make it better? I'd like to modify the end of getRequestUrl to make it look like this:

const urlSegments = [
      endpointUrl,
      baseId,
      encodeURIComponent(tableName),
      ...segments
]

let sort: {
   field: string
   direction?: 'asc' | 'desc'
}[] = []
let sortQuery: string = ''

if ('sort' in query) {
  sort = query.sort
  delete query.sort
}

let queryString = hasAnyKey(query) ? '?' + stringify(query) : ''

if (sort.length) {
  sortQuery = sort
     .map(
       ({ field, direction }, index) =>
         `sort[${index}][field]=${field}&sort[${index}][direction]=${direction}`
     )
     .join('&')
  queryString += '&' + sortQuery
}

return urlSegments.join('/').concat(queryString)

Of course, this is a rough draft.

I could add some tests, maybe. What do you think?

kitcat-dev avatar Apr 21 '21 18:04 kitcat-dev

Hi there! Sorry for the inconvenience. It's been a while since I've done Deno (and open source) stuff, I'd be glad to accept PRs and suggestions!

Probably need to update Deno on my workstation since it's still on *checks version* 1.7.1. 😂

grikomsn avatar Apr 21 '21 18:04 grikomsn

Cool. Thanks for the fast feedback :) I'll write some code tomorrow.

I have one more question. Maybe it is worthwhile to merge @minnacaptain's pull request? That code will be very helpful.

kitcat-dev avatar Apr 21 '21 18:04 kitcat-dev

#5 is squashed! 🎉

grikomsn avatar Apr 21 '21 19:04 grikomsn