Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

belongs_to relationships do not show up as required on rails 7.1+ #1848

Open
siegfault opened this issue Aug 15, 2024 · 0 comments
Open

belongs_to relationships do not show up as required on rails 7.1+ #1848

siegfault opened this issue Aug 15, 2024 · 0 comments

Comments

@siegfault
Copy link

Environment

  • Ruby 3.3.0
  • Rails 7.2.0
  • Simple Form 5.3.1

Current behavior

By default, on rails 7.1+, belongs_to relationships do not show up as required.

Sample App
# frozen_string_literal: true

require 'bundler/inline'

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

  gem 'rails'
  gem 'simple_form'
  gem 'sqlite3'
end

require 'minitest/autorun'
require 'active_record'
require 'action_view'

ActiveRecord::Base.establish_connection(adapter: 'sqlite3', database: ':memory:')
ActiveRecord::Base.logger = Logger.new($stdout)

ActiveRecord::Schema.define do
  create_table :users, force: true
  create_table :payments, force: true do |t|
    t.references :user
  end
end

class User < ActiveRecord::Base
end

class Payment < ActiveRecord::Base
  belongs_to :user
end

class RequiredTest < ActionView::TestCase
  def test_required
    render inline: <<~ERB
      <%= simple_form_for Payment.new, url: '' do |f| %>
        <div>
          <%= f.association :user %>
        </div>
      <% end %>
    ERB

    element = rendered.html.at('div')

    assert_match /\*/, element.text
  end
end

This works if we add a redundant validates :user, presence: true.

Expected behavior

belongs_to relationships should show up as required

Details

This is due to an interaction between how we calculate required, specifically at

validator.options.include?(:if) || validator.options.include?(:unless)
, and a new change in rails 7.1 (https://github.com/rails/rails/blob/9f9deaf6c14e3fdb3134261a9d52c57208c750ee/activerecord/lib/active_record/associations/builder/belongs_to.rb#L127-L140 / rails/rails#46522) to conditionally define the validation.

In the short term, we can fall back to the old behavior by setting config.active_record.belongs_to_required_validates_foreign_key to true (thus unconditionally defining the validation), but my understanding is that this is a short-lived config to support backwards compatibility, and besides, the new behavior is a performance improvement.

Assuming y'all agree this is something that should be fixed (rather than, say, defining the redundant validation, passing required: true to the field in the form, or something else), I'd be happy to work on a patch if I could get some guidance. Something like

-  validator.options.include?(:if) || validator.options.include?(:unless) 
+ if validator.options.include?(:if)
+   file_path, line_number = validator.options[:if].source_location
+   !file_path.match?(%r{lib/active_record/associations/builder/belongs_to.rb}) && line_number != 130
+ else
+   validator.options.include?(:unless)
+ end

would obviously work but be incredibly hacky. How would we want to approach a more robust way to allow this particular conditional validation where the item should always be present?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

1 participant