Skip to content
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

Fixes PostGIS model registration #67

Merged
merged 1 commit into from
Feb 27, 2020

Conversation

sklarsa
Copy link
Contributor

@sklarsa sklarsa commented Feb 25, 2020

The result of generators.get_type_mapping() is being used when registering types in baker.init_type_mapping, but the return value of get_type_mapping is being set to the copied value of the mappings before postgis models have been added. This leads model baker to believe that the PostGis models are invalid, when in fact, they are valid.

Given this model in myapp/models.py...

from django.contrib.gis.db import models as gis_models
from django.db import models

class MyModel(models.Model):
    point = gis_models.PointField(srid=4326)

The code below errors when executed in the Django shell:

from model_bakery import baker
instance = baker.make(MyModel)

with the following error: TypeError: <class 'django.contrib.gis.db.models.fields.PointField'> is not supported by baker.

I've tested this change locally, and it works, but I'm not sure why this is not being picked up in the tests on TravisCI

Copy link
Contributor

@anapaulagomes anapaulagomes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🏅

@@ -143,7 +143,7 @@ def get_type_mapping():

mapping = default_mapping.copy()
mapping[ContentType] = random_gen.gen_content_type
default_mapping.update(default_gis_mapping)
mapping.update(default_gis_mapping)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️ thanks for catching this!!!

@@ -143,7 +143,7 @@ def get_type_mapping():

mapping = default_mapping.copy()
mapping[ContentType] = random_gen.gen_content_type
default_mapping.update(default_gis_mapping)
mapping.update(default_gis_mapping)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️ thanks for catching this!!!

@berinhard
Copy link
Member

I'm merging this since 2/3 maintainers have approved it 👍

@berinhard berinhard merged commit cad6b5c into model-bakers:master Feb 27, 2020
@berinhard
Copy link
Member

@anapaulagomes and @amureki what do you think about a new release adding this PR?

@amureki
Copy link
Collaborator

amureki commented Feb 27, 2020

@berinhard sure, why not? If possible, we should try to add a regression test for this case to not break it in the future again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants