-
Notifications
You must be signed in to change notification settings - Fork 260
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
discriminator property not added to components under oneOf with django-rest-polymorphic #885
Comments
Yes, that is the intended behavior. It is a carefully crafted trade-off between correct schema and
We could come up with a more complicated way of writing the Lets take the test example: https://github.com/tfranzel/drf-spectacular/blob/master/tests/contrib/test_rest_polymorphic.py Written like that export type Person = { resourcetype: 'legal' } & LegalPerson | { resourcetype: 'natural' } & NaturalPerson | { resourcetype: 'nomadic' } & NomadicPerson;
export function PersonFromJSONTyped(json: any, ignoreDiscriminator: boolean): Person {
if ((json === undefined) || (json === null)) {
return json;
}
switch (json['resourcetype']) {
case 'legal':
return {...LegalPersonFromJSONTyped(json, true), resourcetype: 'legal'};
case 'natural':
return {...NaturalPersonFromJSONTyped(json, true), resourcetype: 'natural'};
case 'nomadic':
return {...NomadicPersonFromJSONTyped(json, true), resourcetype: 'nomadic'};
default:
throw new Error(`No variant of Person exists with 'resourcetype=${json['resourcetype']}'`);
}
} So this just works for If you want to have this 100% correct, I recommend adding a constant field to your sub-serializers: class SubModel2Serializer(ModelSerializer):
resourcetype = serializers.SerializerMethodField()
def get_resourcetype(self, object) -> str:
return 'SubModel2'
class Meta:
model = SubModel2
exclude = ("polymorphic_ctype",) |
Could components be created specifically for the polymorphic serializer with the type included to leave the ones they're using unaffected? Or would that cause confusion with their usage? Thinking about it, doesn't the usage through PolymorphicProxySerializer have the property in the components as the discriminator should have a field defined on the serializers? The SerializerMethodField leaves it out of the request schema which is where it's important |
That would be possible, but that means duplication and we would need to come up with additional derived component names that again have the potential to collide. It should not be too difficult to modify
It makes sense for the
ah sry you are right. that was from my spotty memory. This was for the |
removed - it was not well thought out given your statements. I have to think about this. |
Describe the bug
When the schema for a view using the PolymorphicSerializer from django-rest-polymorphic is generated, it uses the serializer's
resource_type_field_name
for the discriminator but the property under the same name is not added to the components generated from the sub-serializers.This causes it to not be included in the schema in swagger/redoc and a mismatch between what the api expects.
To Reproduce
run generation on
Expected behavior
string property under the same name as
resource_type_field_name
added to components generator for polymorphic serializer so tooling picks it upThe text was updated successfully, but these errors were encountered: