django-reversion: Diff view... (django-reversion-compare)

[jens]

Ich nutzt gern django-reversion... In PyLucid sind fast alle Modelle damit verknüpft.

Was mir fehlt ist eine "Diff" Ansicht, damit man sehen kann, was denn nun überhaupt geändert wurde.

Leider gibt es da nichts out-of-the-box, siehe: https://github.com/etianen/django-rever ... ting-Diffs

Bevor ich nun was eigenes mache, dachte ich mir, erweitere ich erstmal django-reversion um eine Allgemeinen Schnittstelle, damit man nicht von Null Anfangen muß. Also hab ich einen branch gemacht: https://github.com/jedie/django-reversion/compare/diff (siehe auch https://github.com/etianen/django-reversion/issues/147 )

Damit direkt etwas Nutzbares raus kommt, habe ich eine einfache "Generischen" Diff Ansicht gemacht: Es wird dabei ein difflib.ndiff() über eine Art Serialisierte Daten String gemacht. (wenn Pygments da ist, dann auch in Bund)

Screenshots, wie das ganze z.Z. aussieht, gibt es hier: http://www.pylucid.org/de/about/screens ... reversion/

Die Idee ist, das der Entwickler nur in seiner von VersionAdmin geerbten Admin Klasse die Methode make_diff() Sinnvoll implementieren muß. Denn ich denke viel mehr als ein diff über ein pformat kann man generisch nicht machen, oder? Sicherlich könnte man das noch weiter Optimieren. z.B. könnte man ein Datetime richtig Formatieren lassen. Aber auch da ist die Frage, ob ein Timedelta interessanter wäre...

Was haltet ihr von dem ganzen? Wer hilft mit?


EDIT: Mittlerweile gibt es: https://github.com/jedie/django-reversion-compare

[Leonidas]

Also ich find ja, dass das auf jeden Fall ne nützliche Sache ist...

Was haltet ihr von dem ganzen? Wer hilft mit?

Ich find das schaut irgendwie recht "user-feindlich" aus. Was den Look angeht, schau dir doch mal an wie GitHub das macht, da sieht man keinen Zahlenspam von Diff sondern Zeilennummern an der Seite und bei kleinen Änderungen wir ebenfalls gehighlightet welcher Part nun tatsächlich geändert wurde.

[jens]

Ja, das wäre sicherlich nett. Allerdings ist das IMHO nicht ohne zusätzliche Abhängigkeiten machbar. Das ist nicht gewollt.

Aber optionale Abhängigkeiten kann man ja nehmen. Schon jetzt wird in den "helpers" https://github.com/etianen/django-rever ... helpers.py auf http://code.google.com/p/google-diff-match-patch/ zurück gegriffen.

EDIT: So, ich hab nochmal daran gearbeitet. Nun sieht es so aus:

ALT: http://www.pylucid.org/media/pylucid.or ... iff_02.png
NEU: http://www.pylucid.org/media/pylucid.or ... iff_04.png

ALT: http://www.pylucid.org/media/pylucid.or ... iff_03.png
NEU: http://www.pylucid.org/media/pylucid.or ... iff_05.png

Das sieht schon einfacher aus, oder?

Doch ich Frage mich, ob ich nicht was besseres als repr() nutzten könnte.

[jens]

Mal abgesehen vom aussehen... Was ich nicht ganz verstehe, warum der Entwickler von django-reversion so darauf aus ist, die Funktionalität in einem separaten Projekt zu verschieben...

Ich sehe da nur Nachteile für alle, die django-reversion nutzten und als einzigen Vorteil fällt mir ein, das der Entwickeler von django-reversion keine Arbeit mit dem zusätzlichen Code hat.

Siehe https://github.com/etianen/django-reversion/issues/147

[Leonidas]

Mal abgesehen vom aussehen... Was ich nicht ganz verstehe, warum der Entwickler von django-reversion so darauf aus ist, die Funktionalität in einem separaten Projekt zu verschieben...

Steht doch da, hat er doch ausführlich erklärt? Ich hab da nicht alles gelesen, weils für mich nur mäßig interessant ist, aber ich lese da ne Reihe guter Gründe raus.

[jens]

Auch wenn ich es für essentiell für django-reversion halte, habe ich nun ein separates Projekt gestartet:

https://github.com/jedie/django-reversion-compare

[Leonidas]

https://github.com/jedie/django-reversion-compare

Warum hast du das von django-reversion geforkt? Das ist ja ein separates Projekt?

[jens]

Ich hatte ursprünglich django-reversion geforkt und im branch "diff" alles gemacht, weil ich ja wollte, das es in django-reversion integriert wird.

Nun hab ich einfach das repository umbenannt und nun ist alles in "master"...

[Leonidas]

Uhm...

[jens]

Was ist daran schlimm? So behalte ich die history... z.B. nutzte ich das beispiel project weiter, wenn auch ein wenig angepasst.

Erstes release: http://pypi.python.org/pypi/django-reversion-compare/

[Leonidas]

Was ist daran schlimm? So behalte ich die history...

Die History von einem anderen Projekt. Na klar, ich kann auch immer wenn ich ein neues Git-Repo mache Django forken und alles rauslöschen, aber sauber ist das irgendwie nicht.

[jens]

Das Projekt ist IMHO auf einen gute weg... Ich arbeite gerade daran, das many-to-many Felder auch im Vergleich vorkommen.

Da hab ich aber noch ein Problem, vielleicht kennt hier jemand eine Lösung:

Irgendwie komme ich nicht an die many-to-many Daten herran. Ich habe drei Varianten:

Code: Alles auswählen

obj = version.object_version.object
related = getattr(obj, field.name)
queryset = related.all()

Code: Alles auswählen

concrete_model = obj._meta.concrete_model
many_to_many_fields = concrete_model._meta.many_to_many
for field in many_to_many_fields:
    queryset = field.value_from_object(obj)

Code: Alles auswählen

obj = version.object_version.object
related = getattr(obj, field.name)
ids = version.field_dict[field.name]
queryset = many_related_manager.filter(pk__in=ids)

Dabei ist:
version: Instanz von reversion.models.Version
field: Instanz von django.db.models.fields.related.ManyToManyField und kommt von obj._meta.concrete_model._meta.many_to_many
many_related_manager: Instanz von django.db.models.fields.related.ManyRelatedManager

Keine davon liefert mir aber die Daten der aktuellen Version. Beim letzten habe ich die Liste der IDs bzw. pk... Die liste stimmt schonmal (kommt ja direkt von den sreializierten Daten) allerdings existieren im queryset nicht immer alle Objekte. Ein Hinweis: many_related_manager.all() liefert ebenfalls nicht alle existierenden Objekte.

Ideen?

EDIT: Ah, hab evtl. das Problem ausgemacht: In den RelatedManager wird anscheinend vorgefiltert bsp:

Code: Alles auswählen

                self.core_filters = {
                    '%s__%s' % (rel_field.name, attname): getattr(instance, attname)
                }
...

            def get_query_set(self):
...
                    return super(RelatedManager, self).get_query_set().using(db).filter(**self.core_filters)

Ich denke man bekommt also mit many_related_manager.all() immer nur die Objekte, die aktuell "verknüpft" sind. Somit fehlen die natürlich, wenn man sich einen alten Versionsstand ansieht.

EDIT2: ja, daran liegt es. work-a-round:

Code: Alles auswählen

obj = version.object_version.object
related = getattr(obj, field.name)
ids = version.field_dict[field.name]

old_core_filters = many_related_manager.core_filters
many_related_manager.core_filters = {}

queryset = many_related_manager.all().filter(pk__in=ids)

many_related_manager.core_filters = old_core_filters

Schön ist das aber nicht. Was kann man tun?

[jens]

Eine bessere Lösung:

Code: Alles auswählen

related_model = field.rel.to
ids = version.field_dict[field.name]
queryset = related_model.objects.all().filter(pk__in=ids)

Siehe: https://github.com/jedie/django-reversi ... 4f2a0f7790

Da hat man einen "frischen" Zugriff auf die Daten

Generell gibt es IMHO mit "related objects" (egal ob ForeignKey oder m2m) ein Problem in django-reversion: Wenn die "alten" Daten geändert oder gelöscht wurden, werden sie auch nicht automatisch rückgesetzt/hergestellt.
Aber das ist IMHO auch nur recht schwer allgemeingültig zu Lösen.

[jens]

Generell gibt es IMHO mit "related objects" (egal ob ForeignKey oder m2m) ein Problem in django-reversion...

Stimmt nicht, es kommt darauf an, wie die Models registriert wurden!

...und wieder was gelernt...

Siehe: https://github.com/etianen/django-rever ... nt-5622318

Test code: https://gist.github.com/2652502

Ausgaben:

Code: Alles auswählen

version 1: Dave [<Pet: Catworth>, <Pet: Dogwoth>]
version 2: Dave [<Pet: Catworth the second>]
revert to version 1: Dave [<Pet: Catworth>, <Pet: Dogwoth>]

[jens]

So, nun ist es endlich geklärt: Es kommt wohl mal wieder daher, das ich die Wörter version, revision und reversion nicht auseinander gehalten hab Dumm das sie sich so sehr optisch ähneln...

Der code tut nun was er soll: https://gist.github.com/2652502

[jens]

So gestern hab ich ein neues Release raus gebracht. Nun werden many-to-many Felder richtig dargestellt. Dabei hab ich mir einige Mühen gemacht um die Änderungen von Felder die nicht mit "follow" auch in django-reversion gespeichert werden richtig an zu zeigen.

many-to-many Felder die nicht mit "follow" mit gespeichert werden, werden mit einem Hinweis gekennzeichnet das die Angaben im History-Vergleich möglicherweise nicht mehr aktuell sind.

Nun gibt es auch einige unittests.

Jetzt fehlt eigentlich nur noch, das ich alle existierenden Model-Felder durchgehe und schaue, ob man die Standart-Darstellung noch verbessern kann.

Das letzte Bild bei https://github.com/jedie/django-reversi ... creenshots zeigt, die ein m2m Vergleich aussieht.

[jens]

Was den Look angeht, schau dir doch mal an wie GitHub das macht, da sieht man keinen Zahlenspam von Diff sondern Zeilennummern an der Seite und bei kleinen Änderungen wir ebenfalls gehighlightet welcher Part nun tatsächlich geändert wurde.

Kennst du ein Python Modul was das kann?

Hab zwar selbst mittels difflib was gebaut, siehe: https://github.com/jedie/django-reversi ... ers.py#L59 aber das funktioniert nicht mit python 2.6: https://github.com/jedie/django-reversi ... e/issues/5
Hab noch nicht nachgesehen, aber vermutlich wurde zwischen den Python Versionen an difflib gearbeitet. Natürlich kann ich nachsehen und anpassen, aber eine externe Lib die das besser kann wäre auch nett.