Skip to content

loadAndAuthorizeResource not correctly loading for edit/show etc #6

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

Closed
sintemaa opened this issue Apr 21, 2014 · 4 comments
Closed
Labels

Comments

@sintemaa
Copy link

According to the documentation if you use loadAndAuthorizeResource in the __construct() method of your controller it should automatically populate the model you need in your method. However while this works ok for the methods that require all items (e.g. index) it doesn't work for my edit & show methods (and probably for all methods of the same type).

My route looks like this:

Route::get('users/{id}/edit', ['as'=>'users.edit', 'uses' =>'UsersController@edit']);

My edit method in UsersController looks like

    /**
     * Show the form for editing the specified resource.
     *
     * @param  int  $id
     * @return Response
     */
    public function edit($id)
    {
        return View::make('users.edit')->with('user', $this->user);
    }

I was expecting $this->user to contain the user with the id set in the route, but I get an error that $this->user is not defined.

I did some digging and found that setting $this->user depends on whether ControllerResource->isMemberAction() returns true. This brings me to ControllerResource->getIdParam(), which gets ControllerResource->getIdKey() and checks if this exists in $this->params.

ControllerResource->getIdKey() returns "id", which is what I was expecting, but for some reason $this->params does not contain a key called "id" with the value from the route, but instead a key called "id_id". I've no idea why this happens.

I did some further digging and ended up in Parameters->fillController() which seems to fill the params array. All seems ok until line 38, but then from line 39 to 49 there is some code that seems to deliberately replace my id parameter with id_id, with some comments about difference between laravel & rails. I don't know rails so this comment is unclear to me.

Is this a bug or am I doing something wrong? I can work around this by adding

$this->user = User::find($id); 

to the start of my edit method but it seems inefficient as I am using this package which should do this automagically.

@tortuetorche
Copy link
Contributor

As a temporary fix, can you try to use this route instead, please?

Route::get('users/{user}/edit', ['as'=>'users.edit', 'uses' =>'UsersController@edit']);

The Parameters class follow the conventions of Laravel controller/RESTful routes.

So you need to replace the {id} route's parameter by {user}.
But I'm agree with you, it's not really intuitive, specially if you add custom routes to your Laravel application. This issue need to be fixed.

@tortuetorche
Copy link
Contributor

By the way, your issue is very well documented, I think you spend some times to write it. Thank you, I appreciate that.
I'm going to push a fix today. Are you available to test it ?

tortuetorche pushed a commit that referenced this issue Apr 22, 2014
…id' routes's parameters.

So you can add routes like this one:
   Route::get('users/{id}', ['as'=>'users.show', 'uses'
=>'UsersController@show']);

Or a nested routes, like this:
    Route::get('users/{user_id}/posts/{id}', ['as'=>'users.posts.show',
'uses' =>'PostsController@show']);

Then the Parameters class should be able to extract the 'user_id' and
'id' parameters to load the User and Post instances.

Should fix issue #6
@tortuetorche
Copy link
Contributor

@sintemaa I think this issue is fixed now, can you update this package in your application ?
With the master version, add this in your composer.json file:

{
  // ...
  "require": {
     //...
     "efficiently/authority-controller": "dev-master",
  //...
}

Then update it, like this:
composer update efficiently/authority-controller

If you have any problems, feel free to reopen this issue.

Otherwise, if it's OK, I'll tag a new version.

Have a good day,
Tortue Torche

@sintemaa
Copy link
Author

Happy to provide the extensive issue documentation. I had done quite some digging in your code to figure out what I did wrong so I thought best to explain what I'd sone to help you understand the root cause, saving you time in fixing it.

The fix seems to work fine for me.
Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants