This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
Thank you for your contributions.

@kamipo ping to check whether you're still interested in this 🙇

Do you have any ideas on @ghiculescu's comment? https://github.com/rails/rails/pull/41730/files#r603533074

I and the people around me are still hoping that this problem will be solved in the Rails side without overriding the attribute writer, but this is a new feature so I'm waiting for feedback from the core team.

I meant that user provided validate option and the original assert_valid_value on the type cast are exclusive, but I admit that it is a bit confusing, so I am open to good suggestions.

@kamipo definitely find @ghiculescu suggestions an improvement. Would love to see them applied and this merged 🙏

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
Thank you for your contributions.

Having user friendly feedback on our model backed forms is a must. Not being able to use enum for this creates a lot of extra code and certainly doesn't seem like the Rails way.

I'm happy to help move this forward. What needs to be done to get this merged?

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
Thank you for your contributions.

What is missing to get this merged? Indeed throwing an exception is not the expected behavior.

Maybe instead of validate the option could be named strict to mirror behavior that is already present for the validate method. Just adding an option to just suppress the ArgumentError could be a way forward? That way it is possible to keep using enum without overriding methods and add validations on top.

class Book < ActiveRecord::Base
  enum :status, [:proposed, :written], strict: false

  validates :status, inclusion:  { in: statuses.keys }
end

I can propose this concern

# models/concern/custom_enum_validation.rb
module CustomEnumValidation
  extend ActiveSupport::Concern

  included do
    self.defined_enums.keys.each do |key|
      validates_inclusion_of key, in: send(key.pluralize).keys

      define_method "#{key}=" do |value|
        @attributes.write_cast_value(key, value)
      end
    end
  end
end

# models/user.rb

class User < ApplicationRecord
  enum status: %i[approved rejected]
  enum some_other_status: %i[foo bar]
  # Must be included after all `enum` declarations
  include CustomEnumValidation
end

With shared example (using Faker + Should-matchers):

shared_examples_for 'custom_enum_validation' do
  let(:factory_name) { model_class.to_s.gsub('::', '').underscore.to_sym }
  let(:model_class) { described_class } # the class that includes the concern

  context 'with custom enum validation' do
    let(:attributes) { attributes_for(factory_name) }
    let(:model_instance) { described_class.new(attributes) }

    described_class.defined_enums.each do |key, values|
      it { is_expected.to validate_inclusion_of(key).in_array(values.keys)}
    end
  end
end

Given this feature is still being fleshed out, in a different issue someone dropped in this as a workaround: #13971 (comment)

It replaces the current behavior of the EnumType type which is:

      def assert_valid_value(value)
        unless value.blank? || mapping.has_key?(value) || mapping.has_value?(value)
          raise ArgumentError, "'#{value}' is not a valid #{name}"
        end
      end

With:

      def assert_valid_value(value)
        unless value.blank? || mapping.has_key?(value) || mapping.has_value?(value)
          nil
        end
      end

And now you can get validation errors with:

class Post < ApplicationRecord
  enum :status, { unpublished: 0, published: 10 }
  validates :status, inclusion: { in: statuses.keys, message: :invalid }
end

Are there any gotchas or things to think about where dropping this into a new project would be an issue until AR is officially patched with whatever solution ends up being ideal?

One thing I noticed is if you use the above patch and don't put in the validator, then the following things occur:

• Trying to write status: 42 on create! results in AR successfully setting status: nil and writing the row in the database but Postgres will end up having 42 written to the status column giving your database an integrity issue based on your enum definition's values, this also bypasses a null: false at the database level since it gets written with the invalid data
• Trying to write status: "yikes" on create! results in Postgres throwing PG::NotNullViolation: ERROR: null value in column "status" of relation "posts" violates not-null constraint with no row being created in the database

I guess the takeaway here is do you want guaranteed database integrity or nice user facing messages? Seems like a case where having an official patch to give us the best of both worlds will be much appreciated!

We merged #49100 for this change so that we can close this PR.

#49100