active-hash / active_hash

A readonly ActiveRecord-esque base class that lets you use a hash, a Yaml file or a custom file as the datasource
MIT License
1.21k stars 179 forks source link

Add column names method #311

Closed hatsu38 closed 3 months ago

hatsu38 commented 4 months ago

This pull request adds the column_names method to the ActiveHash gem.

The column_names method is analogous to the one found in Rails' ActiveRecord, allowing users to retrieve the names of columns defined in an ActiveHash model.

Added column_names method to ActiveHash::Base class. The method returns an array of string column names, similar to ActiveRecord's column_names method.

Country.column_names
# => ["id", "name", "code"]

close #310

flavorjones commented 4 months ago

I would also like to see a short explicit test in spec/active_hash/base_spec.rb like the one I added in #312 for #field_names. Thank you!

kbrock commented 4 months ago

Rails does not have field_names, instead they have column_names. So there is a difference. Rails explicitly states that column_names is an array of strings.

I'd feel more comfortable with just doing the work for column_names. @flavorjones what is your take?

def column_names
  field_names.map(&:name)
end

Also looks like field_names << field is not ensuring that a symbol goes in there, but comparison routines assume it is a symbol. I'll create a PR for that.

flavorjones commented 4 months ago

@kbrock I agree, let's not cache column names, and always iterate over field names when it's called. @hatsu38 Can you make that change?

kbrock commented 4 months ago

@flavorjones Mike, Symbol.name was introduced in ruby 3.0. Do you have an opinion on how to resolve?

I almost think dropping Ruby <3.0 (and therefore rails rails <=6.0. It sounds drastic but they are all EOL since early 2023, and this will work for them, just not the new column_names.

OTOH/ we could use Symbol.to_s and not worry about the frozen world.

flavorjones commented 4 months ago

Oof, hmm. Well for the record: Rails 6.1 is still supported, and it still technically supports Ruby 2.7. Totally annoying.

But: you're right that this is a new feature, and Rails 6.1 is only supported for security updates at this point. So I agree it is feasible and probably preferable to drop support for Ruby < 3.0 in the next minor release.

kbrock commented 4 months ago

w