Skip to content

Commit

Permalink
feat(api): use ValidationError with a field speficied when possible
Browse files Browse the repository at this point in the history
- include field when it is known
- avoid building 400 responses manually
  • Loading branch information
nijel committed Jan 16, 2025
1 parent 48bf8dc commit 26c02cd
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 33 deletions.
1 change: 1 addition & 0 deletions docs/changes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ Not yet released.
* :guilabel:`Synchronize` on shared repository now operates on all its components.
* :ref:`check-punctuation-spacing` ignores markup such as Markdown or reStructuredText.
* :ref:`autofix-punctuation-spacing` does not alter reStructuredText markup.
* Improved validation errors in :doc:`/api`.

**Bug fixes**

Expand Down
60 changes: 27 additions & 33 deletions weblate/api/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
HTTP_200_OK,
HTTP_201_CREATED,
HTTP_204_NO_CONTENT,
HTTP_400_BAD_REQUEST,
HTTP_423_LOCKED,
HTTP_500_INTERNAL_SERVER_ERROR,
)
Expand Down Expand Up @@ -429,12 +428,12 @@ def groups(self, request: Request, **kwargs):

if "group_id" not in request.data:
msg = "Missing group_id parameter"
raise ValidationError(msg)
raise ValidationError({"group_id": msg})

try:
group = Group.objects.get(pk=int(request.data["group_id"]))
except (Group.DoesNotExist, ValueError) as error:
raise ValidationError(str(error)) from error
raise ValidationError({"group_id": str(error)}) from error

Check warning

Code scanning / CodeQL

Information exposure through an exception Medium

Stack trace information
flows to this location and may be exposed to an external user.

if request.method == "POST":
obj.add_team(request, group)
Expand Down Expand Up @@ -589,12 +588,12 @@ def roles(self, request: Request, **kwargs):

if "role_id" not in request.data:
msg = "Missing role_id parameter"
raise ValidationError(msg)
raise ValidationError({"role_id": msg})

try:
role = Role.objects.get(pk=int(request.data["role_id"]))
except (Role.DoesNotExist, ValueError) as error:
raise ValidationError(str(error)) from error
raise ValidationError({"role_id": str(error)}) from error

Check warning

Code scanning / CodeQL

Information exposure through an exception Medium

Stack trace information
flows to this location and may be exposed to an external user.

obj.roles.add(role)
serializer = self.serializer_class(obj, context={"request": request})
Expand All @@ -612,12 +611,12 @@ def languages(self, request: Request, **kwargs):

if "language_code" not in request.data:
msg = "Missing language_code parameter"
raise ValidationError(msg)
raise ValidationError({"language_code": msg})

try:
language = Language.objects.get(code=request.data["language_code"])
except (Language.DoesNotExist, ValueError) as error:
raise ValidationError(str(error)) from error
raise ValidationError({"language_code": str(error)}) from error

Check warning

Code scanning / CodeQL

Information exposure through an exception Medium

Stack trace information
flows to this location and may be exposed to an external user.

obj.languages.add(language)
serializer = self.serializer_class(obj, context={"request": request})
Expand Down Expand Up @@ -650,14 +649,14 @@ def projects(self, request: Request, **kwargs):

if "project_id" not in request.data:
msg = "Missing project_id parameter"
raise ValidationError(msg)
raise ValidationError({"project_id": msg})

try:
project = Project.objects.get(
pk=int(request.data["project_id"]),
)
except (Project.DoesNotExist, ValueError) as error:
raise ValidationError(str(error)) from error
raise ValidationError({"project_id": str(error)}) from error

Check warning

Code scanning / CodeQL

Information exposure through an exception Medium

Stack trace information
flows to this location and may be exposed to an external user.
obj.projects.add(project)
serializer = self.serializer_class(obj, context={"request": request})

Expand Down Expand Up @@ -686,14 +685,14 @@ def componentlists(self, request: Request, **kwargs):

if "component_list_id" not in request.data:
msg = "Missing component_list_id parameter"
raise ValidationError(msg)
raise ValidationError({"component_list_id": msg})

try:
component_list = ComponentList.objects.get(
pk=int(request.data["component_list_id"]),
)
except (ComponentList.DoesNotExist, ValueError) as error:
raise ValidationError(str(error)) from error
raise ValidationError({"component_list_id": str(error)}) from error

Check warning

Code scanning / CodeQL

Information exposure through an exception Medium

Stack trace information
flows to this location and may be exposed to an external user.
obj.componentlists.add(component_list)
serializer = self.serializer_class(obj, context={"request": request})

Expand Down Expand Up @@ -732,14 +731,14 @@ def components(self, request: Request, **kwargs):
self.perm_check(request)
if "component_id" not in request.data:
msg = "Missing component_id parameter"
raise ValidationError(msg)
raise ValidationError({"component_id": msg})

try:
component = Component.objects.filter_access(request.user).get(

Check failure on line 737 in weblate/api/views.py

View workflow job for this annotation

GitHub Actions / mypy

Argument 1 to "filter_access" of "ComponentQuerySet" has incompatible type "User | AnonymousUser"; expected "User"
pk=int(request.data["component_id"])
)
except (Component.DoesNotExist, ValueError) as error:
raise ValidationError(str(error)) from error
raise ValidationError({"component_id": str(error)}) from error

Check warning

Code scanning / CodeQL

Information exposure through an exception Medium

Stack trace information
flows to this location and may be exposed to an external user.
obj.components.add(component)
serializer = self.serializer_class(obj, context={"request": request})

Expand Down Expand Up @@ -768,13 +767,13 @@ def grant_admin(self, request: Request, id): # noqa: A002
user_id = request.data.get("user_id")
if not user_id:
msg = "User ID is required"
raise ValidationError(msg)
raise ValidationError({"user_id": msg})

try:
user = User.objects.get(pk=user_id)
except User.DoesNotExist as error:
msg = "User not found"
raise ValidationError(msg) from error
raise ValidationError({"user_id": msg}) from error
group.admins.add(user)
user.add_team(cast("AuthenticatedHttpRequest", request), group)
return Response({"Administration rights granted."}, status=HTTP_200_OK)
Expand Down Expand Up @@ -1147,17 +1146,15 @@ def machinery_settings(self, request: Request, **kwargs):
if request.method in {"POST", "PATCH"}:
try:
service_name = request.data["service"]
except KeyError:
return Response(
{"errors": ["Missing service name"]}, status=HTTP_400_BAD_REQUEST
)
except KeyError as error:
raise ValidationError({"service": "Missing service name"}) from error

service, configuration, errors = validate_service_configuration(
service_name, request.data.get("configuration", "{}")
)

if service is None or errors:
return Response({"errors": errors}, status=HTTP_400_BAD_REQUEST)
raise ValidationError({"configuration": errors})

Check warning

Code scanning / CodeQL

Information exposure through an exception Medium

Stack trace information
flows to this location and may be exposed to an external user.

if request.method == "PATCH":
if configuration:
Expand All @@ -1178,10 +1175,7 @@ def machinery_settings(self, request: Request, **kwargs):

if request.method == "POST":
if service_name in project.machinery_settings:
return Response(
{"errors": ["Service already exists"]},
status=HTTP_400_BAD_REQUEST,
)
raise ValidationError({"service": ["Service already exists"]})

project.machinery_settings[service_name] = configuration
project.save(update_fields=["machinery_settings"])
Expand All @@ -1199,7 +1193,7 @@ def machinery_settings(self, request: Request, **kwargs):
)

if service is None or errors:
return Response({"errors": errors}, status=HTTP_400_BAD_REQUEST)
raise ValidationError({"configuration": errors})

Check warning

Code scanning / CodeQL

Information exposure through an exception Medium

Stack trace information
flows to this location and may be exposed to an external user.

valid_configurations[service_name] = configuration

Expand Down Expand Up @@ -1310,15 +1304,15 @@ def translations(self, request: Request, **kwargs):

if "language_code" not in request.data:
msg = "Missing 'language_code' parameter"
raise ValidationError(msg)
raise ValidationError({"languge_code": msg})

language_code = request.data["language_code"]

try:
language = Language.objects.get(code=language_code)
except Language.DoesNotExist as error:
msg = f"No language code {language_code!r} found!"
raise ValidationError(msg) from error
raise ValidationError({"language_code": msg}) from error

if not obj.can_add_new_language(request.user):
self.permission_denied(request, message=obj.new_lang_error_message)
Expand All @@ -1330,7 +1324,7 @@ def translations(self, request: Request, **kwargs):
message = "\n".join(m.message for m in storage)
else:
message = f"Could not add {language_code!r}!"
raise ValidationError(message)
raise ValidationError({"language_code": message})

serializer = TranslationSerializer(
translation, context={"request": request}, remove_fields=("component",)
Expand Down Expand Up @@ -1429,7 +1423,7 @@ def add_link(self, request: Request, instance: Component):
self.permission_denied(request, "Can not edit component")
if "project_slug" not in request.data:
msg = "Missing 'project_slug' parameter"
raise ValidationError(msg)
raise ValidationError({"project_slug": msg})

project_slug = request.data["project_slug"]

Expand All @@ -1439,7 +1433,7 @@ def add_link(self, request: Request, instance: Component):
)
except Project.DoesNotExist as error:
msg = f"No project slug {project_slug!r} found!"
raise ValidationError(msg) from error
raise ValidationError({"project_slug": msg}) from error

instance.links.add(project)
serializer = self.serializer_class(instance, context={"request": request})
Expand Down Expand Up @@ -1679,7 +1673,7 @@ def units(self, request: Request, **kwargs):
parse_query(query_string)
except Exception as error:
msg = f"Could not parse query string: {error}"
raise ValidationError(msg) from error
raise ValidationError({"q": msg}) from error

Check warning

Code scanning / CodeQL

Information exposure through an exception Medium

Stack trace information
flows to this location and may be exposed to an external user.

queryset = obj.unit_set.search(query_string).order_by("id").prefetch_full()
page = self.paginate_queryset(queryset)
Expand Down Expand Up @@ -1821,7 +1815,7 @@ def filter_queryset(self, queryset):
parse_query(query_string)
except Exception as error:
msg = f"Could not parse query string: {error}"
raise ValidationError(msg) from error
raise ValidationError({"q": msg}) from error

Check warning

Code scanning / CodeQL

Information exposure through an exception Medium

Stack trace information
flows to this location and may be exposed to an external user.
if query_string:
result = result.search(query_string)
return result
Expand Down Expand Up @@ -1855,7 +1849,7 @@ def perform_update(self, serializer) -> None: # noqa: C901
self.permission_denied(request, "The string is read-only.")
if not new_target or new_state is None:
msg = "Please provide both state and target for a partial update"
raise ValidationError(msg)
raise ValidationError({"state": msg, "target": msg})

if new_state not in {
STATE_APPROVED,
Expand Down

0 comments on commit 26c02cd

Please sign in to comment.