laravel-json-api / laravel

JSON:API for Laravel applications
MIT License
540 stars 42 forks source link

Arguement 3 passed to controller must be an instance of model, string given #118

Open bhushan08 opened 3 years ago

bhushan08 commented 3 years ago

Hello, first of all, great library, very useful, thank you for creating this wonderful library.

I have created a custom controller which consists of one action. I want to update the status of the shop employee on this action. When I post on the action url, that is, shop_employees/2/-actions/activate, I get this error Argument 3 passed to App\\Http\\Controllers\\API\\V1\\ShopEmployeeEditController::activate() must be an instance of App\\Models\\ShopEmployee, string given, called in /home/bhushan/Documents/Projects/Repo/where-hair-backend/vendor/laravel/framework/src/Illuminate/Routing/Controller.php on line 54",

I checked in the string for the third parameter I am getting "shop_employees". I should get the model, where am I going wrong?

Source code as follows, as per the documentation:

// ShopEmployeeEditController.php

<?php

namespace App\Http\Controllers\API\V1;

use App\Http\Controllers\Controller;
use App\JsonApi\V1\ShopEmployees\ShopEmployeeSchema;
use App\Models\ShopEmployee;
use Illuminate\Auth\Access\AuthorizationException;
use Illuminate\Auth\AuthenticationException;
use LaravelJsonApi\Core\Responses\DataResponse;
use LaravelJsonApi\Laravel\Http\Controllers\Actions;

class ShopEmployeeEditController extends Controller {

    use Actions\FetchMany;
    use Actions\FetchOne;
    use Actions\Store;
    use Actions\Update;
    use Actions\Destroy;
    use Actions\FetchRelated;
    use Actions\FetchRelationship;
    use Actions\UpdateRelationship;
    use Actions\AttachRelationship;
    use Actions\DetachRelationship;

    /**
     * @param ShopEmployeeSchema $shopEmployeeSchema
     * @param ShopEmployee $shopEmployee
     * @return mixed
     * @throws AuthorizationException
     * @throws AuthenticationException
     */
    public function activate(ShopEmployeeSchema $shopEmployeeSchema, $query, ShopEmployee $shopEmployee) {
        if (!empty(auth()->user())) {
            $shopEmployee->update([
                "status" => 'active',
            ]);
        }

        return $this->reply($shopEmployeeSchema, $query, $shopEmployee);
    }

    private function reply($schema, $query, $post) {
        $model = $schema
            ->repository()
            ->queryOne($post)
            ->withRequest($query)
            ->first();

        return new DataResponse($model);
    }

}

api.php

$server->resource('shop_employees', \App\Http\Controllers\API\V1\ShopEmployeeEditController::class)->actions('-actions', function ($actions) { $actions->withId()->post('activate'); });

Package I am using: "laravel-json-api/laravel": "^1.0",

lindyhopchris commented 3 years ago

There's something weird going on with this as I'd expect the string to be "shop_employee", but you've said you're getting "shop_employees"?

Can you provide the output of php artisan route:list - only for that route (i.e. don't paste the entire route list output, I only need to see the row for that specific route).

bhushan08 commented 3 years ago

Thank you for quick response. The thing is the third parameter in the documentation is said to be Model instance, but as you as said above, even you expect it to be a string, and yes, I am getting "shop_employees" not "shop_employee", but I should get the model instance not the string correct? I have attached the image of the route:list. I have also added it in the text format, although it won't be clear.

Screenshot from 2021-09-03 10-54-09

POST | api/v1/shop_employees/{shop_employee}/-actions/activate | v1.shop_employees.activate | App\Http\Controllers\API\V1\ShopEmployeeEditController@activate | api | | | | | | | LaravelJsonApi\Laravel\Http\Middleware\BootJsonApi:v1 |

I just saw, I am using an older version of the library, I will update the version today, check the api again and update it here, if I found the same issue. Thank you.

bhushan08 commented 3 years ago

So I updated the library version, but still I am getting the same error and getting "shop_employees" as the third parameter of the action.

ben221199 commented 3 years ago

Did you uncache your routes?

bhushan08 commented 3 years ago

@ben221199 Yes I have tried that as well, still same error.

lindyhopchris commented 3 years ago

Yes I'm aware you should be getting a model. The problem however seems to be with route binding substitution. That should replace the string with the model instance. This is working both in the tests and the production application I'm using the package with, so not really sure what can be causing the bug in your application.

Thinking about it I would expect the string to be the resource id, that needs replacing with the model. It is extremely weird therefore that the string you have is shop_employees - it should be a resource id, e.g. 123 if you use numeric identifiers.

Can you confirm what URL you are trying to access? It should be:

/api/v1/shop_employees/123/-actions/activate

Where 123 is the model id.

I'll be honest, this is going to be almost impossible for me to debug, so I'm reliant on what you can debug.

Basically this line in the JSON:API middleware: https://github.com/laravel-json-api/laravel/blob/develop/src/Http/Middleware/BootJsonApi.php#L94

Should be hit, and you need to inspect what this does at that point: https://github.com/laravel-json-api/laravel/blob/1a2205ee9aefd3edcce70e2ac6dc5dc112b7a0dc/src/Routing/Route.php#L211-L223

bhushan08 commented 2 years ago

@lindyhopchris Yes, I am accessing the same URL with the resource id. I will be checking what you suggested and updating it here if I found anything useful. Although, I am also using

public static function type(): string {
    return 'shop_employees';
}

public function uriType(): string {
    return self::type();
}

In My schema file, will that hamper the result which I am looking for?

Is there anything I should be adding in any of the file like resource or request file to make it compatible with requirements?

Do I have to add the query file as well, because I haven't added it.

bhushan08 commented 2 years ago

@lindyhopchris I tried to debug in hasSubstituteBindings function this is what I got

private function hasSubstitutedBindings(): bool
{
    $expected2 = $this->schema()->model();
    dd([
        "expected" => $expected2,
        "resourceIdName" => $this->resourceIdName(),
        "parameter" => $this->route->parameter($this->resourceIdName()),
        "hasSubstituteBindings" => $this->route->parameter($this->resourceIdName()) instanceof $expected2,
    ]);
    if ($name = $this->resourceIdName()) {
        $expected = $this->schema()->model();
        return $this->route->parameter($name) instanceof $expected;
    }

    return false;
}

I get this result

array:4 [
  "expected" => "App\Models\ShopEmployee"
  "resourceIdName" => "shop_employee"
  "parameter" => App\Models\ShopEmployee {#2254
    #table: "shop_employees"
    #appends: array:1 [
      0 => "user_image"
    ]
    +fillable: array:3 [
      0 => "shop_id"
      1 => "status"
      2 => "user_id"
    ]
    #casts: array:2 [
      "extra" => "array"
      "schedule" => "App\Casts\ScheduleCast"
    ]
    #connection: "mysql"
    #primaryKey: "id"
    #keyType: "int"
    +incrementing: true
    #with: []
    #withCount: []
    +preventsLazyLoading: false
    #perPage: 15
    +exists: true
    +wasRecentlyCreated: false
    #attributes: array:8 [
      "id" => 2
      "user_id" => 22
      "shop_id" => 1
      "status" => "active"
      "description" => null
      "created_at" => "2021-08-29 22:54:28"
      "updated_at" => "2021-09-15 19:17:59"
      "schedule" => null
    ]
    #original: array:8 [
      "id" => 2
      "user_id" => 22
      "shop_id" => 1
      "status" => "active"
      "description" => null
      "created_at" => "2021-08-29 22:54:28"
      "updated_at" => "2021-09-15 19:17:59"
      "schedule" => null
    ]
    #changes: []
    #classCastCache: []
    #dates: []
    #dateFormat: null
    #dispatchesEvents: []
    #observables: []
    #relations: []
    #touches: []
    +timestamps: true
    #hidden: []
    #visible: []
    #guarded: array:1 [
      0 => "*"
    ]
    +mediaConversions: []
    +mediaCollections: []
    #deletePreservingMedia: false
    #unAttachedMediaLibraryItems: []
  }
  "hasSubstituteBindings" => true
]

PS: I apologise, I am new to the laravel development, please let me know if I am doing anything wrong even with the debugging.