Skip to content

The 'beforeSave' trigger breaks the dot notation for subdocuments (cf #567) #3912

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

Merged
merged 5 commits into from
Jun 14, 2017

Conversation

IlyaDiallo
Copy link
Contributor

REST update / create queries setting values in subdocuments fail to update the values if a beforeSave trigger exists.
Example: { "key.subkey" : value }
This is a fix for that issue.

@codecov
Copy link

codecov bot commented Jun 7, 2017

Codecov Report

Merging #3912 into master will decrease coverage by <.01%.
The diff coverage is 94.11%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3912      +/-   ##
==========================================
- Coverage   90.44%   90.43%   -0.01%     
==========================================
  Files         114      114              
  Lines        7669     7682      +13     
==========================================
+ Hits         6936     6947      +11     
- Misses        733      735       +2
Impacted Files Coverage Δ
src/RestWrite.js 92.91% <94.11%> (-0.22%) ⬇️
...dapters/Storage/Postgres/PostgresStorageAdapter.js 95.7% <0%> (-0.14%) ⬇️
src/Adapters/Auth/facebook.js 84% <0%> (+4%) ⬆️

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 f0949a1...2b1f8e6. Read the comment docs.

src/RestWrite.js Outdated
@@ -1104,6 +1102,29 @@ RestWrite.prototype.sanitizedData = function() {
return Parse._decode(undefined, data);
}

// Returns an updated copy of the object
RestWrite.prototype.buildUpdatedObject = function (extraData) {
var updatedObject = triggers.inflate(extraData, this.originalData);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please use const/let instead of var

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, done

Copy link
Contributor

@natanrolnik natanrolnik Jun 14, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You apparently missed this one, @IlyaDiallo , no?
It seems you replaced the other ones but not this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking that the style requirement of let instead of var was only for declarations inside blocks, not at the beginning of a function. Is that wrong ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I understand/can answer. @flovilmart, what do you say?

Copy link
Contributor Author

@IlyaDiallo IlyaDiallo Jun 14, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean than this does not conform to the guideline because the var is visible outside the if block:
function f() { if(test) { var v; } }
... but this is (maybe) OK because var and let have the same scope:
function f() { var v; }

BTW is there a written styling guide for the project ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PS here it's in fact a const variable , so I've replaced var with const anyway, but the style question still stands.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we use const for immutable variable (never reassigned) and let in place of vars as they behave more appropriately.

@natanrolnik natanrolnik merged commit 92fa6f2 into parse-community:master Jun 14, 2017
@flovilmart flovilmart modified the milestone: 2.5.0 Jun 25, 2017
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.

4 participants