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

Support for Rails 7 #92

Closed
kapso opened this issue Jan 12, 2022 · 12 comments
Closed

Support for Rails 7 #92

kapso opened this issue Jan 12, 2022 · 12 comments

Comments

@kapso
Copy link

kapso commented Jan 12, 2022

No description provided.

@paulschreiber
Copy link

One for has added support: healthium@1f3afa4

Can you pull in that commit?

@seanarnold
Copy link

There is a PR here that is open #91.

@NEWeber
Copy link

NEWeber commented Aug 17, 2022

With the merging of #91, you can now pull in master in your Gemfile to support Rails 7:

gem "default_value_for",  github: "FooBarWidget/default_value_for", branch: "master"

(Thank you to @norman for merging!) 🎉

After Norman gets the tests working, he may release or we may need to poke someone else to get a release (he hasn't maintained the project in some time, big thank to him for stepping back briefly for this):
#91 (comment)

@nbcraft
Copy link

nbcraft commented Sep 15, 2022

@chrisarcand If you've got time, would you be able to submit a new release (which would have support for Rails 7) please?
That'd be greatly appreciated by many. 🙏
Cheers!

@igor-drozdov
Copy link
Contributor

@norman @FooBarWidget could you please submit a new release that includes Rails 7 support whenever you have time?

@capripot
Copy link

capripot commented May 11, 2023

It looks like on Rubygems, it still is restricted to Rails < 7.0 which makes it impossible to use with current Rails version directly from Rubygems.

It looks like master is correct but no new version was cut. Could we cut 3.4.1, since it looks like the tests on #91 are passing now?

@jrafanie
Copy link
Collaborator

jrafanie commented Feb 8, 2024

Sorry for the bump but it's been several months. @norman @FooBarWidget @chrisarcand Looks like we have one commit on master since 3.4.0 to add rails 7.0 support. We have another similar PR to add 7.1 support. Can this be merged and released?

@seanarnold
Copy link

seanarnold commented Feb 8, 2024

@jrafanie and others - We found that we could just drop this gem entirely and use the default behaviour that was added in Rails 6: https://api.rubyonrails.org/classes/ActiveRecord/Attributes/ClassMethods.html#method-i-attribute

attribute :foo, :string, default: 'bar'
# or dynamic 
attribute :foo, :datetime, default: -> { Time.now }

@jrafanie
Copy link
Collaborator

@seanarnold thanks! For reference, it's not completely compatible.

I've found the attributes API is different than default_value_for in the following ways:

  1. You can't override the getter/setter methods for the attribute in the same way you could with this gem.
  2. The default value block format doesn't yield the instance, so you can't derive defaults based on the current instance.
  3. There doesn't seem to be a way to set multiple default values all at once. You have to set them individually.
  4. Default values make new objects look changed, unlike default_value_for.
  5. Attributes API default value doesn't support allow_nil: false so if a nil is provided as a value for an attribute, the default value will not be used.

I think you can use before_validation or other mechanisms to manually change your code to deal with allow_nil: false, deriving defaults based on the yielded instance or anything you needed to do in the getter/setter methods.

See also: https://jtway.co/how-to-setup-default-values-for-attributes-in-ruby-on-rails-dd1d2ba38b82

I took a stab at trying to implement some of the attributes API in the default_value_for tests to see what breaks.

See changes below:

diff --git a/test.rb b/test.rb
index 6347a46..8176486 100644
--- a/test.rb
+++ b/test.rb
@@ -19,6 +19,7 @@
 # THE SOFTWARE.
 
 require 'bundler/setup'
+require 'minitest'
 require 'minitest/autorun'
 require 'minitest/around/unit'
 require 'active_record'
@@ -61,7 +62,7 @@ if rails_2_4_or_newer && arel_older_than_7_1
 end
 
 begin
-  TestCaseClass = MiniTest::Test
+  TestCaseClass = Minitest::Test
 rescue NameError
   TestCaseClass = MiniTest::Unit::TestCase
 end
@@ -121,10 +122,15 @@ class DefaultValuePluginTest < TestCaseClass
   end
 
   def test_default_value_on_attribute_methods
+    skip "attributes API default value doesn't work with overridden getter/setter methods"
+
     Book.class_eval do
       serialize :stuff
-      default_value_for :color, :green
-      def color; (self.stuff || {})[:color]; end
+      attribute :color, :string, :default => :green
+      def color
+        (self.stuff || {})[:color] || super
+      end
+
       def color=(val)
         self.stuff ||= {}
         self.stuff[:color] = val
@@ -134,17 +140,17 @@ class DefaultValuePluginTest < TestCaseClass
   end
 
   def test_default_value_can_be_passed_as_argument
-    Book.default_value_for(:number, 1234)
+    Book.class_eval { attribute :number, :integer, default: 1234}
     assert_equal 1234, Book.new.number
   end
 
   def test_default_value_can_be_passed_as_block
-    Book.default_value_for(:number) { 1234 }
+    Book.class_eval { attribute :number, :integer, default: ->{1234} }
     assert_equal 1234, Book.new.number
   end
 
   def test_works_with_create
-    Book.default_value_for :number, 1234
+    Book.class_eval { attribute :number, :integer, default: 1234}
 
     object = Book.create
     refute_nil Book.find_by_number(1234)
@@ -156,7 +162,9 @@ class DefaultValuePluginTest < TestCaseClass
   end
 
   def test_does_not_allow_nil_sets_default_value_on_existing_nils
-    Book.default_value_for(:number, :allows_nil => false) { 1234 }
+    skip "attributes API default value doesn't support :allows_nil => false (it allows provided nils instead of the default)"
+
+    Book.class_eval { attribute :number, :integer, default: 1234 }
     object = Book.create
     object.update_attribute(:number, nil)
     assert_nil Book.find_by_number(1234)
@@ -164,24 +172,24 @@ class DefaultValuePluginTest < TestCaseClass
   end
 
   def test_overwrites_db_default
-    Book.default_value_for :count, 1234
+    Book.class_eval { attribute :count, :integer, default: 1234 }
     assert_equal 1234, Book.new.count
   end
 
   def test_doesnt_overwrite_values_provided_by_mass_assignment
-    Book.default_value_for :number, 1234
+    Book.class_eval { attribute :number, :integer, default: 1234 }
     assert_equal 1, Book.new(:number => 1, :count => 2).number
   end
 
   def test_doesnt_overwrite_values_provided_by_multiparameter_assignment
-    Book.default_value_for :timestamp, Time.mktime(2000, 1, 1)
+    Book.class_eval { attribute :timestamp, :time, default: Time.mktime(2000, 1, 1) }
     timestamp = Time.mktime(2009, 1, 1)
     object = Book.new('timestamp(1i)' => '2009', 'timestamp(2i)' => '1', 'timestamp(3i)' => '1')
     assert_equal timestamp, object.timestamp
   end
 
   def test_doesnt_overwrite_values_provided_by_constructor_block
-    Book.default_value_for :number, 1234
+    Book.class_eval { attribute :number, :integer, default: 1234 }
     object = Book.new do |x|
       x.number = 1
       x.count  = 2
@@ -190,45 +198,49 @@ class DefaultValuePluginTest < TestCaseClass
   end
 
   def test_doesnt_overwrite_explicitly_provided_nil_values_in_mass_assignment
-    Book.default_value_for :number, 1234
+    Book.class_eval { attribute :number, :integer, default: 1234 }
     assert_nil Book.new(:number => nil).number
   end
 
   def test_overwrites_explicitly_provided_nil_values_in_mass_assignment
-    Book.default_value_for :number, :value => 1234, :allows_nil => false
+    skip "attributes API default value doesn't support :allows_nil => false (it allows provided nils instead of the default)"
+
+    Book.class_eval { attribute :number, :integer, default: 1234 }
     assert_equal 1234, Book.new(:number => nil).number
   end
 
   def test_default_values_are_inherited
-    Book.default_value_for :number, 1234
+    Book.class_eval { attribute :number, :integer, default: 1234 }
     assert_equal 1234, Novel.new.number
   end
 
   def test_default_values_in_superclass_are_saved_in_subclass
-    Book.default_value_for :number, 1234
-    Novel.default_value_for :flag, true
+    Book.class_eval { attribute :number, :integer, default: 1234 }
+    Novel.class_eval { attribute :flag, :boolean, default: true }
     object = Novel.create!
     assert_equal object.id, Novel.find_by_number(1234).id
     assert_equal object.id, Novel.find_by_flag(true).id
   end
 
   def test_default_values_in_subclass
-    Novel.default_value_for :number, 5678
+    Novel.class_eval { attribute :number, :integer, default: 5678 }
     assert_equal 5678, Novel.new.number
     assert_nil Book.new.number
   end
 
   def test_multiple_default_values_in_subclass_with_default_values_in_parent_class
     Book.class_eval do
-      default_value_for :other_number, nil
+      attribute :other_number, :integer, default: nil
       attr_accessor :other_number
     end
-    Novel.default_value_for :number, 5678
+
+    Novel.class_eval { attribute :number, :integer, default: 5678 }
 
     # Ensure second call in this class doesn't reset _default_attribute_values,
     # and also doesn't consider the parent class' _default_attribute_values when
     # making that check.
-    Novel.default_value_for :user_id, 9999
+
+    Novel.class_eval { attribute :user_id, :integer, default: 9999 }
 
     object = Novel.new
     assert_nil object.other_number
@@ -237,40 +249,47 @@ class DefaultValuePluginTest < TestCaseClass
   end
 
   def test_override_default_values_in_subclass
-    Book.default_value_for :number, 1234
-    Novel.default_value_for :number, 5678
+    Book.class_eval { attribute :number, :integer, default: 1234 }
+    Novel.class_eval { attribute :number, :integer, default: 5678 }
     assert_equal 5678, Novel.new.number
     assert_equal 1234, Book.new.number
   end
 
   def test_default_values_in_subclass_do_not_affect_parent_class
-    Book.default_value_for :number, 1234
+    skip "attributes API default value doesn't work with overridden getter/setter methods"
+
+    Book.class_eval { attribute :number, :integer, default: 1234 }
     Novel.class_eval do
-      default_value_for :hello, "hi"
+      attribute :hello, :string, default: "hi"
       attr_accessor :hello
     end
 
-    assert Book.new
-    assert !Book._default_attribute_values.include?(:hello)
+    assert !Book.new.respond_to?(:hello)
+    assert_equal "hi", Novel.new.hello
   end
 
   def test_doesnt_set_default_on_saved_records
     Book.create(:number => 9876)
-    Book.default_value_for :number, 1234
+
+    Book.class_eval { attribute :number, :integer, default: 1234 }
     assert_equal 9876, Book.first.number
   end
 
   def test_also_works_on_attributes_that_arent_database_columns
+    skip "attributes API default value doesn't work with overridden getter/setter methods"
+
     Book.class_eval do
-      default_value_for :hello, "hi"
+      attribute :hello, :string, default: "hi"
       attr_accessor :hello
     end
     assert_equal 'hi', Book.new.hello
   end
 
   def test_works_on_attributes_that_only_have_writers
+    skip "attributes API default value doesn't work with overridden getter/setter methods"
+
     Book.class_eval do
-      default_value_for :hello, "hi"
+      attribute :hello, :string, default: "hi"
       attr_writer :hello
     end
     assert_equal 'hi', Book.new.instance_variable_get('@hello')
@@ -283,7 +302,7 @@ class DefaultValuePluginTest < TestCaseClass
         super(:count => 5678)
       end
 
-      default_value_for :number, 1234
+      attribute :number, :integer, default: 1234
     end
     object = Book.new
     assert_equal 1234, object.number
@@ -292,6 +311,8 @@ class DefaultValuePluginTest < TestCaseClass
   end
 
   def test_model_instance_is_passed_to_the_given_block
+    skip "attributes API default value doesn't yield the instance in the block"
+
     instance = nil
     Book.default_value_for :number do |n|
       instance = n
@@ -301,6 +322,8 @@ class DefaultValuePluginTest < TestCaseClass
   end
 
   def test_can_specify_default_value_via_association
+    skip "attributes API default value doesn't yield the instance in the block"
+
     user = User.create(:username => 'Kanako', :default_number => 123)
     Book.default_value_for :number do |n|
       n.user.default_number
@@ -309,6 +332,8 @@ class DefaultValuePluginTest < TestCaseClass
   end
 
   def test_default_values
+    skip "attributes API default value doesn't support setting multiple default_values in this way"
+
     Book.default_values({
       :type      => "normal",
       :number    => lambda { 10 + 5 },
@@ -321,18 +346,20 @@ class DefaultValuePluginTest < TestCaseClass
   end
 
   def test_default_value_order
-    Book.default_value_for :count, 5
-    Book.default_value_for :number do |this|
-      this.count * 2
-    end
+    skip "attributes API default value doesn't yield the instance in the block, so you can't access other attributes"
+
+    Book.class_eval { attribute :count, :integer, default: 5 }
+    Book.class_eval { attribute :number, :integer, default: -> { count * 2} }
+
     object = Book.new
     assert_equal(5, object.count)
     assert_equal(10, object.number)
   end
 
   def test_attributes_with_default_values_are_not_marked_as_changed
-    Book.default_value_for :count, 5
-    Book.default_value_for :number, 2
+    skip "attributes API default value makes new objects look changed"
+    Book.class_eval { attribute :count, :integer, default: 5 }
+    Book.class_eval { attribute :number, :integer, default: 2 }
 
     object = Book.new
     assert(!object.changed?)
@@ -344,7 +371,7 @@ class DefaultValuePluginTest < TestCaseClass
   end
 
   def test_default_values_are_duplicated
-    User.default_value_for :username, "hello"
+    User.class_eval { attribute :username, :string, default: "hello" }
     user1 = User.new
     user1.username << " world"
     user2 = User.new
@@ -352,9 +379,11 @@ class DefaultValuePluginTest < TestCaseClass
   end
 
   def test_default_values_are_shallow_copied
+    skip "attributes API default value... not sure how to set hash default value"
+
     User.class_eval do
       attr_accessor :hash
-      default_value_for :hash, { 1 => [] }
+      attribute :hash, default: { 1 => [] }
     end
     user1 = User.new
     user1.hash[1] << 1
@@ -377,7 +406,9 @@ class DefaultValuePluginTest < TestCaseClass
   end
 
   def test_does_not_see_false_as_blank_at_boolean_columns_for_existing_records
-    Book.default_value_for(:flag, :allows_nil => false) { true }
+    Book.class_eval do
+      attribute :flag, :boolean, default: true
+    end
 
     object = Book.create
 
@@ -387,9 +418,10 @@ class DefaultValuePluginTest < TestCaseClass
   end
 
   def test_works_with_nested_attributes
-    User.accepts_nested_attributes_for :books
-    User.default_value_for :books do
-      [Book.create!(:number => 0)]
+    skip "attributes API default value doesn't yield the instance in the block, so you can't update associations"
+
+    User.class_eval do
+      attribute :books, default: -> { [Book.create!(:number => 0)] }
     end
 
     user = User.create! :books_attributes => [{:number => 1}]
@@ -398,7 +430,9 @@ class DefaultValuePluginTest < TestCaseClass
 
   def test_works_with_stored_attribute_accessors_when_initializing_value_that_does_not_allow_nil
     User.store :settings, :accessors => :bio
-    User.default_value_for :bio, :value => 'None given', :allows_nil => false
+    User.class_eval do
+      attribute :bio, default: -> { {:value => 'None given'} }
+    end
 
     user = User.create!(:bio => 'This is a bio')
     assert_equal 'This is a bio', user.bio
@@ -406,13 +440,19 @@ class DefaultValuePluginTest < TestCaseClass
 
   if ActionPack::VERSION::MAJOR > 3
     def test_doesnt_overwrite_explicitly_provided_nil_values_in_mass_assignment_with_action_controller_parameters
-      Book.default_value_for :number, 1234
+      Book.class_eval do
+        attribute :number, default: 1234
+      end
 
       assert_nil Book.new(ActionController::Parameters.new(:number => nil).permit!).number
     end
 
     def test_overwrites_explicitly_provided_nil_values_in_mass_assignment_with_action_controller_parameters
-      Book.default_value_for :number, :value => 1234, :allows_nil => false
+      skip "attributes API default value doesn't support :allows_nil => false (it allows provided nils instead of the default)"
+
+      Book.class_eval do
+        attribute :number, default: 1234
+      end
 
       assert_equal 1234, Book.new(ActionController::Parameters.new(:number => nil).permit!).number
     end

@ebed
Copy link

ebed commented Feb 20, 2024

Is it possible to create a new tag for this version that allows Rails 7 to avoid using the master branch?

@szeryf
Copy link

szeryf commented Mar 14, 2024

Please bump the version so it can be used with Rails 7. The latest version 3.4.0 says rails < 7.0.

@jrafanie
Copy link
Collaborator

I've merged #96 and pushed 3.5.0 with rails 7.0 support and 3.6.0 with rails 7.1 support. Please open issues if you find any problems. Thanks!

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

No branches or pull requests

10 participants