Skip to content
This repository has been archived by the owner on Sep 16, 2021. It is now read-only.

Fix form for orm #277

Merged
merged 3 commits into from
Apr 1, 2016
Merged

Fix form for orm #277

merged 3 commits into from
Apr 1, 2016

Conversation

dbu
Copy link
Member

@dbu dbu commented Mar 31, 2016

fix #275

@dbu dbu added this to the 1.2 milestone Mar 31, 2016
@wouterj
Copy link
Member

wouterj commented Mar 31, 2016

👍 (maybe add a test to test if everything works correctly for both PHPCR + ORM?)

@ElectricMaxxx
Copy link
Member

There is a broken WebTest. Looks like an other Field is missing now.

@@ -91,6 +91,7 @@ public function configureOptions(OptionsResolver $resolver)
'data_class' => $this->dataClass,
'translation_domain' => 'CmfSeoBundle',
'required' => false,
'by_reference' => false,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

adding this makes the test fail. i checked in the sandbox, and with PHPCR this leads to "A detached document was found through a child relationship during cascading a persist operation: Symfony\Cmf\Bundle\SeoBundle\Doctrine\Phpcr\SeoMetadata@0000000073c1c05d0000000089dcaddb"

i will make the form type aware of whether we use ORM or not.

@dbu dbu force-pushed the fix-form-for-orm branch from 41d44a8 to 09c4440 Compare April 1, 2016 08:38
@dbu dbu force-pushed the fix-form-for-orm branch from 09c4440 to 7568706 Compare April 1, 2016 08:43
@@ -78,6 +78,11 @@ public function load(array $configs, ContainerBuilder $container)
$sonataBundles[] = 'SonataDoctrineORMBundle';
}

$container->setParameter($this->getAlias().'.form_mode_orm',
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can't use the backend_type_* parameters because they must only be set if true (the doctrine compiler pass only checks if the parameter exists, not the value of the parameter). and anyways, we want this logic in case somebody enabled both backends.

@dbu
Copy link
Member Author

dbu commented Apr 1, 2016

okay, getting there. pushed some fixes for the --dev build.

@dbu
Copy link
Member Author

dbu commented Apr 1, 2016

and its green! review anybody?

@ElectricMaxxx
Copy link
Member

Looks good to me. Nice hack with the orm parameter.

@dbu dbu merged commit 2d80990 into master Apr 1, 2016
@dbu dbu deleted the fix-form-for-orm branch April 1, 2016 11:46
@lsmith77 lsmith77 removed the wip/poc label Apr 1, 2016
@ElectricMaxxx
Copy link
Member

@dbu i would like to give you the honor of tagging and releasing :-) (ill at home with ill children next to me)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Don't persist seoMetadata on Sonata Admin update
4 participants