Opened 13 years ago
Closed 13 years ago
#17717 closed Bug (fixed)
django.core.serializers.serialize doesn't deal with proxy models
| Reported by: | lakin | Owned by: | nobody |
|---|---|---|---|
| Component: | Core (Serialization) | Version: | 1.1 |
| Severity: | Normal | Keywords: | |
| Cc: | anssi.kaariainen@…, charette.s@… | Triage Stage: | Ready for checkin |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
If you try to serialize a proxy=True model - you get no fields serialized.
I wish I had more time to make a test case, but I don't at the moment. The easy test case is to make a model, make another model that proxies to the original model. Insert a row of data, and then serialize an instance of both models to see that the proxy one is empty.
Marking it as 1.1 as that is what I'm using and I haven't had time to double check the other versions.
Attachments (3)
Change History (12)
by , 13 years ago
| Attachment: | 17717-test.diff added |
|---|
comment:1 by , 13 years ago
| Triage Stage: | Unreviewed → Accepted |
|---|
comment:2 by , 13 years ago
| Cc: | added |
|---|---|
| Has patch: | set |
In the attached patch the concrete_model._meta.local_fields are used to find out the fields to dump instead of obj._meta.local_fields.
by , 13 years ago
| Attachment: | 17717.diff added |
|---|
comment:5 by , 13 years ago
Now that there's a concrete_class property and that proxy_for_model is actually a proxy chain we should probably just get it from there?
Correct me if I'm wrong but the patch checked in r17640 will fail for proxy of a proxy model.
Should the patch be checked out and corrected with additional tests?
comment:6 by , 13 years ago
| Cc: | added |
|---|
comment:7 by , 13 years ago
| Resolution: | fixed |
|---|---|
| Status: | closed → reopened |
| Triage Stage: | Ready for checkin → Accepted |
Oh, yeah, I didn't see the changes done in r17573, this new "concrete_model" property might indeed fix this easier. Re-opening..
by , 13 years ago
| Attachment: | ticket-17717-proxy-model-serializers.diff added |
|---|
Make sure to rely on newly introduced concrete_model model option
comment:8 by , 13 years ago
| Triage Stage: | Accepted → Ready for checkin |
|---|
Yes, that should be fixed. Now proxy_for_model is the actual proxied model, so as written the committed patch isn't correct any more. That is my failing, as I should have written it using the while opts._proxy_for_model: opts = opts._proxy_for_model._meta idiom instead of assuming the buggy behavior in the patch.
I tested the patch and without the fix the added test fails, with the fix in the patch it succeeds. I quickly skimmed through it and it seems ready for checkin to me.
Test showing bug