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
Fix usage of AliasGenerator
with computed_field
decorator
#8806
Conversation
…dantic into fix-alex-suggestions
Deploying with Cloudflare Pages
|
Please review |
CodSpeed Performance ReportMerging #8806 will not alter performanceComparing Summary
|
# note that we use the serialization_alias with priority over alias, as computed_field | ||
# aliases are used for serialization only (not validation) | ||
if computed_field_info.alias_priority == 1: | ||
computed_field_info.alias = serialization_alias or alias |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would do
computed_field_info.alias = serialization_alias or alias | |
computed_field_info.alias = serialization_alias if serialization_alias is not None else alias |
to handle the case of serialization_alias = ''
and alias != ''
. Probably worth a comment about why, though not a test.
I could be convinced not to do that but I do think there are tests for the alias = ''
case elsewhere in the codebase.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should do this, but in a different PR, to both the field_info logic and the computed_field_info logic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, doesn't seem like an urgent change 👍
Co-authored-by: Alex Hall <alex.mojaki@gmail.com>
Fix #8768
Ensure that
AliasGenerator
is handled properly when using it along with acomputed_field
in a model.Selected Reviewer: @samuelcolvin