Opened 16 years ago

Closed 16 years ago

Last modified 16 years ago

#6117 closed (fixed)

newforms-admin change history needs to be implemented

Reported by: Karen Tracey <kmtracey@…> Owned by: Brian Rosner
Component: contrib.admin Version: newforms-admin
Severity: Keywords: change history nfa-blocker
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

This code in newforms-admin/django/contrib/admin/options.py save_change needs to be replaced with a real implementation:

        # 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.
        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 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)

Until then all the history for changes made using newforms-admin displays "No fields changed."

Attachments (7)

admin_log.diff (2.6 KB ) - added by Alex Gaynor 16 years ago.
This should work, brosner has said this should probably be in modelforms, I'm not familliar enough to work on those, however someone can use this work.
admin_log.2.diff (2.6 KB ) - added by Alex Gaynor 16 years ago.
Made it a bit more efficient
admin_log.3.diff (5.5 KB ) - added by Karen Tracey <kmtracey@…> 16 years ago.
admin_log.4.diff (5.6 KB ) - added by Karen Tracey <kmtracey@…> 16 years ago.
Fix error when adding 1st inline object, include quotes around object values like old code did (noticed during investigation of #6853)
admin_changed.diff (8.5 KB ) - added by Alex Gaynor 16 years ago.
Added some tests, there may be some test failures, I can't tell right now
admin_changed.2.diff (8.0 KB ) - added by Alex Gaynor 16 years ago.
Added some tests, there may be some test failures, I can't tell right now
6117.diff (9.0 KB ) - added by Alex Gaynor 16 years ago.
This fixes the test failures from the previous patch

Download all attachments as: .zip

Change History (19)

comment:1 by Karen Tracey <kmtracey@…>, 16 years ago

comment:2 by Brian Rosner, 16 years ago

Keywords: nfa-blocker added
Triage Stage: UnreviewedAccepted

This should be completed before merging since it will completely break the compatibility with the trunk admin.

comment:3 by Thomas Güttler, 16 years ago

Cc: hv@… added

FYI:

I wrote a small method (form_changed()) which compares the initial values of a form with
cleaned_data. It returns True if the form was changed.

http://www.djangosnippets.org/snippets/621/

May something like this can be used to implement the change history.

by Alex Gaynor, 16 years ago

Attachment: admin_log.diff added

This should work, brosner has said this should probably be in modelforms, I'm not familliar enough to work on those, however someone can use this work.

by Alex Gaynor, 16 years ago

Attachment: admin_log.2.diff added

Made it a bit more efficient

comment:4 by Alex Gaynor, 16 years ago

Has patch: set
Owner: changed from nobody to Alex Gaynor
Status: newassigned

comment:5 by Karen Tracey <kmtracey@…>, 16 years ago

I don't believe either of the existing patches covers logging changes to inline-edited objects. I'll attach what I have working that covers the inline-edited objects as well. Sorry I don't have more time to spend on this but maybe what I have will be useful in some way.

First, it makes the form responsible for reporting what has changed (I updated the logic I had originally used months ago to mirror what is currently in BaseForm's has_changed -- if this goes forward that logic should probably be consolidated in one place rather than being duplicated as I left it in the patch) via a changed_data property. This is something that could potentially be documented for general form users.

Similarly BaseModelFormSet is where the tracking of new/changed/deleted inline-edited objects goes. The formset stuff is not as clean as the single form case -- the information is generated as a side-effect of saving, which seems a bit iffy. Possibly the logic for determining what has been added/changed/deleted should be made independent of save -- save could then just use what added/changed/deleted information is reported by this split-out routine.

One thing this patch does that I really like is it suppresses saving of non-changed inline-edited objects. Thus if you've got upwards of a hundred inline-edited objects and tweak just one (typical for my particular use), you don't have to save all hundred or so, which seems like a win.

by Karen Tracey <kmtracey@…>, 16 years ago

Attachment: admin_log.3.diff added

by Karen Tracey <kmtracey@…>, 16 years ago

Attachment: admin_log.4.diff added

Fix error when adding 1st inline object, include quotes around object values like old code did (noticed during investigation of #6853)

comment:6 by Matthijs Kooijman <matthijs@…>, 16 years ago

Perhaps #4102 (which aims to keep track of changed fields with the model itself) could be of use here?

comment:7 by Brian Rosner, 16 years ago

Needs tests: set

Karen's patch is looking perfect. I can't see anything it is missing. Is there something I am? Otherwise just needs some tests and its RFC.

comment:8 by Brian Rosner, 16 years ago

Once again I don't fully read what Karen types ;) For purposes of inclusion of getting LogEntry working the patch is looking perfect.

by Alex Gaynor, 16 years ago

Attachment: admin_changed.diff added

Added some tests, there may be some test failures, I can't tell right now

by Alex Gaynor, 16 years ago

Attachment: admin_changed.2.diff added

Added some tests, there may be some test failures, I can't tell right now

by Alex Gaynor, 16 years ago

Attachment: 6117.diff added

This fixes the test failures from the previous patch

comment:9 by Brian Rosner, 16 years ago

Owner: changed from Alex Gaynor to Brian Rosner
Status: assignednew

Stealing this ticket. I am going to work on finishing this up and getting it committed.

comment:10 by Brian Rosner, 16 years ago

Status: newassigned

Oh and thanks for all your work Alex!

comment:11 by Brian Rosner, 16 years ago

Resolution: fixed
Status: assignedclosed

(In [7507]) 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.

comment:12 by Thomas Güttler, 16 years ago

Cc: hv@… removed
Note: See TracTickets for help on using tickets.
Back to Top