Use basename as a part of the operationId generation
Closed this issue · 3 comments
Describe the bug
The application throws warnings of collisions for ModelViewsets when the "patch" is overridden by "post" in separate paths.
To Reproduce
## Model & View Setup
class TestModel(models.Model):
name = models.CharField(max_length=100)
class TestModelSerializer(serializers.ModelSerializer):
class Meta:
model = TestModel
fields = "__all__"
class TestModelViewset(ModelViewSet):
queryset = TestModel.objects.all()
serializer_class = TestModelSerializer
## URL
path(
"test-model/<int:pk>",
views.TestModelViewset.as_view({"get": "retrieve", "post": "partial_update"}),
name="test-model-object",
),
path(
"test-model/",
views.TestModelViewset.as_view({"get": "list", "post": "create"}),
name="test-models",
),
Error Msg : Warning: operationId "api_bridge_test_model_create" has collisions [('/api/bridge/test-model/', 'post'), ('/api/bridge/test-model/{id}', 'post')]. resolving with numeral suffixes.
Expected behavior
While the operationIds are autocorrected by the lib, not seeing the warning would be better :)
-
Including the path's name as a part of the operationId might be ideal as they are a functional part of Django's URL config and DRF router doc recommends using name too
-
Or inspect the config mapped to the method in the URL to be added to the operationId
DRF defines a method_mapping
and POST always maps to create
for the operation name. You are misusing the POST when it is supposed to be a PATCH.
So you get the *_create
name twice and therefore a collision because the "test-model/<int:pk>"
and "test-model/"
both generate test_model
. --> 2x test_model_create
I would say this is not a bug because there is a action2name mapping and you create a collision by accident. this should manually fix the collsion by providing an override name.
@extend_schema_view(
partial_update=extend_schema(operation_id="fooo_partial_update")
)
class TestModelViewset(viewsets.ModelViewSet):
queryset = SimpleModel.objects.all()
serializer_class = TestModelSerializer
I am completely on board that it should be a PATCH but I can't afford to update an existing codebase and justify the churn it'll cause. Looks like I'll have to overwrite the operationId as suggested.
i figured you didn't do modification just for fun, but at the end of the day it is a correctly handled from spectaculars perspective. I think the fix is so compact that it is tolerable imho.