Improve exception message for HasManyThroughAssociationPolymorphicSource... #7569
Conversation
| @@ -17,7 +17,7 @@ def initialize(owner_class_name, reflection) | |||
|
|
|||
| class HasManyThroughAssociationPolymorphicSourceError < ActiveRecordError #:nodoc: | |||
| def initialize(owner_class_name, reflection, source_reflection) | |||
| super("Cannot have a has_many :through association '#{owner_class_name}##{reflection.name}' on the polymorphic object '#{source_reflection.class_name}##{source_reflection.name}'.") | |||
| super("Cannot have a has_many :through association '#{owner_class_name}##{reflection.name}' on the polymorphic object '#{source_reflection.class_name}##{source_reflection.name}' without 'source_type'. Try adding ':source_type => \"#{reflection.name.to_s.singularize.camelcase}\"' to 'has_many :through' definition.") | |||
There was a problem hiding this comment.
Use the new hash style inside the message please.
There was a problem hiding this comment.
Is this error only used when source_type is not present?
There was a problem hiding this comment.
Thanks for your feedback.
- Will change to the new hash style. Should I change also the other messages with old hash syntax?
- I'm pretty sure it is the only use case, yes. https://github.com/rails/rails/blob/master/activerecord/lib/active_record/reflection.rb#L529
There was a problem hiding this comment.
Ok. There's no need to change the other messages, it wouldn't be accepted I believe, just this one you're adding now. Thanks.
There was a problem hiding this comment.
Also, wouldn't the message be to the 'has_many :through' definition, with the? (just wondering...)
There was a problem hiding this comment.
I think, it is more like a recommendation than a question.
|
Please squash the commits into one. Thanks. |
|
@carlosantoniodasilva squashed the commits - wondering how the process proceeds from here. |
|
@skorfmann thanks, now we merge :). But something just came to my mind: shouldn't we be using >> 'egg_and_hams'.camelcase
=> "EggAndHams"
>> 'egg_and_hams'.classify
=> "EggAndHam"I think it can give us a more consistent output, wdyt? |
|
That is actually the method I was looking for but couldn't recall :) I will update the pull request tomorrow (it's getting late here in Australia) Thanks On 09.09.2012, at 23:33, Carlos Antonio da Silva notifications@github.com wrote:
|
|
Sure! Just ping me afterwards and I'll merge, thanks. |
…rceError Exception message was misleading, as it is possible to have a polymorphic 'has_many :through' join model.
|
@carlosantoniodasilva I guess i should have mentioned you in the last comment :) |
|
Oh I missed it, thanks! |
…ption-message Improve exception message for HasManyThroughAssociationPolymorphicSourceError
|
No worries - Thanks for your support! |
Exception message was misleading, as it is possible to have a
polymorphic 'has_many :through' join model.