Add to_h and to_json to Granite::ORM::Fields #72
Conversation
|
Let me know if you guys see any areas to improve the I also didn't want to cast all the values to |
spec/granite_orm_spec.cr
Outdated
|
|
||
| it "should provide a to_h method" do | ||
| t = Todo.new(name: "test todo", priority: 20) | ||
| result = { "name" => "test todo", "priority": 20 } |
There was a problem hiding this comment.
This is interesting:
{ "name" => "test todo", "priority": 20 }.class # => Hash(String, Int32 | String)
{ "priority": 20 }.class # => NamedTuple(priority: Int32)
{ "priority": 20, "name" => "test todo" }.class # space not allowed between named argument name and ':'I don't want to be pedantic, but the first case looks like a bug in Crystal or at least it is not documented. Personally, I would prefer to use here official way for hashes to avoid problems in future:
{ "name" => "test todo", "priority" => 20 }There was a problem hiding this comment.
@veelenga My ruby background seems to have seeped in here pushing a fix shortly.
src/granite_orm/fields.cr
Outdated
|
|
||
| if updated_at | ||
| fields["updated_at"] = updated_at.not_nil!.to_s("%F %X") | ||
| end |
There was a problem hiding this comment.
All fields with nil values will be present in a hash, but created_at and updated_at will not. In this intended?
There was a problem hiding this comment.
@veelenga Good point I'll allow the nil values
spec/granite_orm_spec.cr
Outdated
|
|
||
| it "should provide a to_json method" do | ||
| t = Todo.new(name: "test todo", priority: 20) | ||
| result = "{\"name\":\"test todo\",\"priority\":20}" |
There was a problem hiding this comment.
How about %({"name":"test todo","priority":20})?
There was a problem hiding this comment.
I like this better than escaping all the quotes
|
I've made the changes on my branch in migeorge/granite-orm but they don't seem to be propagating here. Anyone experienced this before? |
src/granite_orm/fields.cr
Outdated
| {% end %} | ||
| {% if SETTINGS[:timestamps] %} | ||
| fields["created_at"] = created_at ? created_at.not_nil!.to_s("%F %X") : "" | ||
| fields["updated_at"] = updated_at ? updated_at.not_nil!.to_s("%F %X") : "" |
There was a problem hiding this comment.
👍
but why "" instead of nil ?
fields["created_at"] = created_at.try &.to_s("%F %X")
fields["updated_at"] = updated_at.try &.to_s("%F %X")There was a problem hiding this comment.
@veelenga That makes more sense, fixed.
|
This should be ready to go unless anyone has more feedback |
This PR should satisfy the agreed upon resolution of #50
It adds a
to_handto_jsonmethod to Granite ORM Models.