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

Renamed property name is no longer used in the error report (0.17.0) #306

Open
iensu opened this issue Mar 11, 2024 · 6 comments
Open

Renamed property name is no longer used in the error report (0.17.0) #306

iensu opened this issue Mar 11, 2024 · 6 comments

Comments

@iensu
Copy link

iensu commented Mar 11, 2024

In 0.16.1 the validation error report uses the renamed property name when using #[serde(rename = "...")], but in 0.17.0 the original property name is used.

#[derive(Debug, serde::Deserialize, validator::Validate)]
pub struct User {
    #[validate(length(min = 1, message = "too short"))]
    #[serde(rename = "firstName")]
    pub first_name: Option<String>,
}

In 0.16.1 the error report says firstName: too short, but in 0.17.0 it's first_name: too short. This is problematic since the firstName is what is actually expected to be sent from the client, not first_name.

@iensu iensu changed the title The renamed property name is no longer used in the error report (0.17.0) Renamed property name is no longer used in the error report (0.17.0) Mar 11, 2024
@Keats
Copy link
Owner

Keats commented Mar 11, 2024

This has been removed for now since the support was very limited. It only worked with manual serde renaming, and not with container attributes (eg rename_all). Now that the derive has been rewritten it should be easier to implement it properly.
The change was not documented in the changelog though, I've added it in my PR

@iensu
Copy link
Author

iensu commented Mar 11, 2024

I see! Yes, I noticed the lack of container attribute support. Do you plan to implement the feature in the near future?

@Keats
Copy link
Owner

Keats commented Mar 11, 2024

Yes but in practice I don't know when/if i will have the time for it. I'd definitely take a PR adding that though

@iensu
Copy link
Author

iensu commented Mar 11, 2024

👍 I have no idea how complicated it would be to implement, but I'll gladly take a look at it.

@iensu
Copy link
Author

iensu commented Jul 23, 2024

Hi @Keats! I finally have some time to start looking at this. I noticed the issue is still there in 0.18.1, even though the README still states that at least field level renames should work.

I've had a more thorough look at the code now and think I have an idea how to go about implementing support for both container and field level serde renames.

Just to make sure: you haven't started looking at this yet right?

@Keats
Copy link
Owner

Keats commented Jul 23, 2024

No i haven't. If it's in the README, it's something that should be removed

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

No branches or pull requests

2 participants