From cd3cb7db620df23c671ca04c150ff85a82068f83 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Thu, 5 Jun 2014 12:41:52 +0200 Subject: [PATCH] History: improvements on history app - Handle deleted objects. - Add project to freezers. - Make values implementation optional. - Split choices to separated module. - Add own mixin. --- taiga/projects/history/__init__.py | 3 + taiga/projects/history/choices.py | 29 +++++++++ taiga/projects/history/freeze_impl.py | 23 +++++++ taiga/projects/history/mixins.py | 76 ++++++++++++++++++++++ taiga/projects/history/models.py | 13 +--- taiga/projects/history/services.py | 93 +++++++++++++++++++-------- 6 files changed, 200 insertions(+), 37 deletions(-) create mode 100644 taiga/projects/history/choices.py create mode 100644 taiga/projects/history/mixins.py diff --git a/taiga/projects/history/__init__.py b/taiga/projects/history/__init__.py index e69de29b..ff39480d 100644 --- a/taiga/projects/history/__init__.py +++ b/taiga/projects/history/__init__.py @@ -0,0 +1,3 @@ +from .mixins import HistoryResourceMixin + +__all__ = ["HistoryResourceMixin"] diff --git a/taiga/projects/history/choices.py b/taiga/projects/history/choices.py new file mode 100644 index 00000000..0895ca8c --- /dev/null +++ b/taiga/projects/history/choices.py @@ -0,0 +1,29 @@ +# Copyright (C) 2014 Andrey Antukh +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as +# published by the Free Software Foundation, either version 3 of the +# License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see . + + +import enum + +from django.utils.translation import ugettext_lazy as _ + + +class HistoryType(enum.IntEnum): + change = 1 + create = 2 + delete = 3 + + +HISTORY_TYPE_CHOICES = ((HistoryType.change, _("Change")), + (HistoryType.create, _("Create")), + (HistoryType.delete, _("Delete"))) diff --git a/taiga/projects/history/freeze_impl.py b/taiga/projects/history/freeze_impl.py index b6d79e06..03e469ca 100644 --- a/taiga/projects/history/freeze_impl.py +++ b/taiga/projects/history/freeze_impl.py @@ -150,6 +150,12 @@ def wikipage_values(diff): # Freezes #################### +def _generic_extract(obj:object, fields:list, default=None) -> dict: + result = {} + for fieldname in fields: + result[fieldname] = getattr(obj, fieldname, default) + return result + @as_tuple def extract_attachments(obj) -> list: @@ -162,6 +168,22 @@ def extract_attachments(obj) -> list: "order": attach.order} +def project_freezer(project) -> dict: + fields = ("name", + "slug", + "created_at", + "owner_id", + "public", + "total_milestones", + "total_story_points", + "tags", + "is_backlog_activated", + "is_kanban_activated", + "is_wiki_activated", + "is_issues_activated") + return _generic_extract(project, fields) + + def milestone_freezer(milestone) -> dict: snapshot = { "name": milestone.name, @@ -175,6 +197,7 @@ def milestone_freezer(milestone) -> dict: return snapshot + def userstory_freezer(us) -> dict: rp_cls = get_model("userstories", "RolePoints") rpqsd = rp_cls.objects.filter(user_story=us) diff --git a/taiga/projects/history/mixins.py b/taiga/projects/history/mixins.py new file mode 100644 index 00000000..b6f180e6 --- /dev/null +++ b/taiga/projects/history/mixins.py @@ -0,0 +1,76 @@ +# Copyright (C) 2014 Andrey Antukh +# Copyright (C) 2014 Jesús Espino +# Copyright (C) 2014 David Barragán +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as +# published by the Free Software Foundation, either version 3 of the +# License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see . + +import warnings + +from .services import take_snapshot + + +class HistoryResourceMixin(object): + """ + Rest Framework resource mixin for resources + susceptible to have models with history. + """ + + # This attribute will store the last history entry + # created for this resource. It is mainly used for + # notifications mixin. + __last_history = None + __object_saved = False + + def get_last_history(self): + if not self.__object_saved: + message = ("get_last_history() function called before any object are saved. " + "Seems you have a wrong mixing order on your resource.") + warnings.warn(message, RuntimeWarning) + return self.__last_history + + def get_object_for_snapshot(self, obj): + """ + Method that returns a model instance ready to snapshot. + It is by default noop, but should be overwrited when + snapshot ready instance is found in one of foreign key + fields. + """ + return obj + + def persist_history_snapshot(self, obj=None, delete:bool=False): + """ + Shortcut for resources with special save/persist + logic. + """ + + user = self.request.user + comment = self.request.DATA.get("comment", "") + + if obj is None: + obj = self.get_object() + + sobj = self.get_object_for_snapshot(obj) + if sobj != obj and delete: + delete = False + + self.__last_history = take_snapshot(sobj, comment=comment, user=user, delete=delete) + self.__object_saved = True + + def post_save(self, obj, created=False): + self.persist_history_snapshot() + super().post_save(obj, created=created) + + def pre_delete(self, obj): + self.persist_history_snapshot(obj, delete=True) + super().pre_delete(obj) + diff --git a/taiga/projects/history/models.py b/taiga/projects/history/models.py index 41647313..746aa409 100644 --- a/taiga/projects/history/models.py +++ b/taiga/projects/history/models.py @@ -13,7 +13,6 @@ # along with this program. If not, see . import uuid -import enum from django.utils.translation import ugettext_lazy as _ from django.db import models @@ -21,10 +20,8 @@ from django.db.models.loading import get_model from django.utils.functional import cached_property from django_pgjson.fields import JsonField - -class HistoryType(enum.IntEnum): - change = 1 - create = 2 +from .choices import HistoryType +from .choices import HISTORY_TYPE_CHOICES class HistoryEntry(models.Model): @@ -35,16 +32,12 @@ class HistoryEntry(models.Model): It is used for store object changes and comments. """ - - TYPE_CHOICES = ((HistoryType.change, _("Change")), - (HistoryType.create, _("Create"))) - id = models.CharField(primary_key=True, max_length=255, unique=True, editable=False, default=lambda: str(uuid.uuid1())) user = JsonField(blank=True, default=None, null=True) created_at = models.DateTimeField(auto_now_add=True) - type = models.SmallIntegerField(choices=TYPE_CHOICES) + type = models.SmallIntegerField(choices=HISTORY_TYPE_CHOICES) is_snapshot = models.BooleanField(default=False) key = models.CharField(max_length=255, null=True, default=None, blank=True) diff --git a/taiga/projects/history/services.py b/taiga/projects/history/services.py index 3a61ca5f..b2a18555 100644 --- a/taiga/projects/history/services.py +++ b/taiga/projects/history/services.py @@ -25,23 +25,26 @@ This is possible example: # Do something... history.persist_history(object, user=request.user) """ - +import logging from collections import namedtuple -from functools import partial, wraps, lru_cache from copy import deepcopy +from functools import partial +from functools import wraps +from functools import lru_cache from django.conf import settings +from django.contrib.contenttypes.models import ContentType +from django.core.paginator import Paginator, InvalidPage from django.db.models.loading import get_model from django.db import transaction as tx -from django.core.paginator import Paginator, InvalidPage -from django.contrib.contenttypes.models import ContentType - -from .models import HistoryType from taiga.mdrender.service import render as mdrender from taiga.mdrender.service import get_diff_of_htmls from taiga.base.utils.db import get_typename_for_model_class +from .models import HistoryType + + # Type that represents a freezed object FrozenObj = namedtuple("FrozenObj", ["key", "snapshot"]) FrozenDiff = namedtuple("FrozenDiff", ["key", "diff", "snapshot"]) @@ -52,6 +55,8 @@ _freeze_impl_map = {} # Dict containing registred containing with their values implementation. _values_impl_map = {} +log = logging.getLogger("taiga.history") + def make_key_from_model_object(obj:object) -> str: """ @@ -110,7 +115,16 @@ def freeze_model_instance(obj:object) -> FrozenObj: wrapped into FrozenObj. """ - typename = get_typename_for_model_class(obj.__class__) + model_cls = obj.__class__ + + # Additional query for test if object is really exists + # on the database or it is removed. + try: + obj = model_cls.objects.get(pk=obj.pk) + except model_cls.DoesNotExist: + return None + + typename = get_typename_for_model_class(model_cls) if typename not in _freeze_impl_map: raise RuntimeError("No implementation found for {}".format(typename)) @@ -160,10 +174,13 @@ def make_diff(oldobj:FrozenObj, newobj:FrozenObj) -> FrozenDiff: def make_diff_values(typename:str, fdiff:FrozenDiff) -> dict: """ Given a typename and diff, build a values dict for it. + If no implementation found for typename, warnig is raised in + logging and returns empty dict. """ if typename not in _values_impl_map: - raise RuntimeError("No implementation found for {}".format(typename)) + log.warning("No implementation found of '{}' for values.".format(typename)) + return {} impl_fn = _values_impl_map[typename] return impl_fn(fdiff.diff) @@ -209,7 +226,7 @@ def get_last_snapshot_for_key(key:str) -> FrozenObj: # Public api @tx.atomic -def take_snapshot(obj:object, *, comment:str="", user=None): +def take_snapshot(obj:object, *, comment:str="", user=None, delete:bool=False): """ Given any model instance with registred content type, create new history entry of "change" type. @@ -222,33 +239,53 @@ def take_snapshot(obj:object, *, comment:str="", user=None): typename = get_typename_for_model_class(obj.__class__) new_fobj = freeze_model_instance(obj) - old_fobj, need_snapshot = get_last_snapshot_for_key(key) + old_fobj, need_real_snapshot = get_last_snapshot_for_key(key) + + entry_model = get_model("history", "HistoryEntry") + user_id = None if user is None else user.id + user_name = "" if user is None else user.get_full_name() + + # Determine history type + if delete: + entry_type = HistoryType.delete + elif new_fobj and not old_fobj: + entry_type = HistoryType.create + elif new_fobj and old_fobj: + entry_type = HistoryType.change + else: + raise RuntimeError("Unexpected condition") + + kwargs = { + "user": {"pk": user_id, "name": user_name}, + "key": key, + "type": entry_type, + "comment": "", + "comment_html": "", + "diff": None, + "values": None, + "snapshot": None, + "is_snapshot": False, + } fdiff = make_diff(old_fobj, new_fobj) fvals = make_diff_values(typename, fdiff) # If diff and comment are empty, do # not create empty history entry - if not fdiff.diff and not comment and old_fobj != None: + if (not fdiff.diff and not comment + and old_fobj is not None + and entry_type != HistoryType.delete): + return None - entry_type = HistoryType.change if old_fobj else HistoryType.create - entry_model = get_model("history", "HistoryEntry") - - user_id = None if user is None else user.id - user_name = "" if user is None else user.get_full_name() - - kwargs = { - "user": {"pk": user_id, "name": user_name}, - "type": entry_type, - "key": key, - "diff": fdiff.diff, - "snapshot": fdiff.snapshot if need_snapshot else None, - "is_snapshot": need_snapshot, - "comment": comment, - "comment_html": mdrender(obj.project, comment), + kwargs.update({ + "snapshot": fdiff.snapshot if need_real_snapshot else None, + "is_snapshot": need_real_snapshot, "values": fvals, - } + "comment": comment, + "diff": fdiff.diff, + "comment_html": mdrender(obj.project, comment), + }) return entry_model.objects.create(**kwargs) @@ -267,12 +304,14 @@ def get_history_queryset_by_model_instance(obj:object, types=(HistoryType.change # Freeze implementatitions +from .freeze_impl import project_freezer from .freeze_impl import milestone_freezer from .freeze_impl import userstory_freezer from .freeze_impl import issue_freezer from .freeze_impl import task_freezer from .freeze_impl import wikipage_freezer +register_freeze_implementation("projects.project", project_freezer) register_freeze_implementation("milestones.milestone", milestone_freezer,) register_freeze_implementation("userstories.userstory", userstory_freezer) register_freeze_implementation("issues.issue", issue_freezer)