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

Project defined attribute types should be valid #13

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

CristianCurteanu
Copy link

It can be handy to add valid type to attributes that are defined within a project scope. In this pull request I have tried to make such functionality possible, and to make possible to add value types for classes that are defined. If the type/class is not defined, then UnsupportedTypeError exception still should be thrown

Also, there was made some refactoring to ModelAttribute::Casts module class methods, to be a bit more decoupled from .cast method

Cristian Curteanu and others added 2 commits December 22, 2017 15:30
@msftclas
Copy link

CLA assistant check
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

❌ CristianCurteanu sign now
You have signed the CLA already but the status is still pending? Let us recheck it.

@dwaller
Copy link
Contributor

dwaller commented Dec 29, 2017

Hi, thanks for the contribution. Can you explain a little more what you're trying to achieve with your change?

I'm a little concerned that what you're trying to do might be in opposition to what the gem's initial aims are - to support creating a model that fronts an HTTP web service. Particularly (from the readme):

  • Handles integers, booleans, strings and times - a set of types that are very easy to persist to and parse from JSON.
  • Supports efficient serialization of attributes to JSON.

I think you're trying to allow attributes that have arbitrary Ruby types. But how are these then serialized to JSON? JSON doesn't allow arbitrary types.

If you wanted to have an address attribute with Address type, for example, then I'd suggest setting the attribute to be JSON type, and provide your own accessors on the model that handle converting an Address instance to a JSON friendly hash. If you build the Address model using ModelAttribute then Address#attributes_for_json will easily give you a JSON friendly hash, and Address#set_attributes will allow you to turn that hash back into a populated model.

@CristianCurteanu
Copy link
Author

Hello! Actually instead of Address type there could be set any defined class object, and the purpose of this change is to make it possible to use types that were defined within a scope. If the class is not defined then UnsupportedTypeError will be thrown.

@dwaller
Copy link
Contributor

dwaller commented Jan 31, 2018

Whatever the class of the object you want to use, it will still not have a simple conversion to JSON primitives. This is where your change goes against the intent of this gem.

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

Successfully merging this pull request may close these issues.

3 participants