MONGOID-4921 add discriminator_key as a class variable #4814
Conversation
|
Let's not allow reads of discriminator_key on instance level and add a test for that. |
| super( | ||
| compose_message( | ||
| "invalid_discriminator_key_target", | ||
| { class_name: class_name, superclass: superclass} |
There was a problem hiding this comment.
Nitpick - missing a space before the closing curly brace.
There was a problem hiding this comment.
good one! can't believe I missed that... It's bothering me like crazy just looking at it.
spec/mongoid/traversable_spec.rb
Outdated
| describe "#discriminator_key" do | ||
|
|
||
| context "when the discriminator key is not set on a class" do | ||
| it "equals _type" do |
There was a problem hiding this comment.
Could you make this test description more descriptive?
spec/mongoid/traversable_spec.rb
Outdated
| it "the global discriminator key is _type" do | ||
| expect(Mongoid.discriminator_key).to eq("_type") | ||
| end | ||
|
|
||
| it "child discriminator keys equal _type: Piano" do | ||
| expect(Piano.discriminator_key).to eq("_type") | ||
| end | ||
|
|
||
| it "child discriminator keys equal _type: Guitar" do | ||
| expect(Guitar.discriminator_key).to eq("_type") | ||
| end | ||
| end |
There was a problem hiding this comment.
I don't mean to be a stickler for rspec conventions (lol) but could you change these test descriptions so the line reads like a sentence? (e.g. it "sets the global discriminator key to _type")
spec/mongoid/traversable_spec.rb
Outdated
| it "doesn't change in that class" do | ||
| expect(Guitar.discriminator_key).to eq("_type") | ||
| end | ||
|
|
||
| it "the global discriminator key is _type" do | ||
| expect(Mongoid.discriminator_key).to eq("_type") | ||
| end | ||
|
|
||
| it "doesn't change in the sibling" do | ||
| expect(Piano.discriminator_key).to eq("_type") | ||
| end | ||
|
|
||
| it "doesn't change in the parent" do | ||
| expect(Instrument.discriminator_key).to eq("_type") | ||
| end | ||
| end |
There was a problem hiding this comment.
I think you need to add some sort of before block that tries to set the discriminator key on Guitar.
There was a problem hiding this comment.
I changed it to a let block... does that work? Because I still have to test that it raises an error, and I don't know how kosher it is to use expect in a before block.
spec/mongoid/traversable_spec.rb
Outdated
| end | ||
|
|
||
| context "when the discriminator key is changed in the child" do | ||
| let(:guitar) do |
There was a problem hiding this comment.
Could you give this helper a slightly more descriptive name like set_discriminator_key?
Added discriminator key as an attr_accessor on the class. Pre-req to doing MONGOID-4817.