tfranzel/drf-spectacular

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 :)

  1. 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

  2. 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.