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

error_messages on serializer fields should also be used by validators. #2878

Closed
WhyNotHugo opened this issue Apr 25, 2015 · 13 comments · Fixed by #3435
Closed

error_messages on serializer fields should also be used by validators. #2878

WhyNotHugo opened this issue Apr 25, 2015 · 13 comments · Fixed by #3435

Comments

@WhyNotHugo
Copy link

email = models.EmailField(
    unique=True,
    error_messages={
        'unique': 'my message'
    }
)

This does not show the defined error as expected when the field is not unique. The message is still the default: "This field must be unique.".

@jpadilla
Copy link
Member

I see. We automatically add UniqueValidator for model fields that have unique=True here.

@tomchristie I wonder if we should be forwarding error_messages['unique'] to UniqueValidator, which IMHO seems like we should.

@hobarrera A workaround might be to subclass UniqueValidator and change the message there. Then in your serializer manually add your email field with that validator.

@WhyNotHugo
Copy link
Author

@tomchristie I wonder if we should be forwarding error_messages['unique'] to UniqueValidator, which IMHO seems like we should.

I believe that that makes sense and is what would usually be expected by most developers.

@hobarrera A workaround might be to subclass UniqueValidator and change the message there. Then in your serializer manually add your email field with that validator.

Yes, that's what I did. In case anybody else needs a bit more details on this workaround:

class ClientSerializer(serializers.ModelSerializer):

    email = serializers.EmailField(validators=[
        UniqueValidator(
            queryset=Client.objects.all(),
            message="This email is already in use.",
        )]
    )
    # ...

@ConorMcGee
Copy link

Is the proposed enhancement to take the error messages from what's defined on the model level and forward them on?
Edit: I'd be interested in working on this, if so.

@WhyNotHugo
Copy link
Author

Is the proposed enhancement to take the error messages from what's defined on the model level and forward them on?

Yes, that's what's expected (and it's done for other validators, unless I'm mistaken).

@tomchristie
Copy link
Member

Seems reasonable to me for us to check the validator codes and apply the field error message if one exists, yup. No objection to this from me.

@tomchristie tomchristie changed the title unique=True field can't have a customized message error_messages on serializer fields should also be used by validators. Jun 23, 2015
@erichaus
Copy link

+1 any progress on this one? is this saying we can define error message on the Model level as in https://docs.djangoproject.com/en/1.8/ref/models/fields/#error-messages?

@tomchristie
Copy link
Member

+1 any progress on this one?

No, if there's progress you'd see it here.

is this saying we can define error message on the Model level

Yup

@erichaus
Copy link

cool, if you tell me the general idea for how that should work or where I need to make updates I can take a stab at it...

@stianjensen
Copy link
Contributor

You'd need to change something here, at least:
https://github.com/tomchristie/django-rest-framework/blob/master/rest_framework/utils/field_mapping.py#L195

That's where a UniqueValidator is automatically added to the serializer for fields with unique constraints.
It takes an optional message parameter.

I'm not quite sure how to get the message off of the model field, though.

@kevin-brown
Copy link
Member

I'm not quite sure how to get the message off of the model field, though.

Error messages for a specific field are available as field.error_messages. The unique message uses the key unique.

stianjensen added a commit to stianjensen/django-rest-framework that referenced this issue Sep 23, 2015
In the automatically applied UniqueValidator, use the error message from
error_messages defined in the model instead of the generic default
UniqueValidator message.

This fixes encode#2878.
stianjensen added a commit to stianjensen/django-rest-framework that referenced this issue Sep 23, 2015
In the automatically applied UniqueValidator, use the error message from
error_messages defined in the model instead of the generic default
UniqueValidator message.

This fixes encode#2878.
@elizabethadegbaju
Copy link

@tomchristie I wonder if we should be forwarding error_messages['unique'] to UniqueValidator, which IMHO seems like we should.

I believe that that makes sense and is what would usually be expected by most developers.

@hobarrera A workaround might be to subclass UniqueValidator and change the message there. Then in your serializer manually add your email field with that validator.

Yes, that's what I did. In case anybody else needs a bit more details on this workaround:

class ClientSerializer(serializers.ModelSerializer):

    email = serializers.EmailField(validators=[
        UniqueValidator(
            queryset=Client.objects.all(),
            message="This email is already in use.",
        )]
    )
    # ...

you should have indicated what file you added this to. It isn't really helpful just like this

@WhyNotHugo
Copy link
Author

@iampicca I added this to my serializers, which are in my serializers.py file. I think that if you're writing a project using DRF, it should be pretty obvious is which file your serializers are.

@nimeshtntra
Copy link

from rest_framework.validators import UniqueValidator

email = serializers.EmailField(validators=[
UniqueValidator(
queryset=User.objects.all(),
message="This email is already register by user.",
)]
)
This will work for you...###

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants