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

Fill props from route parameters #341

Merged
merged 22 commits into from
Feb 10, 2023

Conversation

ragulka
Copy link
Contributor

@ragulka ragulka commented Feb 1, 2023

Re-opening in place of #303 which was prematurely closed by GH.

Note: I have updated the usage examples below to reflect the latest version of this PR.

This PR introduces 2 new attributes that data properties can leverage to fill them from route parameters:

  • FromRouteParameter - fills the data property with the route parameter (can be string, integer, object, array, etc)
  • FromRouteParameterProperty - fills the data property with a property from the route parameter

These attributes are handled in the FillRouteParameterPropertiesDataPipe. The implementation is based on the discussion here, however the details have evolved quite a bit during the original PR's lifetime.

Usage examples

See tests for all the possible usage examples and how different configurations are handled.

Docs

Please review the docs on how to use this feature.

@ragulka ragulka marked this pull request as ready for review February 1, 2023 15:20
@ragulka
Copy link
Contributor Author

ragulka commented Feb 1, 2023

@rubenvanassche I've updated the implementation based on our discussion and added docs in b87c4ca.

Would you mind giving this (and especially the docs) a review and see if this all makes sense? Did I miss anything?

@rubenvanassche
Copy link
Member

This is looking wonderful, made some small changes in code readability and removed a few sections from the docs. Thanks!

@rubenvanassche rubenvanassche merged commit b87c4ca into spatie:main Feb 10, 2023
@ragulka ragulka deleted the fill-props-from-route-models branch February 10, 2023 11:54
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