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

@@ -0,0 +1,3 @@
+from .mixins import HistoryResourceMixin
+
+__all__ = ["HistoryResourceMixin"]

taiga/projects/history/choices.py

@@ -0,0 +1,29 @@
+# Copyright (C) 2014 Andrey Antukh <niwi@niwi.be>
+# 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 <http://www.gnu.org/licenses/>.
+
+
+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")))

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)

taiga/projects/history/mixins.py

@@ -0,0 +1,76 @@
+# Copyright (C) 2014 Andrey Antukh <niwi@niwi.be>
+# Copyright (C) 2014 Jesús Espino <jespinog@gmail.com>
+# Copyright (C) 2014 David Barragán <bameda@dbarragan.com>
+# 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 <http://www.gnu.org/licenses/>.
+
+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)
+

taiga/projects/history/models.py

@@ -13,7 +13,6 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 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
 
-
+from .choices import HistoryType
-class HistoryType(enum.IntEnum):
+from .choices import HISTORY_TYPE_CHOICES
-    change = 1
-    create = 2
 
 
 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)

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
+    kwargs.update({
-    entry_model = get_model("history", "HistoryEntry")
+        "snapshot": fdiff.snapshot if need_real_snapshot else None,
-
+        "is_snapshot": need_real_snapshot,
-    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),
         "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)