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

Fix prepareQueryOrNewObj with object values #2189

Merged
merged 1 commit into from
May 29, 2020

Conversation

alanpoulain
Copy link
Contributor

@alanpoulain alanpoulain commented May 29, 2020

Q A
Type bug
BC Break no
Fixed issues N/A

Summary

Following #2104.

In API Platform, since the release of the 2.1 version, the following test was failing: https://github.com/api-platform/core/blob/d83d50228e2686b7a2f76bfbadf2dec3fd83fdcd/tests/Bridge/Doctrine/MongoDbOdm/Filter/DateFilterTest.php#L264-L289

You can see it here: https://github.com/api-platform/core/pull/3571/checks?check_run_id=718581504 and here: https://github.com/api-platform/core/pull/3571/checks?check_run_id=718581453.

It's because #2104 has removed the array_map on the $preparedValue: DateTime is not converted anymore to the DB type if it is the type of a value in an array and if the field is not present in the class (for instance with a lookup).

To fix this, I've changed the convertToDatabaseValue to fallback to Type::convertPHPToDatabaseValue if the field doesn't have a mapping.

It's also a better behavior than the previous one (before #2104): the previous one didn't work if the value was too deep nested.

In convertToDatabaseValue, fallback to Type::convertPHPToDatabaseValue
if the field doesn't have a mapping.
@alanpoulain alanpoulain force-pushed the fix-prepare-query-object-value branch from 3fb3ba0 to 6a86b9d Compare May 29, 2020 09:14
@malarzm malarzm added the Bug label May 29, 2020
@malarzm malarzm added this to the 2.1.1 milestone May 29, 2020
@malarzm malarzm merged commit 91e90a4 into doctrine:2.1.x May 29, 2020
@malarzm
Copy link
Member

malarzm commented May 29, 2020

Thanks a lot @alanpoulain for taking time to fix this! 2.1.1 is now available :)

@alanpoulain
Copy link
Contributor Author

It was very quick, thank you!

However, I've made some CS changes, I think it will be better like this: #2192.

@malarzm
Copy link
Member

malarzm commented May 29, 2020

Yeah, looks a bit better :) But important thing is that bug is gone, CS can wait for next patch release

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.

None yet

2 participants