owowagency/laravel-resources

A package to create projects faster

laravel-resources's Issues

Dynamic resource response in `ResourceController`

Currently when a custom manager method needs to be called to only retrieve and display its contents, a custom controller needs to be created. 90% of the cases the only difference between the ResourceController and this custom controller is the method that is being called to retrieve these results.

Eg:

class PostController extends ResourceController
{
    /**
     * Display a listing of the resource.
     *
     * @return \Illuminate\Http\Response
     *
     * @throws \Illuminate\Auth\Access\AuthorizationException
     */
    public function index()
    {
        $this->authorize('view', $this->resourceModelClass);

        $resourcesPaginated = $this->resourceManager->paginateForUser(currentUser()); // <- the only difference

        $resources = resource($resourcesPaginated, true);

        $resourcesPaginated->setCollection($resources->collection);

        return ok($resourcesPaginated);
    }
}

A suggestion would be to implement the __call method to allow a non Response type to be returned by all resource methods. The following pseudo example could make things a little more clearer:

class PostController extends ResourceController
{
    public function index()
    {
        // Instead of implementing all duplicate code we will just return a
        // LengthAwarePaginator type.
        return $this->resourceManager->paginateForUser(currentUser());
    }
}

class ResourceController
{
    public function __call()
    {
        // Call the authorization
        $this->authorizeForMethod($method);

        // We call the intended method. If it does not exist it should throw an
        // non existing method call exception like PHP does by default we do
        // not want to change this behaviour.
        $result = $this->$method(...);

        // We check if the $result is of instance response. If so we return this.
        if ($result instanceof Response) {
            return $result;
        }

        // Otherwise we will make a response with the results.
        else {
            // Depening on the method is being called we return different responses.
            return $this->respondForMethod($method, $results);
        }
    }
}

This change will result in less duplicate code in our controllers and keeps the possibility of returning a custom response.

Auto-discovery of resources.

Currently Laravel supports auto-discovery.

Would be nice to be able to do the same with resource classes.

Separate policy of index and show.

Currently index and show share the same policy: view.

Let's separate them into viewAny and view.

UPDATE:
Use the authorizeResource method in the ResourceController.

https://laravel.com/docs/6.x/authorization#via-controller-helpers

Move ResourceController model actions to separate methods.

As mentioned in #7 we constantly have to copy the whole method when only changing the model "action" part. The model action could be retrieving, storing, updating...

A new proposal is to separate these action calls in new methods so the overwriting is minimal. The following example would make this clearer.

// ResourceController
public function index()
{
    ... default stuff

    $models = $this->indexModel();

    ... default stuff
}

public function indexModel()
{
    return $this->resourceModelClass::paginate();
}

// My custom controller
public function indexModel()
{
    return $this->resourceModelClass::with('relation')->paginate();
}

Resource controller authorize

Currently the ResourceController@authorize uses $this->resourceModelClass to find a policy. However the model that is used to find a policy might not always be the resourceModelClass and thus might result in a wrong authorization.

Suggestion to change code:

public function authorize($ability, $arguments = [])
{
    $model = is_array($arguments)
        ? Arr::first($arguments)
        : $arguments;

    // Do not try to authorize when policy does not exist.
    if (is_null(Gate::getPolicyFor($model))) {
        return;
    }

    return $this->traitAuthorize($ability, $arguments);
}

This code will use the model class that is actually authorized.

Do not extend from App namespaces.

Some classes extend from the App namespace. Example is that the ResourceController extends from App\Http\Controller class. Not sure if this is done in more than one location.

The classes should extend other classes provided by Laravel.

get_class() expects parameter 1 to be object, array given

message": "get_class() expects parameter 1 to be object, array given",
"exception": "ErrorException",
"file": "/src/Resources/ResourceFactory.php",
"line": 100,

Modify ResourceController to allow storing multiple models at once

If we want to store multiple models at once, we can overwrite the storeModel method in ResourceController.

However, currently the store method will always return 1 single resource, due to this code:

$model = $this->storeModel($request);

$resource = resource($model);

return created($resource);

Todo:

• Check whether the return of storeModel is a single resource or a collection. Return the correct JSON resource accordingly.

Modify storeModel() to receive validated request data

    public function storeModel(Request $request, array $data)
    {
        return $this->resourceModelClass::create($data);
    }

This way, storeModel() doesnt have to call $request->validated() anymore.

Make getModel work with getRouteKeyName

When a developer chooses to have a different route key name, eg: slug, the ResourceController@getModel will fail. getModel uses findOrFail wich uses the primaryKey property of the model.

Maybe use where($theRouteKeyName, $model)->firstOrFail()