Skip to content

Problems with build-filters which not manage n-n relations. #130

Closed
ComOcean opened this Issue Apr 2, 2012 · 13 comments

5 participants

@ComOcean
ComOcean commented Apr 2, 2012

Since last release, sfGuardPlugin doesn't work with sfPropelORMPlugin

The build-filters doesn't create the sfWidgetFormPropelChoice widget and validators for n-n relation.

Then I have in sfGuardPlugin :

Fatal error: Call to a member function setLabel() on a non-object in /MyProject/site/plugins/sfGuardPlugin/lib/filter/sfGuardPermissionFormFilter.class.php on line 17

@bugbyte
bugbyte commented Apr 2, 2012

I'm having the same problem with the forms.
I've created a sfGuardPlugin_schema.custom.yml that takes cares of the missing crossRefs (which appear to have become mandatory):

propel:
_attributes: { package: plugins.sfGuardPlugin.lib.model }

sf_guard_group_permission:
_attributes: { phpName: sfGuardGroupPermission, isCrossRef: true }

sf_guard_user_permission:
_attributes: { phpName: sfGuardUserPermission, isCrossRef: true }

sf_guard_user_group:
_attributes: { phpName: sfGuardUserGroup, isCrossRef: true }

However, sfGuardPlugin now works correctly but in other places I seem to have it mixing up the crossRef data in forms, creating widgets whose name does not correspond to the model-parameter of sfWidgetFormPropelChoice.
In some places the widget is missing where it should clearly be, in other it creates one choice widget with mixed up data instead of creating two or more widgets.

@ComOcean
ComOcean commented Apr 2, 2012

Ok thanks a lot for this information.

I think, either

  • users will have to be warned and these change documented because it concerns a lot and a lot of plugins.
  • or it has to work like before (no need to update or customize all plugins)
@bugbyte
bugbyte commented Apr 2, 2012

Looks like other are having similar problems: #129

@willdurand
Propel member

@jaugustin I think we need to revert your changes, or at least, to think a bit more on that...

@jaugustin
Propel member

yes we can revert the changes and wrote another patch but I think we will have to do choices:

  • no BC break and keep bugs
  • little BC Break and fix bugs ;)

Maybe my patch could handle many-to-many without isCrossRef parameters but it will probably leave some bugs

@bugbyte
bugbyte commented Apr 2, 2012

IMO having to explicitly set isCrossRef to enable it's behavior is fine, but I do have a problem with the mixing of several crossrefs, a I described in the second comment. If that can be fixed I'd be very happy.

@ComOcean
ComOcean commented Apr 3, 2012

@jaugustin I understand but keep in mind that it introduce bugs in a lot of Symfony plugins (open-source or not) and projects.

Then it is not a "little" BC Break, it is a major BC Break that will have to be warned to all sfPropelORMPlugin users for next official release. Ok it is not difficult to add this property to all schemas but users will not have to spend hours to find where is the problem.

@bdujon
bdujon commented Apr 3, 2012

i also see this as a major bc break especially since isCrossRef method to add relation seems unstable

@jaugustin
Propel member

I will work on a better patch,
For now the solution is to use the tagged version of the sfPropelORMPlugin 1.4

maybe @willdurand could revert my patch until I made a new one.

@bdujon
bdujon commented Apr 4, 2012

a revert would be nice for the svn users.

@jaugustin
Propel member

you could use this svn url to checkout 1.4 tag

https://github.com/propelorm/sfPropelORMPlugin/tags/1.4

@bugbyte
bugbyte commented Apr 4, 2012

@jaugustin I was just about to ask that question :)

@bdujon
bdujon commented Apr 4, 2012

thx @jaugustin i didnt know tags were accessible via github for svn users

@jaugustin jaugustin added a commit to jaugustin/sfPropelORMPlugin that referenced this issue Apr 4, 2012
@jaugustin jaugustin fix issue #129 and #130 with BC break of generated form for M2M witho…
…ut isCrossRef parameters
d8e35ef
@jaugustin jaugustin added a commit to jaugustin/sfPropelORMPlugin that referenced this issue Apr 5, 2012
@jaugustin jaugustin fix many-to-many form generation when middle table join one table twice,
(only without isCrossRef=true) issues #130 #129
15e733a
@willdurand willdurand closed this Apr 10, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.