brainspec / enumerize

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

Unexpected behaviour of STI models after upgrading to 2.8.0 #447

Closed viralpraxis closed 3 months ago

viralpraxis commented 4 months ago

An MRE:

class Theme < ApplicationRecord
  extend Enumerize

  enumerize :variant, in: %w[accent border]
end

class ThemeSpecific < Theme
  enumerize :variant, in: %w[accent border third_variant]
end

ThemeSpecific is an STI-subclass of Theme.

On 2.7.0 we had this behaviour:

theme_specific = ThemeSpecific.find(...)

theme_specific.update!(variant: "third_variant") # OK, variant is `third_variant`

On 2.8.0:

theme_specific = ThemeSpecific.find(...)

theme_specific.update!(variant: "third_variant") # updates the record, sets `variant` to nil

I'm not sure if it matters but variant is not an ordinary database column, it's a JSONB field (we use jsonb_accessor gem to make it an AR attribute).

Also, it seems like the issue is not STI-specific and happens due to subclassing

nashby commented 4 months ago

Hey @viralpraxis. What Rails version are you using?

viralpraxis commented 4 months ago

@nashby ruby MRI 3.3.0, rails 7.1.3.2

nashby commented 4 months ago

@viralpraxis I did a quick test with STI and everything seems to work fine. So my guess is that something is fishy with JSONB/jsonb_accessor gem. Will try to reproduce it tonight (unless you want to try to add a failing spec, that would be really helpful)

viralpraxis commented 4 months ago

@nashby I made a repo with reproducible behaviour https://github.com/viralpraxis/enumerize-mre

Running with 2.8.0:

$ bundle exec rails test
Running 1 tests in a single process (parallelization threshold is 50)
Run options: --seed 60429

# Running:

F

Failure:
ThemeSpecificTest#test_creation [test/models/theme/specific_test.rb:11]:
Expected: "border"
  Actual: nil

Running with 2.7.0:

(test passes)

(the only non-default files are Gemfile, app/models/theme.rb and app/models/theme/specific.rb)

nashby commented 4 months ago

@viralpraxis thanks! I noticed if you add

jsonb_accessor :data, menu_variant: [:string, default: "default"]

to Specific model the test passes. So there's something strange with how jsonb_accessor works with STI. Will keep investigating

nashby commented 4 months ago

@viralpraxis this should be fixed by https://github.com/brainspec/enumerize/commit/47f0dc569a9dd2bc73e2b9fd5773fb37fe65634a. Could you please test master branch of enumerize with your app?

viralpraxis commented 4 months ago

@nashby I can confirm that our tests are green when running against master 👍🏻

viralpraxis commented 4 months ago

Hey @nashby 👋🏻 Thanks for the fix, any plans on releasing new version?

nashby commented 3 months ago

@viralpraxis 2.8.1 has been released! Thanks for reporting this issue and proving a sample app! ❤️