Adding an "exclude" condition causes undefined behaviour

[robgolding]

This is a pretty serious bug introduced by https://github.com/disqus/gargoyle/pull/53.

---

Since this commit, the order of the conditions is now important, and since this library is backed by modeldict, that order can not be guaranteed. Say you have multiple conditions over multiple fields:

• IP Address: 127.0.0.1
• User email: exclude [email protected]

If the conditions come out in the order above, then the condition set is always true. If they come out in the opposite order, the condition set is only true if the IP address is 127.0.0.1.

What we're seeing is that the conditions are sometimes respected correctly (resulting in the expected behaviour), but the vast majority of the time the switch is just enabled. It's almost impossible to reproduce by running the code in a shell, because the order of a dictionary changes based on how the memory is laid out, and that's unique for each process.

Furthermore, I don't think the "default" should ever be that a switch is on. If a switch is in selective mode and has a single exclude condition, then that switch should not be enabled for anyone. You should have to selectively opt-in groups users to the switch, then opt-out individuals if necessary.

I don't expect there's any chance of the default behaviour being changed again, so I'll just update our fork to revert this change, but I wanted to raise this in case anyone comes across this crazy bug in the future!

[jlward]

I just ran into this updating from v0.10.8 (the version right before the bug)