-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Pass deeply nested keys correctly to afterSave triggers #7385
base: alpha
Are you sure you want to change the base?
Conversation
Also added a regression test
Codecov Report
@@ Coverage Diff @@
## master #7385 +/- ##
==========================================
- Coverage 93.92% 84.62% -9.30%
==========================================
Files 181 181
Lines 13209 13201 -8
==========================================
- Hits 12406 11172 -1234
- Misses 803 2029 +1226
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice cleanup and test!
@mstniy Can you add this fix to the ChangeLog? |
This looks suspicious. This PR removes lot of code from RestWrite and replaces it with a line that was already within the removed code. Did you research why the logic around the line that you intend to keep was there previously? Is it possible that the logic was there to accommodate certain use cases? |
@mtrezza ParseObject.set is capable of handling nested keys, no matter the depth. The regression tests actually makes use of this capability. I am not sure why the previous snippet did the split manually. Perhaps it was written before ParseObject.set gained that ability, or the contributor wasn't aware it could do that? But that's pure speculation. Thus, the else branch also becomes a simple call to ParseObject.set, so I have removed the branch entirely. I am not sure why the code checks for a dot, so I have left that part intact. |
Done |
@mstniy @mtrezza Support for nested objects was recently improved in the SDK That’s the reason why this cleanup works so well. |
Aha, that may explain it indeed, thanks. |
Solves problems with nested keys Expanded the regression test
ParseUser.get('password') used to be undefined. Now, password is not an attribute
Blocked on parse-community/Parse-SDK-JS#1364 |
|
New Pull Request Checklist
Issue Description
Related issue: #7384
Approach
RestWrite.buildUpdatedObject now functions correctly even for deeply nested keys.
TODOs before merging