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

Omit redundant parent ID validation #732

Conversation

nicodevs
Copy link
Contributor

@nicodevs nicodevs commented Jan 5, 2025

Since #725, we can use meta.parent to generate routes with nested model binding. For example, this draft:

controllers:
  Comment:
    resource: api
    meta:
      parent: post

... generates the following routes:

  GET|HEAD        api/posts/{post}/comments
  POST            api/posts/{post}/comments
  GET|HEAD        api/posts/{post}/comments/{comment}
  PUT|PATCH       api/posts/{post}/comments/{comment}
  DELETE          api/posts/{post}/comments/{comment}

Laravel automatically resolves the Post model matching the provided {post} ID or returns a 404 if not found. For this reason, receiving and validating post_id in the FormRequest for these routes is redundant.

This PR removes the generation of form requests validation rules for columns named after the controller meta.parent plus the _id suffix (e.g., post_id).

Before and after

Validation Rules Before:

return [
    'body' => ['required', 'string'],
    'post_id' => ['required', 'integer', 'exists:posts,id'],
];

Validation Rules After:

return [
    'body' => ['required', 'string'],
];

Testing

This PR includes a test to verify the generated form request for a controller with a parent. All existing tests remain unchanged and are passing, ensuring form requests are correctly generated in all other scenarios.

@jasonmccreary jasonmccreary merged commit a748d8a into laravel-shift:master Jan 5, 2025
25 checks passed
@nicodevs nicodevs deleted the omit-redundant-parent-id-validation branch January 5, 2025 21:18
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.

2 participants