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

Improve dot notation for updating nested objects #729

Merged
merged 4 commits into from
Feb 8, 2019

Conversation

dplewis
Copy link
Member

@dplewis dplewis commented Feb 6, 2019

No description provided.

@codecov
Copy link

codecov bot commented Feb 6, 2019

Codecov Report

Merging #729 into master will increase coverage by 0.05%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #729      +/-   ##
==========================================
+ Coverage   88.59%   88.64%   +0.05%     
==========================================
  Files          53       53              
  Lines        4541     4562      +21     
  Branches     1054     1060       +6     
==========================================
+ Hits         4023     4044      +21     
  Misses        518      518
Impacted Files Coverage Δ
src/ParseObject.js 89.64% <100%> (+0.16%) ⬆️
src/ObjectStateMutations.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fcd43da...784c278. Read the comment docs.

@dplewis dplewis requested a review from flovilmart February 6, 2019 21:12
@dplewis
Copy link
Member Author

dplewis commented Feb 6, 2019

@flovilmart This test case works for PG also

@flovilmart
Copy link
Contributor

Well, there is to test also all the other cases: setting a value on ‘key.path’ and increment when it’s not an object stored behind, unset(‘key.path’) etc...

Sent with GitHawk

@flovilmart
Copy link
Contributor

I am very worry bad things will happen with that much decoupling but if you beliebe this is ‘needed’ go ahead

Sent with GitHawk

@dplewis
Copy link
Member Author

dplewis commented Feb 7, 2019

@flovilmart I did run into two issues with this.

You can't set or update nested documents on new objects.

const obj = new TestObject();
obj.set('objectField.number', 20); // Throws internal server error because field doesn't exist
// we could ignore this or separate this into an object

If field exist the local data doesn't update until saved

Don't know much about internal states of parse objects to update local data

const obj = new TestObject({ objectField: { number: 5 } });
await obj.save();
// obj.get('objectField').number is 5
obj.set('objectField.number', 20);
// obj.get('objectField').number is still 5
await obj.save();
// obj.get('objectField').number is now 20

@flovilmart
Copy link
Contributor

Do what you feel is best

Sent with GitHawk

@dplewis dplewis requested a review from acinader February 7, 2019 15:25
@dplewis
Copy link
Member Author

dplewis commented Feb 8, 2019

@flovilmart I resolved both issues I stated on the client side. Can you look this over?

@flovilmart
Copy link
Contributor

I still have bad feelings about this, but go ahead.

Sent with GitHawk

@dplewis dplewis changed the title Allow dot in field names Improve dot notation for updating nested objects Feb 8, 2019
@dplewis dplewis merged commit a4ba856 into parse-community:master Feb 8, 2019
@dplewis dplewis deleted the set-validation-this branch February 8, 2019 01:05
@dplewis
Copy link
Member Author

dplewis commented Feb 8, 2019

Thanks! I’m going to play around with it more. There a lot of features that use set() like add, addUnique etc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants