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

String attr_json attributes does not get stripped using strip_attributes #151

Closed
rekapap opened this issue Jun 8, 2022 · 4 comments
Closed

Comments

@rekapap
Copy link

rekapap commented Jun 8, 2022

Hi,

I wanted to use the strip_attributes gem to strip white spaces from a string attr_json attribute,
which surprisingly does not work.

Current behaviour:
Given a model using the strip_attributes gem for all attributes,
When a string attr_json field is changed with a string including whitespaces,
Then the string attr_json attribute is not stripped of white spaces.

My expected behaviour:
Given a model using the strip_attributes gem for all attributes,
When a string attr_json field is changed with a string including whitespaces,
Then the string attr_son attribute is stripped of white spaces.

Example of the current behaviour:

Model

# frozen_string_literal: true

# == Schema Information
#
# Table name: accounts
#
#  id                             :bigint           not null, primary key
#  full_name                      :string

class Account < ApplicationRecord
  strip_attributes

  attr_json :foo, :string
end

Testing from the console to keep is simple:

> account.full_name = ' white space '
=> " white space "
> account.foo = ' spacey attr json '
=> " spacey attr json "
> account.save
=> true
> account.full_name
=> "white space"
> account.foo
=> " spacey attr json "

I'm not sure why it does not work with attr_json attributes, the gem itself is stripping the attributes before validation.
Any thoughts?

@jrochkind
Copy link
Owner

jrochkind commented Jun 8, 2022

Hi, thanks for the question, and the clear observed/expected/example bug report!

What do you mean by "the gem itself is stripping the attributes before validation"?

Can you try setting rails_attribute: true when you define the attributes, and see if that makes any difference? (I mean to make this always on in a future 2.0 release).

Otherwise, I'm not sure, it's possible that the strip_attributes gem just works using Rails API that doesn't apply to the way attr_json works. I don't really have time to debug it myself at the moment, but would be happy to hear and respond to anything anyone else finds from more debugging, or review a PR!

@rekapap
Copy link
Author

rekapap commented Jun 20, 2022

Hi,
Thank you for your fast reply.
I added the rails_attribute: true, unfortunately, it did not work.
I will investigate a bit further, thank you.

@jrochkind
Copy link
Owner

Looks like the strip_attributes gem uses the []= method to set the new value...

https://github.com/rmm5t/strip_attributes/blob/37c1cb257638ea5075f970e8beb763eb3ea06bc8/lib/strip_attributes.rb#L47

I am doing some refactoring of internal attr_json things for a 2.x release, and I was wondering if this might be enough to make strip_attributes work, but I'm afraid that I still haven't managed to make AttrJson compatible with setting values with []=.

if strip_attributes were changed to use record.send("#{attr}=", value) instead, it should work compatibly with AttrJson. (Although wouldn't help you get into things like array attributes and such).

Alternately, you could try to write your own code using strip_attributes, re-use it's code in a slightly different way. It would take writing some code.

As it is, they are not compatible, and there isn't any great path on this end to making them compatible.

I'm going to close this ticket for now! Thanks for the report!

@jrochkind
Copy link
Owner

Note that a change to strip_attributes was proposed at rmm5t/strip_attributes#34 a few years ago that would make it work with attr_json, among other gems; but it was rejected.

I do think we need some good answer for stripping with attr_json, I'm thinking on it and hope to have something in upcoming v2 release.

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

2 participants