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

rollbackInvalid & rollbackProperty functions #179

Merged
merged 8 commits into from
Jul 30, 2018
Merged

rollbackInvalid & rollbackProperty functions #179

merged 8 commits into from
Jul 30, 2018

Conversation

XaserAcheron
Copy link
Contributor

Though I'm pretty over the moon about this addon's approach to validations, I ran into a case where I needed to ditch all invalid changes and keep (+execute) just the valid ones, but couldn't find a good way to do it with the provided API.

To that extent, here's two new functions:

Tests and docs included.

I was a bit unsure whether to make rollbackProperty a new function or just extend rollback to optionally accept a key parameter (similar to validate) -- I went the new function route, but will update it to go the other way if desired.

Copy link
Collaborator

@poteto poteto left a comment

Choose a reason for hiding this comment

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

This looks good, just a minor comment! Thanks :)


```js
changeset.rollback(); // returns changeset
```

**[⬆️ back to top](#api)**

#### `rollbackInvalid`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you also add an entry in the API TOC above?

@XaserAcheron
Copy link
Contributor Author

Oops! Missed a spot -- fixed now. :P

Thanks for taking a look!

@urbany
Copy link

urbany commented May 25, 2017

Thanks @XaserAcheron was just looking for a way to rollback a single attribute.

@XaserAcheron
Copy link
Contributor Author

Glad to help!

At risk of slight annoyance, pinging @poteto for re-review. Should all be kosher now.

@XaserAcheron
Copy link
Contributor Author

Just checking back in on this -- dropping off my monthly @poteto ping, I suppose. :P

@Dhaulagiri
Copy link
Collaborator

Anything we can do to get this landed? rollbackProperty would be really helpful

@XaserAcheron
Copy link
Contributor Author

It's really unfortunate this has been delayed for nearly a year because of a single-line documentation issue that was corrected within a day. 9_6

@nucleartide
Copy link
Contributor

Will try to take a look at this over the weekend (but no promises since other things might come up). Sorry it's taken so long!

@Dhaulagiri
Copy link
Collaborator

@XaserAcheron would you mind rebasing this so it's ready to go?

@XaserAcheron
Copy link
Contributor Author

Happy 1-year anniversary, everyone. D:

I've made this merge-able again, but there's a bunch of new stuff (relayCache & Flow types) in the base rollback method, so now I suppose I've got to go back and figure out what this is and account for it in the new ones. 9_6

Copy link
Collaborator

@snewcomer snewcomer left a comment

Choose a reason for hiding this comment

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

I can definitely see the use case here for finer grain control, esp for complex use cases.
@XaserAcheron You have done a lot, so sry to make one more request...Could you merge master in one last time (to make sure master is a-ok) with your branch and we will merge this in tomorrow as long as the tests pass! 🎉

@XaserAcheron
Copy link
Contributor Author

Merged n' pushed. Tests worked on my end -- we'll see if Travis behaves today. :P

@snewcomer snewcomer merged commit ad6d928 into adopted-ember-addons:master Jul 30, 2018
@XaserAcheron
Copy link
Contributor Author

Woohoo! Thanks there.

@XaserAcheron XaserAcheron deleted the rollback-additions branch May 14, 2019 18:46
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.

Support rollback for individual properties
6 participants