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

Incorrect value in Postman Doc for class passed into route #151

Closed
1 task done
jamespavett opened this issue Dec 2, 2020 · 9 comments
Closed
1 task done

Incorrect value in Postman Doc for class passed into route #151

jamespavett opened this issue Dec 2, 2020 · 9 comments
Labels
done in v3 enhancement New feature or request

Comments

@jamespavett
Copy link

jamespavett commented Dec 2, 2020

What happened?

  1. I set my configuration to...
  2. Then I ran php artisan scribe:generate ...
  3. But I saw...

I am making use of Laravel apiResource groups for some of my models.

Within the controller I am using Laravel's dependency injection to get access to the class (in this instance User) straight away in the method. For example the following update function:

public function update(Request $request, User $user)
{
    ....
}

However inside the collection.json generated by running php artisan scribe:generate, the value assigned to the variable is just some random string. In the below screenshots for the user model it is getting the string velit passed to it, while in reality that should be an integer reflecting the ID of the resource.

Right now I am not sure if there is fault in the package of whether I am making a mistake somewhere, but I cannot see what else I can change on my end. Any help would be appreciated.

Screenshots and stack traces:

image

My environment:

  • PHP version (from php -v): 7.4.3
  • Framework (Laravel/Lumen): Laravel
  • Laravel/Lumen version (from composer show laravel/framework or composer show laravel/lumen-framework): 8.16.1
  • Scribe version (from composer show knuckleswtf/scribe): 2.3.0
@jamespavett jamespavett added the bug Something isn't working label Dec 2, 2020
@shalvah
Copy link
Contributor

shalvah commented Dec 2, 2020

That's expected behaviour. That value does not come from the $user function parameter, but the {user} URL parameter. Scribe automatically picked that parameter up from your route's path and guessed that its type is string. If you don't want that, then document the parameter properly with @urlParam.

@shalvah shalvah closed this as completed Dec 2, 2020
@jamespavett
Copy link
Author

Cheers for getting back to me.

Is there no other way to get around this? Only reason I ask is because this sort of messes up with the PHP Doc comments I already have, they still request me pass any parameters through @param.

@carestad
Copy link

Wouldn't it be possible for Scribe to parse this differently? Maybe check the type hinted class in the controller rmethod and if it is a class that extends Laravel's Model, one can assume it is that model's primary key that is actually expected there and one could check the model's $keyType property to see if the primary key is an int (default) or string.

@shalvah
Copy link
Contributor

shalvah commented May 21, 2021

Interesting suggestion that I can consider for v3, but how often do models have a $keyType? I've never seen it before, and I don't think Laravel sets it by default. Is it always:

  • if no $keyType, assume int
  • otherwise, use $keyType?

@shalvah shalvah reopened this May 21, 2021
@shalvah shalvah added enhancement New feature or request and removed bug Something isn't working labels May 21, 2021
@carestad
Copy link

carestad commented May 21, 2021

Interesting suggestion that I can consider for v3, but how often do models have a $keyType? I've never seen it before, and I don't think Laravel sets it by default. Is it always:

* if no `$keyType`, assume `int`

* otherwise, use `$keyType`?

Yep, that sounds about right. It is declared in Laravel's base model here: https://github.com/laravel/framework/blob/8.x/src/Illuminate/Database/Eloquent/Model.php#L61

And it can be fetched using ->getKeyType(), so for example: $keyType = (new User)->getKeyType().

I believe it can be string too in some circumstances, which is why I considered it more safe to just check it, otherwise I would just assume int anyway.

@shalvah
Copy link
Contributor

shalvah commented May 21, 2021 via email

@carestad
Copy link

carestad commented May 21, 2021 via email

@shalvah
Copy link
Contributor

shalvah commented May 24, 2021

Alright. Noted for v3.

@shalvah shalvah added needs backporting Done in v3; needs backporting to v2 done in v3 and removed needs backporting Done in v3; needs backporting to v2 labels May 25, 2021
@shalvah shalvah closed this as completed May 27, 2021
@shalvah
Copy link
Contributor

shalvah commented Jun 7, 2021

🎉v3 is out now! Should be smarter with routes and URL parameters now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
done in v3 enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants