Fix coercion of PK value in replaceById method
#1763
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bajtos
requested review from
b-admike,
dhmlau,
emonddr,
jannyHou and
zbarbuto
as code owners
bajtos
commented
Jul 25, 2019
| }); | ||
|
|
||
| // Verify that no warnings were triggered | ||
| Object.keys(Post._warned).should.be.empty(); |
There was a problem hiding this comment.
Without my fix in place, this checks passes when we run the tests in juggler, but fails against the MongoDB connector. I have verified that myself by running the MongoDB test suite against npm link-ed version of juggler.
4 tasks
hacksparrow
approved these changes
Jul 25, 2019
bajtos
force-pushed
the
fix/PK-coercion-in-replaceById
branch
from
c1fb686 to
bb1cc70
Compare
Before this change, when both the PK value (`id`) and the `data` object
were provided as plain-data values (e.g. as received in a JSON request),
and the connector was using a complex PK type (e.g. `ObjectID`
in MongoDB), then `replaceById` operation was printing confusing
warnings:
WARNING: id property cannot be changed from 5d39775a59f5f541513c5e05
to 5d39775a59f5f541513c5e05 for model:Post
in 'before save' operation hook
WARNING: id property cannot be changed from 5d39775a59f5f541513c5e05
to 5d39775a59f5f541513c5e05 for model:Post
in 'loaded' operation hook
This commit fixes the problem by applying the same type coercion on the
PK value (`id`) as has been applied by the model constructor on the PK
property (`data.id`).
Signed-off-by: Miroslav Bajtoš <mbajtoss@gmail.com>
bajtos
force-pushed
the
fix/PK-coercion-in-replaceById
branch
from
bb1cc70 to
f1fa976
Compare
I checked the console logs and these downstream failures do not seem to be caused by my changes. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Before this change, when both the PK value (
id) and thedataobject were provided as plain-data values (e.g. as received in a JSON request), and the connector was using a complex PK type (e.g.ObjectIDin MongoDB), thenreplaceByIdoperation was printing confusing warnings:This commit fixes the problem by applying the same type coercion on the PK value (
id) as has been applied by the model constructor on the PK property (data.id).Related issues
I discovered this problem while working on the spike for Inclusion of related models (see loopbackio/loopback-next#3387), but the relation is not really relevant.
Checklist
guide