Add config options to allow fields to be updated using a setter method. #2645
Conversation
|
I would like to add tests for those changes as well. Do you want a separate/duplicated test for every affected annotation/listener? |
fwolfsjaeger
force-pushed
the
feature/use-setter-methods
branch
from
d5ba3a4
to
54b65be
Compare
|
I'm interested with this functionality. I hope soon will merged to master. |
fwolfsjaeger
force-pushed
the
feature/use-setter-methods
branch
from
54b65be
to
f86b03b
Compare
|
I'm also interested in this functionality. At the moment I override and extend the BlameableListener to add this functionality but it was made |
|
I'll try to find the time to add tests for the changes this week. |
fwolfsjaeger
force-pushed
the
feature/use-setter-methods
branch
from
46e3b15
to
9ffa94d
Compare
|
Alright, all done:
Let me know if you need anything else ;) |
fwolfsjaeger
force-pushed
the
feature/use-setter-methods
branch
from
4968c59
to
0f4649b
Compare
|
Any chance that this PR is getting merged? Even though there is no BC breaking change right now: I could refactor the changes to auto-detect a setter method by simply checking for setFieldName(). That however could break BC as there's no way to stick to the original way of setting the property directly. |
|
I'll rebase the latest changes right away. Many thanks for taking the time to review the changes! |
fwolfsjaeger
force-pushed
the
feature/use-setter-methods
branch
from
463bd96
to
bd614cc
Compare
This change is applied in Blameable, IpTraceable, SoftDeleteable & Timestampable annotations. The new attribute "setterMethod" can be passed to specify the setter method to be used for a field. If no setter method is specified, the property value would be set directly as before.
Fixed using setterMethod for SoftDeleteable annotations Added Unit tests for the setter methods Amended Code Style
franmomu
force-pushed
the
feature/use-setter-methods
branch
from
bd614cc
to
4f2c498
Compare
|
Thanks for keeping working into this @fwolfsjaeger, in general I see the code fine, but I would like to avoid |
| if (isset($mappingProperty['setterMethod'])) { | ||
| $this->setSetterMethod($field, $mappingProperty['setterMethod'], $config); | ||
| } |
There was a problem hiding this comment.
Since YAML drivers are deprecated, I'd rather don't add more code in the drivers
|
|
||
| /** | ||
| * Set the setter method for the given field. | ||
| */ | ||
| protected function setSetterMethod(string $field, ?string $method, array &$config): void | ||
| { | ||
| if (empty($method)) { | ||
| return; | ||
| } | ||
|
|
||
| if (!isset($config['setterMethod'])) { | ||
| $config['setterMethod'] = []; | ||
| } | ||
|
|
||
| $config['setterMethod'][$field] = $method; | ||
| } |
There was a problem hiding this comment.
This code doesn't seem to belong to File, if we don't use it in Yaml drivers, I'd move it to Xml
|
Sorry, but I'm no longer using DoctrineExtensions. I've created this and another PR about a year ago. That and the incompatibility with DBAL 4 forced me to just add my own listeners for the 3 attributes I was using. I'll look into the recent deprecations and requested changes, but it might take some time. |
Feature request: #2644
This change is applied in Blameable, IpTraceable, SoftDeleteable & Timestampable annotations. The new attribute "setterMethod" can be passed to specify the setter method to be used for a field. If no setter method is specified, the property value would be set directly as before.