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

Clean authData of null values on _User update #1199

Merged
merged 3 commits into from
Mar 29, 2016

Conversation

yuzeh
Copy link
Contributor

@yuzeh yuzeh commented Mar 26, 2016

Note: I haven't been able to test this because I'm still trying to figure out how to get my project to depend on my fork of parse-server.


Adds a step to the RestWrite#execute chain: it cleans the response
authData object of null values.

For example, this:

{"authData": {"anonymous": null}, "updatedAt", ...}

will be transformed to this:

{"updatedAt", ...}

And this:

{"authData": {"anonymous": null, "twitter": ...}, "updatedAt", ...}

will be transformed to this:

{"authData": {"twitter": ...}, "updatedAt", ...}

Fixing this issue will fix anonymous user upgrades from the Android SDK.

@codecov-io
Copy link

Current coverage is 93.00%

Merging #1199 into master will increase coverage by +0.03% as of b6201b6

@@            master   #1199   diff @@
======================================
  Files           84      84       
  Stmts         5265    5275    +10
  Branches       956     960     +4
  Methods          0       0       
======================================
+ Hit           4895    4906    +11
  Partial         11      11       
+ Missed         359     358     -1

Review entire Coverage Diff as of b6201b6

Powered by Codecov. Updated on successful CI builds.

@flovilmart
Copy link
Contributor

@yuzeh please add unit tests to make sure the fix is correct and doing what's expected the cleaning is currently tested here https://github.com/ParsePlatform/parse-server/blob/master/spec/OAuth.spec.js#L263

@yuzeh
Copy link
Contributor Author

yuzeh commented Mar 29, 2016

Hi @flovilmart, I've added a test case that exercises this functionality (if I undo the code in src/, the test will fail). I'm not sure if this is the best place to put it, though. Please advise!

@facebook-github-bot
Copy link

@yuzeh updated the pull request.

@flovilmart
Copy link
Contributor

Can you rebase / squash all those commits?

flovilmart and others added 2 commits March 29, 2016 14:42
Adds a step to the RestWrite#execute chain: it cleans the response
authData object of null values.

For example, this:

{"authData": {"anonymous": null}, "updatedAt", ...}

will be transformed to this:

{"updatedAt", ...}

And this:

{"authData": {"anonymous": null, "twitter": ...}, "updatedAt", ...}

will be transformed to this:

{"authData": {"twitter": ...}, "updatedAt", ...}

Fixing this issue will fix anonymous user upgrades from the Android SDK.
@yuzeh
Copy link
Contributor Author

yuzeh commented Mar 29, 2016

@flovilmart done.

BTW, I noticed I currently can't use my fork of parse-server as a dependency in NPM. Is there an easy way to fix this?

@flovilmart
Copy link
Contributor

There is still an extra commit....

@flovilmart
Copy link
Contributor

And to answer your question, there is an 'easy' way through git submodules and npm link

@flovilmart flovilmart merged commit aa5ebd4 into parse-community:master Mar 29, 2016
@yuzeh yuzeh deleted the fix-authData-null-value branch March 29, 2016 22:28
@j-koenig
Copy link

I'm still having a problem with a user having authData set to {"facebook": null} running parse-server 2.2.10

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.

5 participants