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

Delete collection from Elasticsearch index #490

Merged

Conversation

prabalsingh24
Copy link
Contributor

Problem

After deleting the collection from Database, it has to be deleted from ES index as well

@coveralls
Copy link

coveralls commented Aug 13, 2020

Coverage Status

Coverage increased (+1.1%) to 59.182% when pulling d0d59a9 on prabalsingh24:delete-collection-from-ES into ef27871 on bookbrainz:UserCollection.

Copy link
Contributor

@MonkeyDo MonkeyDo left a comment

Choose a reason for hiding this comment

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

Hole in one !
Nice catch on remembering to remove from the index, and nice PR with nothing to change!

Well done ! 💯

For my own curiosity, could you explain why you changed a few instances of UserCollectionItem({collectionId: collection.get('id')}) into UserCollectionItem().where('collectionId', collection.get('id'))?
I saw the same in another PR, and I thought they were pretty much interchangeable (and now I'm thinking i've been missing something this whole time)

@MonkeyDo MonkeyDo merged commit 6db64d4 into metabrainz:UserCollection Aug 13, 2020
@prabalsingh24
Copy link
Contributor Author

I've seen that sometimes UserCollection({id: 1}).fetchAll() gives incorrect result. whereas with fetch it works fine.

When i have to use fetch , I use UserCollection({id: 1}).fetch() whereas when i have to do fetchAll I use UserCollection().where('id', 1).fetchAll

@MonkeyDo
Copy link
Contributor

Huh, interesting. I wonder why that is.
Thanks for the tip!

@prabalsingh24
Copy link
Contributor Author

I could be wrong too. But i experienced it at many places.

@prabalsingh24
Copy link
Contributor Author

Next time i see a similar issue, I'll check it properly. Will use debug true and see what happens

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants