From 738d9af1e8e38b0289b7dfa7c8a5413f5d0e20d1 Mon Sep 17 00:00:00 2001 From: Jacob Kaplan-Moss Date: Wed, 8 Mar 2006 17:53:55 +0000 Subject: [PATCH] magic-removal: fixed #1330: edit-inline works again on magic-removal. Note that the API will change *substantailly* before we're done (for example, this reintroduces core fields, which suck) but this at least gives us a place to start with. Many many thanks for Christopher Lenz, my new hero. git-svn-id: http://code.djangoproject.com/svn/django/branches/magic-removal@2502 bcc190cf-cafb-0310-a4f2-bffc1f526a37 --- .../templates/admin/edit_inline_stacked.html | 8 +- .../templates/admin/edit_inline_tabular.html | 84 +++++++++---------- django/core/management.py | 10 +++ django/db/models/fields/__init__.py | 47 +++++++---- django/db/models/fields/related.py | 36 ++++---- django/db/models/manipulators.py | 12 +-- django/db/models/related.py | 53 ++++++++++++ django/forms/__init__.py | 40 ++++----- 8 files changed, 177 insertions(+), 113 deletions(-) diff --git a/django/contrib/admin/templates/admin/edit_inline_stacked.html b/django/contrib/admin/templates/admin/edit_inline_stacked.html index 8c1c73752f..45aa0a4f58 100644 --- a/django/contrib/admin/templates/admin/edit_inline_stacked.html +++ b/django/contrib/admin/templates/admin/edit_inline_stacked.html @@ -12,11 +12,5 @@ {% admin_field_line bound_field %} {% endif %} {% endfor %} -
- -
{% endfor %} -
- -
- \ No newline at end of file + diff --git a/django/contrib/admin/templates/admin/edit_inline_tabular.html b/django/contrib/admin/templates/admin/edit_inline_tabular.html index 95aaf0fefb..e9535df02c 100644 --- a/django/contrib/admin/templates/admin/edit_inline_tabular.html +++ b/django/contrib/admin/templates/admin/edit_inline_tabular.html @@ -1,45 +1,43 @@ {% load admin_modify %} +
+

{{ bound_related_object.relation.opts.verbose_name_plural|capfirst }}

+ + {% for fw in bound_related_object.field_wrapper_list %} + {% if fw.needs_header %} + {{ fw.field.verbose_name|capfirst }} + {% endif %} + {% endfor %} + {% for fcw in bound_related_object.form_field_collection_wrappers %} + {% if change %}{% if original_row_needed %} + {% if fcw.obj.original %} + + {% endif %} + {% endif %}{% endif %} + {% if fcw.obj.errors %} + + {% endif %} + + {% for bound_field in fcw.bound_fields %} + {% if not bound_field.hidden %} + + {% endif %} + {% endfor %} + {% if bound_related_object.show_url %}{% endif %} + -
-

{{ bound_related_object.relation.opts.verbose_name_plural|capfirst }}

-
{{ fcw.obj.original }}
+ {{ fcw.obj.html_combined_error_list }} +
+ {% field_widget bound_field %} + + {% if fcw.obj.original %}View on site{% endif %} +
- - - {% for fw in bound_related_object.field_wrapper_list %} - {% if fw.needs_header %} - {{ fw.field.verbose_name|capfirst }} - {% endif %} - {% endfor %} - - - - - - - - - - {% for fcw in bound_related_object.form_field_collection_wrappers %} - - {% for bound_field in fcw.bound_fields %} - {% if not bound_field.hidden %} - - {% endif %} - {% endfor %} - - - {% endfor %} - -
 
- -
- {{ bound_field.html_error_list }} - {% field_widget bound_field %} - - -
-
\ No newline at end of file + {% endfor %} + + {% for fcw in bound_related_object.form_field_collection_wrappers %} + {% for bound_field in fcw.bound_fields %} + {% if bound_field.hidden %} + {% field_widget bound_field %} + {% endif %} + {% endfor %} + {% endfor %} + diff --git a/django/core/management.py b/django/core/management.py index 37d699a725..c78ebc4c28 100644 --- a/django/core/management.py +++ b/django/core/management.py @@ -954,6 +954,16 @@ def get_validation_errors(outfile, app=None): except models.FieldDoesNotExist: e.add(opts, '"ordering" refers to "%s", a field that doesn\'t exist.' % field_name) + # Check core=True, if needed. + for related in opts.get_followed_related_objects(): + try: + for f in related.opts.fields: + if f.core: + raise StopIteration + e.add(related.opts, "At least one field in %s should have core=True, because it's being edited inline by %s.%s." % (related.opts.object_name, opts.module_name, opts.object_name)) + except StopIteration: + pass + # Check unique_together. for ut in opts.unique_together: for field_name in ut: diff --git a/django/db/models/fields/__init__.py b/django/db/models/fields/__init__.py index 4c66e364ad..2dacd1cc55 100644 --- a/django/db/models/fields/__init__.py +++ b/django/db/models/fields/__init__.py @@ -74,7 +74,7 @@ class Field(object): self.primary_key = primary_key self.maxlength, self.unique = maxlength, unique self.blank, self.null = blank, null - self.rel, self.default = rel, default + self.core, self.rel, self.default = core, rel, default self.editable = editable self.validator_list = validator_list or [] self.prepopulate_from = prepopulate_from @@ -88,10 +88,6 @@ class Field(object): # Set db_index to True if the field has a relationship and doesn't explicitly set db_index. self.db_index = db_index - self.deprecated_args = [] - if core: - self.deprecated_args.append('core') - # Increase the creation counter, and save our local copy. self.creation_counter = Field.creation_counter Field.creation_counter += 1 @@ -219,14 +215,29 @@ class Field(object): params['validator_list'].append(curry(manipulator_validator_unique, self, opts, manipulator)) # Only add is_required=True if the field cannot be blank. Primary keys - # are a special case. - params['is_required'] = not self.blank and not self.primary_key + # are a special case, and fields in a related context should set this + # as False, because they'll be caught by a separate validator -- + # RequiredIfOtherFieldGiven. + params['is_required'] = not self.blank and not self.primary_key and not rel # BooleanFields (CheckboxFields) are a special case. They don't take # is_required or validator_list. if isinstance(self, BooleanField): del params['validator_list'], params['is_required'] + # If this field is in a related context, check whether any other fields + # in the related object have core=True. If so, add a validator -- + # RequiredIfOtherFieldsGiven -- to this FormField. + if rel and not self.blank and not isinstance(self, AutoField) and not isinstance(self, FileField): + # First, get the core fields, if any. + core_field_names = [] + for f in opts.fields: + if f.core and f != self: + core_field_names.extend(f.get_manipulator_field_names(name_prefix)) + # Now, if there are any, add the validator to this FormField. + if core_field_names: + params['validator_list'].append(validators.RequiredIfOtherFieldsGiven(core_field_names, gettext_lazy("This field is required."))) + # Finally, add the field_names. field_names = self.get_manipulator_field_names(name_prefix) return [man(field_name=field_names[i], **params) for i, man in enumerate(field_objs)] @@ -239,9 +250,8 @@ class Field(object): Given the full new_data dictionary (from the manipulator), returns this field's data. """ - #if rel: - # return new_data.get(self.name, [self.get_default()])[0] - #else: + if rel: + return new_data.get(self.name, [self.get_default()])[0] val = new_data.get(self.name, self.get_default()) if not self.empty_strings_allowed and val == '' and self.null: val = None @@ -397,12 +407,12 @@ class DateTimeField(DateField): def get_manipulator_new_data(self, new_data, rel=False): date_field, time_field = self.get_manipulator_field_names('') - #if rel: - # d = new_data.get(date_field, [None])[0] - # t = new_data.get(time_field, [None])[0] - #else: - d = new_data.get(date_field, None) - t = new_data.get(time_field, None) + if rel: + d = new_data.get(date_field, [None])[0] + t = new_data.get(time_field, [None])[0] + else: + d = new_data.get(date_field, None) + t = new_data.get(time_field, None) if d is not None and t is not None: return datetime.datetime.combine(d, t) return self.get_default() @@ -492,7 +502,10 @@ class FileField(Field): upload_field_name = self.get_manipulator_field_names('')[0] if new_data.get(upload_field_name, False): func = getattr(new_object, 'save_%s_file' % self.name) - func(new_data[upload_field_name]["filename"], new_data[upload_field_name]["content"]) + if rel: + func(new_data[upload_field_name][0]["filename"], new_data[upload_field_name][0]["content"]) + else: + func(new_data[upload_field_name]["filename"], new_data[upload_field_name]["content"]) def get_directory_name(self): return os.path.normpath(datetime.datetime.now().strftime(self.upload_to)) diff --git a/django/db/models/fields/related.py b/django/db/models/fields/related.py index 461a4ec162..d35e652b86 100644 --- a/django/db/models/fields/related.py +++ b/django/db/models/fields/related.py @@ -418,6 +418,10 @@ class ForeignKey(RelatedField, Field): kwargs['edit_inline'] = kwargs.pop('edit_inline_type') kwargs['rel'] = ManyToOne(to, to_field, + num_in_admin=kwargs.pop('num_in_admin', 3), + min_num_in_admin=kwargs.pop('min_num_in_admin', None), + max_num_in_admin=kwargs.pop('max_num_in_admin', None), + num_extra_on_change=kwargs.pop('num_extra_on_change', 1), edit_inline=kwargs.pop('edit_inline', False), related_name=kwargs.pop('related_name', None), limit_choices_to=kwargs.pop('limit_choices_to', None), @@ -427,10 +431,6 @@ class ForeignKey(RelatedField, Field): self.db_index = True - for name in ('num_in_admin', 'min_num_in_admin', 'max_num_in_admin', 'num_extra_on_change'): - if name in kwargs: - self.deprecated_args.append(name) - def get_attname(self): return '%s_id' % self.name @@ -501,6 +501,7 @@ class OneToOneField(RelatedField, IntegerField): kwargs['edit_inline'] = kwargs.pop('edit_inline_type') kwargs['rel'] = OneToOne(to, to_field, + num_in_admin=kwargs.pop('num_in_admin', 0), edit_inline=kwargs.pop('edit_inline', False), related_name=kwargs.pop('related_name', None), limit_choices_to=kwargs.pop('limit_choices_to', None), @@ -511,10 +512,6 @@ class OneToOneField(RelatedField, IntegerField): self.db_index = True - for name in ('num_in_admin',): - if name in kwargs: - self.deprecated_args.append(name) - def get_attname(self): return '%s_id' % self.name @@ -534,6 +531,7 @@ class ManyToManyField(RelatedField, Field): def __init__(self, to, **kwargs): kwargs['verbose_name'] = kwargs.get('verbose_name', None) kwargs['rel'] = ManyToMany(to, kwargs.pop('singular', None), + num_in_admin=kwargs.pop('num_in_admin', 0), related_name=kwargs.pop('related_name', None), filter_interface=kwargs.pop('filter_interface', None), limit_choices_to=kwargs.pop('limit_choices_to', None), @@ -542,9 +540,6 @@ class ManyToManyField(RelatedField, Field): if kwargs["rel"].raw_id_admin: kwargs.setdefault("validator_list", []).append(self.isValidIDList) Field.__init__(self, **kwargs) - for name in ('num_in_admin'): - if name in kwargs: - self.deprecated_args.append(name) if self.rel.raw_id_admin: msg = gettext_lazy('Separate multiple IDs with commas.') @@ -641,15 +636,17 @@ class ManyToManyField(RelatedField, Field): pass class ManyToOne: - def __init__(self, to, field_name, edit_inline=False, + def __init__(self, to, field_name, num_in_admin=3, min_num_in_admin=None, + max_num_in_admin=None, num_extra_on_change=1, edit_inline=False, related_name=None, limit_choices_to=None, lookup_overrides=None, raw_id_admin=False): try: to._meta - except AttributeError: + except AttributeError: # to._meta doesn't exist, so it must be RECURSIVE_RELATIONSHIP_CONSTANT assert isinstance(to, basestring), "'to' must be either a model, a model name or the string %r" % RECURSIVE_RELATIONSHIP_CONSTANT self.to, self.field_name = to, field_name - self.edit_inline = edit_inline - self.related_name = related_name + self.num_in_admin, self.edit_inline = num_in_admin, edit_inline + self.min_num_in_admin, self.max_num_in_admin = min_num_in_admin, max_num_in_admin + self.num_extra_on_change, self.related_name = num_extra_on_change, related_name self.limit_choices_to = limit_choices_to or {} self.lookup_overrides = lookup_overrides or {} self.raw_id_admin = raw_id_admin @@ -660,22 +657,23 @@ class ManyToOne: return self.to._meta.get_field(self.field_name) class OneToOne(ManyToOne): - def __init__(self, to, field_name, edit_inline=False, + def __init__(self, to, field_name, num_in_admin=0, edit_inline=False, related_name=None, limit_choices_to=None, lookup_overrides=None, raw_id_admin=False): self.to, self.field_name = to, field_name - self.edit_inline = edit_inline + self.num_in_admin, self.edit_inline = num_in_admin, edit_inline self.related_name = related_name self.limit_choices_to = limit_choices_to or {} self.lookup_overrides = lookup_overrides or {} self.raw_id_admin = raw_id_admin self.multiple = False - + class ManyToMany: - def __init__(self, to, singular=None, related_name=None, + def __init__(self, to, singular=None, num_in_admin=0, related_name=None, filter_interface=None, limit_choices_to=None, raw_id_admin=False, symmetrical=True): self.to = to self.singular = singular or None + self.num_in_admin = num_in_admin self.related_name = related_name self.filter_interface = filter_interface self.limit_choices_to = limit_choices_to or {} diff --git a/django/db/models/manipulators.py b/django/db/models/manipulators.py index a17973e454..0196496263 100644 --- a/django/db/models/manipulators.py +++ b/django/db/models/manipulators.py @@ -139,6 +139,9 @@ class AutomaticManipulator(forms.Manipulator): if child_follow: obj_list = expanded_data[related.var_name].items() + if not obj_list: + continue + obj_list.sort(lambda x, y: cmp(int(x[0]), int(y[0]))) # For each related item... @@ -187,15 +190,8 @@ class AutomaticManipulator(forms.Manipulator): if param != None: params[f.attname] = param - # Related links are a special case, because we have to - # manually set the "content_type_id" and "object_id" fields. - if self.opts.has_related_links and related.opts.module_name == 'relatedlinks': - contenttypes_mod = get_module('core', 'contenttypes') - params['content_type_id'] = contenttypes_mod.get_object(package__label__exact=self.opts.app_label, python_module_name__exact=self.opts.module_name).id - params['object_id'] = new_object.id - # Create the related item. - new_rel_obj = related.opts.get_model_module().Klass(**params) + new_rel_obj = related.model(**params) # If all the core fields were provided (non-empty), save the item. if all_cores_given: diff --git a/django/db/models/related.py b/django/db/models/related.py index b00088031e..64b6cb2880 100644 --- a/django/db/models/related.py +++ b/django/db/models/related.py @@ -41,6 +41,35 @@ class RelatedObject(object): """ return data # TODO + def get_list(self, parent_instance=None): + "Get the list of this type of object from an instance of the parent class." + if parent_instance is not None: + attr = getattr(parent_instance, self.get_accessor_name()) + if self.field.rel.multiple: + # For many-to-many relationships, return a list of objects + # corresponding to the xxx_num_in_admin options of the field + objects = list(attr.all()) + + count = len(objects) + self.field.rel.num_extra_on_change + if self.field.rel.min_num_in_admin: + count = max(count, self.field.rel.min_num_in_admin) + if self.field.rel.max_num_in_admin: + count = min(count, self.field.rel.max_num_in_admin) + + change = count - len(objects) + if change > 0: + return objects + [None for _ in range(change)] + if change < 0: + return objects[:change] + else: # Just right + return objects + else: + # A one-to-one relationship, so just return the single related + # object + return [attr] + else: + return [None for _ in range(self.field.rel.num_in_admin)] + def editable_fields(self): "Get the fields in this class that should be edited inline." return [f for f in self.opts.fields + self.opts.many_to_many if f.editable and f != self.field] @@ -62,6 +91,30 @@ class RelatedObject(object): over[self.field.name] = False return self.opts.get_follow(over) + def get_manipulator_fields(self, opts, manipulator, change, follow): + if self.field.rel.multiple: + if change: + attr = getattr(manipulator.original_object, self.get_accessor_name()) + count = attr.count() + count += self.field.rel.num_extra_on_change + if self.field.rel.min_num_in_admin: + count = max(count, self.field.rel.min_num_in_admin) + if self.field.rel.max_num_in_admin: + count = min(count, self.field.rel.max_num_in_admin) + else: + count = self.field.rel.num_in_admin + else: + count = 1 + + fields = [] + for i in range(count): + for f in self.opts.fields + self.opts.many_to_many: + if follow.get(f.name, False): + prefix = '%s.%d.' % (self.var_name, i) + fields.extend(f.get_manipulator_fields(self.opts, manipulator, change, + name_prefix=prefix, rel=True)) + return fields + def __repr__(self): return "" % (self.name, self.field.name) diff --git a/django/forms/__init__.py b/django/forms/__init__.py index 4d3c24c373..ba42706c9e 100644 --- a/django/forms/__init__.py +++ b/django/forms/__init__.py @@ -131,10 +131,10 @@ class FormWrapper: def fill_inline_collections(self): if not self._inline_collections: ic = [] - children = self.manipulator.children.items() - for rel_obj, child_manips in children: + related_objects = self.manipulator.get_related_objects() + for rel_obj in related_objects: data = rel_obj.extract_data(self.data) - inline_collection = InlineObjectCollection(self.manipulator, rel_obj, child_manips, data, self.error_dict) + inline_collection = InlineObjectCollection(self.manipulator, rel_obj, data, self.error_dict) ic.append(inline_collection) self._inline_collections = ic @@ -213,12 +213,11 @@ class FormFieldCollection(FormFieldWrapper): class InlineObjectCollection: "An object that acts like a sparse list of form field collections." - def __init__(self, parent_manipulator, rel_obj,child_manips, data, errors): + def __init__(self, parent_manipulator, rel_obj, data, errors): self.parent_manipulator = parent_manipulator self.rel_obj = rel_obj self.data = data self.errors = errors - self.child_manips = child_manips self._collections = None self.name = rel_obj.name @@ -240,7 +239,7 @@ class InlineObjectCollection: def __iter__(self): self.fill() - return self._collections.values().__iter__() + return iter(self._collections.values()) def items(self): self.fill() @@ -250,22 +249,25 @@ class InlineObjectCollection: if self._collections: return else: - #var_name = self.rel_obj.opts.object_name.lower() - cols = {} - #orig = hasattr(self.parent_manipulator, 'original_object') and self.parent_manipulator.original_object or None - #orig_list = self.rel_obj.get_list(orig) + var_name = self.rel_obj.opts.object_name.lower() + collections = {} + orig = None + if hasattr(self.parent_manipulator, 'original_object'): + orig = self.parent_manipulator.original_object + orig_list = self.rel_obj.get_list(orig) - for i, manip in enumerate(self.child_manips) : - if manip and not manip.needs_deletion: - collection = {'original': manip.original_object} - for field in manip.fields: - errors = self.errors.get(field.field_name, []) + for i, instance in enumerate(orig_list): + collection = {'original': instance} + for f in self.rel_obj.editable_fields(): + for field_name in f.get_manipulator_field_names(''): + full_field_name = '%s.%d.%s' % (var_name, i, field_name) + field = self.parent_manipulator[full_field_name] data = field.extract_data(self.data) - last_part = field.field_name[field.field_name.rindex('.') + 1:] - collection[last_part] = FormFieldWrapper(field, data, errors) + errors = self.errors.get(full_field_name, []) + collection[field_name] = FormFieldWrapper(field, data, errors) + collections[i] = FormFieldCollection(collection) + self._collections = collections - cols[i] = FormFieldCollection(collection) - self._collections = cols class FormField: """Abstract class representing a form field.