apragacz / django-rest-registration

User-related REST API based on the awesome Django REST Framework
https://django-rest-registration.readthedocs.io/
MIT License
544 stars 84 forks source link

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

Closed akhayyat closed 1 year ago

akhayyat commented 1 year ago

Checklist

Optional checklist

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": "no-reply@example.com",
    "REGISTER_SERIALIZER_CLASS": "users_api.serializers.RegisterSerializer",
}
akhayyat commented 1 year ago

A minimal example Django project reproducing the issue:

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

akhayyat commented 1 year ago

Here is another potential solution: Use a custom model __init__ method, although this is recommended against in Django docs.

Here is working code: https://github.com/akhayyat/django-rest-registration-nested-register-serializer/tree/fix-using-custom-model-init

Diff: https://github.com/akhayyat/django-rest-registration-nested-register-serializer/compare/fix-using-custom-model-init

apragacz commented 1 year ago

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

apragacz commented 1 year ago

Hi @akhayyat

sorry for the delayed response.

I have evaluated multiple options, my current prefered one is this one, which is a hybrid approach: https://github.com/apragacz/django-rest-registration/pull/269 (details in the description)

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

The one you proposed: https://github.com/apragacz/django-rest-registration/pull/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 commented 1 year ago

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