Skip to content

volatileClasses are masterKey only #3916

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Jun 13, 2017
Merged

volatileClasses are masterKey only #3916

merged 3 commits into from
Jun 13, 2017

Conversation

@pungme
Copy link
Contributor

@pungme pungme commented Jun 8, 2017 •
edited
Loading

  • Hey! this is my first PR ever, feel free to suggest anything. I'm still learning the best practice!
  • basically, all the volatileClasses are masterKey only
  • potentially fix VolatileClass ignores the CLP #3915

@pungme
Copy link
Contributor Author

pungme commented Jun 8, 2017

I think I should add some test about this case also ...

@codecov
Copy link

codecov bot commented Jun 8, 2017 •
edited
Loading

Codecov Report

Merging #3916 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3916      +/-   ##
==========================================
+ Coverage   90.24%   90.24%   +<.01%     
==========================================
  Files         114      114              
  Lines        7584     7588       +4     
==========================================
+ Hits         6844     6848       +4     
  Misses        740      740
Impacted Files Coverage Δ
src/Controllers/SchemaController.js 97.04% <ø> (ø) ⬆️
src/rest.js 97.18% <100%> (+0.16%) ⬆️
src/RestWrite.js 93.12% <0%> (ø) ⬆️

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 c4fbc1f...c81e6e7. Read the comment docs.

@pungme pungme changed the title volatileClass should respect the schema if defined volatileClass is masterKey only Jun 8, 2017
@natanrolnik
Copy link
Contributor

natanrolnik commented Jun 8, 2017

@pungme Would you mind adding a few tests to check this behavior?

@pungme
Copy link
Contributor Author

pungme commented Jun 8, 2017 •
edited
Loading

@natanrolnik sure! I'll add some test

@pungme pungme changed the title volatileClass is masterKey only volatileClasses is masterKey only Jun 8, 2017
@pungme pungme changed the title volatileClasses is masterKey only volatileClasses are masterKey only Jun 8, 2017
@pungme
Copy link
Contributor Author

pungme commented Jun 12, 2017

Hey, Is it ok like this? Or should I add more test to it?

@flovilmart
Copy link
Contributor

flovilmart commented Jun 12, 2017

I'll have a look later today! Thanks

@flovilmart
Copy link
Contributor

flovilmart commented Jun 13, 2017

Alright this is looking great and I was about to need it for something else!

@flovilmart flovilmart merged commit 7d70af6 into parse-community:master Jun 13, 2017
@pungme
Copy link
Contributor Author

pungme commented Jun 13, 2017

awesome, cheers!

@montymxb montymxb mentioned this pull request Jun 21, 2017
Closed
@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

Reviewers

1 more reviewer

@natanrolnik natanrolnik natanrolnik approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

2.5.0

Development

Successfully merging this pull request may close these issues.

VolatileClass ignores the CLP

3 participants

@pungme @natanrolnik @flovilmart