Fixing groups API validation #864
Conversation
|
Attached issue: https://pulp.plan.io/issues/7329 |
pulpcore/app/viewsets/user.py
Outdated
| try: | ||
| permission = Permission.objects.get(**data) | ||
| except (Permission.MultipleObjectsReturned, Permission.DoesNotExist, FieldError) as exc: | ||
| raise ValidationError(_(str(exc))) |
There was a problem hiding this comment.
I do not believe, gettext works in this situation. How is it supposed to extract the translatable string here?
There was a problem hiding this comment.
I do not know how gettext works, str(exc) returns a string I imagined it would work
There was a problem hiding this comment.
Well, I thought it must be able to extract the strings without calling the code. So it probably looks for calls to ’_("...")’.
There was a problem hiding this comment.
The exception text should already be translated if a translation is available. Please remove the _() around str(exc).
fao89
force-pushed
the
7329
branch
3 times, most recently
from
14af945
to
2dc22f4
Compare
pulpcore/app/viewsets/user.py
Outdated
| obj_pk = request.data["obj"].strip("/").split("/")[-1] | ||
| uuid.UUID(obj_pk) | ||
| except (AttributeError, ValueError): | ||
| raise ValidationError(_(f"Invalid value for 'obj': {request.data['obj']}")) |
There was a problem hiding this comment.
Stackoverflow says nope:
https://stackoverflow.com/questions/49797658/how-to-use-gettext-with-python-3-6-f-strings
| raise ValidationError(_(f"Invalid value for 'obj': {request.data['obj']}")) | |
| raise ValidationError(_("Invalid value for 'obj': " + str(request.data['obj']))) |
Or the good old "format" of course (Usually this helps people translating if they see the whole string template).
| raise ValidationError(_(f"Invalid value for 'obj': {request.data['obj']}")) | |
| raise ValidationError(_("Invalid value for 'obj': {obj}").format(request.data['obj'])) |
pulpcore/app/viewsets/user.py
Outdated
| try: | ||
| permission = Permission.objects.get(codename=codename) | ||
| except (Permission.MultipleObjectsReturned, Permission.DoesNotExist, FieldError) as exc: | ||
| raise ValidationError(_(str(exc))) |
There was a problem hiding this comment.
| raise ValidationError(_(str(exc))) | |
| raise ValidationError(str(exc)) |
Won't work at all, but the exception should only use translatable strings anyway.
pulpcore/app/viewsets/user.py
Outdated
| try: | ||
| user = User.objects.get(**request.data) | ||
| except (User.DoesNotExist, FieldError) as exc: | ||
| raise ValidationError(_(str(exc))) |
There was a problem hiding this comment.
same here
| raise ValidationError(_(str(exc))) | |
| raise ValidationError(str(exc)) |
| @@ -274,7 +309,7 @@ def destroy(self, request, group_pk, pk): | |||
| Remove a user from a group. | |||
| """ | |||
| group = Group.objects.get(pk=group_pk) | |||
| user = get_object_or_404(get_user_model(), pk=pk) | |||
| user = get_object_or_404(User, pk=pk) | |||
There was a problem hiding this comment.
Is this going to be a problem for galaxy_ng?
There was a problem hiding this comment.
nope, at the beginning of this file (line 23) I declared:
User = get_user_model()
https://pulp.plan.io/issues/7329
closes #7329
Please be sure you have read our documentation on creating PRs:
https://docs.pulpproject.org/contributing/pull-request-walkthrough.html