[Archetype] Parent archetype choices list should not contain current archetype #3381

CoderMaggie · 2015-10-02T09:10:35Z

Q	A
Bug fix?	yes
New feature?	no
BC breaks?	no
Deprecations?	no
Fixed tickets	#3359
License	MIT

stloyd · 2015-10-02T09:22:48Z

src/Sylius/Bundle/ArchetypeBundle/Form/Type/ArchetypeType.php

@@ -11,6 +11,7 @@

 namespace Sylius\Bundle\ArchetypeBundle\Form\Type;

+use Sylius\Bundle\ResourceBundle\Doctrine\ORM\EntityRepository;


This should be interface to not blocking people using ODM.

pjedrzejewski · 2015-10-02T10:10:38Z

src/Sylius/Bundle/ArchetypeBundle/Form/Type/ArchetypeType.php

@@ -44,6 +45,8 @@ function __construct($dataClass, array $validationGroups, $subject)
     */
    public function buildForm(FormBuilderInterface $builder, array $options)
    {
+        $currentArchetype = $builder->getForm()->getData();


We should do a listener, cause it only works when you pass the data when creating the form. If someone calls setData later, it will not be taken into account. :)

Changed! :)

Zales0123 · 2015-10-02T12:57:26Z

src/Sylius/Bundle/ArchetypeBundle/Form/EventListener/ParentArchetypeListener.php

+                    ->createQueryBuilder('parentArchetype')
+                    ->where('parentArchetype.id != :id')
+                    ->setParameter('id', $currentArchetype->getId())
+                    ;


pjedrzejewski · 2015-10-07T08:12:41Z

src/Sylius/Bundle/ArchetypeBundle/Form/EventListener/ParentArchetypeListener.php

+use Symfony\Component\Form\FormEvents;
+
+
+/**


Extra blank line.

pjedrzejewski · 2015-10-07T08:14:56Z

👍

michalmarcinkowski · 2015-10-08T08:40:24Z

src/Sylius/Bundle/ArchetypeBundle/Form/EventListener/ParentArchetypeListener.php

+    {
+        $currentArchetype = $event->getData();
+        if (!$currentArchetype instanceof ArchetypeInterface) {
+           throw new UnexpectedTypeException($currentArchetype, 'Sylius\Component\Archetype\Model\ArchetypeInterface');


Instead of hardcoded string we should use ArchetypeInterface::class

michalmarcinkowski · 2015-10-09T09:47:04Z

src/Sylius/Bundle/ArchetypeBundle/Form/EventListener/ParentArchetypeListener.php

+            'property' => 'name'
+        );
+
+        if (null != $currentArchetype->getId()) {


Does it break anything if we remove this if statement?

Yes :) We need to check if the current archetype already has ID, because if it doesn't (for instance while creating new archetype) the query for possible parent archetype is failing because it uses Null in the where statement.

[Archetype] Parent archetype choices list should not contain current archetype

pjedrzejewski · 2015-10-19T21:45:59Z

Thank you Magda! 👍

stloyd reviewed Oct 2, 2015
View reviewed changes

CoderMaggie force-pushed the current-archetype-as-parent branch from f812012 to d5633ee Compare October 2, 2015 09:41

pjedrzejewski reviewed Oct 2, 2015
View reviewed changes

CoderMaggie force-pushed the current-archetype-as-parent branch from d5633ee to 9432f57 Compare October 2, 2015 12:43

Zales0123 reviewed Oct 2, 2015
View reviewed changes

CoderMaggie force-pushed the current-archetype-as-parent branch from 9432f57 to a235155 Compare October 2, 2015 13:55

michalmarcinkowski added the Bug Fix label Oct 7, 2015

michalmarcinkowski added this to the v0.16.0 milestone Oct 7, 2015

pjedrzejewski reviewed Oct 7, 2015
View reviewed changes

CoderMaggie force-pushed the current-archetype-as-parent branch from a235155 to 6e6a01f Compare October 7, 2015 13:24

michalmarcinkowski reviewed Oct 8, 2015
View reviewed changes

CoderMaggie force-pushed the current-archetype-as-parent branch from 6e6a01f to 41a51ca Compare October 9, 2015 08:17

michalmarcinkowski reviewed Oct 9, 2015
View reviewed changes

Parent archetype choices list should not contain current archetype

6f934a3

CoderMaggie force-pushed the current-archetype-as-parent branch from 41a51ca to 6f934a3 Compare October 12, 2015 13:43

pjedrzejewski pushed a commit that referenced this pull request Oct 19, 2015

Merge pull request #3381 from TheMadeleine/current-archetype-as-parent

061a435

[Archetype] Parent archetype choices list should not contain current archetype

pjedrzejewski merged commit 061a435 into Sylius:master Oct 19, 2015

pjedrzejewski mentioned this pull request Oct 20, 2015

[Archetype] I shouldn't be able to set current archetype as parent #3359

Closed

CoderMaggie deleted the current-archetype-as-parent branch April 6, 2016 08:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Archetype] Parent archetype choices list should not contain current archetype #3381

[Archetype] Parent archetype choices list should not contain current archetype #3381

CoderMaggie commented Oct 2, 2015

stloyd Oct 2, 2015

pjedrzejewski Oct 2, 2015

CoderMaggie Oct 2, 2015

Zales0123 Oct 2, 2015

pjedrzejewski Oct 7, 2015

pjedrzejewski commented Oct 7, 2015

michalmarcinkowski Oct 8, 2015

pjedrzejewski Oct 19, 2015

michalmarcinkowski Oct 9, 2015

CoderMaggie Oct 9, 2015

pjedrzejewski commented Oct 19, 2015

		@@ -11,6 +11,7 @@

		namespace Sylius\Bundle\ArchetypeBundle\Form\Type;

		use Sylius\Bundle\ResourceBundle\Doctrine\ORM\EntityRepository;

[Archetype] Parent archetype choices list should not contain current archetype #3381

[Archetype] Parent archetype choices list should not contain current archetype #3381

Conversation

CoderMaggie commented Oct 2, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pjedrzejewski commented Oct 7, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pjedrzejewski commented Oct 19, 2015