Overriding `@extend_schema` decorator in inherited class does not override `exclude=True`

[dashdanw]

Describe the bug
If a base view or viewset class is decorated with @extend_schema(exclude=True) any inherited class will be excluded as well, and overriding by decorating that class with @extend_schema(exclude=False) does not seem to enable it to be included

To Reproduce

class AuthorizationMixin(APIView):
    # a view that requires that a user be logged in
    authentication_classes = [SomeAuthenticationClass,]

@extend_schema(exclude=True)
class SomeView(APIView):
    pass

@extend_schema(exclude=False)
class SomeAdminView(AuthorizationMixin, SomeView):
   # this class will be excluded from the documentation

Expected behavior
An inheriting class should be able to override the configuration of it's base class, in our implementation the base class is used as an endpoint as well which we DO want excluded.

as a note if given direction I would be happy to take a look at this and make a PR

[tfranzel]

Hi @dashdanw,

that was indeed a gap in the interface. The last one I hope 😄 Thanks for offering to do a PR, but while trying to figure out the exact problem, I already implemented the fix.

[dashdanw]

tfranzel I just wanted to extend a thanks, I rely on this package at work and this is actually huge for me. I'll continue to try to submit useful issues and hopefully I'll be able to contribute!

[alexandernst]

I just hit this bug and IMHO it's not quite fixed.

@extend_schema(exclude=True)
class SomeView(APIView):
    pass

@extend_schema(get=extend_schema(operation_id="foo"))
class SomeAdminView(SomeView):
   pass

I'd expect this to generate the "SomeAdminView" endpoint in my swagger, but it won't. I must explicitly add , exclude=False to make it work as expected. I find this counterintuitive since exclude is False by default. Moreover, I'm adding an operation_id, which really should give drf-spectacular a clue that I really do want to add this endpoint to the swagger.

[tfranzel]

Hey @alexandernst, I get your point but I would argue that it is not a bug.

I find this counterintuitive since exclude is False by default.

Not entirely true, because the decorator argument is tri-state. The default is False at the base implementation, true, but the decorations do nothing by default a.k.a. None.

So you basically exclude all endpoints in SomeView and then fail to explicitly turn on the ones you want. Sure, you could make the argument that giving anything in @extend_schema should implicitly set exclude=False, but that would break overall consistency. Think of @extend_schema as stacking layers with all arguments as "neutral" by default. If you change something in a lower layer, that change propagates up unless you explicitly change it again in a layer above. We handle exclude just like all other arguments in this case.

tldr: I think explicit is better than implicit, and a change would break consistency. I am for leaving it as is.

[alexandernst]

The problem (in this case's scenario) is that it's not immediately clear (to me?) that inheriting from SomeView will also inherit (and overwrite!) the extend_schema decorator (and it's properties). Which is why I wasted some time debugging what appeared to me like a bug.

The rest of your statement is true (and I could agree with it). It's just that I wasn't expecting this inheriting behavior.

[tfranzel]

The problem (in this case's scenario) is that it's not immediately clear (to me?) that inheriting from SomeView will also inherit (and overwrite!) the extend_schema decorator

hehe... in the beginning when this was not a feature yet, people complained that not also inheriting the decorators would be unexpected and weird. 😄 As you see, users come with different use-cases/mindsets and thus have different expectations.

I would say it currently is very close to what you would expect from regular idiomatic python code in terms of what would be inherited and how. We just had to take the concept one step further to maintain internal consistency.