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 docChanges removed, modified and added with proper index values #284

Merged

Conversation

KholmatovS
Copy link
Contributor

Fix for #224 .
Also fixed proper snapshot query emitting when document modified/removed preserving old and new indexes.

@KholmatovS KholmatovS changed the title Fix document delete, modified and add changes with proper index values Fix docChanges removed, modified and added with proper index values Dec 15, 2023
Comment on lines +1720 to +1723
// after adding doc 2 (ordered by name)
['Gwen', 'Norman'],
// after updating doc 2 (Gwen -> Peter, order by name )
['Norman', 'Peter'],
Copy link
Owner

Choose a reason for hiding this comment

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

Very nice and thorough test case, and great documentation

…e_change

# Conflicts:
#	lib/src/query_snapshot_stream_manager.dart
@KholmatovS
Copy link
Contributor Author

@atn832 resolved merge conflicts

Copy link
Owner

@atn832 atn832 left a comment

Choose a reason for hiding this comment

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

Looks great!!

@atn832 atn832 merged commit 5e66d3c into atn832:master Dec 26, 2023
4 checks passed
@atn832
Copy link
Owner

atn832 commented Dec 26, 2023

Just published your pull requests to https://pub.dev/packages/fake_cloud_firestore/changelog#245. Thanks for the great update!

@diegogarciar
Copy link

hey, after upgrading to this version my snapshot listener is not receiving events when calling reference.update(). I think the might be extra

image

@diegogarciar
Copy link

Nevermind, I think the issue is somewhere else. I think now we're not checking the type when creating new instances but here we are, so if we use a converter in the app but a simple reference in the tests, I think this will drop the update

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.

None yet

3 participants