Skip to content

Update 2.0.0 appears to break for POROs #81

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

Open
cjhutchi opened this issue Feb 27, 2025 · 3 comments
Open

Update 2.0.0 appears to break for POROs #81

cjhutchi opened this issue Feb 27, 2025 · 3 comments

Comments

@cjhutchi
Copy link

cjhutchi commented Feb 27, 2025

I recently updated to the latest version (2.0.0), and in our app we use this gem on some POROs. Here's an example of how we use it:

class PatientForm
  include ActiveModel::Model
  include ActiveModel::Attributes
  include ActiveModel::Validations::Callbacks

  strip_attributes only: :last_name, collapse_spaces: true

  ...
end

After the update, the callback to collapse_spaces seemed to break with this error:

undefined method '[]=' for an instance of PatientForm

I was able to fix it by manually just defining the [] and []= methods on the model

class PatientForm
  include ActiveModel::Model
  include ActiveModel::Attributes
  include ActiveModel::Validations::Callbacks

  strip_attributes only: :last_name, collapse_spaces: true

  def [](key)
    send(key)
  end

  def []=(key, value)
    send("#{key}=", value)
  end
  ...
end

Is this an actual bug in the release, or are the callbacks not intended to be used with objects that don't get persisted to the database like this and we should use the gem directly instead?

@rmm5t
Copy link
Owner

rmm5t commented Feb 27, 2025

@cjhutchi Are you saying that things worked for you prior to v2.0.0 and they no longer work? I'm a little confused why that would be the case, because nothing changed between 1.x and 2.0 that would affect that exact scenario.
https://github.com/rmm5t/strip_attributes/compare/v1.14.1..v2.0.0

Officially, strip_attributes doesn't have any direct tests for the exact scenario you're working under.

There is an open PR (#66) to better support raw ActiveModel and I would like to incorporate that, but not at the expense of losing ActiveAttr.

@cjhutchi
Copy link
Author

@rmm5t Sorry, yes, this was working prior to v2.0.0. We have a workaround for this, so its not a huge issue, I just wanted to bring it to your attention. Thanks for the quick response!

@justinko
Copy link

justinko commented Feb 28, 2025

value.dup is the cause of the regression: https://github.com/rmm5t/strip_attributes/pull/80/files#diff-d6d7fbebc68691eb492b2d3dff74c7841e33653fe5aae5f5f44efc99905cc5a5R64

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

3 participants