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

Fix Rails 3 serialized attributes #14

Merged
merged 6 commits into from
Feb 13, 2018
Merged

Conversation

einzige
Copy link
Owner

@einzige einzige commented Jul 31, 2016

  • Add spec

@ritikesh

Going forward, let's have our updates separated in commits.

For a spec, you'll need to update spec/spe_helper.rb where you'll find a test class called "Fake", just add a new column to it and serialize the way you did in your app.

It may also require you to add conditional spec / code depending on a currently running rails/AR version, eg: if ActiveRecord.version < 4 then add serialization, run this spec

@@ -25,6 +26,8 @@ class Fake < ActiveRecord::Base
before_save :before_save_callback

belongs_to :belonger

Choose a reason for hiding this comment

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

Trailing whitespace detected.

@ritikesh
Copy link
Collaborator

ritikesh commented Aug 2, 2016

It may also require you to add conditional spec / code depending on a currently running rails/AR version, eg: if ActiveRecord.version < 4 then add serialization, run this spec.

Doubt that it would require version checks. Serialized columns should work irrespective of whichever version we're on right?
Let me know your thoughts. Thanks.

@einzige
Copy link
Owner Author

einzige commented Aug 3, 2016

@ritikesh yep, sounds right.

Let's check if it is really saved in specs

@ritikesh
Copy link
Collaborator

@einzige when I ran specs, it ran with 0 failures. How about you?

@einzige
Copy link
Owner Author

einzige commented Aug 17, 2016

@ritikesh 0 failures, but we need to make sure a serialized column gets saved. For instance:

it 'updates serialized column' do
  expect do
     fake.config = {one: :two}
     fake.sneaky_save!
  end.to change { fake.reload.config }.from(nil).to(one: :two)
end

@@ -25,6 +26,8 @@ class Fake < ActiveRecord::Base
before_save :before_save_callback

belongs_to :belonger

Choose a reason for hiding this comment

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

Trailing whitespace detected.

expect(subject.sneaky_save).to eq(true)
subject.reload
expect(subject.name).to eq("new name")
expect(subject.sneaky_save).to eq(true)
expect(subject.config).to eq({test: "test"})

Choose a reason for hiding this comment

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

Space inside { missing.
Redundant curly braces around a hash parameter.
Space inside } missing.

@@ -71,10 +78,11 @@

it "stores attributes in database" do
subject.name = "new name"
subject.config = {test: "test"}

Choose a reason for hiding this comment

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

Space inside { missing.
Space inside } missing.


it "updates serialized column" do
expect do
fake.config = {one: :two}

Choose a reason for hiding this comment

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

Use 2 (not 3) spaces for indentation.
Space inside { missing.
Space inside } missing.


# Serialize values for rails3 before updating
unless sneaky_new_rails?
serialized_fields = self.class.serialized_attributes.keys & changed_attributes.keys

Choose a reason for hiding this comment

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

Line is too long. [89/80]

@ritikesh ritikesh merged commit db42aa9 into master Feb 13, 2018
@ritikesh ritikesh deleted the fix-rails3-serialization branch February 13, 2018 14:31
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

Successfully merging this pull request may close these issues.

None yet

3 participants