From 983c158da7723eb00a376bd31db76709da4d0260 Mon Sep 17 00:00:00 2001 From: Claude Paroz Date: Thu, 10 Mar 2016 18:21:25 +0100 Subject: [PATCH] Refs #24227 -- Replaced M2M isinstance checks by field.many_to_many Thanks Markus Holtermann, Collin Anderson and Tim Graham for the reviews. --- django/contrib/admin/checks.py | 27 ++++++++++----------------- django/contrib/admin/options.py | 8 ++++---- django/contrib/sites/managers.py | 4 ++-- django/db/migrations/autodetector.py | 15 ++++----------- django/forms/models.py | 4 +--- docs/ref/checks.txt | 16 ++++++++-------- tests/admin_checks/tests.py | 6 +++--- tests/model_meta/tests.py | 5 ++--- tests/modeladmin/tests.py | 10 +++++----- tests/sites_framework/tests.py | 2 +- 10 files changed, 40 insertions(+), 57 deletions(-) diff --git a/django/contrib/admin/checks.py b/django/contrib/admin/checks.py index 47e40d62ab..3186d8a752 100644 --- a/django/contrib/admin/checks.py +++ b/django/contrib/admin/checks.py @@ -104,8 +104,8 @@ class BaseModelAdminChecks(object): return refer_to_missing_field(field=field_name, option=label, model=model, obj=obj, id='admin.E002') else: - if not isinstance(field, (models.ForeignKey, models.ManyToManyField)): - return must_be('a ForeignKey or ManyToManyField', + if not field.many_to_many and not isinstance(field, models.ForeignKey): + return must_be('a foreign key or a many-to-many field', option=label, obj=obj, id='admin.E003') else: return [] @@ -218,11 +218,10 @@ class BaseModelAdminChecks(object): # be an extra field on the form. return [] else: - if (isinstance(field, models.ManyToManyField) and - not field.remote_field.through._meta.auto_created): + if field.many_to_many and not field.remote_field.through._meta.auto_created: return [ checks.Error( - "The value of '%s' cannot include the ManyToManyField '%s', " + "The value of '%s' cannot include the many-to-many field '%s' " "because that field manually specifies a relationship model." % (label, field_name), obj=obj.__class__, @@ -295,8 +294,8 @@ class BaseModelAdminChecks(object): return refer_to_missing_field(field=field_name, option=label, model=model, obj=obj, id='admin.E019') else: - if not isinstance(field, models.ManyToManyField): - return must_be('a ManyToManyField', option=label, obj=obj, id='admin.E020') + if not field.many_to_many: + return must_be('a many-to-many field', option=label, obj=obj, id='admin.E020') else: return [] @@ -389,23 +388,17 @@ class BaseModelAdminChecks(object): is a name of existing field and the field is one of the allowed types. """ - forbidden_field_types = ( - models.DateTimeField, - models.ForeignKey, - models.ManyToManyField - ) - try: field = model._meta.get_field(field_name) except FieldDoesNotExist: return refer_to_missing_field(field=field_name, option=label, model=model, obj=obj, id='admin.E027') else: - if isinstance(field, forbidden_field_types): + if field.many_to_many or isinstance(field, (models.DateTimeField, models.ForeignKey)): return [ checks.Error( "The value of '%s' refers to '%s', which must not be a DateTimeField, " - "ForeignKey or ManyToManyField." % (label, field_name), + "a foreign key, or a many-to-many field." % (label, field_name), obj=obj.__class__, id='admin.E028', ) @@ -629,10 +622,10 @@ class ModelAdminChecks(BaseModelAdminChecks): id='admin.E108', ) ] - elif isinstance(field, models.ManyToManyField): + elif getattr(field, 'many_to_many', False): return [ checks.Error( - "The value of '%s' must not be a ManyToManyField." % label, + "The value of '%s' must not be a many-to-many field." % label, obj=obj.__class__, id='admin.E109', ) diff --git a/django/contrib/admin/options.py b/django/contrib/admin/options.py index 6eb5060414..bdd988f4c7 100644 --- a/django/contrib/admin/options.py +++ b/django/contrib/admin/options.py @@ -133,8 +133,8 @@ class BaseModelAdmin(six.with_metaclass(forms.MediaDefiningClass)): if db_field.choices: return self.formfield_for_choice_field(db_field, request, **kwargs) - # ForeignKey or ManyToManyFields - if isinstance(db_field, (models.ForeignKey, models.ManyToManyField)): + # Foreign key or many-to-many fields + if db_field.many_to_many or isinstance(db_field, models.ForeignKey): # Combine the field kwargs with any options for formfield_overrides. # Make sure the passed in **kwargs override anything in # formfield_overrides because **kwargs is more specific, and should @@ -145,7 +145,7 @@ class BaseModelAdmin(six.with_metaclass(forms.MediaDefiningClass)): # Get the correct formfield. if isinstance(db_field, models.ForeignKey): formfield = self.formfield_for_foreignkey(db_field, request, **kwargs) - elif isinstance(db_field, models.ManyToManyField): + elif db_field.many_to_many: formfield = self.formfield_for_manytomany(db_field, request, **kwargs) # For non-raw_id fields, wrap the widget with a wrapper that adds @@ -1385,7 +1385,7 @@ class ModelAdmin(BaseModelAdmin): except FieldDoesNotExist: continue # We have to special-case M2Ms as a list of comma-separated PKs. - if isinstance(f, models.ManyToManyField): + if f.many_to_many: initial[k] = initial[k].split(",") return initial diff --git a/django/contrib/sites/managers.py b/django/contrib/sites/managers.py index a4f83777c5..a3a0f07fc3 100644 --- a/django/contrib/sites/managers.py +++ b/django/contrib/sites/managers.py @@ -34,10 +34,10 @@ class CurrentSiteManager(models.Manager): ) ] - if not isinstance(field, (models.ForeignKey, models.ManyToManyField)): + if not field.many_to_many and not isinstance(field, (models.ForeignKey)): return [ checks.Error( - "CurrentSiteManager cannot use '%s.%s' as it is not a ForeignKey or ManyToManyField." % ( + "CurrentSiteManager cannot use '%s.%s' as it is not a foreign key or a many-to-many field." % ( self.model._meta.object_name, field_name ), obj=self, diff --git a/django/db/migrations/autodetector.py b/django/db/migrations/autodetector.py index d5b2666575..b7716b208f 100644 --- a/django/db/migrations/autodetector.py +++ b/django/db/migrations/autodetector.py @@ -799,8 +799,7 @@ class MigrationAutodetector(object): # You can't just add NOT NULL fields with no default or fields # which don't allow empty strings as default. preserve_default = True - if (not field.null and not field.has_default() and - not isinstance(field, models.ManyToManyField) and + if (not field.null and not field.has_default() and not field.many_to_many and not (field.blank and field.empty_strings_allowed)): field = field.clone() field.default = self.questioner.ask_not_null_addition(field_name, model_name) @@ -861,19 +860,13 @@ class MigrationAutodetector(object): old_field_dec = self.deep_deconstruct(old_field) new_field_dec = self.deep_deconstruct(new_field) if old_field_dec != new_field_dec: - both_m2m = ( - isinstance(old_field, models.ManyToManyField) and - isinstance(new_field, models.ManyToManyField) - ) - neither_m2m = ( - not isinstance(old_field, models.ManyToManyField) and - not isinstance(new_field, models.ManyToManyField) - ) + both_m2m = old_field.many_to_many and new_field.many_to_many + neither_m2m = not old_field.many_to_many and not new_field.many_to_many if both_m2m or neither_m2m: # Either both fields are m2m or neither is preserve_default = True if (old_field.null and not new_field.null and not new_field.has_default() and - not isinstance(new_field, models.ManyToManyField)): + not new_field.many_to_many): field = new_field.clone() new_default = self.questioner.ask_not_null_alteration(field_name, model_name) if new_default is not models.NOT_PROVIDED: diff --git a/django/forms/models.py b/django/forms/models.py index 4422882315..067f601493 100644 --- a/django/forms/models.py +++ b/django/forms/models.py @@ -79,8 +79,6 @@ def model_to_dict(instance, fields=None, exclude=None): fields will be excluded from the returned dict, even if they are listed in the ``fields`` argument. """ - # avoid a circular import - from django.db.models.fields.related import ManyToManyField opts = instance._meta data = {} for f in chain(opts.concrete_fields, opts.virtual_fields, opts.many_to_many): @@ -90,7 +88,7 @@ def model_to_dict(instance, fields=None, exclude=None): continue if exclude and f.name in exclude: continue - if isinstance(f, ManyToManyField): + if f.many_to_many: # If the object doesn't have a primary key yet, just use an empty # list for its m2m fields. Calling f.value_from_object will raise # an exception. diff --git a/docs/ref/checks.txt b/docs/ref/checks.txt index c2849d5f74..e50dd5d959 100644 --- a/docs/ref/checks.txt +++ b/docs/ref/checks.txt @@ -271,8 +271,8 @@ with the admin site: * **admin.E001**: The value of ``raw_id_fields`` must be a list or tuple. * **admin.E002**: The value of ``raw_id_fields[n]`` refers to ````, which is not an attribute of ````. -* **admin.E003**: The value of ``raw_id_fields[n]`` must be a ``ForeignKey`` or - ``ManyToManyField``. +* **admin.E003**: The value of ``raw_id_fields[n]`` must be a foreign key or + a many-to-many field. * **admin.E004**: The value of ``fields`` must be a list or tuple. * **admin.E005**: Both ``fieldsets`` and ``fields`` are specified. * **admin.E006**: The value of ``fields`` contains duplicate field(s). @@ -284,7 +284,7 @@ with the admin site: ``fields``. * **admin.E012**: There are duplicate field(s) in ``fieldsets[n][1]``. * **admin.E013**: ``fields[n]/fieldsets[n][m]`` cannot include the - ``ManyToManyField`` ````, because that field manually specifies a + many-to-many field ````, because that field manually specifies a relationship model. * **admin.E014**: The value of ``exclude`` must be a list or tuple. * **admin.E015**: The value of ``exclude`` contains duplicate field(s). @@ -294,7 +294,7 @@ with the admin site: * **admin.E019**: The value of ``filter_vertical[n]/filter_vertical[n]`` refers to ````, which is not an attribute of ````. * **admin.E020**: The value of ``filter_vertical[n]/filter_vertical[n]`` must - be a ``ManyToManyField``. + be a many-to-many field. * **admin.E021**: The value of ``radio_fields`` must be a dictionary. * **admin.E022**: The value of ``radio_fields`` refers to ````, which is not an attribute of ````. @@ -308,8 +308,8 @@ with the admin site: * **admin.E027**: The value of ``prepopulated_fields`` refers to ````, which is not an attribute of ````. * **admin.E028**: The value of ``prepopulated_fields`` refers to - ````, which must not be a ``DateTimeField``, ``ForeignKey`` or - ``ManyToManyField``. + ````, which must not be a ``DateTimeField``, a foreign key or a + many-to-many field. * **admin.E029**: The value of ``prepopulated_fields[]`` must be a list or tuple. * **admin.E030**: The value of ``prepopulated_fields`` refers to @@ -343,7 +343,7 @@ with the admin site: which is not a callable, an attribute of ````, or an attribute or method on ````. * **admin.E109**: The value of ``list_display[n]`` must not be a - ``ManyToManyField``. + many-to-many field. * **admin.E110**: The value of ``list_display_links`` must be a list, a tuple, or ``None``. * **admin.E111**: The value of ``list_display_links[n]`` refers to ``