search

firebase / geofire-js

GeoFire for JavaScript - Realtime location queries with Firebase
MIT License
1.44k stars 345 forks source link

geo.removeById() does not remove all references #6

Closed abinop closed 10 years ago

abinop commented 10 years ago

I have managed to insert multiple records with geo.insertByLocWithId for the same id

Then, upon calling geo.removeById() not all references were removed.

robertdimarco commented 10 years ago

Hi @abinop -

Appreciate reaching out to us with this issue. When you get a moment, can you include a small snippet to reproduce this case? We can use it in testing to ensure that your issue is fixed correctly. Thanks!

abinop commented 10 years ago

Sure, see this http://snipt.org/AiKa3.

robertdimarco commented 10 years ago

@abinop Thanks - appreciate the repro case.

To confirm, you would expect the second insertByLocWithId() call, with the same ID, to replace the prior one?

kav-ya commented 10 years ago

Hi @abinop,

To add to Rob's question, does the updateLocForId() function provide the behavior you expect? For example, if you wanted to update the location of the user with uid=111? If so, would you want insertByLocWithId() to behave the same too?

Thanks!

abinop commented 10 years ago

Hello @kav-ya and @robertdimarco , I would expect that the second insertByLocWithId would replace the first call, yes. As I see it, it is the same object that now as a different location, it cannot be in the different locations at the same time.

Even if that was true, deleting it should delete both locations, not only the first one.

I'd love to hear your thoughts.

goggledefogger commented 10 years ago

I'm having a similar issue with removeById(), it's not deleting all the entries. Even weirder, I've narrowed it down to correctly removing the entry every other time - it works the first time, and then not the next, but then works again, and back and forth. I guess that could be something in my calling code, but I figured if I call removeById() with the correct id it would remove them all no matter what, and it now it only works half the time.

jwngr commented 10 years ago

Hey @abinop - we are releasing version 2.0.0 of GeoFire on Monday. It brings a brand new API which solves this problem and more. You can check it out in the v2 branch or in the master branch come Monday.