[Django] #26521: CreateModel allows duplicate field names
Django
-------------------------------+--------------------
Reporter: w0rp | Owner: nobody
Type: Uncategorized | Status: new
Component: Migrations | Version: 1.9
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------
I hit this bug while manually editing a migration file after squashing
files. If you define the same field twice in a list of fields for
CreateModel, then migrate will take the second field definition without
reporting any errors.
{{{#!python
class Migration(migrations.Migration):
operations = [
migrations.CreateModel(
name='Foo',
fields=[
('id', models.AutoField(verbose_name='ID',
serialize=False, auto_created=True, primary_key=True)),
('created', models.DateTimeField(auto_now_add=True,
db_index=True)),
# This one will be used.
('created', models.DateField(auto_now_add=True,
db_index=True)),
],
),
]
}}}
I think the migration code should check for duplicate fields, and then
report an error if you have mistakenly defined the same field twice for
the fields array.
--
Ticket URL: <https://code.djangoproject.com/ticket/26521>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: w0rp | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Migrations | Version: 1.9
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: => 0
* needs_docs: => 0
* type: Uncategorized => Cleanup/optimization
* needs_tests: => 0
* stage: Unreviewed => Accepted
Comment:
Probably wouldn't hurt.
--
Ticket URL: <https://code.djangoproject.com/ticket/26521#comment:1>
Django
Reporter: w0rp | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Migrations | Version: 1.9
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/6482 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/26521#comment:2>
Django
Reporter: w0rp | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Migrations | Version: 1.9
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by MarkusH):
I don't know if we really need this check, but if we do that, the manager
names need to be unique and so do the base classes.
--
Ticket URL: <https://code.djangoproject.com/ticket/26521#comment:3>
Django
Reporter: w0rp | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by MarkusH):
* needs_better_patch: 0 => 1
* version: 1.9 => master
--
Ticket URL: <https://code.djangoproject.com/ticket/26521#comment:4>
Django
Reporter: w0rp | Owner: nobody
Type: Cleanup/optimization | Status: closed
Component: Migrations | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Changes (by Simon Charette <charettes@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"417e083e5521766e73419fb02cac88996ea3b9e7" 417e083e]:
{{{
#!CommitTicketReference repository=""
revision="417e083e5521766e73419fb02cac88996ea3b9e7"
Fixed #26521 -- Validated CreateModel bases, fields and managers for
duplicates.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26521#comment:5>
Django
Reporter: w0rp | Owner: nobody
Type: Cleanup/optimization | Status: closed
Component: Migrations | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Comment (by Simon Charette <charettes@…>):
In [changeset:"a877a2f83da384944ad6530eb951bcd55dcd8605" a877a2f8]:
{{{
#!CommitTicketReference repository=""
revision="a877a2f83da384944ad6530eb951bcd55dcd8605"
Refs #26521 -- Added the duplicated value to CreateModel validation
messages.
Thanks Tim for the suggestion.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26521#comment:6>
Django
Reporter: w0rp | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Changes (by collinanderson):
* status: closed => new
* resolution: fixed =>
Comment:
I think this broke non-model base classes:
{{{
File
"/home/collin/comcenter/../comcenter/category/migrations/0001_initial.py",
line 9, in <module>
class Migration(migrations.Migration):
File
"/home/collin/comcenter/../comcenter/category/migrations/0001_initial.py",
line 35, in Migration
bases=(models.Model, comcenter.product.models.WhiteBlackListMixin),
File "/home/collin/comcenter/django/db/migrations/operations/models.py",
line 62, in __init__
for base in self.bases
File "/home/collin/comcenter/django/db/migrations/operations/models.py",
line 17, in _check_for_duplicates
for val in objs:
File "/home/collin/comcenter/django/db/migrations/operations/models.py",
line 63, in <genexpr>
if base is not models.Model)
AttributeError: type object 'WhiteBlackListMixin' has no attribute 'lower'
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26521#comment:7>
Django
Reporter: w0rp | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Comment (by charettes):
Will take care of this. I'm a bit surprised `CreateModel` is untested with
mixins.
--
Ticket URL: <https://code.djangoproject.com/ticket/26521#comment:8>
Django
Reporter: w0rp | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Comment (by Simon Charette <charettes@…>):
In [changeset:"f951bb78cbf179f0fb70fe74ae0c218925fd7ede" f951bb78]:
{{{
#!CommitTicketReference repository=""
revision="f951bb78cbf179f0fb70fe74ae0c218925fd7ede"
Refs #26521 -- Adjusted CreateModel bases validation to account for
mixins.
Thanks Collin for the report.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26521#comment:9>
Django
Reporter: w0rp | Owner: nobody
Type: Cleanup/optimization | Status: closed
Component: Migrations | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Changes (by charettes):
* status: new => closed
* resolution: => fixed
--
Ticket URL: <https://code.djangoproject.com/ticket/26521#comment:10>