Merge lp:~robru/bileto/unify-audit-logging into lp:bileto

[Robert Bruce Park]

[lp:~robru/bileto/unify-audit-logging]

517. By Robert Bruce Park on 2016-06-10

Fix tests.

518. By Robert Bruce Park on 2016-06-10

Simplify.

=== modified file 'bileto/models.py'
--- bileto/models.py	2016-06-08 16:11:29 +0000
+++ bileto/models.py	2016-06-10 21:49:56 +0000
@@ -16,7 +16,7 @@
 from sqlalchemy.inspection import inspect
 from sqlalchemy.orm import relationship
 from sqlalchemy.dialects import sqlite
-from flask import session
+from flask import request, session
 
 from bileto.app import db
 
@@ -331,9 +331,41 @@
         """Allow requests to be sorted."""
         return self.request_id < other.request_id
 
-    def update(self, *args, **kwargs):
-        """Cache computed values in the db."""
-        super().update(*args, **kwargs)
+    def _audit(self, status, job_log, author='bileto-bot', **kwargs):
+        """Create a Comment object which records a change in the ticket."""
+        db.session.add(Comment(
+            log_url=job_log,
+            published_versions=kwargs.get('published_versions', EMPTY),
+            siloname=kwargs.get('siloname', self.siloname),
+            request_id=self.request_id,
+            author=author,
+            text=status,
+        ))
+
+    def audit(self, **kwargs):
+        """Create audit log messages as appropriate."""
+        no_audit = self.verboten | {'author', 'job_log'}
+        nick = environ.get('BLAME') or session.get('nickname')
+        kwargs['author'] = nick or kwargs.get('author') or 'bileto-bot'
+        status = kwargs.get('status')
+        if status and status != self.status:
+            self._audit(**kwargs)
+        else:
+            comment = dict(kwargs)
+            comment.setdefault('job_log', self.path)
+            for key, val in comment.items():
+                if key not in no_audit and hasattr(self, key):
+                    if self[key] != val:
+                        explicit = '{}: {}'.format(key, val)
+                        brief = 'Updated {}.'.format(key)
+                        self._audit(
+                            status=explicit if len(explicit) < 100 else brief,
+                            **comment)
+
+    def update(self, **kwargs):
+        """Ensure changes are logged & compute computed values."""
+        self.audit(**kwargs)
+        super().update(**kwargs)
         tokens = set(WORD.split((self.status or EMPTY).lower()))
         keys = sorted(set(super().keys()) - self.verboten)
         self.search = N.join([str(self[key] or EMPTY) for key in keys])
@@ -348,17 +380,11 @@
 
     def set_status(self, message):
         """Update status immediately."""
-        request_path = environ.get('LOG_PATH') or environ['REQUEST_PATH']
-        self.job_log = request_path
-        self.update(status=message)
-        # Add audit log comment.
-        db.session.add(Comment(
-            log_url=request_path,
-            published_versions=EMPTY,  # TODO
-            siloname=self.siloname,
-            request_id=self.request_id,
-            author=environ['BLAME'],
-            text=message))
+        self.update(
+            status=message,
+            job_log=environ.get('LOG_PATH') or
+            environ.get('REQUEST_PATH') or
+            request.path)
         self.commit()
         return message
 

=== modified file 'bileto/v1.py'
--- bileto/v1.py	2016-06-01 01:08:28 +0000
+++ bileto/v1.py	2016-06-10 21:49:56 +0000
@@ -52,29 +52,6 @@
         return 0
 
 
-def diff_dicts(alpha, beta):
-    """Identify which keys differ between dicts.
-
-    Only considers keys from the first dict.
-    """
-    fields = {key for key, val in alpha.items() if beta.get(key) != val}
-    fields -= set(Request.verboten)
-    fields -= {'author', 'job_log'}
-    return fields
-
-
-def audit(status, request_id, job_log, author='bileto-bot', **kwargs):
-    """Create an audit record of something that happened."""
-    comment = Comment(
-        log_url=job_log,
-        published_versions=kwargs.get('published_versions', ''),
-        siloname=kwargs.get('siloname', ''),
-        request_id=request_id,
-        author=author,
-        text=status)
-    db.session.add(comment)
-
-
 def do_pagination(args, pages, page, limit):
     """Calculate some pagination magic."""
     remaining = max(pages.total - (page * limit), 0)
@@ -126,9 +103,9 @@
     requestid = request.json.get('request_id')
     success = 200 if requestid else 201
     if not requestid:
-        req = Request()
+        request.json['creator'] = session['nickname']
+        req = Request(**request.json)
         db.session.add(req)
-        request.json['creator'] = session['nickname']
     else:
         try:
             req = Request.query.get(requestid)
@@ -136,20 +113,8 @@
         except Exception:
             db.session.rollback()
             abort(400)
-        nick = session.get('nickname')
-        diff = diff_dicts(request.json, dict(req))
-        if 'status' in diff:
-            audit(**request.json)
-        else:
-            kwargs = dict(request.json)
-            kwargs.setdefault('job_log', req.path)
-            kwargs['author'] = nick or kwargs.get('author') or 'bileto-bot'
-            for key in diff:
-                msg = '{}: {}'.format(key, request.json[key])
-                kwargs['status'] = msg if len(msg) < 100 else 'Updated ' + key
-                audit(**kwargs)
     req.update(**request.json)
-    db.session.commit()
+    req.commit()
     return jsonify(dict(request_id=req.request_id)), success