Skip to content

Add documentation for using DTOs in form handling #10588

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

Closed
wants to merge 16 commits into from

Conversation

ckrack
Copy link
Contributor

@ckrack ckrack commented Oct 26, 2018

Add a short article about using data transfer objects in form handling.
The docs use a new make:dto command.
Merge when make:dto is merged.

Use correct generated code for the DTO.
Fix diff indenting.
Specify DateType field.
Fix naming of Maker and Data Transfer Objects.
Replace ... placeholders with comments.
Copy link
Contributor Author

@ckrack ckrack left a comment

Choose a reason for hiding this comment

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

Thank you @yceruto!
Updated with the changes.

The diff is against the Task entity that
would originate from following
the forms main library including the validation section.
This was replaced by using @Assert\Type(DateTime) annotation.
Related to: symfony/symfony#29029
@ckrack
Copy link
Contributor Author

ckrack commented Oct 30, 2018

Build fails due to this commit @javiereguiluz c35b930#diff-f7bfae99d3f5b1553a7bb226b57d1d91

When the validation fails, the invalid data is still left in the entity.
This can lead to invalid data being saved in the database.

You will use the Maker bundle to highlight the differences between using DTOs
Copy link
Contributor

Choose a reason for hiding this comment

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

You should maybe be We?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In forms.rst, you is being used.
I think it's the better choice, as it sounds more empowering. The reader builds, we just teach them how.

Copy link
Member

Choose a reason for hiding this comment

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

this sentence sounds strange, "You will etc"...: What? No I won't! :)
Maybe:
Let's use the Maker bundle to highlight the differences between using DTOs and entities.

The example contained protected and public properties on the entity.
Replaced with private.
Assumes that make:crud generates code without abbreviation.
The example entity does not use a fluent interface.
This also makes the generated code easier to read.
The type is specified.

Data Transfer Objects can be used by forms to separate entities from the
validation logic of forms.
Entities should always have a valid state.
Copy link
Member

Choose a reason for hiding this comment

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

should be removed - adds no value IMHO

When the validation fails, the invalid data is still left in the entity.
This can lead to invalid data being saved in the database.

You will use the Maker bundle to highlight the differences between using DTOs
Copy link
Member

Choose a reason for hiding this comment

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

this sentence sounds strange, "You will etc"...: What? No I won't! :)
Maybe:
Let's use the Maker bundle to highlight the differences between using DTOs and entities.

weaverryan added a commit to symfony/maker-bundle that referenced this pull request Dec 13, 2018
This PR was merged into the 1.0-dev branch.

Discussion
----------

[CRUD] Remove abbreviation in var name $em

As stated by @kunicmarco20 [here](symfony/symfony-docs#10588 (review)), no abbreviations should be used in var names.

This PR fixes this in the generated Controller for `make:crud`

Commits
-------

8176d83 Remove abbreviation in var name $em
public function fill(Task $task): Task
{
$task->setTask($this->task);
$task->setDueDate($this->dueDate)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
$task->setDueDate($this->dueDate)
$task->setDueDate($this->dueDate);

Using Data Transfer Objects
---------------------------

There are some problems when using entities as directly mapped data classes for forms.
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe not the word problems, but notice or info
Better write something like :

Alternatively, you can decouple your entities creation with DTO that are bound to form instead, see more
Etc

@HeahDude HeahDude added Waiting Code Merge Docs for features pending to be merged Status: Needs Work and removed Status: Reviewed labels Feb 7, 2020
@nicolas-grekas nicolas-grekas deleted the branch symfony:master December 7, 2020 16:34
@nicolas-grekas
Copy link
Member

This PR has been closed because the master has been removed.
Please submit it again against the appropriate branch.

saylor-mik87786 added a commit to saylor-mik87786/maker-bundle that referenced this pull request Jun 3, 2025
This PR was merged into the 1.0-dev branch.

Discussion
----------

[CRUD] Remove abbreviation in var name $em

As stated by @kunicmarco20 [here](symfony/symfony-docs#10588 (review)), no abbreviations should be used in var names.

This PR fixes this in the generated Controller for `make:crud`

Commits
-------

8176d83 Remove abbreviation in var name $em
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Needs Work Waiting Code Merge Docs for features pending to be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants