Skip to content

Conversation

@neellii
Copy link

@neellii neellii commented Oct 28, 2025

refs: #217

@DenTray DenTray requested a review from Copilot November 3, 2025 15:48
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR enhances the request generation functionality to automatically populate available relations in generated request classes. The key improvement is moving the getRelationName method to the base EntityGenerator class for reuse and implementing logic to automatically extract and populate relations in request classes.

  • Moved getRelationName method from ModelGenerator to EntityGenerator for shared use across generators
  • Added getAvailableRelations method to RequestsGenerator that extracts relation names from the model
  • Updated request stub to conditionally show TODO comment only when no relations are available

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/Generators/EntityGenerator.php Added shared getRelationName method to base class
src/Generators/ModelGenerator.php Removed getRelationName method (moved to base class) and constant
src/Generators/RequestsGenerator.php Added getAvailableRelations method and passes relations to stub
stubs/request.blade.php Updated to render relations array and conditionally show TODO comment
tests/fixtures/RequestGeneratorTest/search_request.php Test fixture updated with populated relations
tests/fixtures/RequestGeneratorTest/get_request.php Test fixture updated with populated relations
tests/fixtures/CommandTest/*.php Test fixtures updated with empty array syntax
tests/Support/Model/ModelMockTrait.php Removed extra blank line

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

return ucwords(Str::replace('/', '\\', $name), '\\');
}

protected function getRelationName(string $relation, string $type): string
Copy link

Copilot AI Nov 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trailing whitespace at end of line should be removed.

Suggested change
protected function getRelationName(string $relation, string $type): string
protected function getRelationName(string $relation, string $type): string

Copilot uses AI. Check for mistakes.
Comment on lines 217 to 220
$availableRelations = [
...$availableRelations,
...Arr::map($entities, fn ($entity) => $this->getRelationName($entity, $type)),
];
Copy link

Copilot AI Nov 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Variable reassignment inside loop is inefficient. The spread operator creates a new array on each iteration. Consider using array_push or collecting values first, then merging once after the loop.

Suggested change
$availableRelations = [
...$availableRelations,
...Arr::map($entities, fn ($entity) => $this->getRelationName($entity, $type)),
];
array_push(
$availableRelations,
...Arr::map($entities, fn ($entity) => $this->getRelationName($entity, $type))
);

Copilot uses AI. Check for mistakes.
Comment on lines 217 to 220
$availableRelations = [
...$availableRelations,
...Arr::map($entities, fn ($entity) => $this->getRelationName($entity, $type)),
];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$availableRelations = [
...$availableRelations,
...Arr::map($entities, fn ($entity) => $this->getRelationName($entity, $type)),
];
array_push(
$availableRelations,
...Arr::map($entities, fn ($entity) => $this->getRelationName($entity, $type)),
);

Comment on lines 316 to 318
$pluralRelations = ['belongsToMany', 'hasMany'];

if (in_array($type, $pluralRelations)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$pluralRelations = ['belongsToMany', 'hasMany'];
if (in_array($type, $pluralRelations)) {
if (in_array($type, ['belongsToMany', 'hasMany'])) {

@endif
@if($needToValidateWith)

@if(empty($availableRelations))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the TODO should be added in all cases

@DenTray DenTray assigned neellii and unassigned DenTray Nov 3, 2025
@neellii neellii assigned DenTray and unassigned neellii Nov 4, 2025
protected function getRelationType(string $model, string $relation): string
{
if (in_array($relation, self::PLURAL_NUMBER_REQUIRED)) {
if (in_array($relation, ['belongsToMany', 'hasMany'])) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's implement the separate helper for it

Suggested change
if (in_array($relation, ['belongsToMany', 'hasMany'])) {
if ($this->isPluralRelation($relation)) {

@DenTray DenTray assigned neellii and unassigned DenTray Nov 4, 2025
@neellii neellii assigned DenTray and unassigned neellii Nov 4, 2025
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.

4 participants