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

Allow specific keys to be reverted in unsaved objects #565

Merged
merged 5 commits into from
Nov 13, 2018

Conversation

omairvaiyani
Copy link
Contributor

Currently, the Parse.Object prototype method revert resets ALL unsaved changes. With this PR, specific fields can be provided to the function to allow selective reversion.

Currently:

var person =  ParseObject.fromJSON({
      className: 'Person',
      objectId: 'xxxx',
      name: 'Omair'
 });
person.set('gender', 'male');
person.set('age', 25);
// 'age' is ignored, all fields will be reverted
person.revert('age');
// person -> { name: 'Omair' }

With PR (test cases in commit):
Example 1 (single field):

var person =  ParseObject.fromJSON({
      className: 'Person',
      objectId: 'xxxx',
      name: 'Omair'
 });
person.set('gender', 'male');
person.set('age', 25);
person.revert('age');
// person -> { name: 'Omair', gender: 'male' }

Example 2 (multiple fields):

var person =  ParseObject.fromJSON({
      className: 'Person',
      objectId: 'xxxx',
      name: 'Omair'
 });
person.set('gender', 'male');
person.set('age', 25);
person.set('occupation', 'Doctor');
person.revert(['gender', 'occupation']);
// person -> { name: 'Omair', age: '25' }

Example 3 (no fields, works as now):

var person =  ParseObject.fromJSON({
      className: 'Person',
      objectId: 'xxxx',
      name: 'Omair'
 });
person.set('gender', 'male');
person.set('age', 25);
person.set('occupation', 'Doctor');
person.revert();
// person -> { name: 'Omair' }

@flovilmart
Copy link
Contributor

Can we make this backwards compatible please? Now if no keys are passed the SDK an exception will be thrown.

@omairvaiyani
Copy link
Contributor Author

@flovilmart I'm getting 584/584 passed on the branch which submitted this PR, is it failing after you attempt a merge with master?

Contains general cleanup and type fixes.
@codecov
Copy link

codecov bot commented Oct 17, 2018

Codecov Report

Merging #565 into master will increase coverage by 0.02%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #565      +/-   ##
==========================================
+ Coverage   85.69%   85.71%   +0.02%     
==========================================
  Files          48       48              
  Lines        3880     3886       +6     
  Branches      882      885       +3     
==========================================
+ Hits         3325     3331       +6     
  Misses        555      555
Impacted Files Coverage Δ
src/ParseObject.js 88.57% <100%> (+0.08%) ⬆️

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 1084fb7...49146e7. Read the comment docs.

@flovilmart flovilmart closed this Nov 10, 2018
@flovilmart flovilmart reopened this Nov 10, 2018
@flovilmart
Copy link
Contributor

LGTM’ thanks!

Copy link
Contributor

@flovilmart flovilmart left a comment

Choose a reason for hiding this comment

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

There seems to be issues in the tests / lint. Can you adress those?

@omairvaiyani
Copy link
Contributor Author

@flovilmart should be good to go now

Copy link
Contributor

@flovilmart flovilmart left a comment

Choose a reason for hiding this comment

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

Let’s go! Thanks for bearing with me on this one. It took a while, but it was worth it I believe!

@flovilmart flovilmart merged commit e0c51ab into parse-community:master Nov 13, 2018
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