Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The search is not very usable #53

Closed
cdimitroulas opened this issue Aug 21, 2018 · 2 comments
Closed

The search is not very usable #53

cdimitroulas opened this issue Aug 21, 2018 · 2 comments

Comments

@cdimitroulas
Copy link
Contributor

cdimitroulas commented Aug 21, 2018

I think it would be really nice if using this SDK you did not have to write the search string yourself.

Ideally the API of this library would allow you to do something like:

timekit.getBookings({
  search: {
    'meta.some_property': 'someValue'
  }
})

And this would be converted to a call to /bookings?search=meta.some_property:someValue automatically.

A function like this should do the trick (quickly tested and tried in my own code using lodash/fp):

function createSearchString(search) {
  const keyValues = fp.map.convert({ cap: false })(
      (val, key) => `${key}:${val}`,
      search
    )

  return keyValues.join(';')
}
@laander
Copy link
Contributor

laander commented Aug 22, 2018

Good call @cdimitroulas, I agree that passing search query strings could be simpler.

Just for the record, we already have the carry function that passes stuff to the following endpoint call. Still not super smooth, but in case you didn't know about it, see https://github.com/timekit-io/js-sdk#usage-carry

A few comment regarding your solution:

  • We currently don't transpile ES6+ES7 (babel) so we'd have to make the implemenntation in ES5 for the time being. That means no template literals and const.
  • If we we're to pull in lodash, I'd prefer that we do some treeshaking using babel-plugin-lodash to make sure we don't include the whole lodash lib. Again, that depends on babel

If you want to give a PR a shot taking the above comments into considerations, I'd be happy to have a look at it :)

@cdimitroulas
Copy link
Contributor Author

Hey @laander, yes I have seen the carry method after I wrote this issue 😅

However, it would still be worth having the string creation automatically so that user's of this lib don't have to do it. I can try to come up with an ES5, non-lodash version of my function which would work for this case when I find some time.

@laander laander closed this as completed Dec 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants