Fixed a bug in normalizing URL parameters

[Khaled-Farhat]

This pull request fixes #275. The bug is that Scribe uses the id as the default URL parameter when no inline binding is applied, but the user might be using another column by overriding the getRouteKeyName() method in the model.

Behavior

Old behavior

1. If there is an inline binding in the route, use it.
2. Else, if it is a resource route parameter, use the id.
3. Otherwise, keep the same parameter name.

New behavior

1. If there is an inline binding in the route, use it.
2. Else, If there is a type-hinted variable in the controller action, with the same name as the route segment name, use it to call getRouteKeyName().
3. Else, if it is a resource route parameter, use the id.
4. Otherwise, keep the same parameter name.

Examples of the differences can be found in a previous discussion.

Code

Before

Camel\Extraction\ExtractedEndpointData@normalizeResourceParamName is called in the constructor before calling the parent constructor and initializing the object, so you cannot access the method property in the normalization process, as it has not been initialized yet.

After

Camel\Extraction\ExtractedEndpointData@normalizeResourceParamName is called after calling the parent constructor.
If there was no inline binding in the route, Camel\Extraction\ExtractedEndpointData@getFieldBindingForUrlParam will search for a type-hinted variable whose name matches the route segment name, and will use it to call getRouteKeyName() on the model.

Testing

Created two feature tests in Knuckles\Scribe\Tests\GenerateDocumentationTest to test generating URL params from resource and non-resource routes with model binding. Created some fixtures to achieve that: A TestPost model that overrides getRouteKeyName() to return slug, A partial resource controller with an injected TestPost argument, and another partial resource controller for a nested resource (posts.users) with injected TestPost and TestUser models.

Possible Refactor

Maybe we can create a new class Src\Extracting\UrlParamsNormalizer and move the normalization logic into it. We can use that class inside the constructor of Camel\Extraction\ExtractedEndpointData.

[shalvah]

Ah, sweet!

Sure, go for the UrlParamsNormalizer . Sounds like a good idea too. Better to extract that code into its own thing.

[shalvah]

So, is your other PR still needed?

[Khaled-Farhat]

No. I will close it.

[shalvah]

Implemented the UrlParamsNormalizer and did a bunch of refactoring in 97065d9. The logic for the whole thing is now (hopefully) simpler, and there's a better separation between the jobs of ExtractedEndpointData and GetFromLaravelAPI.