Skip to content

Conversation

@flavorjones
Copy link
Collaborator

@flavorjones flavorjones commented Jul 3, 2024

There is code that assumes that @field_names will contain symbols, like this method in ActiveHash::Base:

def respond_to?(method_name, include_private=false)
  super ||
    begin
      config = configuration_for_custom_finder(method_name)
      config && config[:fields].all? do |field|
        field_names.include?(field.to_sym) || field.to_sym == :id
      end
    end
end

This also introduces an explicit test for #field_names.

There is code that assumes that `@field_names` will contain symbols,
like this method in ActiveHash::Base:

    def respond_to?(method_name, include_private=false)
      super ||
        begin
          config = configuration_for_custom_finder(method_name)
          config && config[:fields].all? do |field|
            field_names.include?(field.to_sym) || field.to_sym == :id
          end
        end
    end
Copy link
Collaborator

@kbrock kbrock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yours makes fewer changes.

I'm a little amused that I asked if we could use strings for field/column names, yet we both doubled down on using symbols for column names.

@kbrock kbrock merged commit cd64cf3 into master Jul 7, 2024
@kbrock kbrock mentioned this pull request Jul 8, 2024
@flavorjones flavorjones deleted the flavorjones-field-names-should-always-be-symbols branch July 8, 2024 18:16
@kbrock kbrock mentioned this pull request Jul 14, 2024
kbrock added a commit to kbrock/active_hash that referenced this pull request Jul 29, 2025
- Add i18n support active-hash#230 @ryu-sato
- Add `column_names` method active-hash#311 @hatsu38
- Support Active Record 7.2 active-hash#317 @ashleyHutton
- Support ruby 3.4 active-hash#328 @flavorjones
- Add `:alias` to `has_many :through` active-hash#329 @alexgriff
- Add Active Record 8.0 active-hash#324 @flavorjones

- Fix Do not suppress load errors#309 @andreynering
- Ensure `field_names` are all strings active-hash#312 @flavorjones
- Hide private `add_default_value` active-hash#314 @kbrock
- Fix `exists?(nil)` active-hash#320 @y-yagi
- Enance Enum support active-hash#321 @hatsu38
- Updated docs active-hash#326 @y-yagi

- Drop Active Record < 6.1. Ruby < 3.0 active-hash#324 @flavorjones
kbrock added a commit to kbrock/active_hash that referenced this pull request Jul 29, 2025
Added
=====

- Add i18n support active-hash#230 @ryu-sato
- Add `column_names` method active-hash#311 @hatsu38
- Support Active Record 7.2 active-hash#317 @ashleyHutton
- Support ruby 3.4 active-hash#328 @flavorjones
- Add `:alias` to `has_many :through` active-hash#329 @alexgriff
- Add Active Record 8.0 active-hash#324 @flavorjones

Fixed
=====

- Fix Do not suppress load errors#309 @andreynering
- Ensure `field_names` are all strings active-hash#312 @flavorjones
- Hide private `add_default_value` active-hash#314 @kbrock
- Fix `exists?(nil)` active-hash#320 @y-yagi
- Enance Enum support active-hash#321 @hatsu38
- Updated docs active-hash#326 @y-yagi

Removed
=======

- Drop Active Record < 6.1. Ruby < 3.0 active-hash#324 @flavorjones
@kbrock kbrock mentioned this pull request Jul 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants