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

Can't delete connected entities ! #32

Open
jstoeffler opened this issue Jan 15, 2015 · 2 comments
Open

Can't delete connected entities ! #32

jstoeffler opened this issue Jan 15, 2015 · 2 comments

Comments

@jstoeffler
Copy link

Hi there!

(J'ai traduit en français, voir plus bas)

I'm using your bundle, great project !

The preRemove event listener calls hasConnections without the second argument ($filters)

$this->getConnectionManager()->hasConnections($entity)

And this method calls validateFilters with an empty filter array:

 $this->filterValidator->validateFilters($filters);

The validate filters requires the type field to be not blank and not null.

    $filterConstraint = new Collection(array(
        'type' => array(
            new NotBlank(),
            new NotNull(),
        ),
        'depth' => new Type('integer'),
    ));

This filter is useful when creating connections I guess (and even then, if I just have one connection type for a given entity, I wouldn't need it), but it's not useful when fetching constraints.

Bon, en français...

Je pense que vous avez compris avec le code, quand je supprime un objet qui a une question, le Listener est appelé. Ce listener regarde s'il y a des connections pour les supprimer le cas échéant, ce qui est très bien.

Sauf que hasConnection valide le deuxième argument (qui est vide dans ce cas, et devient un tableau vide par défaut), et que le validateur demande à ce que le champ 'type' soit défini (NotBlank, NotNull).

C'est pratique de valider le champ type quand on crée une connection (et encore, si je n'ai qu'un seul type de connection entre deux types d'entités, je n'en aurais pas besoin, mais bon), par contre quand on veut voir s'il y a une connection entre deux objets, ce n'est pas forcément utile.

Et surtout, ça fait planter le Listener, et je ne peux pas supprimer mes objets ! :D

Je vais utiliser un fork du projet, et faire une PR. Mon choix va être d'enlever validateFilters de hasConnection, ainsi que de toutes les méthodes en lecture. Pour que le bundle soit encore plus flexible, on pourrait carrément enlever ce filtre qui n'apporte pas grand chose.

Ça va sans doute faire planter les tests, dites-moi si ça vous intéresse de voulez merger la PR si vous voulez que je m'en occupe (et dites-moi quelle est la stratégie que vous préférez).

@armetiz
Copy link
Contributor

armetiz commented Jan 16, 2015

Hello;
Ce bundle n'est plus maintenu depuis plusieurs mois (Avril 2013 !). Nous avions fait avec ce bundle un gros POC autours de la notion de connection entre objet.

Si tu peux nous faire un retour d'experience; ce serai très apprécié !
Dans quel cadre t'en sers-tu ? Qu'en penses-tu ? As-tu des remarques ?

Aussi;
Après une relecture très rapide, je pense qu'il faut faire évoluer le Filtre qui est trop restrictif. Il doit permettre d'avoir un tableau vide.
Car la notion de "type" n'a de sens que pour la création de connection.

@benjamindulau @choomz qu'en pensez-vous ?

@jstoeffler
Copy link
Author

Merci pour la réponse ! C'est un très bon POC je trouve. Ça ressemble à ce qu'ils ont fait chez Etsy pour leur architecture de BDD : http://fr.slideshare.net/danmckinley/etsy-activity-feeds-architecture

Je développe un réseau social, du coup je l'utilise pour faire des liens :

  • utilisateur / post (like, partage)
  • utilisateur / utilisateur (follow)

Effectivement, j'avais vu qu'il n'est plus maintenu... Je pense que c'est un type de bundle qui peut être utile.

Pour le filtre, je l'ai carrément supprimé du coup... Je pense que par flexibilité on peut même laisser créer des connections sans type. Si je n'ai qu'un type de connexion par couple d'entités, ce n'est pas nécessaire.

Le seul avantage du filtre, c'est d'éviter une erreur dans le code. Je pense que ce n'est pas nécessaire, puisque ce qu'on valide en général c'est des données utilisateurs.

Du coup je l'ai court-circuité dan un fork :D

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

No branches or pull requests

2 participants