-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Using Doctrine LifecycleCallbacks #1439
base: main
Are you sure you want to change the base?
Conversation
This is the modern solution to automatically update timestamps in Doctrine. I have added createdAt and updatedAt to all entities using LifecycleCallbacks.
src/Entity/Tag.php
Outdated
if ($this->getUpdatedAt() == null) { | ||
$this->setUpdatedAt(new \DateTimeImmutable('now')); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The null check does not make sense here. With this logic, we will basically set the updatedAt
once when creating the record and never update it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True, sorry as mentioned I copied from an older project that needed only one time to update.
src/Entity/Tag.php
Outdated
if ($this->getCreatedAt() == null) { | ||
$this->setCreatedAt(new \DateTimeImmutable('now')); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why does a PreUpdate
handler deal with the createdAt
property? That property should by definition be set once and never be updated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I just copied from an older project. The point was to demonstrate how it can be used and should be used in an official demo. I will update shortly.
{ | ||
$this->updatedAt = $updatedAt; | ||
|
||
return $this; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there is no reason to make those setters fluent. Other setters in the entity are not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
More over, there is not reason to add theses public method
composer.json
Outdated
@@ -50,6 +50,7 @@ | |||
"require-dev": { | |||
"dama/doctrine-test-bundle": "^7.0", | |||
"doctrine/doctrine-fixtures-bundle": "^3.4", | |||
"friendsofphp/php-cs-fixer": "^3.35", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unrelated change, please revert.
@@ -51,6 +56,12 @@ class Comment | |||
#[ORM\JoinColumn(nullable: false)] | |||
private ?User $author = null; | |||
|
|||
#[ORM\Column(type: 'datetime_immutable')] | |||
private $createdAt; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Our linters rightfully complain about missing property types. Please add them.
src/Entity/Comment.php
Outdated
@@ -51,6 +56,12 @@ class Comment | |||
#[ORM\JoinColumn(nullable: false)] | |||
private ?User $author = null; | |||
|
|||
#[ORM\Column(type: Types::DATETIMETZ_IMMUTABLE)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think that we want to use the timezone-aware datetime field here.
hello @Mecanik , I like the PR and I also use doctrine logic, but I fail/dont get what this PR adds as value on the side of this "symfony" demo? |
Thanks. My idea is to guide users to use not just Symfony "the right way" but it's core components like Doctrine. Realistically, nobody will read the doc unless they have to. I have seen thousands of projects developed the "same way" without any improvements or using modern functionality. I would like the demo to slowly become as modern as possible, using latest features, so when a newbie starts coding he follows the right path. |
This is the modern solution to automatically update timestamps in Doctrine. I have added
createdAt
andupdatedAt
to all entities using LifecycleCallbacks.