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

Update undoChanges method to accept an only option. #25

Merged
merged 4 commits into from Mar 25, 2016

Conversation

Projects
None yet
2 participants
@stallings
Copy link
Contributor

stallings commented Mar 23, 2016

The only option is either a string or an array of strings of owned
association names that limit the undoing of changes to only the
provided associations.

Update undoChanges method to accept an only option.
The only option is either a string or an array of strings of owned
association names that limit the undoing of changes to only the
provided associations.

@stallings stallings assigned stallings and burrows and unassigned stallings Mar 23, 2016

@@ -1059,6 +1061,9 @@ var Model = TransisObject.extend(function() {
if (opts.except === name) { continue; }
if (Array.isArray(opts.except) && opts.except.indexOf(name) >= 0) { continue; }

if (opts.only && opts.only !== name) { continue; }

This comment has been minimized.

@burrows

burrows Mar 23, 2016

Member

If opts.only is an array and name is in the array, then this will skip processing name.

This comment has been minimized.

@stallings

stallings Mar 23, 2016

Contributor

Good point. How does this look?

        if(opts.only) {
          if(Array.isArray(opts.only)) {
            if (opts.only.indexOf(name) === -1) { continue; }
          }
          else {
            if (opts.only !== name) { continue; }
          }
        }

This comment has been minimized.

@burrows

burrows Mar 23, 2016

Member

That looks better. Make sure you add a spec for the scenario I described.

@stallings stallings force-pushed the update-undo-changes branch from fe5c04a to 3520684 Mar 24, 2016

@@ -2143,9 +2143,10 @@ describe('Model', function () {
});

it('does not undo changes to owned associations specified in the exception option as an array', function() {

This comment has been minimized.

@stallings

stallings Mar 24, 2016

Contributor

@burrows While working on the only option, noticed a bug with the except option. If you write the test this way it would fail. This is because the check was not happing in for (let prop in this.ownChanges) {

This comment has been minimized.

@burrows

burrows Mar 25, 2016

Member

@stallings Originally the except option was designed to only avoid recursively calling undoChanges on associated objects. This change makes it work with attributes as well. I think this makes sense, but please update the documentation to reflect that.

@burrows burrows merged commit 95a09f6 into master Mar 25, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment