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

AttributeValues are required to be strings for Metadata #416

Closed
jhubert opened this issue Sep 11, 2017 · 1 comment
Closed

AttributeValues are required to be strings for Metadata #416

jhubert opened this issue Sep 11, 2017 · 1 comment

Comments

@jhubert
Copy link

jhubert commented Sep 11, 2017

We came across this when updating the to latest version.

The metadata generation is using .to_str to get the attribute value. to_str is only defined on String values, which is throwing a 500 error for our configuration that has boolean attribute values.

This may be the intended behavior, and perhaps we should always be using a string but I wanted to check.

Our configuration has a line like this:

settings.attribute_consuming_service.configure do
  ...
  add_attribute({ name: 'is_active', name_format: NAME_FORMAT, friendly_name: 'Is Active?', attribute_value: true })
end

This line causes a 500 error because attribute_value is boolean, even though true.to_s would work just fine and convert true to "true".

Personally, I like the way it reads being a boolean value and it allows an easy way to support methods that return boolean responses like this:

add_attribute({ name: 'is_active', name_format: NAME_FORMAT, friendly_name: 'Is Active?', attribute_value: default_user_active_setting })

The impact of using .to_s vs .to_str seems very minimal, but I'd love to know if there is a more complicated reason for the choice.

If there isn't, I'm happy to submit a PR.

@pitbulk
Copy link
Collaborator

pitbulk commented Sep 12, 2017

There is no reason, please provide a PR with the change and some test to cover the new behavior and I will be happy to merge ;).

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