Display and update extra data #2
Conversation
this prevents annoying errors on machines with newer versions of bunder
(most machines, since 1.17 is pretty old)
i was seeing an error like so:
```
Bundler could not find compatible versions for gem "bundler":
In Gemfile:
bundler (~> 1.17)
Current Bundler version:
bundler (2.1.4)
This Gemfile requires a different version of Bundler.
Perhaps you need to update Bundler by running `gem install bundler`?
Could not find gem 'bundler (~> 1.17)' in any of the relevant sources:
the local ruby installation
```
this looks like a local alias for the original author
we display extra data fields in the form. we allow updating them. we keep the type of data the same -- if we want to change data type, we should use the console. this will allow rapid updates of extra data, which helps when rollup is being used as a config store
There was a problem hiding this comment.
Thanks for your contribution, I made a few comments about things that we should address before merging this.
Also have you consider just having a data and textarea with nicely formatted json? that might work even better and would not require funky conditions to convert from string to int/booleans
| input.appearance-none.border.rounded-sm.w-full.py-2.px-4.text-gray-600.leading-relaxed.bg-white.mt-1.sm:mt-0.sm:col-span-2( | ||
| name=name | ||
| id=name | ||
| value=(val == true ? 'true' : (val == false ? 'false' : val)) |
There was a problem hiding this comment.
I believe you can simply write this as value=val.to_s
| new_val = params[name] | ||
|
|
||
| # keep type the same (for integers/boolean/strings) | ||
| new_val = if old_val.is_a? Integer |
There was a problem hiding this comment.
I think it would be more bulletproof and ellegant to use a regexp to see if it's a number, eg https://medium.com/launch-school/number-validation-with-regex-ruby-393954e46797
| new_val = if old_val.is_a? Integer | ||
| new_val.to_i | ||
| elsif !!old_val == old_val | ||
| new_val == 'true' ? true : (new_val == 'false' ? false : !!new_val) |
There was a problem hiding this comment.
nested ternary operators are crazy hard to read... using regex and case here might also be more readable
|
would love to see this PR merged 👍. |
|
@igor47 @reneklacan Our team would love to see this PR merged as well We would be happy to push the requested changes to unblock merge @igor47 if you don't mind |
|
feel free to take it over! i'm not using this project any more so i don't have a need for it nor any way to test it |
rollup allows storing arbitrary extra data attributes. from the readme:
however, rollout-ui does not display the extra data (in this case,
:release_dataand:whatever) or allow updating it. in this PR, we enable both. it looks like this:when updating, we try to keep the type of extra data the same -- if it was an integer, it will remain an integer, if it was a boolean, it will remain a boolean, and otherwise it will be a string.