pusher / chatkit-client-js

JavaScript client SDK for Pusher Chatkit
https://pusher.com/chatkit
MIT License
90 stars 15 forks source link

combinedQueryParams aren't being used in currentUser.fetchMessagesFromRoom #17

Closed apalmer0 closed 6 years ago

apalmer0 commented 6 years ago

It looks like fetchMessagesFromRoom is accepting options and correctly joining them together in a query string, but then not actually appending that string to the request path:


    const combinedQueryParams = [
      initialIdQueryParam,
      limitQueryParam,
      directionQueryParam,
    ].join('&');

    this.apiInstance
      .request({
        method: 'GET',
        path: `/rooms/${room.id}/messages`,
      })

I think it should be like below?


    const combinedQueryParams = [
      initialIdQueryParam,
      limitQueryParam,
      directionQueryParam,
    ].join('&');

    this.apiInstance
      .request({
        method: 'GET',
        path: `/rooms/${room.id}/messages?${combinedQueryParams}`,
      })

Thanks!

vivangkumar commented 6 years ago

Hey there!

You're indeed correct. I'll create a PR to fix this. Thanks for bringing this up!

Edit: I've created a PR here (https://github.com/pusher/chatkit-client-js/pull/18) which should be merged in soon!