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

fix(Entity): updateOne/updateMany should not change ids state on existing entity #581

Merged
merged 1 commit into from
Nov 28, 2017
Merged

Conversation

bbaia
Copy link
Contributor

@bbaia bbaia commented Nov 18, 2017

Closes #571

Needed to create a DidMutate enum, tell me what your think.
This PR only fixes the unsorted state adpater.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.04%) to 92.655% when pulling 3644258 on bbaia:entity-ids-state into 4125914 on ngrx:master.

@MikeRyanDev
Copy link
Member

This seems like a reasonable way to address the issue. Do you want to fix both adapters here or merge this one in and then fix the other in a followup PR?

@bbaia
Copy link
Contributor Author

bbaia commented Nov 20, 2017

The sorted state adpater is a bit more complex, the same code is used for both add and update methods.
I will try to fix it and merge with this one when PR #550 will be merged.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.08%) to 92.274% when pulling e18d560 on bbaia:entity-ids-state into a467bd6 on ngrx:master.

@bbaia
Copy link
Contributor Author

bbaia commented Nov 24, 2017

Fixed sorted state adpater, amend and rebase done.

@brandonroberts brandonroberts merged commit b989e4b into ngrx:master Nov 28, 2017
@brandonroberts
Copy link
Member

Thanks!

@bbaia bbaia deleted the entity-ids-state branch December 11, 2020 09:24
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.

updateOne/updateMany should not change ids state on existing entity
4 participants