From abb7a7ff0fa9440d833434a52328f56ab0c425e3 Mon Sep 17 00:00:00 2001 From: Brian Rosner Date: Tue, 29 Apr 2008 05:45:46 +0000 Subject: [PATCH] newforms-admin: Fixed #6117 -- Implemented change history for the admin. This includes the ability to track changes on a newform. Model formsets now only return the changed/new objects saved. A big thanks to Karen Tracey and Alex Gaynor. git-svn-id: http://code.djangoproject.com/svn/django/branches/newforms-admin@7507 bcc190cf-cafb-0310-a4f2-bffc1f526a37 --- django/contrib/admin/options.py | 29 +++++++++++------- django/newforms/forms.py | 33 +++++++++++++-------- django/newforms/models.py | 17 +++++++---- tests/modeltests/model_formsets/models.py | 36 +++++++++++++++++++---- 4 files changed, 80 insertions(+), 35 deletions(-) diff --git a/django/contrib/admin/options.py b/django/contrib/admin/options.py index 29ce10abbd..0fe0bf071c 100644 --- a/django/contrib/admin/options.py +++ b/django/contrib/admin/options.py @@ -389,17 +389,26 @@ class ModelAdmin(BaseModelAdmin): for formset in formsets: formset.save() - # Construct the change message. TODO: Temporarily commented-out, - # as manipulator object doesn't exist anymore, and we don't yet - # have a way to get fields_added, fields_changed, fields_deleted. + # Construct the change message. change_message = [] - #if manipulator.fields_added: - #change_message.append(_('Added %s.') % get_text_list(manipulator.fields_added, _('and'))) - #if manipulator.fields_changed: - #change_message.append(_('Changed %s.') % get_text_list(manipulator.fields_changed, _('and'))) - #if manipulator.fields_deleted: - #change_message.append(_('Deleted %s.') % get_text_list(manipulator.fields_deleted, _('and'))) - #change_message = ' '.join(change_message) + if form.changed_data: + change_message.append(_('Changed %s.') % get_text_list(form.changed_data, _('and'))) + + if formsets: + for formset in formsets: + for added_object in formset.new_objects: + change_message.append(_('Added %s "%s".') + % (added_object._meta.verbose_name, added_object)) + for changed_object, changed_fields in formset.changed_objects: + change_message.append(_('Changed %s for %s "%s".') + % (get_text_list(changed_fields, _('and')), + changed_object._meta.verbose_name, + changed_object)) + for deleted_object in formset.deleted_objects: + change_message.append(_('Deleted %s "%s".') + % (deleted_object._meta.verbose_name, deleted_object)) + + change_message = ' '.join(change_message) if not change_message: change_message = _('No fields changed.') LogEntry.objects.log_action(request.user.id, ContentType.objects.get_for_model(model).id, pk_value, force_unicode(new_object), CHANGE, change_message) diff --git a/django/newforms/forms.py b/django/newforms/forms.py index 42dacec6ce..300afb2a5b 100644 --- a/django/newforms/forms.py +++ b/django/newforms/forms.py @@ -81,6 +81,7 @@ class BaseForm(StrAndUnicode): self.label_suffix = label_suffix self.empty_permitted = empty_permitted self._errors = None # Stores the errors after clean() has been called. + self._changed_data = None # The base_fields class attribute is the *class-wide* definition of # fields. Because a particular *instance* of the class might want to @@ -243,19 +244,25 @@ class BaseForm(StrAndUnicode): """ Returns True if data differs from initial. """ - # XXX: For now we're asking the individual widgets whether or not the - # data has changed. It would probably be more efficient to hash the - # initial data, store it in a hidden field, and compare a hash of the - # submitted data, but we'd need a way to easily get the string value - # for a given field. Right now, that logic is embedded in the render - # method of each widget. - for name, field in self.fields.items(): - prefixed_name = self.add_prefix(name) - data_value = field.widget.value_from_datadict(self.data, self.files, prefixed_name) - initial_value = self.initial.get(name, field.initial) - if field.widget._has_changed(initial_value, data_value): - return True - return False + return bool(self.changed_data) + + def _get_changed_data(self): + if self._changed_data is None: + self._changed_data = [] + # XXX: For now we're asking the individual widgets whether or not the + # data has changed. It would probably be more efficient to hash the + # initial data, store it in a hidden field, and compare a hash of the + # submitted data, but we'd need a way to easily get the string value + # for a given field. Right now, that logic is embedded in the render + # method of each widget. + for name, field in self.fields.items(): + prefixed_name = self.add_prefix(name) + data_value = field.widget.value_from_datadict(self.data, self.files, prefixed_name) + initial_value = self.initial.get(name, field.initial) + if field.widget._has_changed(initial_value, data_value): + self._changed_data.append(name) + return self._changed_data + changed_data = property(_get_changed_data) def _get_media(self): """ diff --git a/django/newforms/models.py b/django/newforms/models.py index f12b53dacb..79a9e1a457 100644 --- a/django/newforms/models.py +++ b/django/newforms/models.py @@ -328,8 +328,11 @@ class BaseModelFormSet(BaseFormSet): return self.save_existing_objects(commit) + self.save_new_objects(commit) def save_existing_objects(self, commit=True): + self.changed_objects = [] + self.deleted_objects = [] if not self.get_queryset(): return [] + # Put the objects from self.get_queryset into a dict so they are easy to lookup by pk existing_objects = {} for obj in self.get_queryset(): @@ -338,23 +341,25 @@ class BaseModelFormSet(BaseFormSet): for form in self.initial_forms: obj = existing_objects[form.cleaned_data[self.model._meta.pk.attname]] if self.can_delete and form.cleaned_data[DELETION_FIELD_NAME]: + self.deleted_objects.append(obj) obj.delete() else: - saved_instances.append(self.save_existing(form, obj, commit=commit)) + if form.changed_data: + self.changed_objects.append((obj, form.changed_data)) + saved_instances.append(self.save_existing(form, obj, commit=commit)) return saved_instances def save_new_objects(self, commit=True): - new_objects = [] + self.new_objects = [] for form in self.extra_forms: if not form.has_changed(): continue # If someone has marked an add form for deletion, don't save the - # object. At some point it would be nice if we didn't display - # the deletion widget for add forms. + # object. if self.can_delete and form.cleaned_data[DELETION_FIELD_NAME]: continue - new_objects.append(self.save_new(form, commit=commit)) - return new_objects + self.new_objects.append(self.save_new(form, commit=commit)) + return self.new_objects def add_fields(self, form, index): """Add a hidden field for the object's primary key.""" diff --git a/tests/modeltests/model_formsets/models.py b/tests/modeltests/model_formsets/models.py index 386cd04f03..89a4e0bf5c 100644 --- a/tests/modeltests/model_formsets/models.py +++ b/tests/modeltests/model_formsets/models.py @@ -50,9 +50,9 @@ Charles Baudelaire Gah! We forgot Paul Verlaine. Let's create a formset to edit the existing -authors with an extra form to add him. This time we'll use formset_for_queryset. -We *could* use formset_for_queryset to restrict the Author objects we edit, -but in that case we'll use it to display them in alphabetical order by name. +authors with an extra form to add him. We *could* pass in a queryset to +restrict the Author objects we edit, but in this case we'll use it to display +them in alphabetical order by name. >>> qs = Author.objects.order_by('name') >>> AuthorFormSet = _modelformset_factory(Author, extra=1, can_delete=False) @@ -79,8 +79,9 @@ but in that case we'll use it to display them in alphabetical order by name. >>> formset.is_valid() True +# Only changed or new objects are returned from formset.save() >>> formset.save() -[, , ] +[] >>> for author in Author.objects.order_by('name'): ... print author.name @@ -124,8 +125,9 @@ deltetion, make sure we don't save that form. >>> formset.is_valid() True +# No objects were changed or saved so nothing will come back. >>> formset.save() -[, , ] +[] >>> for author in Author.objects.order_by('name'): ... print author.name @@ -133,6 +135,28 @@ Arthur Rimbaud Charles Baudelaire Paul Verlaine +Let's edit a record to ensure save only returns that one record. + +>>> data = { +... 'form-TOTAL_FORMS': '4', # the number of forms rendered +... 'form-INITIAL_FORMS': '3', # the number of forms with initial data +... 'form-0-id': '2', +... 'form-0-name': 'Walt Whitman', +... 'form-1-id': '1', +... 'form-1-name': 'Charles Baudelaire', +... 'form-2-id': '3', +... 'form-2-name': 'Paul Verlaine', +... 'form-3-name': '', +... 'form-3-DELETE': '', +... } + +>>> formset = AuthorFormSet(data=data, queryset=qs) +>>> formset.is_valid() +True + +# One record has changed. +>>> formset.save() +[] # Inline Formsets ############################################################ @@ -199,7 +223,7 @@ book. True >>> formset.save() -[, ] +[] As you can see, 'Le Spleen de Paris' is now a book belonging to Charles Baudelaire.