brainspec / enumerize

Enumerated attributes with I18n and ActiveRecord/Mongoid support
MIT License
1.73k stars 190 forks source link

Unexpected behaviour of `@subtype.cast` ?? #452

Closed chrhsmt closed 3 weeks ago

chrhsmt commented 3 months ago

I have found some behavior changes as a result of updating from 2.7.0 to 2.8.0. When assigning to a field using enum key, the value is no longer set, is this the expected behavior? It just seemed like a bug to me....

2.7.0

class Sample < ApplicationRecord
  extend Enumerize
  enumerize :code, in: {
    initial: 1,
    start: 2,
    finish: 3,
  }
end

o = Sample.new
o["code"] = "start"
o
#<Sample:0x0000000129696db8> {
              :id => nil,
            :code => "start",
      :created_at => nil,
      :updated_at => nil
}

2.8.0

class Sample < ApplicationRecord
  extend Enumerize
  enumerize :code, in: {
    initial: 1,
    start: 2,
    finish: 3,
  }
end

o = Sample.new
o["code"] = "start"
o
#<Sample:0x000000012244f570> {
              :id => nil,
            :code => nil, # 👈
      :created_at => nil,
      :updated_at => nil
}

The following commit seems to have affected this. https://github.com/brainspec/enumerize/commit/018c5a4b936fc72ec6dbe362c4a89ad4c7ed3196

https://github.com/brainspec/enumerize/blob/018c5a4b936fc72ec6dbe362c4a89ad4c7ed3196/lib/enumerize/activerecord.rb#L119

chrhsmt commented 3 months ago

This is code to reproduce.

# frozen_string_literal: true

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

  gem "rails"
  # If you want to test against edge Rails replace the previous line with this:
  # gem "rails", github: "rails/rails", branch: "main"

  gem "sqlite3", "~> 1.4"
  gem "enumerize", "2.8.0"
end

require "active_record"
require "minitest/autorun"
require "logger"

# This connection will do for database-independent bug reports.
ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")
ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord::Schema.define do
  create_table :samples, force: true do |t|
    t.integer :code
  end
end

class Sample < ActiveRecord::Base
  extend Enumerize
  enumerize :code, in: {
    initial: 1,
    start: 2,
    finish: 3,
  }
end

class BugTest < Minitest::Test
  def test_enum_assignment
    sample = Sample.new
    sample["code"] = "start"
    assert_equal "start", sample.code
  end
end
nashby commented 3 months ago

hey @chrhsmt! Any reason why you use o["code"] = "start" instead of proper setter as o.code = "start"? By using hash directly you bypass setters Enumerize defines. Even though it worked before it's not something Enumerize supports.

chrhsmt commented 2 months ago

@nashby

Because we have a process that reads multiple csv files and insert them to the database, so we use hash access.

setters Enumerize defines

https://github.com/brainspec/enumerize/blob/5a2b168db6fae53802e2d868a689dcc13d269f11/lib/enumerize/attribute.rb#L107-L122

This area of processing above is that setter definition.

My question is, is there a proactive reason not to support hash access?

It seems like we could implement a setter for hash access in Enumerize::ActiveRecordSupport::InstanceMethods! I've only tried a few things, but I'll send you a PR if I think I can implement it.

      def []=(key, value)
        if self.class.enumerized_attributes[key.to_sym]
          self.send("#{key}=".to_sym, value)
        else
          super
        end
      end
nashby commented 3 weeks ago

I think for your particular use-case you can use public_send to call methods dynamically and it'll solve your issue. I'd rather keep it simple without adding support for []=. Sorry!