Skip to content
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

Problems customising Block entity #36

Closed
sweoggy opened this issue Oct 13, 2017 · 10 comments
Closed

Problems customising Block entity #36

sweoggy opened this issue Oct 13, 2017 · 10 comments
Labels

Comments

@sweoggy
Copy link
Contributor

sweoggy commented Oct 13, 2017

When I try to customise the Block entity, using the http://docs.sylius.org/en/latest/customization/model.html guide, I get an exception when trying to updated the database schema

[Doctrine\DBAL\Schema\SchemaException]
The table with name 'persiennbutiken_ng.bitbag_cms_block' already exists.

My config looks the following:

sylius_resource:
    resources:
        bitbag.block:
            classes:
                model: AppBundle\Entity\CmsPlugin\Block

And my entity:

/**
 * @ORM\Table(name="bitbag_cms_block")
 * @ORM\Entity(repositoryClass="BitBag\CmsPlugin\Repository\BlockRepository")
 */
class Block extends BaseBlock implements BlockInterface
@bitbager
Copy link
Member

Weird. I am always using the YAML mapping, but it should not make difference. Could you override all the sylius_resource definition content, not only the model like below?

sylius_resource:
    resources:
        bitbag.block:
            driver: doctrine/orm
            classes:
                model: AppBundle\Entity\CmsPlugin\Block
                form: BitBag\CmsPlugin\Form\Type\BlockType
                repository: BitBag\CmsPlugin\Repository\BlockRepository
            translation:
                classes:
                    model: BitBag\CmsPlugin\Entity\BlockTranslation

Maybe if you are implementing a useful feature, it can be included in the core, so there's no need to override it, can I ask for the specific use-case? 🙂

bitbager pushed a commit to bitbager/SyliusCmsPlugin that referenced this issue Nov 5, 2017
@bitbager
Copy link
Member

bitbager commented Nov 6, 2017

Have you solved the issue? 😉

@sweoggy
Copy link
Contributor Author

sweoggy commented Nov 7, 2017

I will try to have a look at this, this week or next

@kochen
Copy link
Contributor

kochen commented Nov 7, 2017

I can confirm that this issue still exist:

  The table with name 'bitbag_cms_block' already exists.

AppBundle\Entity\Block.php:

class Block extends BaseBlock
{

    /**
     * @var string
     */
    protected $my_field;
}

Block.orm.yml:

AppBundle\Entity\Block:
    type: entity
    table: bitbag_cms_block
    fields:
        my_field:
            type: string

@pamil
Copy link
Contributor

pamil commented Nov 9, 2017

Have you tried using mapped superclass instead of entity?

@bitbager bitbager added Bug and removed help wanted labels Nov 9, 2017
@diimpp
Copy link

diimpp commented Nov 24, 2017

Not really related, but I think it's more appropriate to use DI extension and configuration to define resources, rather than rely on sylius_resource config.

So far no official plugins use DI resource configuration, which even led me to believe it's not supported for some reason, but turned out to be working alright.

P.S. Could it be, that bundles initialization race condition is a source of this issue?

Like AppBundle creates new resource and BitBagCms redefines it instead of the other way around.

@bitbager
Copy link
Member

CC @pamil, @lchrusciel

Yeap, we believed that it'd be more comfortable for the contributors to understand, as it's the workflow described in Sylius documentation.

I think that switching to the DI extension config can be overcomplicated for some basic CRUD use-cases like the ones we're using in our plugin.

Sylius guys, what do you think? Can we refer it to Sylius/PluginSkeleton#58?

@diimpp
Copy link

diimpp commented Nov 24, 2017

I think that switching to the DI extension config can be overcomplicated for some basic CRUD use-cases like the ones we're using in our plugin.

All Sylius bundles are basic crud apps in standalone usage. :)

Once you will introduce bundle parameters, like mentioned in cke editor issue, it will get really awkward to modify bundle details in several places.

bitbag_cms:
    withCke: true

sylius_resource:
    resources:
        bitbag.block ~

In my experience it's only complicated to devise working BitBagSyliusCmsPlugin.php, that will support DI resources in single bundle for the first time and it's business as usual afterwards.
So I don't think it would be a problem for contributors.

@bitbager
Copy link
Member

I'm not talking about the DI extension configuration, but about the Sylius resource config. IMHO https://github.com/BitBagCommerce/SyliusCmsPlugin/blob/master/src/Resources/config/sylius_resource/block.yml is way much more straightforward than the setup you can find here - https://github.com/Sylius/Sylius/blob/master/src/Sylius/Bundle/ProductBundle/DependencyInjection/SyliusProductExtension.php#L64.

For now, overriding Sylius resources is done by overriding them in the directory structure, and I think that as @pamil said, the general problem is in Doctrine configuration as our entities are not tagged as mapped superclasses.

@bitbager
Copy link
Member

bitbager commented Dec 2, 2017

Guys, the problem should be fixed on 1.3.0-dev version. Upgrade your plugins, don't forget to read the UPGRADE.md guide and let me know if it works. For now, I'm closing this issue 🙂

@bitbager bitbager closed this as completed Dec 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants