Blanket IntoActiveValue implementations so custom db types are supported

[wdcocq]

PR Info

Currently it doesn't seem possible to use DeriveIntoActiveModel with a field with Option<Option<CustomDbType>> or Option<CustomDbType> because the IntoActiveValue<Option<CustomDbType>> trait is missing.

error[E0277]: the trait bound `std::option::Option<std::option::Option<Language>>: IntoActiveValue<_>` is not satisfied
   |
10 |     Clone, Default, Debug, Hash, PartialEq, Eq, DeriveIntoActiveModel, Serialize, Deserialize,
   |                                                 ^^^^^^^^^^^^^^^^^^^^^ the trait `IntoActiveValue<_>` is not implemented for `std::option::Option<std::option::Option<Language>>

Because of the orphan rule you can't implement IntoActiveValue<Option<CustomDbType>> for Option<CustomDbType>. You can for just CustomDbType of course.

error[E0117]: only traits defined in the current crate can be implemented for types defined outside of the crate
   |
17 | impl IntoActiveValue<Option<Language>> for Option<Option<Language>> {
   | ^^^^^----------------------------------^^^^^-------------------------
   | |    |                                      |
   | |    |                                      `std::option::Option` is not defined in the current crate
   | |    `std::option::Option` is not defined in the current crate
   | impl doesn't use only types from inside the current crate

Changes

By changing the Option<T> and Option<Option<T>> from being implemented by the impl_into_active_value macro, to blanket implementations this will work for every type that implements IntoActiveValue<T>

impl IntoActiveValue<Language> for Language {
    fn into_active_value(self) -> ActiveValue<Language> {
        ActiveValue::set(self)
    }
}

Will make it possible to use DeriveIntoActiveModel on structs with a Option<Option<CustomDbType>> field

[tyt2y3]

I agree that we should support Option<CustomDbType> but what's the usecase for Option<Option<<CustomDbType>>> ?

[wdcocq]

see the discussion here: #323
It's to be able to set a field to NULL in the database

[tyt2y3]

Ah thank you for the context

[tyt2y3]

Sorry but I need more information on exactly what you are trying to do that does not compile, and how this change fixes it now.

It'd be great if you can add a test case somewhere under tests/features. It will also ensure that it will be well maintained in the future

[wdcocq]

Sorry i linked the wrong issue, i changed it in my previous comment

[tyt2y3]

Ah I see, well it's a complicated topic - type system! Definitely needed some test cases then

[wdcocq]

There doesn't seem to be any current test cases around DeriveIntoActiveModel where i can plug in..

For more context, this currently doesn't work:

#[derive(Debug, Clone, DeriveIntoActiveModel)]
pub struct User {
    pub name: Option<String>,
    pub email: Option<Option<String>>,
    pub language: Option<Option<Language>>,
}

because of the error:

error[E0277]: the trait bound std::option::Option<std::option::Option<Language>>: IntoActiveValue<_> is not satisfied

where Language is:

#[derive(Hash, Eq, Debug, Clone, PartialEq, EnumIter, DeriveActiveEnum, Serialize, Deserialize)]
#[sea_orm(rs_type = "String", db_type = "Enum", enum_name = "language")]
pub enum Language {
    #[sea_orm(string_value = "en_US")]
    EnUs,
    #[sea_orm(string_value = "fr_FR")]
    FrFr,
    #[sea_orm(string_value = "nl_NL")]
    NlNl,
    ...
}

If really necessary i can still add some tests

[billy1624]

Hey @wdcocq, sorry for the delay! Thank you so much for the contributions!!

Yes, please! Adding more test cases would be appreciated :D

[billy1624]

Hey @tyt2y3, this PR shouldn't change behaviour of the IntoActiveValiue as it's just refactoring a concrete trait implementation to blanket implementation. However, I'll flag this PR as breaking changes. Because any downstream implemented IntoActiveValue for Option<V> and Option<Option<V>> will be forbidden.

[wdcocq]

Because any downstream implemented IntoActiveValue for Option<V> and Option<Option<V>> will be forbidden.

Is this breaking though, as you already can't do this anyway? Unless I'm missing something :)

Yes, please! Adding more test cases would be appreciated :D

I already added a test that should test whether it compiles with custom types (and some primitives). Not sure what else to add for this PR that's not testing the functionality of IntoActiveValue itself, which this PR isn't really targeting.

[tyt2y3]

Thank you