Skip to content

Conversation

@zuzana-kunckova
Copy link
Contributor

I would like to propose to change User in the Generating Resources section example to UserResource to avoid conflict with the User model namespace when importing the User resource in the UserController.

@taylorotwell
Copy link
Member

If we do this there may be a few other spots in this piece of documentation that need to be updated. Are you able to check for those? Thanks.

@zuzana-kunckova
Copy link
Contributor Author

Yes, I'll go over the whole section and update the PR.

@driesvints driesvints changed the title change User to UserResource [8.x] Change User to UserResource Jan 5, 2021
@taylorotwell
Copy link
Member

taylorotwell commented Jan 5, 2021

Reading over this section gave me a bit of pause:

image

I think we will need to make a slight change to the framework if we make this documentation change since people will use the Resource suffix when generating resources which means the default resource collections will no longer be able to find the resource they map to.

I think I'll need to update this method in the framework to also look for FooResource:

image

@driesvints
Copy link
Member

Isn't the more simpler solution to alias the User resource import in the controller?

@zuzana-kunckova
Copy link
Contributor Author

I didn't realise it would mean making more extensive changes, sorry, Taylor.

If you don't make the change and continue to alias the User resource import in the controller, it might be good to mention in the docs that this is what we are doing and why. I didn't notice the alias when I first started working with API resources. Also, when I started creating resources by giving them more descriptive names, e.g. UserResource instead of just User, it wasn't clear whether that wouldn't break something somewhere else, whether the resource HAD to be same as the model name.

@taylorotwell
Copy link
Member

Personally I think it would be nice to add the $collects property to the resource collection stub class and make that connection a bit more explicit and de-emphasize this magical convention. I may look into that today.

@taylorotwell
Copy link
Member

I've pushed a change so that resource collections will look for a class with a Resource suffix now as well as the current behavior of no suffix.

image

@zuzana-kunckova
Copy link
Contributor Author

Awesome, thank you, Taylor!

@taylorotwell taylorotwell merged commit 37303ad into laravel:8.x Jan 12, 2021
@zuzana-kunckova zuzana-kunckova deleted the update-api-resources branch January 12, 2021 18:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants