8000 [2.7] [WIP] Fix for #14583 by phansys · Pull Request #14617 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[2.7] [WIP] Fix for #14583 #14617

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

[2.7] [WIP] Fix for #14583 #14617

wants to merge 1 commit into from

Conversation

phansys
Copy link
Contributor
@phansys phansys commented May 12, 2015
Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #14583
License MIT
Doc PR

Fixes #14583.
I'm not sure if this is the right fix, so I need someone to share thoughts about this.

TODO

  • Add tests

@xabbuh
Copy link
Member
xabbuh commented May 12, 2015

At least, this is breaking the fix from #14465.

@phansys
Copy link
Contributor Author
phansys commented May 12, 2015

You're right @xabbuh, I didn't notice this issue in my test suite.
I'll will update the PR when I have some better idea.

Thank you.

@xabbuh xabbuh added the Form label May 12, 2015
@benji07
Copy link
Contributor
benji07 commented May 22, 2015

any news ?

@phansys
Copy link
Contributor Author
phansys commented May 22, 2015

I'm sorry @benji07, I'm very busy these days, but I'll try to work on it this weekend. BTW, if you have some idea, feel free to share your thoughts.

@phansys phansys force-pushed the ticket_14583 branch 2 times, most recently from 95dbac6 to e1497e7 Compare May 24, 2015 14:57
@phansys
Copy link
Contributor Author
phansys commented May 24, 2015

I just updated the PR. Can somenone else experiencing this issue test if this fix works? /cc @benji07, @mbartok
BTW, I don't know if this is the right place for this fix, so any help would be much appreciated.
ping @webmozart

@phansys
[2.7] [WIP] Fix for symfony#14583
f1a38cb 
| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | symfony#14583
| License       | MIT
| Doc PR        |

Fixes symfony#14583.
@weaverryan
Copy link
Member

I think we need to hear what @webmozart thinks - I also can't tell if this is the right way/place to fix this, or if it should be done in the Propel bridge like you mentioned in #14583. @webmozart if you can offer some insight, @phansys or someone else can probably push this the rest of the way.

@phansys
Copy link
Contributor Author
phansys commented May 27, 2015

Thank you @weaverryan.
I think its nothing to do with Propel (at least not in an isolated way), since I have this issue with Doctrine 2.5 + PostgreSQL 9.3.

@benji07
Copy link
Contributor
benji07 commented Jun 1, 2015

@webmozart @fabpot so Symfony 2.7 was release this weekend, and we can't create an entity choice with an empty value.

@webmozart
Copy link
Contributor

Sorry, I didn't see this issue until now. Will look into it.

@phansys
Copy link
Contributor Author
phansys commented Jun 1, 2015

Thank you @webmozart.

@fabpot fabpot mentioned this pull request Jun 11, 2015
Merged
@webmozart
Copy link
Contributor

Hi @phansys, thanks again for this PR! I think the right solution for the problem is #14950. The same should also be done for the Propel bundle. Do you want to work on that?

I'll close this issue as any further work should go to a new PR.

@webmozart webmozart closed this Jun 12, 2015
@phansys
Copy link
Contributor Author
phansys commented Jun 12, 2015

Thank you @webmozart. It would be nice to have #14950 merged ASAP. BTW, about the Propel bundle, I would prefer to delegate the task to somebody who know and have worked with Propel since I never used it.

@phansys phansys deleted the ticket_14583 branch June 12, 2015 12:56
@guilhermecvmnsj
Copy link

@webmozart your solution doesn't work when using 'uuids'. @phansys solution resolves this problem.

with params ["1", "9359da6d-55ff-4da0-8e82-e3b318ecd633", ""]:

SQLSTATE[22P02]: Invalid text representation: 7 ERROR: invalid input syntax for uuid: ""

@podhy
Copy link
Contributor
podhy commented Jan 7, 2016

Any news with this pull request? Because PR #14950 doesnt resolve problem with UUIDs.

Using symfony 2.8

@jakzal
Copy link
Contributor
jakzal commented Jan 8, 2016

@podhy @guilhermecvmnsj instead of commenting on closed pull requests, please open a new issue if you think there's an unresolved bug. Don't forget to provide steps to reproduce your problem.

@javiereguiluz
Copy link
Member

@jakzal maybe we could improve our "GitHub bot" to lock conversations automatically some time after the issue has been closed or the PR has been merged (say 6 months).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Form
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@phansys @xabbuh @benji07 @weaverryan @webmozart @guilhermecvmnsj @podhy @jakzal @javiereguiluz @althaus
0