search

solnic / virtus

[DISCONTINUED ] Attributes on Steroids for Plain Old Ruby Objects
MIT License
3.77k stars 228 forks source link

Add The `nullify_blank` option to nullify empty Strings when coercion fails #314

Closed lucasmazza closed 9 years ago

lucasmazza commented 9 years ago

Closes #310.

I did a simple implementation based on what the code that supports the strict option, so let me know if I shouldn't take down this road. I still want to give a review of my own on this later and update the README documenting this feature before getting this merged :smile:.

solnic commented 9 years ago

wow that was fast :) I left one comment, tell me what you think, cheers!

lucasmazza commented 9 years ago

Just pushed the tests of having nullify_blank and strict working together, but looks like required might not work as expected - since it is a piece of the strict logic. Maybe extracting it to a module that has precedence over both strict and nullify_blank might improve this but I haven't looked deeper enough through the code.

solnic commented 9 years ago

required flag means whether nil is an acceptable value or not for a specific attribute, I suppose setting nullify_blank implies that required should be false.

lucasmazza commented 9 years ago

@solnic OK, so I think the current tests cover this case when all options are true and we don't raise a coercion error when the nullable input is provided. I guess we are done with the implementation here, so let me know if we need to change/add something before merging this.

solnic commented 9 years ago

@lucasmazza awesome, thanks! I'm gonna merge it in and see how it works in my apps and then we could push it out in the next release. This will simplify rom-rails so that's really cool.

lucasmazza commented 9 years ago

@solnic nice! I tested in my current project and it worked as expected - it isn't a large app but I've managed to remove our implementation of custom attribute classes that supported this behaviour with the configuration option directly.