Add uniqueness check for rule types #64
Conversation
I did this because we do not want the rule types to be stored with the same name within a rule type registry. This would make it difficult to find the correct rule type if it was given the same name.
I did this because we don't want users to use different ids to reference the same class name. This might cause us to not identify the correct type id when looking them up by class name. This ensures that, along with type id, they are unique.
| end | ||
| end | ||
| end | ||
|
|
||
| describe 'get registered rule type class' do | ||
| it 'returns the associated rule type class' do | ||
| Togls.rule_types do | ||
| register(:test_rule_one, Togls::Rules::Boolean) | ||
| register(:test_rule_one, String) |
There was a problem hiding this comment.
Since Togls::Rules::Boolean is defined when included I couldn't use it. I switched it to Class but the previous test still had it defined. To get the test to pass I had to pick other class names since I couldn't get them to clear out of the rule type registry.
I tried setting the rule type repository and registry to nil but it introduced a bunch of other problems. I'm still playing with it.
Edit: you can see in the following tests i used Integer, Hash, and Array to get it to pass.
There was a problem hiding this comment.
can you get any more specific with the other problems the nil reset introduced?
There was a problem hiding this comment.
Dug into why setting the rule type registry/repository to nil is causing a bunch of failures...
Looks like because we are using the included hook in the manager to register the 'boolean' and 'group' rule types, they are getting cleared out when the repository/registry is being set to nil. Since the included hook is only being triggered one time when we require 'togls' in spec_helper.rb, those rule types are not being registered again. As a result a bunch of tests fail because it tries to call .new on nil after trying to look up the 'boolean' rule type.
Why you made the change: I did this because everything else was already setup to lazy load and this part wasn't which broken our conventions around testing and expected behavior.
This adds uniqueness for rule type ids and their class names.