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 dirty nil assignments #107

Merged
merged 1 commit into from
Sep 22, 2020
Merged

Fix dirty nil assignments #107

merged 1 commit into from
Sep 22, 2020

Conversation

chrisarcand
Copy link
Member

Description

Fixes a regression caused by 6c5b1a3

The usage of instance_variable_get means that if an attribute is assigned to nil, the variable to signal that attributes are loaded is false - meaning that any intentional setting of nil results in attributes getting reloaded on the next read and wiping that nil value back to whatever it was before. I've added some specs to describe this behavior.

Fixes a regression caused by
6c5b1a3

The usage of instance_variable_get means that if an attribute is
assigned to `nil`, the variable to signal that attributes are loaded is
`false` - meaning that any intentional setting of `nil` results in
attributes getting reloaded on the next read and wiping that `nil` value
back to whatever it was before. I've added a spec to describe this
behavior.
Copy link
Contributor

@Valarissa Valarissa left a comment

Choose a reason for hiding this comment

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

Great catch! This looks great.

Copy link
Member

@radditude radditude left a comment

Choose a reason for hiding this comment

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

Nice!

@chrisarcand chrisarcand merged commit a9866a3 into master Sep 22, 2020
@chrisarcand chrisarcand deleted the fix-nil-assignments branch September 22, 2020 14:32
@gdemonie
Copy link

gdemonie commented Oct 11, 2021

This doesn't seem to fix the case where vault_single_decrypt! is enabled?
After assigning nil to an encrypted attribute, the return value for the attribute is still the previous value.
There is a test for that case with LazyPerson:

  it "allows dirty attributes to be unset" do
      person = LazyPerson.create!(ssn: "123-45-6789")
      person.ssn = nil
      expect(person.ssn).to be_nil
      person2 = LazyPerson.create!(ssn: "123-45-6789")
      person2.assign_attributes(ssn: nil)
      expect(person2.ssn).to be_nil
    end

The same case with LazySinglePerson doesn't have the same test.

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.

4 participants