Skip to content

Commit

Permalink
Fixed #24099 -- Removed contenttype.name deprecated field
Browse files Browse the repository at this point in the history
This finsishes the work started on #16803.
Thanks Simon Charette, Tim Graham and Collin Anderson for the
reviews.
  • Loading branch information
claudep authored and MarkusH committed Jan 16, 2015
1 parent 374c241 commit b4ac232
Show file tree
Hide file tree
Showing 15 changed files with 110 additions and 49 deletions.
2 changes: 1 addition & 1 deletion django/contrib/admin/templates/admin/index.html
Expand Up @@ -69,7 +69,7 @@ <h3>{% trans 'My Actions' %}</h3>
{% endif %}
<br/>
{% if entry.content_type %}
<span class="mini quiet">{% filter capfirst %}{% trans entry.content_type.name %}{% endfilter %}</span>
<span class="mini quiet">{% filter capfirst %}{{ entry.content_type }}{% endfilter %}</span>
{% else %}
<span class="mini quiet">{% trans 'Unknown content' %}</span>
{% endif %}
Expand Down
5 changes: 3 additions & 2 deletions django/contrib/auth/management/__init__.py
Expand Up @@ -110,8 +110,9 @@ def create_permissions(app_config, verbosity=2, interactive=True, using=DEFAULT_
for perm in perms:
if len(perm.name) > permission_name_max_length:
raise exceptions.ValidationError(
"The verbose_name of %s is longer than %s characters" % (
perm.content_type,
"The verbose_name of %s.%s is longer than %s characters" % (
perm.content_type.app_label,
perm.content_type.model,
verbose_name_max_length,
)
)
Expand Down
2 changes: 1 addition & 1 deletion django/contrib/auth/tests/test_management.py
Expand Up @@ -568,7 +568,7 @@ def test_verbose_name_length(self):
models.Permission._meta.verbose_name = "some ridiculously long verbose name that is out of control" * 5

six.assertRaisesRegex(self, exceptions.ValidationError,
"The verbose_name of permission is longer than 244 characters",
"The verbose_name of auth.permission is longer than 244 characters",
create_permissions, auth_app_config, verbosity=0)


Expand Down
4 changes: 0 additions & 4 deletions django/contrib/auth/tests/test_models.py
Expand Up @@ -52,24 +52,20 @@ def test_load_data_with_user_permissions(self):
default_objects = [
ContentType.objects.db_manager('default').create(
model='examplemodela',
name='example model a',
app_label='app_a',
),
ContentType.objects.db_manager('default').create(
model='examplemodelb',
name='example model b',
app_label='app_b',
),
]
other_objects = [
ContentType.objects.db_manager('other').create(
model='examplemodelb',
name='example model b',
app_label='app_b',
),
ContentType.objects.db_manager('other').create(
model='examplemodela',
name='example model a',
app_label='app_a',
),
]
Expand Down
2 changes: 0 additions & 2 deletions django/contrib/contenttypes/management.py
@@ -1,7 +1,6 @@
from django.apps import apps
from django.db import DEFAULT_DB_ALIAS, router
from django.db.migrations.loader import is_latest_migration_applied
from django.utils.encoding import smart_text
from django.utils import six
from django.utils.six.moves import input

Expand Down Expand Up @@ -50,7 +49,6 @@ def update_contenttypes(app_config, verbosity=2, interactive=True, using=DEFAULT

cts = [
ContentType(
name=smart_text(model._meta.verbose_name_raw),
app_label=app_label,
model=model_name,
)
Expand Down
@@ -0,0 +1,41 @@
# -*- coding: utf-8 -*-
from __future__ import unicode_literals

from django.db import models, migrations


def add_legacy_name(apps, schema_editor):
ContentType = apps.get_model('contenttypes', 'ContentType')
for ct in ContentType.objects.all():
try:
ct.name = apps.get_model(ct.app_label, ct.model)._meta.object_name
except:
ct.name = ct.model
ct.save()


class Migration(migrations.Migration):

dependencies = [
('contenttypes', '0001_initial'),
]

operations = [
migrations.AlterModelOptions(
name='contenttype',
options={'verbose_name': 'content type', 'verbose_name_plural': 'content types'},
),
migrations.AlterField(
model_name='contenttype',
name='name',
field=models.CharField(max_length=100, null=True),
),
migrations.RunPython(
migrations.RunPython.noop,
add_legacy_name,
),
migrations.RemoveField(
model_name='contenttype',
name='name',
),
]
36 changes: 19 additions & 17 deletions django/contrib/contenttypes/models.py
@@ -1,11 +1,13 @@
from __future__ import unicode_literals

import warnings

from django.apps import apps
from django.db import models
from django.db.utils import OperationalError, ProgrammingError
from django.utils.translation import ugettext_lazy as _
from django.utils.encoding import force_text
from django.utils.encoding import python_2_unicode_compatible
from django.utils.deprecation import RemovedInDjango20Warning
from django.utils.encoding import force_text, python_2_unicode_compatible


class ContentTypeManager(models.Manager):
Expand Down Expand Up @@ -34,6 +36,14 @@ def _get_from_cache(self, opts):
key = (opts.app_label, opts.model_name)
return self.__class__._cache[self.db][key]

def create(self, **kwargs):
if 'name' in kwargs:
del kwargs['name']
warnings.warn(
"ContentType.name field doesn't exist any longer. Please remove it from your code.",
RemovedInDjango20Warning, stacklevel=2)
return super(ContentTypeManager, self).create(**kwargs)

def get_for_model(self, model, for_concrete_model=True):
"""
Returns the ContentType object for a given model, creating the
Expand Down Expand Up @@ -66,7 +76,6 @@ def get_for_model(self, model, for_concrete_model=True):
ct, created = self.get_or_create(
app_label=opts.app_label,
model=opts.model_name,
defaults={'name': opts.verbose_name_raw},
)
self._add_to_cache(self.db, ct)
return ct
Expand Down Expand Up @@ -108,7 +117,6 @@ def get_for_models(self, *models, **kwargs):
ct = self.create(
app_label=opts.app_label,
model=opts.model_name,
name=opts.verbose_name_raw,
)
self._add_to_cache(self.db, ct)
results[ct.model_class()] = ct
Expand Down Expand Up @@ -148,7 +156,6 @@ def _add_to_cache(self, using, ct):

@python_2_unicode_compatible
class ContentType(models.Model):
name = models.CharField(max_length=100)
app_label = models.CharField(max_length=100)
model = models.CharField(_('python model class name'), max_length=100)
objects = ContentTypeManager()
Expand All @@ -157,22 +164,17 @@ class Meta:
verbose_name = _('content type')
verbose_name_plural = _('content types')
db_table = 'django_content_type'
ordering = ('name',)
unique_together = (('app_label', 'model'),)

def __str__(self):
# self.name is deprecated in favor of using model's verbose_name, which
# can be translated. Formal deprecation is delayed until we have DB
# migration to be able to remove the field from the database along with
# the attribute.
#
# We return self.name only when users have changed its value from the
# initial verbose_name_raw and might rely on it.
return self.name

@property
def name(self):
model = self.model_class()
if not model or self.name != model._meta.verbose_name_raw:
return self.name
else:
return force_text(model._meta.verbose_name)
if not model:
return self.model
return force_text(model._meta.verbose_name)

def model_class(self):
"Returns the Python model class for this type of content."
Expand Down
27 changes: 22 additions & 5 deletions django/contrib/contenttypes/tests/tests.py
@@ -1,5 +1,7 @@
from __future__ import unicode_literals

import warnings

from django.contrib.contenttypes.models import ContentType
from django.contrib.contenttypes.views import shortcut
from django.contrib.sites.shortcuts import get_current_site
Expand Down Expand Up @@ -230,11 +232,10 @@ def test_missing_model(self):
is defined anymore.
"""
ct = ContentType.objects.create(
name='Old model',
app_label='contenttypes',
model='OldModel',
)
self.assertEqual(six.text_type(ct), 'Old model')
self.assertEqual(six.text_type(ct), 'OldModel')
self.assertIsNone(ct.model_class())

# Make sure stale ContentTypes can be fetched like any other object.
Expand All @@ -243,9 +244,6 @@ def test_missing_model(self):
ct_fetched = ContentType.objects.get_for_id(ct.pk)
self.assertIsNone(ct_fetched.model_class())


class MigrateTests(TestCase):

@skipUnlessDBFeature('can_rollback_ddl')
def test_unmigrating_first_migration_post_migrate_signal(self):
"""
Expand All @@ -260,3 +258,22 @@ def test_unmigrating_first_migration_post_migrate_signal(self):
call_command("migrate", "contenttypes", "zero", verbosity=0)
finally:
call_command("migrate", verbosity=0)

def test_name_deprecation(self):
"""
ContentType.name has been removed. Test that a warning is emitted when
creating a ContentType with a `name`, but the creation should not fail.
"""
with warnings.catch_warnings(record=True) as warns:
warnings.simplefilter('always')
ContentType.objects.create(
name='Name',
app_label='contenttypes',
model='OldModel',
)
self.assertEqual(len(warns), 1)
self.assertEqual(
str(warns[0].message),
"ContentType.name field doesn't exist any longer. Please remove it from your code."
)
self.assertTrue(ContentType.objects.filter(model='OldModel').exists())
3 changes: 3 additions & 0 deletions docs/internals/deprecation.txt
Expand Up @@ -164,6 +164,9 @@ details on these changes.
* ``GeoQuerySet`` aggregate methods ``collect()``, ``extent()``, ``extent3d()``,
``makeline()``, and ``union()`` will be removed.

* Ability to specify ``ContentType.name`` when creating a content type instance
will be removed.

.. _deprecation-removed-in-1.9:

1.9
Expand Down
14 changes: 9 additions & 5 deletions docs/ref/contrib/contenttypes.txt
Expand Up @@ -59,7 +59,7 @@ The ``ContentType`` model
.. class:: ContentType

Each instance of :class:`~django.contrib.contenttypes.models.ContentType`
has three fields which, taken together, uniquely describe an installed
has two fields which, taken together, uniquely describe an installed
model:

.. attribute:: app_label
Expand All @@ -74,12 +74,19 @@ The ``ContentType`` model

The name of the model class.

Additionally, the following property is available:

.. attribute:: name

The human-readable name of the model. This is taken from the
The human-readable name of the content type. This is taken from the
:attr:`verbose_name <django.db.models.Field.verbose_name>`
attribute of the model.

.. versionchanged:: 1.8

Before Django 1.8, the ``name`` property was a real field on the
``ContentType`` model.

Let's look at an example to see how this works. If you already have
the :mod:`~django.contrib.contenttypes` application installed, and then add
:mod:`the sites application <django.contrib.sites>` to your
Expand All @@ -96,9 +103,6 @@ created with the following values:
* :attr:`~django.contrib.contenttypes.models.ContentType.model`
will be set to ``'site'``.

* :attr:`~django.contrib.contenttypes.models.ContentType.name`
will be set to ``'site'``.

.. _the verbose_name attribute: ../model-api/#verbose_name

Methods on ``ContentType`` instances
Expand Down
4 changes: 4 additions & 0 deletions docs/releases/1.8.txt
Expand Up @@ -1101,6 +1101,10 @@ Miscellaneous
the newly introduced ``hints`` parameter for these operations, but it can
also be used to disable migrations from running on a particular database.

* The ``name`` field of :class:`django.contrib.contenttypes.models.ContentType`
has been removed by a migration and replaced by a property. That means it's
not possible to query or filter a ``ContentType`` by this field any longer.

.. _deprecated-features-1.8:

Features deprecated in 1.8
Expand Down
6 changes: 3 additions & 3 deletions tests/admin_views/tests.py
Expand Up @@ -1974,7 +1974,7 @@ def test_recentactions_without_content_type(self):
self.assertContains(response, should_contain)
should_contain = "Model with string primary key" # capitalized in Recent Actions
self.assertContains(response, should_contain)
logentry = LogEntry.objects.get(content_type__name__iexact=should_contain)
logentry = LogEntry.objects.get(content_type__model__iexact='modelwithstringprimarykey')
# http://code.djangoproject.com/ticket/10275
# if the log entry doesn't have a content type it should still be
# possible to view the Recent Actions part
Expand All @@ -1989,8 +1989,8 @@ def test_recentactions_without_content_type(self):

def test_logentry_get_admin_url(self):
"LogEntry.get_admin_url returns a URL to edit the entry's object or None for non-existent (possibly deleted) models"
log_entry_name = "Model with string primary key" # capitalized in Recent Actions
logentry = LogEntry.objects.get(content_type__name__iexact=log_entry_name)
log_entry_model = "modelwithstringprimarykey" # capitalized in Recent Actions
logentry = LogEntry.objects.get(content_type__model__iexact=log_entry_model)
model = "modelwithstringprimarykey"
desired_admin_url = "/test_admin/admin/admin_views/%s/%s/" % (model, iri_to_uri(quote(self.pk)))
self.assertEqual(logentry.get_admin_url(), desired_admin_url)
Expand Down
6 changes: 3 additions & 3 deletions tests/contenttypes_tests/tests.py
Expand Up @@ -11,7 +11,7 @@
from django.db import connections, models
from django.test import TestCase, override_settings
from django.test.utils import captured_stdout
from django.utils.encoding import force_str
from django.utils.encoding import force_str, force_text

from .models import Author, Article, SchemeIncludedURL

Expand Down Expand Up @@ -87,7 +87,7 @@ class Meta:
ct = ContentType.objects.get_for_model(ModelCreatedOnTheFly)
self.assertEqual(ct.app_label, 'my_great_app')
self.assertEqual(ct.model, 'modelcreatedonthefly')
self.assertEqual(ct.name, 'a model created on the fly')
self.assertEqual(force_text(ct), 'modelcreatedonthefly')


class IsolatedModelsTestCase(TestCase):
Expand Down Expand Up @@ -364,7 +364,7 @@ class InvalidBookmark(models.Model):
class UpdateContentTypesTests(TestCase):
def setUp(self):
self.before_count = ContentType.objects.count()
ContentType.objects.create(name='fake', app_label='contenttypes_tests', model='Fake')
ContentType.objects.create(app_label='contenttypes_tests', model='Fake')
self.app_config = apps.get_app_config('contenttypes_tests')

def test_interactive_true(self):
Expand Down
2 changes: 1 addition & 1 deletion tests/generic_relations/models.py
Expand Up @@ -29,7 +29,7 @@ class TaggedItem(models.Model):
content_object = GenericForeignKey()

class Meta:
ordering = ["tag", "content_type__name"]
ordering = ["tag", "content_type__model"]

def __str__(self):
return self.tag
Expand Down
5 changes: 0 additions & 5 deletions tests/i18n/contenttypes/tests.py
Expand Up @@ -28,8 +28,3 @@ def test_verbose_name(self):
self.assertEqual(six.text_type(company_type), 'Company')
with translation.override('fr'):
self.assertEqual(six.text_type(company_type), 'Société')

def test_field_override(self):
company_type = ContentType.objects.get(app_label='i18n', model='company')
company_type.name = 'Other'
self.assertEqual(six.text_type(company_type), 'Other')

0 comments on commit b4ac232

Please sign in to comment.