-
Notifications
You must be signed in to change notification settings - Fork 272
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
Fix readonly related fields generating incorrect schema #275
Fix readonly related fields generating incorrect schema #275
Conversation
Codecov Report
@@ Coverage Diff @@
## master #275 +/- ##
==========================================
+ Coverage 98.37% 98.39% +0.01%
==========================================
Files 53 53
Lines 4438 4476 +38
==========================================
+ Hits 4366 4404 +38
Misses 72 72
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@diesieben07 awesome catch! funny how such a bug can go unnoticed for so long.
however the bugfix does break in some cases. this test shows it with indirect_referenced_model_ro
. field.field_name
does not always apply.
for regressions we prefer consise minimalistic tests and put it in test_regressions.py
.
also, i prefer to not add full yamls dumps for single bugs. i prepared this for your convenience. please use it instead.
def test_incorrect_foreignkey_type_on_readonly_field(no_warnings):
class ReferencingModel(models.Model):
id = models.UUIDField(primary_key=True)
referenced_model = models.ForeignKey(SimpleModel, on_delete=models.CASCADE)
referenced_model_ro = models.ForeignKey(SimpleModel, on_delete=models.CASCADE)
referenced_model_m2m = models.ManyToManyField(SimpleModel)
referenced_model_m2m_ro = models.ManyToManyField(SimpleModel)
class ReferencingModelSerializer(serializers.ModelSerializer):
indirect_referenced_model_ro = serializers.PrimaryKeyRelatedField(
source='referenced_model',
read_only=True,
)
class Meta:
fields = '__all__'
read_only_fields = ['id', 'referenced_model_ro', 'referenced_model_m2m_ro']
model = ReferencingModel
class ReferencingModelViewset(viewsets.ModelViewSet):
serializer_class = ReferencingModelSerializer
queryset = ReferencingModel.objects.all()
schema = generate_schema('/x/', ReferencingModelViewset)
properties = schema['components']['schemas']['ReferencingModel']['properties']
assert properties['referenced_model']['type'] == 'integer'
assert properties['referenced_model_ro']['type'] == 'integer'
assert properties['referenced_model_m2m']['items']['type'] == 'integer'
assert properties['referenced_model_m2m_ro']['items']['type'] == 'integer'
assert properties['indirect_referenced_model_ro']['type'] == 'integer'
Thanks for the note about |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
excellent work! thank you so much. still puzzled how this went unnoticed for so long.
This fixes the issue outlined in #274. I've added a test for this case as well.