8000 [DoctrineBridge][Form] Fix IdReader when indexing by primary foreign key by giosh94mhz · Pull Request #15251 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

[DoctrineBridge][Form] Fix IdReader when indexing by primary foreign key #15251

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

Conversation

giosh94mhz
Copy link
Contributor
Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets
License MIT
Doc PR

Port to Symfony 2.7 of PR #14372 . The IdReader class can now resolve association and determine the real id value.

There is still room for improvement, though. Since I've added a new IdReader in the constructor, it is better to declare IdReader as final just to be safe.

PS: sorry to keep you waiting, @webmozart . When merging both commit don't forget to add the @deprecated annotation, and that SingleAssociationToIntIdEntity.php is duplicated.

@giosh94mhz giosh94mhz force-pushed the entity_choice_list_with_primary_key_association_27 branch from 9297c0f to 799d0e0 Compare July 9, 2015 12:55
@xabbuh xabbuh added the Doctrine label Jul 9, 2015
@cursedcoder
Copy link

👍

@akovalyov
Copy link
Contributor

fingers crossed that it will be merged soon ;)

@fabpot
Copy link
Member
fabpot commented Aug 12, 2015

Thank you @giosh94mhz.

@fabpot fabpot merged commit 799d0e0 into symfony:2.7 Aug 12, 2015
fabpot added a commit that referenced this pull request Aug 12, 2015
…ry foreign key (giosh94mhz)

This PR was merged into the 2.7 branch.

Discussion
----------

[DoctrineBridge][Form] Fix IdReader when indexing by primary foreign key

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

Port to Symfony 2.7 of PR #14372 . The `IdReader` class can now resolve association and determine the real id value.

There is still room for improvement, though. Since I've added a `new IdReader` in the constructor, it is better to declare `IdReader` as `final` just to be safe.

PS: sorry to keep you waiting, @webmozart . When merging both commit don't forget to add the `@deprecated` annotation, and that `SingleAssociationToIntIdEntity.php` is duplicated.

Commits
-------

799d0e0 [DoctrineBridge][Form] Fix IdReader when indexing by primary foreign key
@fabpot
Copy link
Member
fabpot commented Aug 12, 2015

@giosh94mhz Can you make an additional PR now that both PRs are merged to manage the remaining changes?

@giosh94mhz
Copy link
Contributor Author

Thank you @fabpot. You mean the test class and the deprecated annotation? No problem, I'll do it ASAP.

fabpot added a commit that referenced this pull request Aug 13, 2015
…osh94mhz)

This PR was merged into the 2.7 branch.

Discussion
----------

[DoctrineBridge][Form] Add old tests to legacy group

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

@fabpot this are the minor fixes required after merging PR  #15251. You are already managed the duplicated class during merge, so only some `@group legacy` annotation were missing.

Commits
-------

724a54b [DoctrineBridge][Form] Add old tests to legacy group
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants
0