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

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

Open
mallison opened this issue Aug 9, 2015 · 12 comments
Open
Assignees

Comments

@mallison
Copy link
Contributor

mallison commented Aug 9, 2015

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.

@mallison mallison self-assigned this Aug 9, 2015
@bhoomit
Copy link
Contributor

bhoomit commented Mar 2, 2016

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
Copy link
Contributor Author

mallison commented Mar 3, 2016

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
Copy link
Contributor

bhoomit commented Mar 3, 2016

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
Copy link
Contributor Author

mallison commented Mar 3, 2016

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
Copy link
Contributor

bhoomit commented Mar 3, 2016

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

@mallison
Copy link
Contributor Author

mallison commented Mar 3, 2016

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
#3 (comment).

@bhoomit
Copy link
Contributor

bhoomit commented Mar 3, 2016

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
Copy link
Contributor Author

mallison commented Mar 3, 2016

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
#3 (comment).

@bhoomit
Copy link
Contributor

bhoomit commented Mar 3, 2016

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
Copy link
Contributor Author

mallison commented Mar 5, 2016

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
#3 (comment).

@bhoomit
Copy link
Contributor

bhoomit commented Mar 5, 2016

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
Copy link
Contributor Author

mallison commented Mar 5, 2016

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
#3 (comment).

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