NotificationsYou must be signed in to change notification settings
Fork9.7k
Star30.8k

add 'guid' to list of exception to filter out#17668

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Closed

garak wants to merge3 commits intosymfony:2.3fromgarak:patch-4

Closed

add 'guid' to list of exception to filter out#17668

garak wants to merge3 commits intosymfony:2.3fromgarak:patch-4

Conversation

Copy link

Contributor

garak commentedFeb 3, 2016

Q	A
Bug fix?	yes
New feature?	no
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	#14583
License	MIT
Doc PR	none

See#14583 (comment)

add 'guid' to list of exception to filter out

0b8edcb

Seesymfony#14583 (comment)

carsonbot added the Status: Needs Review label

Feb 3, 2016

garak added a commit to garak/symfony that referenced this pull request

Feb 3, 2016

apply patch from PR 17688

313f275

Seesymfony#17668

fix guid case

91f9444

stof reviewed

Feb 3, 2016

View reviewed changes

src/Symfony/Bridge/Doctrine/Form/ChoiceList/ORMQueryBuilderLoader.php Outdated

Copy link

Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

this is wrong. You should not change this to INT_ARRAY. Uuids are not bindable as integers

Copy link

ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I'm open to suggestions. What should I do?

Copy link

Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Well, the binding type should be a string array, not an int array

Copy link

Member

xabbuh commentedFeb 3, 2016

seems to be related to#17488 and#17494

javiereguiluz added Form Enhancement labels

Feb 4, 2016

Copy link

Member

fabpot commentedMar 2, 2016

I think this one should be closed for the same reasons#17494 was closed as well.

Copy link

ContributorAuthor

Well, my proposal is similar to the one in#17494, but much simpler: I'd like not to fail due to a common situation. The situation is anEntity type inside a form, whererequired option is false (orplaceholder option is set). In this situation, theguid type fails on Postgres.
I don't mean to add any complex validation, like the referenced issue, just this super-simple one. I don't care about edge cases, database configuration, you name it. I'd just like to get an optional relation working.

Fix parameter type

141eb23

Copy link

Member

fabpot commentedMar 2, 2016

LGTM now 👍

Copy link

Member

fabpot commentedMar 2, 2016

Thank you@garak.

fabpot added a commit that referenced this pull request

Mar 2, 2016

bug#17668add 'guid' to list of exception to filter out (garak)

59362a9

This PR was squashed before being merged into the 2.3 branch (closes#17668).Discussion----------add 'guid' to list of exception to filter out| Q             | A| ------------- | ---| Bug fix?      | yes| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets |#14583| License       | MIT| Doc PR        | noneSee#14583 (comment)Commits-------0de86ff add 'guid' to list of exception to filter out

fabpot closed this

Mar 2, 2016