Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions ci/Gemfile-rails-7-2
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
source 'https://rubygems.org'

gem 'activerecord', '~> 7.2.0'

gemspec :path => "../"
6 changes: 5 additions & 1 deletion lib/rails-properties/property_object.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,11 @@ class PropertyObject < ActiveRecord::Base
end
end

serialize :value, Hash
if ActiveRecord.version >= Gem::Version.new('7.1')
serialize :value, coder: YAML, type: Hash
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing default value for serialize may return nil

High Severity

The new Rails 7.1+ serialize syntax serialize :value, coder: YAML, type: Hash may not provide the same default value behavior as the old serialize :value, Hash. In pre-7.1 Rails, YAMLColumn with a Hash class argument returns {} for nil database values. The new syntax with raw YAML coder and type: Hash may return nil instead, causing NoMethodError when code calls value[name] or value.delete(name), and failing the validation value.is_a? Hash. The default: {} option appears to be missing.

Fix in Cursor Fix in Web

else
serialize :value, Hash
end
Comment thread
JoeDupuis marked this conversation as resolved.

if RailsProperties.can_protect_attributes?
# attr_protected can not be used here because it touches the database which is not connected yet.
Expand Down
4 changes: 2 additions & 2 deletions spec/properties_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@
end

it "should add properties" do
user.properties(:dashboard).update_attributes! :smart => true
user.properties(:dashboard).update! :smart => true

user.reload
expect(user.properties(:dashboard).smart).to eq(true)
Expand Down Expand Up @@ -179,7 +179,7 @@
end

it "should update properties" do
user.properties(:dashboard).update_attributes! :smart => true
user.properties(:dashboard).update! :smart => true
user.reload

expect(user.properties(:dashboard).smart).to eq(true)
Expand Down
8 changes: 4 additions & 4 deletions spec/property_object_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,9 @@
end
end

describe "update_attributes" do
describe "update" do
it 'should save' do
expect(new_property_object.update_attributes(:foo => 42, :bar => 'string')).to be_truthy
expect(new_property_object.update(:foo => 42, :bar => 'string')).to be_truthy
new_property_object.reload

expect(new_property_object.foo).to eq(42)
Expand All @@ -115,12 +115,12 @@
end

it 'should not save blank hash' do
expect(new_property_object.update_attributes({})).to be_truthy
expect(new_property_object.update({})).to be_truthy
end

if RailsProperties.can_protect_attributes?
it 'should not allow changing protected attributes' do
new_property_object.update_attributes!(:var => 'calendar', :foo => 42)
new_property_object.update!(:var => 'calendar', :foo => 42)

expect(new_property_object.var).to eq('dashboard')
expect(new_property_object.foo).to eq(42)
Expand Down
2 changes: 1 addition & 1 deletion spec/queries_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@

it "should update properties by one SQL query" do
expect {
user.properties(:dashboard).update_attributes! :foo => 'bar'
user.properties(:dashboard).update! :foo => 'bar'
}.to perform_queries(1)
end
end
Expand Down
2 changes: 1 addition & 1 deletion spec/serialize_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@

describe 'updated properties' do
it 'should be serialized' do
user.properties(:dashboard).update_attributes! :smart => true
user.properties(:dashboard).update! :smart => true

dashboard_properties = user.property_objects.where(:var => 'dashboard').first
calendar_properties = user.property_objects.where(:var => 'calendar').first
Expand Down