-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Create Archetype Component and Bundle to replace Product Prototype and allow for inheritance #2225
Conversation
@adamelso I looked around briefly, awesome work! But why we need another set of classes in the archetype component? Can't it just use the ones that we define in Variation and Attribute component? |
@pjedrzejewski Cool! Thank you. Yeah, I wasn't sure if I needed to do this in the same way that the classes were being extended for the Product component but we don't need them and I've now removed those intermediate models. |
), | ||
); | ||
|
||
$metadata->mapManyToOne($parentMapping); |
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.
Unfortunately, I am unable to use the Tree Doctrine extension like Taxonomy does, because the parent mapping has to be done dynamically for each subject (eg. Product
) and trying to hook into the DoctrineExtensions metadata config is really hard: https://github.com/Atlantic18/DoctrineExtensions/blob/master/lib/Gedmo/Tree/Mapping/Driver/Xml.php#L170
e212b69
to
d69af4d
Compare
'referencedColumnName' => 'id', | ||
'nullable' => false, | ||
'unique' => false, | ||
// 'onDelete' => 'CASCADE', ? |
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.
Do we need to casade on delete for archetype options?
a1c5bf9
to
de7d000
Compare
Conflicts: composer.lock src/Sylius/Bundle/ProductBundle/DependencyInjection/Configuration.php src/Sylius/Bundle/ProductBundle/Resources/config/doctrine/model/Attribute.orm.xml src/Sylius/Bundle/ProductBundle/Resources/config/doctrine/model/Option.orm.xml
@adamelso I've sent you a PR with few tweaks and simplifications. Really nice work! If you accept my PR, we can rebase this one and merge it into the core! :) adamelso#4 |
@pjedrzejewski Awesome! I see that you've create a new PR #2298 . Do you still need me to do anything on my part? In the meantime I'm going to test this with my classifieds ads project to see how it works. |
Few simplifications and new building system
Conflicts: composer.lock features/backend/product_prototypes.feature phpspec.yml src/Sylius/Bundle/ArchetypeBundle/spec/Sylius/Bundle/ArchetypeBundle/Form/Type/ArchetypeTypeSpec.php src/Sylius/Bundle/AttributeBundle/Resources/translations/messages.de.yml src/Sylius/Bundle/AttributeBundle/Resources/translations/messages.es.yml src/Sylius/Bundle/AttributeBundle/Resources/translations/messages.fr.yml src/Sylius/Bundle/AttributeBundle/Resources/translations/messages.nl.yml src/Sylius/Bundle/AttributeBundle/Resources/translations/messages.pl.yml src/Sylius/Bundle/AttributeBundle/Resources/translations/messages.ru.yml src/Sylius/Bundle/AttributeBundle/Resources/translations/messages.sk.yml src/Sylius/Bundle/AttributeBundle/Resources/translations/messages.sr.yml src/Sylius/Bundle/AttributeBundle/Resources/translations/validators.de.yml src/Sylius/Bundle/AttributeBundle/Resources/translations/validators.es.yml src/Sylius/Bundle/AttributeBundle/Resources/translations/validators.fr.yml src/Sylius/Bundle/AttributeBundle/Resources/translations/validators.nl.yml src/Sylius/Bundle/AttributeBundle/Resources/translations/validators.ru.yml src/Sylius/Bundle/ProductBundle/DependencyInjection/Configuration.php src/Sylius/Bundle/ProductBundle/Resources/config/services.xml src/Sylius/Bundle/ProductBundle/Resources/translations/messages.de.yml src/Sylius/Bundle/ProductBundle/Resources/translations/messages.tr.yml src/Sylius/Bundle/ProductBundle/Resources/translations/validators.de.yml src/Sylius/Bundle/ProductBundle/spec/Form/Type/PrototypeTypeSpec.php src/Sylius/Bundle/ProductBundle/spec/Sylius/Bundle/ProductBundle/Form/Type/PrototypeTypeSpec.php src/Sylius/Bundle/VariationBundle/Resources/translations/messages.de.yml src/Sylius/Bundle/VariationBundle/Resources/translations/messages.es.yml src/Sylius/Bundle/VariationBundle/Resources/translations/messages.fr.yml src/Sylius/Bundle/VariationBundle/Resources/translations/messages.nl.yml src/Sylius/Bundle/VariationBundle/Resources/translations/messages.pl.yml src/Sylius/Bundle/VariationBundle/Resources/translations/messages.ru.yml src/Sylius/Bundle/WebBundle/Resources/translations/menu.de.yml src/Sylius/Bundle/WebBundle/Resources/translations/messages.de.yml src/Sylius/Component/Product/spec/Builder/PrototypeBuilderSpec.php src/Sylius/Component/Product/spec/Model/PrototypeSpec.php src/Sylius/Component/Product/spec/Sylius/Component/Product/Model/ArchetypeSpec.php src/Sylius/Component/Product/spec/Sylius/Component/Product/Model/PrototypeSpec.php
We'll need to add Translations for the Archetype entity, but this should be easy enough (and shouldn't hold up this PR). |
* | ||
* @param ArchetypeSubjectInterface $subject | ||
*/ | ||
public function build(ArchetypeSubjectInterface $subject); |
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.
Should ArchetypeSubjectInterface
perhaps not extend the AttributeSubjectInterface
and VariableInterface
? I'm thinking a scenario where an archetype may only use attributes, but not options, yet the ArchetypeSubjectInterface
requires the implementation of methods that manipulate options.
Instead, the building of attributes or options can be taking care in an implementation of ArchetypeBuilderInterface::build()
.
@peteward I can start working on making this translatable during this week. |
Conflicts: composer.lock src/Sylius/Bundle/ProductBundle/Resources/config/services.xml src/Sylius/Component/Product/Model/ProductInterface.php
Create Archetype Component and Bundle to replace Product Prototype and allow for inheritance
Thank you Adam! Awesome work. 👍 |
@adamelso, I've asked @gonzalovilaseca to implement translations for this new component. You haven't started it have you? |
@peteward Perfect, I'll leave it for him to implement as he'll be able to deliver it faster than I would; I don't with Sylius full time, only evenings and weekends. :)
|
PR sent! #2402 |
The new Archetype component and bundle is designed to replace the existing Product Prototypes, introducing inheritance between archetypes, and to be abstract, so any model other than a product can be the subject of archetyping.
I'll probably need some assistance with resolving the interfaces to the correct Doctrine mapping, or whether my implement is correct or not.