Pr 558 560 (#601)

* pdb_stats command fixes github-558, github-560

* stabilize key order in pdb_stats output (order differed on travis, causing tests to fail, and good to have it stable anyways)

config/facsimile/peeringdb.yaml

@@ -149,6 +149,7 @@ install:
     - $SRC_DIR$/peeringdb_server/management/commands/pdb_maintenance.py
     - $SRC_DIR$/peeringdb_server/management/commands/pdb_process_admin_tool_command.py
     - $SRC_DIR$/peeringdb_server/management/commands/pdb_load_data.py
+    - $SRC_DIR$/peeringdb_server/management/commands/pdb_fix_status_history.py
     - $SRC_DIR$/peeringdb_server/migrations/__init__.py
     - $SRC_DIR$/peeringdb_server/migrations/0001_initial.py
     - $SRC_DIR$/peeringdb_server/migrations/0002_partnernship_model.py

peeringdb_server/admin.py

@@ -926,8 +926,10 @@ class VerificationQueueAdmin(ModelAdminWithUrlActions):
                                                     opts.model_name))
 
     def vq_approve(self, request, queryset):
-        for each in queryset:
-            each.approve()
+        with reversion.create_revision():
+            reversion.set_user(request.user)
+            for each in queryset:
+                each.approve()
 
     vq_approve.short_description = _("APPROVE selected items")
 

peeringdb_server/management/commands/pdb_fix_status_history.py

@@ -0,0 +1,91 @@
+import json
+
+from django.core.management.base import BaseCommand
+from django.core import serializers
+
+import reversion
+
+from peeringdb_server.models import REFTAG_MAP
+
+
+class Command(BaseCommand):
+    help = "Fix object status in reversion archived data (#558)"
+
+    tags = ["fac", "net", "ix"]
+
+    def add_arguments(self, parser):
+        parser.add_argument(
+            '--commit', action='store_true',
+            help="commit changes, otherwise run in pretend mode")
+
+    def log(self, msg):
+        if not self.commit:
+            self.stdout.write(u"[pretend] {}".format(msg))
+        else:
+            self.stdout.write(msg)
+
+    def handle(self, *args, **options):
+        self.commit = options.get("commit", False)
+        for tag in self.tags:
+            self.process(tag)
+
+    def process(self, tag):
+        """
+        check model for instances where current object status
+        diverges from the status in the most recent archived
+        version of the object.
+
+        Then fix those instances by creating a new revision
+        with the correct status
+        """
+
+        model = REFTAG_MAP[tag]
+        self.log("Processing {} ...".format(model.__name__))
+
+        # no data is getting updated but object versions will be bumped
+        # by one because of the revision fix - in order to not spam "recently updated"
+        # lists with hundreds of empty updates we take out the automatic
+        # update for the `updated` field
+
+        model._meta.get_field("updated").auto_now = False
+
+        fixed = 0
+        for entity in model.objects.filter(status="ok"):
+            versions = reversion.models.Version.objects.get_for_object(entity)
+            version = versions.order_by("-revision_id").first()
+
+            if not version:
+                continue
+
+            # see what status is stored in the most recent archived version
+
+            archived_data = json.loads(version.serialized_data)
+            archived_status = archived_data[0].get("fields").get("status")
+
+            # if archived status is different than current object status
+            # create a new revision for the correct status
+
+            if archived_status != entity.status:
+                fixed += 1
+                self.log("Fixing {}-{} {} archived status: {}".format(
+                    tag, entity.id, version.id, entity.status))
+                if self.commit:
+                    self.process_entity(entity, version)
+
+        self.log("{} revisions fixed for {}".format(fixed, model.__name__))
+
+    @reversion.create_revision()
+    def process_entity(self, entity, most_recent_version):
+        # force revision date to be same as that of the most recent version
+        # so status change is archived at the correct date
+
+        reversion.set_date_created(most_recent_version.revision.date_created)
+
+        # note in comment why this revision was created
+
+        reversion.set_comment(
+            "Fixing status in object archives (script, #558)")
+
+        # add entity to revision
+
+        reversion.add_to_revision(entity)

peeringdb_server/management/commands/pdb_stats.py

@@ -35,16 +35,102 @@ class Command(BaseCommand):
         else:
             return obj.status
 
+
     def handle(self, *args, **options):
         date = options.get('date', None)
         if date:
             dt = datetime.datetime.strptime(date, "%Y%m%d")
+            stats = self.generate_for_past_date(dt)
         else:
-            dt = datetime.datetime.now()
+            stats = self.generate_for_current_date()
+
+        self.print_stats(stats, output_format=options.get("format"))
+
+    def stats(self, dt):
+        """
+        Generates and returns a fresh stats dict with user count
+        for the specified date
+
+        Argument(s)
+
+        - dt: datetime object
+
+        Return(s)
+
+        `dict`
+        """
+
+        stats = {"users": 0}
+
+        for user in get_user_model().objects.filter(created__lte=dt):
+            if user.is_verified:
+                stats["users"] += 1
+
+        return stats
+
+    def print_stats(self, stats, output_format="text"):
+        """
+        Output generated stats in a userfriendly format
+
+        Argument(s)
+
+        - stats: `dict` generated via `generate_for_current_date`
+
+        Keyword Argument(s)
+
+        - output_format: `str` ("text" or "json")
+        """
+
+        dt = stats["dt"]
+        stats = stats["stats"]
+
+        date = dt.replace(tzinfo=None).strftime("%Y-%m-%d")
+        if output_format == "text":
+            self.stdout.write(date)
+            self.stdout.write("-------------")
+            for each in self.tags + ["users"]:
+                self.stdout.write("{}: {}".format(each, stats[each]))
+        elif output_format == "json":
+            self.stdout.write(json.dumps({date: stats}))
+        else:
+            raise Exception("unknown format {}".format(output_format))
+
+    def generate_for_current_date(self):
+        """
+        Generate and return stats for current date
+
+        Returns
+
+        `dict` with `stats` and `dt` keys
+        """
+
+        dt = datetime.datetime.now().replace(tzinfo=UTC())
+
+        stats = self.stats(dt)
+
+        for tag in self.tags:
+            model = REFTAG_MAP[tag]
+            stats[tag] = model.objects.filter(status="ok").count()
+
+
+        return {"stats": stats, "dt": dt}
+
+
+    def generate_for_past_date(self, dt):
+        """
+        Generate and return stats for past date
+
+        Argument(s)
+
+        - dt: `datetime` instance
+
+        Returns
+
+        `dict` with `stats` and `dt` keys
+        """
 
         dt = dt.replace(hour=23, minute=23, second=59, tzinfo=UTC())
-        date = dt.replace(tzinfo=None).strftime("%Y-%m-%d")
-        stats = {"users": 0}
+        stats = self.stats(dt)
 
         for tag in self.tags:
             model = REFTAG_MAP[tag]
@@ -53,19 +139,5 @@ class Command(BaseCommand):
                 if self.status_at_date(obj, dt) == "ok":
                     stats[tag] += 1
 
-        for user in get_user_model().objects.filter(created__lte=dt):
-            if user.is_verified:
-                stats["users"] += 1
+        return {"stats": stats, "dt": dt}
 
-        codec = options.get("format")
-        if codec == "text":
-            print(date)
-            print("-------------")
-            for each in stats.keys():
-                print("{}: {}".format(each, stats[each]))
-
-        elif codec == "json":
-            print(json.dumps({date: stats}))
-
-        else:
-            raise Exception("unknown format {}".format(codec))

peeringdb_server/models.py

@@ -417,6 +417,7 @@ class VerificationQueueItem(models.Model):
                                      self._meta.model_name), args=(self.id,
                                                                    "vq_deny"))
 
+    @reversion.create_revision()
     def approve(self):
         """
         Approve the verification queue item

tests/data/stats/current/output.txt

@@ -0,0 +1,5 @@
+fac: 4
+ix: 4
+net: 4
+org: 7
+users: 4

tests/data/stats/past/output.txt

@@ -0,0 +1,7 @@
+2019-11-01
+-------------
+fac: 1
+ix: 1
+net: 1
+org: 1
+users: 2

tests/test_stats.py

@@ -0,0 +1,64 @@
+import StringIO
+import datetime
+import pytest
+
+from util import ClientCase, Group
+
+from django.core.management import call_command
+from django.conf import settings
+from django.contrib.auth import get_user_model
+
+from peeringdb_server.models import REFTAG_MAP, UTC
+
+
+DATE_PAST = datetime.datetime(year=2019,month=11,day=1)
+
+def setup_data():
+
+    call_command("pdb_generate_test_data", limit=3, commit=True)
+
+    date_past = DATE_PAST.replace(tzinfo=UTC())
+
+    # one object of each type moved to the past
+
+    for tag in ["fac","net","org","ix"]:
+        for obj in REFTAG_MAP[tag].objects.all():
+            obj.created = obj.updated = date_past
+            obj.save()
+            break
+
+    # create users
+
+    User = get_user_model()
+
+    for i in range(1,7):
+        User.objects.create_user("user_{}".format(i),
+                                 "user_{}@localhost".format(i),
+                                 "secret")
+
+    # move users 4, 5, 6 to the past
+
+    User.objects.filter(pk__in=[4,5,6]).update(created=date_past)
+
+    # verify all users except 1 and 4
+
+    for user in User.objects.exclude(pk__in=[1,4]):
+        user.set_verified()
+
+
+@pytest.mark.djangodb
+def test_generate_for_past_date(db, data_stats_past):
+    output = StringIO.StringIO()
+    setup_data()
+    call_command("pdb_stats", date=DATE_PAST.strftime("%Y%m%d"), stdout=output)
+    assert output.getvalue() == data_stats_past.txt
+
+
+@pytest.mark.djangodb
+def test_generate_for_current_date(db, data_stats_current):
+    output = StringIO.StringIO()
+    setup_data()
+    call_command("pdb_stats", stdout=output)
+    assert output.getvalue().find(data_stats_current.txt) > -1
+
+

tests/test_veriqueue.py

@@ -1,3 +1,4 @@
+import json
 import pytest
 
 from django.test import TestCase
@@ -6,6 +7,8 @@ from django.db import IntegrityError
 
 import peeringdb_server.models as models
 
+import reversion
+
 
 class VeriQueueTests(TestCase):
     """
@@ -83,6 +86,11 @@ class VeriQueueTests(TestCase):
         ix.refresh_from_db()
         self.assertEqual(ix.status, "ok")
 
+        # check that the status in the archive is correct (#558)
+
+        version = reversion.models.Version.objects.get_for_object(ix).order_by("-revision_id").first()
+        self.assertEqual(json.loads(version.serialized_data)[0]["fields"]["status"], "ok")
+
         # after approval vqi should no longer exist
         with self.assertRaises(models.VerificationQueueItem.DoesNotExist):
             vqi.refresh_from_db()