owen2345 / camaleon-cms

Camaleon CMS is a dynamic and advanced content management system based on Ruby on Rails
https://camaleon.website
MIT License
1.21k stars 285 forks source link

Typo in CamaleonCms::CustomFieldsRead (post -> Post) ? #376

Closed gcrofils closed 8 years ago

gcrofils commented 8 years ago

in models/concerns/camaleon_cms/custom_fields_read.rb, line 28

It seems default kind must be Post instead of post.

args = args.is_a?(String) ? {kind: args, include_parent: false } : {kind: "post", include_parent: false }.merge(args)

with the current code :

CamaleonCms::PostType.find_by_slug('post').get_field_groups
=> #<ActiveRecord::AssociationRelation []>

CamaleonCms::PostType.find_by_slug('post').get_field_groups('Post').count
 => 2

with the modified code :

CamaleonCms::PostType.find_by_slug('post').get_field_groups.count
=> 2

CamaleonCms::PostType.find_by_slug('post').get_field_groups('Post').count
 => 2

I didn't make a pull request as I'm not 100% sure of the consequences of this change and how to test it extensively. Nevertheless I made the change on my fork and it didn't break anything.

owen2345 commented 8 years ago

Hi @gcrofils I am sorry, I can not see the change, could you please explain me more?

Regards,

gcrofils commented 8 years ago

in models/concerns/camaleon_cms/custom_fields_read.rb line 141

The default value for kind is Post (capital P) def add_custom_field_group(values, kind = "Post")

but in the same file, line 27, the default value is post (lower case) args = args.is_a?(String) ? {kind: args, include_parent: false } : {kind: "post", include_parent: false }.merge(args)

It's not consistent.

It's not a big deal as you can always use the method get_field_groups with arguments but it won't work without any argument (see examples in my previous comment).

owen2345 commented 8 years ago

ooohhh I can see, I will review....