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

Added pushedData hook to root.deleted.uncommitted state. #3031

Merged

Conversation

simonexmachina
Copy link
Contributor

Resolves #3023.

test("root.deleted.uncommitted supports pushedData", function() {
equal(typeof get(rootState, "deleted.uncommitted.pushedData"), 'function',
"root.deleted.uncommitted should be a function");
});
Copy link
Member

Choose a reason for hiding this comment

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

Instead of testing that the deleted.uncommitted.pushedData function exists could you add a new test to the model-test.js file that creates a record with an id using store.push, calls record.deleteRecord() to move it into the deleted.uncommitted state then call store.push again with the same id then assert that the record is still in the root.deleted.uncommitted state and no errors are thrown?

This should help make the test more resilient against future refactors.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep good call, will do. I didn't like this approach but wasn't sure how to correctly test it.

@igorT
Copy link
Member

igorT commented May 2, 2015

No idea why Appveyor failed, I think it's due to a npm issue, will try and restart

@danbell
Copy link

danbell commented May 10, 2015

@igorT Any movement on this PR?

@simonexmachina simonexmachina force-pushed the pushed-data-in-deleted-uncommitted branch from 4115cdf to 556827e Compare May 11, 2015 08:24
@simonexmachina
Copy link
Contributor Author

I've improved the test based on the feedback from @bmac.

@bmac
Copy link
Member

bmac commented May 11, 2015

Looks good to me.

igorT added a commit that referenced this pull request May 12, 2015
…mitted

Added pushedData hook to root.deleted.uncommitted state.
@igorT igorT merged commit eaf9aa8 into emberjs:master May 12, 2015
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.

Attempted to handle event pushedData on model while in state root.deleted.uncommitted
4 participants