Fix deserialization of burst_colours on MessageReaction

[jamesbt365]

Fixes #2892.

[GnomedDev]

I'm not merging this as-is, we cannot duplicate logic like this. There must be a better way to do this.

[jamesbt365]

I couldn't think of a nicer way at the time, It would probably involve the creation of another module (so that it doesn't reexport the function in serenity::all, or changing it carefully so it reexports everything but this function) alongside somehow handling the difference between the optional and non optional version.

Do you have any suggestions in mind?

[GnomedDev]

Can this not go in /src/model/utils.rs?

[jamesbt365]

I didn't think about moving it there to be fair, _{I don't think much}, that only leaves one issue with one being an option and the other not.

[jamesbt365]

Would this pass?

fn discord_colours_opt<'de, D>(deserializer: D) -> Result<Option<Vec<Colour>>, D::Error>
where
    D: Deserializer<'de>,
{
    let vec_str: Option<Vec<CowStr<'_>>> = Deserialize::deserialize(deserializer)?;

    let Some(vec_str) = vec_str else { return Ok(None) };

    if vec_str.is_empty() {
        return Ok(None);
    }

    deserialize_colours::<D>(vec_str).map(Some)
}

fn discord_colours<'de, D>(deserializer: D) -> Result<Vec<Colour>, D::Error>
where
    D: Deserializer<'de>,
{
    let vec_str: Vec<CowStr<'_>> = Deserialize::deserialize(deserializer)?;

    deserialize_colours::<D>(vec_str)
}

fn deserialize_colours<'de, D>(vec_str: Vec<CowStr<'_>>) -> Result<Vec<Colour>, D::Error>
where
    D: Deserializer<'de>,
{
    vec_str
        .into_iter()
        .map(|s| {
            let s = s.0.strip_prefix('#').ok_or_else(|| DeError::custom("Invalid colour data"))?;

            if s.len() != 6 {
                return Err(DeError::custom("Invalid colour data length"));
            }

            u32::from_str_radix(s, 16)
                .map(Colour::new)
                .map_err(|_| DeError::custom("Invalid colour data"))
        })
        .collect()
}

[GnomedDev]

Sure

[AlexCzar]

Do I understand correctly, that 0.12.* won't work with reactions until this is merged?
I'm trying to update my bot to latest version and see following error:

missing field `burst_colours` at line 1 column 816

at

    ctx.http
        .get_message(reaction.channel_id.into(), message_id.into())
        .await

[jamesbt365]

This is a 2 part fix, the 1st part was #2887.

[AlexCzar]

Can't wait :) I tried applying it myself and specifying git dependency, but some magic happens, and my shuttle app refuses to recognize serenity::Client 🤷🏽

[FichteFoll]

Hi, I'd just like to mention that a release with this fix would be much appreciated, also considering the number of duplicate issues of #2892 since people keep running into it. For now, I'm staying on 0.10 which still works but there have been some breaking API changes since then, so I presume not everyone would be happy to revert to that.

[Friendly-Banana]

@FichteFoll you can use a newer version, this issue was introduced in 0.12.2. serenity = { version = "=0.12.1" } in Cargo,.toml also works if you use Poise