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

Serialize time types as serde_json::Value #1042

Merged
merged 6 commits into from
Oct 17, 2022
Merged

Serialize time types as serde_json::Value #1042

merged 6 commits into from
Oct 17, 2022

Conversation

billy1624
Copy link
Member

@billy1624 billy1624 commented Sep 15, 2022

PR Info

Adds

jimmycuadra and others added 4 commits September 14, 2022 22:58
I tried to implement a [custom active
model](https://www.sea-ql.org/SeaORM/docs/advanced-query/custom-active-model/),
and one of the columns was `Option<TimeDateTimeWithTimeZone>`. I got a
compiler error:

```
error[E0277]: the trait bound `std::option::Option<sea_orm::prelude::TimeDateTimeWithTimeZone>: IntoActiveValue<_>` is not satisfied
```

Looking into the source code, it seemed a simple oversight that this
trait was implemented for the `chrono` types but not the `time` types,
and it was easy enough to fix since there's already a macro to implement
it for new types.

I also noticed that the `time` types are not accounted for in
`src/query/json.rs` while the `chrono` types are, which I assume is also
an oversight. However, I don't have a need for that at this point and
the fix for that seemed less trivial, so I'm just bringing it to your
attention.

Thanks for SeaORM!
@billy1624 billy1624 self-assigned this Sep 15, 2022
@billy1624 billy1624 marked this pull request as ready for review October 17, 2022 08:00
@billy1624 billy1624 added this to the 0.10.x milestone Oct 17, 2022
@billy1624 billy1624 requested a review from tyt2y3 October 17, 2022 08:00
"id": 1,
"date": "2022-03-13",
"time": "16:24:00",
"date_time": "2022-03-13T16:24:00",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect we'd have the sub-second component?

pub fn it_impl_try_from_u64<T: TryFromU64>() {}

#[allow(unused_macros)]
macro_rules! it_impl_traits {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is smart

@tyt2y3 tyt2y3 merged commit a0fd72e into master Oct 17, 2022
@tyt2y3 tyt2y3 deleted the into-json-time branch October 17, 2022 16:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants