Kvoti / redux-rest

Automatically create Redux action constants, action creators, and reducers for your REST API
MIT License
180 stars 11 forks source link

POST/PUT response handling expects full object is returned in repsonse #3

Open mallison opened 9 years ago

mallison commented 9 years ago

Again, this is specific to Django Rest Framework's defaults. API's may only return the object ID on successful create or update. We need to handle that case.

bhoomit commented 8 years ago

On similar note my list API returns custom object containing multiple lists. Right now there's no way to accept such response. Is it intentional?

mallison commented 8 years ago

It's intentional in the sense that I only did enough to get things working with my backend. Adding setCSRF (now SetHeader!) was a start at generalising things.

Do you have an example of returning multiple lists? We could allow further customisation if there's a good use case.

bhoomit commented 8 years ago

There's a home screen where I return multiple type of cards(data). E.g. Region, City, Hotel, Staff Picks, etc. The response is something like

{ "regions": [{},{},{}], "cities": [{},{}], "hotels": [{},{}] }

mallison commented 8 years ago

I think, strictly speaking, that isn't quite RESTful but, for pragmatic reasons, most APIs aren't. I could imagine something like:

const API = {
    '/api/': {
         "regions": types.List(...)
         "cities": types.List(...)
    }
};

but that's not really RESTful and outside the scope of this library I think. The best thing is probably to make sure you can easily use redux-rest, for 'regular' endpoints, along side your own code for special cases.

bhoomit commented 8 years ago

I totally understand that. But on mobile devices for India like demographics we try to minimize number of API calls.

mallison commented 8 years ago

Yes, I understand that (hence my pragmatism point). It's also why Relay/GraphQL is the probably the way forward! It's something I (we) can thing about. Did my pseudo-code make sense? Something like that might work. It would add a layer of complexity but maybe not too much.

On 3 March 2016 at 09:21, Bhoomit notifications@github.com wrote:

I totally understand that. But on mobile devices for India like demographics we try to minimize number of API calls.

— Reply to this email directly or view it on GitHub https://github.com/Kvoti/redux-rest/issues/3#issuecomment-191672318.

bhoomit commented 8 years ago

Yes, It does makes sense. I didn't know about Relay/GraphQL. So went ahead and read about it. And yes it does seem like the way forward. Unfortunately, right now its hard for us to move to that as there are apps which are already using the APIs. Anyway thanks for the tip.

mallison commented 8 years ago

No problem. I think REST APIs will be around for a while which is why I started redux-rest. I'm open to exploring aggregate endpoints like your example. As you say, it must be pretty common for low-bandwidth areas.

On 3 March 2016 at 13:44, Bhoomit notifications@github.com wrote:

Yes, It does makes sense. I didn't know about Relay/GraphQL. So went ahead and read about it. And yes it does seem like the way forward. Unfortunately, right now its hard for us to move to that as there are apps which are already using the APIs. Anyway thanks for the tip.

— Reply to this email directly or view it on GitHub https://github.com/Kvoti/redux-rest/issues/3#issuecomment-191764988.

bhoomit commented 8 years ago

For now what I did in my fork is this:

In ItemReducer

} else if (action.type === this.actionTypes.list_success) {
      if (Array.isArray(action.payload)){
        return [...action.payload];  
      }
      return action.payload;
}
mallison commented 8 years ago

Ok, that's fine for retrieving items. I guess creating, updating items doesn't work?

On 3 March 2016 at 14:15, Bhoomit notifications@github.com wrote:

For now what I did in my fork is this:

In ItemReducer

} else if (action.type === this.actionTypes.list_success) { if (Array.isArray(action.payload)){ return [...action.payload]; } return action.payload; }

— Reply to this email directly or view it on GitHub https://github.com/Kvoti/redux-rest/issues/3#issuecomment-191780031.

bhoomit commented 8 years ago

This was just a quick fix. I didn't test it with other operations. Will check when I get there. Once I'm sure about it, I will send a PR.

mallison commented 8 years ago

Great, thanks.

On 5 March 2016 at 16:28, Bhoomit notifications@github.com wrote:

This was just a quick fix. I didn't test it with other operations. Will check when I get there. Once I'm sure about it, I will send a PR.

— Reply to this email directly or view it on GitHub https://github.com/Kvoti/redux-rest/issues/3#issuecomment-192681278.