Sylius / SyliusFixturesBundle

Configurable fixtures for Symfony applications.
MIT License
50 stars 24 forks source link

Ability to decorate `*Fixture` and `*ExampleFactory` #21

Open igormukhingmailcom opened 5 years ago

igormukhingmailcom commented 5 years ago

TLDR Context When implementing plugins for Sylius, I usually add fixtures - that way app developer can "in one click" try plugin (without eating time to add some data manually).

Sometime plugins extending core resources (Product, Order, etc) and I believe we should have safe way to extend or even decorate *ExampleFactory for core resources.

Safe means that solution should work even if 2 or more plugins will be installed that make changes to same *ExampleFactory. This can be done with decorators I believe.

But methods at *ExampleFactory and *Factory protected which make that impossible to create safe solutions.

Describe the proposed solution

  1. Replace:
protected function configureResourceNode(ArrayNodeDefinition $resourceNode): void;

with

public function configureResourceNode(ArrayNodeDefinition $resourceNode): void;

at AbstractResourceFixture and related core resources fixtures.

  1. Add ConfigurableExampleFactoryInterface (or DecorableExampleFactoryInterface, not sure about name) - optional
interface ConfigurableExampleFactoryInterface extends ExampleFactoryInterface
{
    public function configureOptions(OptionsResolver $resolver): void;
}
  1. Replace AbstractExampleFactory with:
abstract class AbstractExampleFactory implements ConfigurableExampleFactoryInterface
{
    abstract public function configureOptions(OptionsResolver $resolver): void;
}

Additional context As far as currently we have some BC breaking changes in this area ("Make FixturesBundle standalone") - I believe this is good time to do proposed change too.

I will make PR - just let me know please to what branch I should base it to.

loevgaard commented 5 years ago

What's your take on this, @pamil, @Zales0123? :)

igormukhingmailcom commented 5 years ago

Hi, @pamil . Noticed you guys released 1.5 with extracted fixture bundle (which probably good time to make changes like described).

What about this small but very important (for plugin developers at least) change (now probably at 1.6)? With it - we can override fixtures with decorators - that way different plugin's fixtures overrides will not conflict with each other.

Thanks :)

vvasiloi commented 5 years ago

Until any changes to facilitate this process are made, I will leave here a few tips I use to decorate both Fixture and ExampleFactory classes.

Docorating ExampleFactory classes: https://gist.github.com/vvasiloi/a3a8a7b151775bb58cd122a47b2cc4dd

Decorating Fixture classes: since these implement the Symfony\Component\Config\Definition\ConfigurationInterface, you have the public method getConfigTreeBuilder which can be used to make any modification you want to the configuration of the decorated class. The Sylius\Bundle\FixturesBundle\Fixture\FixtureInterface::load public method is a bit harder to work with, but most of the time you don't need to change what it does because it's mainly about bulk processing and persistence, while the actual object creation is delegated to the factories. In the worst case you will have to override it completely (not call the decorated method).

stale[bot] commented 5 years ago

This issue has been automatically marked as stale because it has not had any recent activity. It will be closed in a week if no further activity occurs. Thank you for your contributions.

igormukhingmailcom commented 5 years ago

Do not stale

lchrusciel commented 5 years ago

@igormukhingmailcom there are attempts to make it better like Sylius/Sylius#10616. But the core of the issue is related to https://github.com/Sylius/SyliusFixtureBundle, can we move this conversation there?

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had any recent activity. It will be closed in a week if no further activity occurs. Thank you for your contributions.