Add SQLFilter parameter in the configuration for Doctrine ORM about doctrinebundle HOT 22 CLOSED

Good proposition.
enabled booleanNode should have "true" as default value.

from doctrinebundle.

I think filters should be disabled by default, otherwise the proposition is good. You can implement this and send a PR.

Whats your take @asm89 ?

from doctrinebundle.

exactly how could you force the filter to be abled using the container builder?

right now i'm trying this in a compiler pass, which i could port to this bundle once I get it working

        $container->getDefinition('doctrine.orm.default_configuration')
            ->addMethodCall('addFilter', array('filtername', 'classname'))
        ;
        $container->getDefinition('doctrine.orm.default_entity_manager')
            ->addMethodCall('getFilters', array())
            ->addMethodCall('enable', array('filtername'))
        ;

but that doesn't result in the desired code in the container class

        $instance->getFilters();
        $instance->enable('filtername');

rather than

        $instance->getFilters()->enable('filtername');

from doctrinebundle.

Looks fine imo. I agree with @beberlei that filters should be disabled by default, because I think there are a lot of use cases where filters need additional parameters in order to work properly.

from doctrinebundle.

I agree with @beberlei and @asm89 : this boolean should have a false default value.

@Burgov I'm investigating right now on how to do that too, if you have any suggestions or ideas, feel free to mail me.

from doctrinebundle.

I couldn't think of a way without changing Doctrine ORM code, for example a third optional boolean argument to addFilter, or an en-/disableFilter method on the entitymanager...

from doctrinebundle.

@Burgov I can look at it at the end of the week if you have issues implementing it

from doctrinebundle.

@stof that'd be great. I'd write a PR if I knew how to do this. Unfortunately I'll be out of the country next week, so I won't be able to make any PR's soon

from doctrinebundle.

@stof did you have some time to explore this issues? I'm still blocking on the same issue as @Burgov ... if you have any tips, you're welcome!

from doctrinebundle.

I had no time at all for the Symfony2 community this weekend.

from doctrinebundle.

I found a solution! I implement it now, make tests and send you the PR as all tests will pass.

I just have a question about the configuration.

I first made a proposal as follow :

doctrine:
    ...
    orm:
        filters:
            my_first_filter:
                class: 'Fully\Qualified\Class\Name'
                enabled: true
            my_second_filter:
                class: 'Fully\Qualified\Class\Name2'
                enabled: true

I think that will work for the default entity manager, but if someone has different entity managers, he may want to have different filters for each of his entity managers right?

So I was thinking of the following conf :

doctrine:
    ...
    orm:
        filters:
            my_first_filter:
                class: 'Fully\Qualified\Class\Name'
                enabled: true
            my_second_filter:
                class: 'Fully\Qualified\Class\Name2'
                enabled: true
        entity_managers:
            em1:
                filters:
                    my_em1_filter:
                        class: 'Fully\Qualified\Class\Name'
                        enable: true
            em2:
                filters:
                    my_first_em2_filter:
                        class: 'Fully\Qualified\Class\Name'
                        enabled: true
                    my_scnd_em2_filter:
                        class: 'Fully\Qualified\Class\Name'
                        enabled: true

or do you think the following conf will be better :

doctrine:
    ...
    orm:
        entity_managers:
            default:
                filters:
                    my_first_filter:
                        class: 'Fully\Qualified\Class\Name'
                        enabled: true
                    my_second_filter:
                        class: 'Fully\Qualified\Class\Name2'
                        enabled: true
            em1:
                filters:
                    my_em1_filter:
                        class: 'Fully\Qualified\Class\Name'
                        enable: true
            em2:
                filters:
                    my_first_em2_filter:
                        class: 'Fully\Qualified\Class\Name'
                        enabled: true
                    my_scnd_em2_filter:
                        class: 'Fully\Qualified\Class\Name'
                        enabled: true

?

from doctrinebundle.

it should be done at the em level, like for DQL functions

from doctrinebundle.

so your last example (and the first one will correspond to the short syntax which is rewritten)

from doctrinebundle.

Ok, thanks for your advice. So I implement it to handle both the last example and the short syntax one (the first I gave).

from doctrinebundle.

No, you implement the last example and you add the needed key in the list of rewritten keys for the short syntax. Look at how it is handled

from doctrinebundle.

@beberlei @asm89 does it make sense to give the possibility to enable a sql filter through the config ? It would assume that the sql filter can work without any parameters.

from doctrinebundle.

Yes. For example a SoftDeleteFilter could query for and <alias>.deleted = 0. There is no need to configure the parameters.

from doctrinebundle.

I finally took some time (over my sleeping time obviously) to implement it

from doctrinebundle.

Very nice! Need to remember these configurator classes, I can imagine it'll come in handy more often.

from doctrinebundle.

@Burgov in most cases, they are overkill. The best way to see this is that I never saw a use case for them until yesterday

from doctrinebundle.

@stof Thanks for your time, for my part i've always had a big use case for this feature, restrict user_id on a multi-user product (SAAS).

from doctrinebundle.

@stof thank you very much for your time, I've been (and still am) overloaded theses weeks, and couldn't deal with it.

from doctrinebundle.