Skip to content

Add support for source with attributes in extra_kwargs of ModelSerializer - #9077

Open
BergLucas wants to merge 22 commits into
encode:mainfrom
BergLucas:improvement/source-attributes-in-extra_kwargs
Open

Add support for source with attributes in extra_kwargs of ModelSerializer#9077
BergLucas wants to merge 22 commits into
encode:mainfrom
BergLucas:improvement/source-attributes-in-extra_kwargs

Conversation

@BergLucas

Copy link
Copy Markdown

refs #4688

Description

Hello dear maintainers, it's my first contribution to django rest framework so I hope I didn't do anything wrong.

In the pull request referenced above, there is someone that mentioned that we could not use source with attributes in extra_kwargs of a ModelSerializer.

For example, the following code would create an error:

class MyUser(models.Model):

    user = models.OneToOneField(User, on_delete=models.DO_NOTHING)

class MyUserSerializer(serializers.ModelSerializer):

    class Meta:
        model = MyUser
        fields = (
            "username",
        )
        extra_kwargs = {
            "username": {"source": "user.username"},
        }

I think it could be a very interesting feature because at the moment, when we have a foreign key to another model, we're obliged to specify the fields explicitly in the serializer. However, this means that if we have special validators on the fields of our model, we're obliged to put them back on the serializer fields.

For example, the username field of Django's default User has a special validator so if we just define a basic CharField, it would not validate the data the same way as the model would validate it:

class MyUser(models.Model):

    user = models.OneToOneField(User, on_delete=models.DO_NOTHING)

class MyUserSerializer(serializers.ModelSerializer):

    username = serializers.CharField(source="user.username")

    class Meta:
        model = MyUser
        fields = (
            "username",
        )

This could create a difference between the way the model validates data and the way the serializer validates data if we forgot a validator or if we change the model without changing the serializer.

In this pull request, I added this feature so that we could just specify the model field we want in the source of extra_kwargs and it will generate the right field on the serializer.

The code may seem a little odd, but I've tried to keep the changes in one place only. I've also tried to maintain a good error message so that, if at any point the path to the field is wrong, it returns the full path in the error message and not just part of it.

Thanks for reading and please let me know if there are any changes that could be made.

@auvipy
auvipy self-requested a review August 19, 2023 15:05

@auvipy auvipy left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

could you please add proper test cases to validate the changes proposed?

@auvipy
auvipy requested review from a team and auvipy August 23, 2023 14:25
Comment thread tests/test_model_serializer.py Outdated
Comment thread tests/test_model_serializer.py Outdated
@auvipy

auvipy commented Sep 14, 2023

Copy link
Copy Markdown
Collaborator

___________________________________ summary ____________________________________
ERROR: py36-django30: commands failed
py36-django31: commands succeeded
py36-django32: commands succeeded

@auvipy auvipy left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

that was a nice example

@auvipy auvipy added this to the 3.15 milestone Sep 14, 2023

@auvipy auvipy left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

And this should also require documentation update as far as I can understand!

@BergLucas
BergLucas force-pushed the improvement/source-attributes-in-extra_kwargs branch from b10dfc2 to c1ccc37 Compare September 14, 2023 15:36
@BergLucas

Copy link
Copy Markdown
Author

Hi @auvipy,

And this should also require documentation update as far as I can understand!

Do you mean to add a section here (or elsewhere) to document the feature?

@auvipy

auvipy commented Oct 2, 2023

Copy link
Copy Markdown
Collaborator

Hi @auvipy,

And this should also require documentation update as far as I can understand!

Do you mean to add a section here (or elsewhere) to document the feature?

yup there or somewhere else where it would be relevant

@auvipy
auvipy requested a review from a team October 2, 2023 05:52

@auvipy auvipy left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

does this change conform with the principle of adding compatibility with django? I'm not fully convinced about the merit of the new change

Comment thread tests/test_model_serializer.py Outdated
@auvipy auvipy removed this from the 3.15 milestone Apr 27, 2025
@auvipy
auvipy requested a review from a team April 28, 2025 03:26
@stale

stale Bot commented Jun 27, 2025

Copy link
Copy Markdown

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale Bot added the stale label Jun 27, 2025
@ulgens

ulgens commented Jun 27, 2025

Copy link
Copy Markdown
Contributor

I think this is still relevant and shouldn't be marked as stale.

Comment on lines +1114 to +1115
if attr not in attr_info.relations:
break

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This seems to cause trailing attributes to be ignored if they are "after" other relation attributes, but are not a relation themselves. Is this intended?

If so, I think this scenario should also have a test case.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

It is not really intended. Is it possible to have attributes other than relations with sufficient information in this function to be able to follow them? As far as I know, this isn't the case, so I just did it this way. It's also possible that I misunderstood your comment

Comment thread rest_framework/serializers.py Outdated
@ulgens

ulgens commented Jun 27, 2025

Copy link
Copy Markdown
Contributor

@auvipy Do you know why the bot doesn't remove stale label when there is a new comment?

@ulgens

ulgens commented Jul 18, 2025

Copy link
Copy Markdown
Contributor

@BergLucas Hey 👋🏻 Is it possible for you to rebase this branch from the latest main, so it won't be closed as stale and we can push for another tour of review & testing? Thanks in advance.

@BergLucas

Copy link
Copy Markdown
Author

Hi @ulgens ,

Thanks for your reviews! I've rebased the branch and responded to the comments.

(Sorry for the delay, I've been quite busy these last few weeks.)

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds support for using dotted source paths (e.g. user.username) in ModelSerializer.Meta.extra_kwargs so that DRF can infer the correct serializer field type/validators from related model fields, without requiring explicit field declarations.

Changes:

  • Update ModelSerializer.get_fields() to resolve dotted source paths across model relations when building fields.
  • Add a regression test asserting the generated field representation includes validators from related model fields (e.g. User.username).
  • Document the new extra_kwargs capability with an example mapping serializer fields to related model attributes.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
rest_framework/serializers.py Adds dotted-source traversal to choose the correct related model metadata when building auto-generated fields.
tests/test_model_serializer.py Adds coverage for dotted source paths in extra_kwargs and expected field mapping/validators.
docs/api-guide/serializers.md Documents using extra_kwargs with source to map serializer fields to related model fields.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread rest_framework/serializers.py Outdated
Comment thread rest_framework/serializers.py Outdated
Comment thread docs/api-guide/serializers.md Outdated
Comment thread tests/test_model_serializer.py
auvipy and others added 5 commits September 7, 2026 12:43
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@auvipy
auvipy requested a balanced review from Copilot and removed request for Copilot September 7, 2026 06:46

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Critical test defects and unsupported to-many traversal must be addressed before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

rest_framework/serializers.py:1174

  • A dotted target field can bring in a UniqueValidator whose queryset belongs to the related model, but that validator receives the root serializer instance. On an update it therefore excludes User(pk=profile.pk) rather than profile.user; an unchanged value may be rejected when those PKs differ, and a real conflict may be skipped when they coincide with another user's PK. Contextual validators need to resolve the related instance (or be omitted for dotted fields).
            field_class, field_kwargs = self.build_field(
                source, source_info, source_model, depth
            )

rest_framework/serializers.py:1165

  • Treating a related model's URL field as traversable produces the wrong field: build_url_field() returns HyperlinkedIdentityField, whose constructor forcibly resets source='*'. Thus source='user.url' serializes the root object with the user's view name instead of traversing to user. Remove this special case unless nested identity URLs are implemented explicitly.
                        or attr == self.url_field_name
  • Files reviewed: 3/3 changed files
  • Comments generated: 5
  • Review effort level: Balanced

Comment thread tests/test_model_serializer.py
Comment thread tests/test_model_serializer.py Outdated
Comment thread rest_framework/serializers.py Outdated
Comment thread docs/api-guide/serializers.md Outdated
Comment thread rest_framework/serializers.py
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

@auvipy auvipy left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

push some changes. please cross check again

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

A critical indentation error prevents imports, and dotted fields mishandle uniqueness validation.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

tests/test_model_serializer.py:881

  • The PR explicitly preserves the full dotted path in configuration errors, but this assertion only checks the exception type, so an error mentioning only name would still pass. Assert the dotted source in the message to protect that behavior.
        with self.assertRaises(ImproperlyConfigured):
            InvalidUserProfileSerializer().fields

tests/test_model_serializer.py:881

  • Add two blank lines before the next top-level class. The current adjacency violates E305, which is enabled by the repository's flake8 configuration (pyproject.toml:101-104); surrounding top-level classes in this file also consistently use two blank lines.
            InvalidUserProfileSerializer().fields
  • Files reviewed: 3/3 changed files
  • Comments generated: 2
  • Review effort level: Balanced

Comment thread rest_framework/serializers.py Outdated
Comment on lines 1172 to 1174
field_class, field_kwargs = self.build_field(
source, info, model, depth
source, source_info, source_model, depth
)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@BergLucas please cross check this

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

5 participants