Fixed #26304 -- Handled unmanaged through table between two managed models in table introspection. #6231
Conversation
schinckel
force-pushed
the
unmanaged-through-model
branch
from
14c1c98
to
4d57b9b
Compare
| tables.update(f.m2m_db_table() for f in model._meta.local_many_to_many | ||
| if not f.remote_field or | ||
| not f.remote_field.through or | ||
| f.remote_field.through._meta.managed) |
There was a problem hiding this comment.
All m2m fields should have remote_field.through defined.
There was a problem hiding this comment.
Ah, okay. I originally wrote the patch on 1.4 (don't ask), which has f.rel: which wasn't always set (or f.rel.through wasn't always there), and the subsequent checks were also required.
There was a problem hiding this comment.
I think f.remote_field.through should always be available.
|
Actually I think the test should be added to the |
|
Yeah, there is already stuff there. I'll move it. Thanks. |
| @@ -6,7 +6,7 @@ | |||
| from django.db.utils import DatabaseError | |||
| from django.test import TransactionTestCase, mock, skipUnlessDBFeature | |||
|
|
|||
| from .models import Article, City, Reporter | |||
| from .models import Article, City, Reporter, ArticleReporter | |||
There was a problem hiding this comment.
Put ArticleReporter after Article to appease isort.
MarkusH
changed the title
MarkusH
changed the title
| def test_unmanaged_through_model(self): | ||
| table_name = ArticleReporter._meta.db_table | ||
| connection.cursor().execute('CREATE VIEW {} AS SELECT 1 AS id'.format(table_name)) | ||
| tables = connection.introspection.django_table_names(only_existing=True) |
There was a problem hiding this comment.
I tested locally and confirmed the view creation is not necessary.
Make sure you don't pass only_existing=True to the django_table_names call and the test will fail without the fix applied and pass with it.
|
Please squash all your commits into a single one with the PR's title as body. |
schinckel
force-pushed
the
unmanaged-through-model
branch
2 times, most recently
from
c7033a5
to
0383e28
Compare
| @@ -45,6 +45,7 @@ class Article(models.Model): | |||
| body = models.TextField(default='') | |||
| reporter = models.ForeignKey(Reporter, models.CASCADE) | |||
| response_to = models.ForeignKey('self', models.SET_NULL, null=True) | |||
| unmanaged_reporters = models.ManyToManyField(Reporter, through='introspection.ArticleReporter') | |||
There was a problem hiding this comment.
No need to specify the 'introspection' prefix here.
schinckel
force-pushed
the
unmanaged-through-model
branch
from
0383e28
to
4b44603
Compare
timgraham
changed the title
|
merged in 60633ef, thanks! |
https://code.djangoproject.com/ticket/26304