Remove SnapshotPageProxy hard coded dependancy #552
Conversation
| @@ -33,6 +32,8 @@ class Transformer implements TransformerInterface | |||
|
|
|||
| protected $blockManager; | |||
|
|
|||
| protected $snapshotPageProxyClass; | |||
FabienD
force-pushed
the
snproxy
branch
2 times, most recently
from
2bc8d16 to
2f19a19
Compare
| { | ||
| $this->snapshotManager = $snapshotManager; | ||
| $this->transformer = $transformer; | ||
| $this->snapshotPageProxyClass = $snapshotPageProxyClass; |
There was a problem hiding this comment.
Is this should be mandatory?
In this case, you should throw a deprecated error.
There was a problem hiding this comment.
Yes, it is. A default value is provided in the constructor.
Could you please elaborate? Do you have some concrete cases where this class should be extended / overridden? |
|
Hi, I've made this PR, just because, it's the only way I found to add a new property on the Page entity (cf. #551). I want to have the possibility to set a specific css class by page. Thx. |
|
@rande what do you think? |
FabienD
force-pushed
the
snproxy
branch
2 times, most recently
from
d5aac2d to
fad0074
Compare
|
No feedback ? |
|
I will prefer a factory to avoid injecting class name. |
|
Ok, I'll try to do that. Thank you. |
FabienD
force-pushed
the
snproxy
branch
2 times, most recently
from
902eace to
e632fea
Compare
|
Hi, I've refactored the PR in order to use a factory to instantiate the SnapshotProxy class. Now, the SnapshotManager takes a 4th argument, the SnapshotProxyFactory. I'm not confortable with "tests", I've updated the SnapshotManagerTest but I don't know how to fix the failed test in CmsSnapshotManagerTest. I need help to fix the failed tests. In addition, I've removed the getPageByName method from the SnapshotManager, this method seems not to be used, the snapshotProxy is instantiated with only 2 args, the class needs 3. What do you think about this refactoring ? Thx. |
| */ | ||
| public function __construct($class, ManagerRegistry $registry, $templates = array()) | ||
| public function __construct($class, ManagerRegistry $registry, $templates = array(), SnapshotPageProxyFactory $snapshotPageProxyFactory) |
|
According to the new Sonata version management and next major release plan, this project has been refactored regarding branching and versioning. If you see this message, your PR concerns a patch or a minor release and is not targeting the right branch. So I'm closing this one, but don't see it as a refusal. If you think your work is still relevant and want to continue, feel free to reopen it on the right branch (e.g. the default one). Regards. |
According what I've explained in the issue #551, I've built this PR in order to make SnapshotPageProxy customisable in an easy way (I think).