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

Use verbose_name instead of object_name in error messages #4299

Merged
merged 3 commits into from
Jul 26, 2016
Merged

Use verbose_name instead of object_name in error messages #4299

merged 3 commits into from
Jul 26, 2016

Conversation

sassanh
Copy link
Contributor

@sassanh sassanh commented Jul 24, 2016

Note: Before submitting this pull request, please review our contributing guidelines.

Description

It should use verbose_name, not object_name, took me 2 hours to find out why parts of error were not translated as I didn't expect it to happen in drf.

@sassanh
Copy link
Contributor Author

sassanh commented Jul 24, 2016

Related to #4300.

@tomchristie
Copy link
Member

Yup this looks good, thanks!
Note that there's a test case that also needs to be updated as part of this pull request.

tests/test_validators.py:80: in test_is_not_unique
assert serializer.errors == {'username': ['UniquenessModel with this username already exists.']}
E   AssertionError: assert {'username': ...ady exists.']} == {'username': [...ady exists.']}
E     Differing items:
E     {'username': ['uniqueness model with this username already exists.']} != {'username': ['UniquenessModel with this username already exists.']}

@sassanh
Copy link
Contributor Author

sassanh commented Jul 26, 2016

I'll upgrade the test, could you please review my 2 commits and tell me which one is the right one?

@codecov-io
Copy link

codecov-io commented Jul 26, 2016

Current coverage is 91.19% (diff: 100%)

Merging #4299 into master will increase coverage by <.01%

@@             master      #4299   diff @@
==========================================
  Files            52         52          
  Lines          5776       5781     +5   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           5267       5272     +5   
  Misses          509        509          
  Partials          0          0          

Powered by Codecov. Last update 0f61c9e...09d5f85

@tomchristie
Copy link
Member

tomchristie commented Jul 26, 2016

I'll upgrade the test, could you please review my 2 commits and tell me which one is the right one?

Sorry, could you be more explicit?
The change in the PR looks correct, as it currently stands.

@@ -59,7 +59,7 @@ def get_detail_view_name(model):
"""
return '%(model_name)s-detail' % {
'app_label': model._meta.app_label,
'model_name': model._meta.verbose_name.lower()
'model_name': model._meta.object_name.lower()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tomchristie is it correct the way it is or should I convert this object_name to verbose_name too?

Copy link
Member

Choose a reason for hiding this comment

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

This should remain as object_name, as you currently have it.

@tomchristie tomchristie added this to the 3.4.1 Release milestone Jul 26, 2016
@tomchristie
Copy link
Member

Closes #4300.

@tomchristie tomchristie merged commit f0f61aa into encode:master Jul 26, 2016
@tomchristie tomchristie changed the title use verbose_name instead of object_name in field_mapping Use verbose_name instead of object_name in error messages Jul 26, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants