Skip to content

Conversation

@ckrack
Copy link
Contributor

@ckrack ckrack commented Oct 23, 2018

Before this modification the make:form command allowed passing a fully specified classname.
It would generate a form with the correct bound class, but not read the fields.
It generated a form with one "field_name" field instead of something more useful.
This also affects DTOs.
This modification makes our lives easier by reading all fields of the bound class, except the id field.

Before this modification the make:form command allowed passing a fully specified classname.
It would generate a form with the correct bound class,  but not read the fields.
This also affects DTOs.
This modification makes our lives easier by reading all fields of the bound class.
Copy link
Member

@weaverryan weaverryan left a comment

Choose a reason for hiding this comment

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

Nice improvement! And in such little code! Minor comments

$this->fullClassName = $fullClassName;
}

public function getProperties():? array
Copy link
Member

Choose a reason for hiding this comment

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

Make this private and move to the bottom. At least right now, we don't need this to be public.

return $propertiesList;
}

public function getFormFields()
Copy link
Member

Choose a reason for hiding this comment

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

: array return type


namespace Symfony\Bundle\MakerBundle\Util;

use Symfony\Bundle\MakerBundle\Str;
Copy link
Member

Choose a reason for hiding this comment

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

Not needed?

Make method private,
Mark class as internal.
Add DocComment to public method.
Remove copied use statement.
@weaverryan
Copy link
Member

Thank you @ckrack! This is an obvious, but wonderful improvement. Great work!

@weaverryan weaverryan closed this in 4d2743d Nov 2, 2018
weaverryan added a commit that referenced this pull request Jul 8, 2020
This PR was merged into the 1.0-dev branch.

Discussion
----------

Docs: improve make:form helper

Add information about the second argument (bound-class).
Give hints on using the bound-class with classes other than entities.

Improves on #301 and fixes #477

Commits
-------

6a9b1c9 docs: improve make:form helper
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