search

goldibex / targaryen

Test Firebase security rules without connecting to Firebase.
ISC License
242 stars 36 forks source link

newData should contain all the merged data #108

Closed georgesboris closed 7 years ago

georgesboris commented 7 years ago

Hey man, thanks for developing this.

We've been trying to use targaryan for a while now with my team. Although it helps a lot we're encountering some issues, specially since the updates made recently.

Take a look at this for instance.

In Firebase, when you update some paths, the newData holds all the information for the current tree, not only the information that is been sent.

Like so:

captura de tela 2016-12-22 as 4 49 46 pm captura de tela 2016-12-22 as 4 45 41 pm

When trying to do the same with targaryan:

captura de tela de 2016-12-22 16-43-08

I will try to post here when we find any other issues. And we will gladly contribute to their solutions, it's just that we're in a crazy end of year sprint. We're kind of being forced to "let the tests slip for a while" instead of being allowed to fix this.

georgesboris commented 7 years ago

Note that when the newData holds any value, like if we were trying to update the createdBy prop instead of passing null to it, it appears to work as expected.

dinoboff commented 7 years ago

Thanks for the report. I could reproduce it and I am working on a fix.

dinoboff commented 7 years ago

ps: note that newData reference the all new tree; one location in that tree. It seems there's a bug where the parent node got deleted instead of just createdBy.

dinoboff commented 7 years ago

@georgesboris It should be fixed in 3.0.0-rc.4.

Thanks.