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

Overhauling Controller #63

Closed
jasonlewis opened this issue May 23, 2014 · 7 comments
Closed

Overhauling Controller #63

jasonlewis opened this issue May 23, 2014 · 7 comments

Comments

@jasonlewis
Copy link
Contributor

Okay the plan is to overhaul the controller that the API ships with to provide a bunch of helper methods to easily return better responses.

Currently responses are kind of "magically" transformed. I'd like to keep that, but perhaps add a configuration option to disable it (perhaps by default?) so that you can return raw responses (in regards to #50) and also make use of the helper methods.

Possible helper methods:

  • withPaginator
  • withCollection
  • withModel
  • withArray

Usage would be:

return $this->withModel(User::find(1));

Or something to that affect. Just after some thoughts and other possible methods.

/cc @philsturgeon

@terion-name
Copy link

maybe better something like return Response::make($data)->withPaginator()->format('xml')?

@jasonlewis
Copy link
Contributor Author

I'm trying to keep any sort of formatting related stuff outside of the
controller. The format is set via the Accept header.

On Fri, May 23, 2014 at 9:34 PM, Terion notifications@github.com wrote:

maybe better something like return
Response::make($data)->withPaginator()->format('xml')?


Reply to this email directly or view it on GitHubhttps://github.com//issues/63#issuecomment-44001189
.

@jasonlewis
Copy link
Contributor Author

I'm thinking this might also need to belong in Dingo\Api\Http\Response so that if you are using closures for routes you can replace the Response facade and use it much like was shown above.

return Response::withPaginator($paginator);

@philsturgeon
Copy link

Sounds good. I only shoved them in the controller because it worked and I didn’t care about closures. Your way sounds better.

@jasonlewis
Copy link
Contributor Author

So I'm just looking at this now and I'm not too sure what these methods should be doing. I know in your case Phil you'd like to have more control over how Fractal is handled. For others though they might not want that.

My train of thought at the moment is that simply allowing the "magic responses" to be disabled would be the best way forward, for now. That way you can still retain complete control over the responses that are returned.

I may be thinking about this all wrong though. So. Much. Thinking. 😁

@philsturgeon
Copy link

Sorry to bump you here mate, I know you've been working hard on Dingo. Where are we at?

@rizqidjamaluddin
Copy link

I know I prefer explicit responses whenever possible. In general I don't like the idea of too many layers doing things between the controller response and the actual response. A config switch, or supporting both, sound reasonable.

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

4 participants