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.

Sign up for GitHub

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

Jump to bottom

Should Active Model's ImmutableString type observe the limit parameter? #51494

Open
freesteph opened this issue Apr 4, 2024 · 0 comments
Open

Should Active Model's ImmutableString type observe the limit parameter? #51494

freesteph opened this issue Apr 4, 2024 · 0 comments
Labels
activemodel

Comments

@freesteph
Copy link

freesteph commented Apr 4, 2024 •
edited

TL;DR: should attribute :foobar, :string, limit: 30 chop the value to 30 characters?

ActiveModel::Type::String inherits
ActiveModel::Type::ImmutableString inherits
ActiveModel::Type::Value which accepts three limit, scale, precision options but lets the base type decide what to do with them.

I was wondering whether it would make sense for the string types to interpret limit as a max length and chop on cast or serialize. I've got some custom initializer code at the moment which does:

class ActiveModel::Type::String
  def cast(value)
    if @limit
      super(value.to_s.first(@limit))
    else
      super
    end
  end
end

Which reuses the ImmutableString#cast logic and chops the value, not great but works well, well enough to ask the question here.

What do people think? Happy to submit a PR if we can agree it's nice to have.

NOTE: ActiveModel::Type::Integer interprets limit as the number of bytes to store the integer with, which makes the option a bit confusing. However, a t.string :column_name, limit: 42 will effectively limit the length of that string column in Active Record.
freesteph added a commit to betagouv/aplypro that referenced this issue Apr 4, 2024
@freesteph
asp: truncate the persphysique names attributes
e091ee3 
Piggyback the `limit` option for ActiveModel's attribute method (which
might be a thing one day[1]) to chop the value of the string directly
on the XML-generating class.

This keeps the mapping classes in charge of translating our data into
the ASP stuff, semantically, but keeps the actual XML-maker in control
of the formatted length, which makes more sense to me: all the XML
tags have an associated length in the ASP documentation, so keeping it
as close as possible to the XML generation code is nicer.

Closes #678.

[1]: rails/rails#51494
freesteph added a commit to betagouv/aplypro that referenced this issue Apr 4, 2024
@freesteph
asp: truncate the persphysique names attributes
e457694 
Piggyback the `limit` option for ActiveModel's attribute method (which
might be a thing one day[1]) to chop the value of the string directly
on the XML-generating class.

This keeps the mapping classes in charge of translating our data into
the ASP stuff, semantically, but keeps the actual XML-maker in control
of the formatted length, which makes more sense to me: all the XML
tags have an associated length in the ASP documentation, so keeping it
as close as possible to the XML generation code is nicer.

Closes #678.

[1]: rails/rails#51494
freesteph added a commit to betagouv/aplypro that referenced this issue Apr 4, 2024
@freesteph
asp: truncate the persphysique names attributes
099802c 
Piggyback the `limit` option for ActiveModel's attribute method (which
might be a thing one day[1]) to chop the value of the string directly
on the XML-generating class.

This keeps the mapping classes in charge of translating our data into
the ASP stuff, semantically, but keeps the actual XML-maker in control
of the formatted length, which makes more sense to me: all the XML
tags have an associated length in the ASP documentation, so keeping it
as close as possible to the XML generation code is nicer.

It's also coherent with the custom :asp_date type which we use in a
couple places (ie: prestadoss.rb) and takes care of formatting a date
object in the format dictated by the ASP.

Closes #678.

[1]: rails/rails#51494
freesteph added a commit to betagouv/aplypro that referenced this issue Apr 4, 2024
@freesteph
asp: truncate the persphysique names attributes
7b55dbc 
Piggyback the `limit` option for ActiveModel's attribute method (which
might be a thing one day[1]) to chop the value of the string directly
on the XML-generating class.

This keeps the mapping classes in charge of translating our data into
the ASP stuff, semantically, but keeps the actual XML-maker in control
of the formatted length, which makes more sense to me: all the XML
tags have an associated length in the ASP documentation, so keeping it
as close as possible to the XML generation code is nicer.

It's also coherent with the custom :asp_date type which we use in a
couple places (ie: prestadoss.rb) and takes care of formatting a date
object in the format dictated by the ASP.

Closes #678.

[1]: rails/rails#51494
freesteph added a commit to betagouv/aplypro that referenced this issue Apr 8, 2024
@freesteph
asp: truncate the persphysique names attributes
0577ca6 
Piggyback the `limit` option for ActiveModel's attribute method (which
might be a thing one day[1]) to chop the value of the string directly
on the XML-generating class.

This keeps the mapping classes in charge of translating our data into
the ASP stuff, semantically, but keeps the actual XML-maker in control
of the formatted length, which makes more sense to me: all the XML
tags have an associated length in the ASP documentation, so keeping it
as close as possible to the XML generation code is nicer.

It's also coherent with the custom :asp_date type which we use in a
couple places (ie: prestadoss.rb) and takes care of formatting a date
object in the format dictated by the ASP.

Closes #678.

[1]: rails/rails#51494
freesteph added a commit to betagouv/aplypro that referenced this issue Apr 8, 2024
@freesteph
asp: truncate the persphysique names attributes
51452ce 
Piggyback the `limit` option for ActiveModel's attribute method (which
might be a thing one day[1]) to chop the value of the string directly
on the XML-generating class.

This keeps the mapping classes in charge of translating our data into
the ASP stuff, semantically, but keeps the actual XML-maker in control
of the formatted length, which makes more sense to me: all the XML
tags have an associated length in the ASP documentation, so keeping it
as close as possible to the XML generation code is nicer.

It's also coherent with the custom :asp_date type which we use in a
couple places (ie: prestadoss.rb) and takes care of formatting a date
object in the format dictated by the ASP.

Closes #678.

[1]: rails/rails#51494
freesteph added a commit to betagouv/aplypro that referenced this issue Apr 8, 2024
@freesteph
asp: truncate the persphysique names attributes
31a8a5c 
Piggyback the `limit` option for ActiveModel's attribute method (which
might be a thing one day[1]) to chop the value of the string directly
on the XML-generating class.

This keeps the mapping classes in charge of translating our data into
the ASP stuff, semantically, but keeps the actual XML-maker in control
of the formatted length, which makes more sense to me: all the XML
tags have an associated length in the ASP documentation, so keeping it
as close as possible to the XML generation code is nicer.

It's also coherent with the custom :asp_date type which we use in a
couple places (ie: prestadoss.rb) and takes care of formatting a date
object in the format dictated by the ASP.

Closes #678.

[1]: rails/rails#51494
freesteph added a commit to betagouv/aplypro that referenced this issue Apr 8, 2024
@freesteph
asp: truncate the persphysique names attributes
f14a28e 
Piggyback the `limit` option for ActiveModel's attribute method (which
might be a thing one day[1]) to chop the value of the string directly
on the XML-generating class.

This keeps the mapping classes in charge of translating our data into
the ASP stuff, semantically, but keeps the actual XML-maker in control
of the formatted length, which makes more sense to me: all the XML
tags have an associated length in the ASP documentation, so keeping it
as close as possible to the XML generation code is nicer.

It's also coherent with the custom :asp_date type which we use in a
couple places (ie: prestadoss.rb) and takes care of formatting a date
object in the format dictated by the ASP.

Closes #678.

[1]: rails/rails#51494
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
activemodel
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@freesteph @akhilgkrishnan