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

Cannot use a custom registration serializer that has a nested serializer #259

Closed
4 of 5 tasks
akhayyat opened this issue Jul 12, 2023 · 5 comments
Closed
4 of 5 tasks
Assignees
Labels
state:needs-answer Needs additional information from the issue creator type:bug

Comments

@akhayyat
Copy link

akhayyat commented Jul 12, 2023

Checklist

  • I read Contribution Guidelines
  • I searched existing issues before opening this one
  • This is not a major security issue
  • I reproduced the bug with the newest version

Optional checklist

  • I attached a PR with a test case reproducing the problem

Describe the bug

I am using a custom registration serializer (REGISTER_SERIALIZER_CLASS) that has a nested serializer. The nested serializer represents a related object linked through a required (not nullable) foreign key in the user object - each user must be linked to such object.

The custom registration serializer has a custom create() method that takes care of this: it created the related object first, then passes the related object as the value of the required foreign key field on the user model:

def create(self, validated_data):
    related_data = validated_data.pop("related")
    related_object = Related.objects.create(**related_data)
    validated_data["related"] = related_object
    user = super().create(validated_data)
    return user

However, registration fails before it gets to the create() method: it fails during serializer validation, which appears to try to instantiate a user object on its own:

rest_registration/utils/validation.py:

def validate_user_password(user_data: Dict[str, Any]) -> None:
    password = user_data['password']
    user = build_initial_user(user_data)
    ...

which then calls build_initial_user():

def build_initial_user(data: Dict[str, Any]) -> 'AbstractBaseUser':
    ...
    user_class(**user_data)
    ...

Expected behavior

Using a registration serializer with a nested serializer should work without errors.

If creating a user object before invoking the serializer is absolutely required, then leverage the registration serializer to create it correctly.

Actual behavior

Registration fails since the serialized attributes that should be consumed by the nested serializer are passed as is to the user model constructor, resulting in an error that looks like this:

rest_registration/api/serializers.py", line 130, in validate
    run_validators(validators, attrs)
rest_registration/utils/validation.py", line 76, in run_validators
    validator(value)
rest_registration/utils/validation.py", line 31, in wrapper
    func(value)
rest_registration/utils/validation.py", line 52, in validate_user_password
    user = build_initial_user(user_data)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
rest_registration/utils/users.py", line 191, in build_initial_user
    return user_class(**user_data)
           ^^^^^^^^^^^^^^^^^^^^^^^
django/db/models/base.py", line 543, in __init__
    _setattr(self, field.name, rel_obj)
django/db/models/fields/related_descriptors.py", line 369, in __set__
    super().__set__(instance, value)
django/db/models/fields/related_descriptors.py", line 266, in __set__
    raise ValueError(
ValueError: Cannot assign "OrderedDict([('field1', 'value1'), ('field2', 'value2')])": "User.related" must be a "Related" instance

Steps to reproduce

Steps to reproduce the behavior:

  1. Define a custom registration serializer that has a nested serializer
  2. Configure the custom registration serializer using the setting REGISTER_SERIALIZER_CLASS
  3. Use the registration endpoint (POST /register)

Diagnostic info

My settings.py:

# (Please paste here contents of your Django settings.py file
# (after removing all sensitive information like secrets and domains),
# or at least provide specific settings mentioned below):

REST_REGISTRATION = {
    "REGISTER_VERIFICATION_URL": "https://frontend-host/verify-user/",
    "RESET_PASSWORD_VERIFICATION_URL": "https://frontend-host/reset-password/",
    "REGISTER_EMAIL_VERIFICATION_URL": "https://frontend-host/verify-email/",
    "VERIFICATION_FROM_EMAIL": "[email protected]",
    "REGISTER_SERIALIZER_CLASS": "users_api.serializers.RegisterSerializer",
}
@akhayyat
Copy link
Author

A minimal example Django project reproducing the issue:

https://github.com/akhayyat/django-rest-registration-nested-register-serializer

@akhayyat
Copy link
Author

@apragacz
Copy link
Owner

@akhayyat thank you for your detailed report. I will look at the issue when I'm back from vacation.

@apragacz
Copy link
Owner

Hi @akhayyat

sorry for the delayed response.

I have evaluated multiple options, my current prefered one is this one, which is a hybrid approach:
#269
(details in the description)

It has some downsides, but it looks the best for me now.

The one you proposed:
#260
has the advantage of building the user object in the right shape, but relies too much on the database transaction rollback feature (creates unnecessary/risky interactions with the DB).

Let me know what you think about this.

@apragacz apragacz added the state:needs-answer Needs additional information from the issue creator label Sep 27, 2023
@apragacz
Copy link
Owner

@akhayyat just letting you know: I'm gonna merge #269 and close this issue within few days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
state:needs-answer Needs additional information from the issue creator type:bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants