Sketched more of the new-process permissions

backend/models.py

@@ -131,6 +131,8 @@ class PersonVisitorPermissions(VisitorPermissions):
 
         This is used only when starting old-style processes
         """
+        # TODO: remove this once old-style processes get deprecated
+
         # Nothing can happen while the person is pending confirmation
         if self.person.pending: return []
         # Anonymous visitors cannot advocate

backend/unittest.py

@@ -8,6 +8,7 @@ from backend.models import Person, Process, AM
 from backend import const
 from django.utils.timezone import now
 from django.test import Client
+from collections import defaultdict
 import datetime
 import os
 import io
@@ -246,6 +247,33 @@ class PersonFixtureMixin(BaseFixtureMixin):
         cls.ams.create("dam", person=dam, is_fd=True, is_dam=True)
 
 
+class TestSet(set):
+    """
+    Set of strings that can be initialized from space-separated strings, and
+    changed with simple text patches.
+    """
+    def __init__(self, initial=""):
+        if initial: self.update(initial.split())
+
+    def set(self, vals):
+        self.clear()
+        self.update(vals.split())
+
+    def patch(self, diff):
+        for change in diff.split():
+            if change[0] == "+":
+                self.add(change[1:])
+            elif change[0] == "-":
+                self.discard(change[1:])
+            else:
+                raise RuntimeError("Changes {} contain {} that is nether an add nor a remove".format(repr(text), repr(change)))
+
+    def clone(self):
+        res = TestSet()
+        res.update(self)
+        return res
+
+
 class PatchExact(object):
     def __init__(self, text):
         if text:
@@ -279,11 +307,12 @@ class PatchDiff(object):
         return cur
 
 
-class ExpectedSets(dict):
+class ExpectedSets(defaultdict):
     """
     Store the permissions expected out of a *VisitorPermissions object
     """
     def __init__(self, action_msg="{visitor}", issue_msg="{problem} {mismatch}"):
+        super(ExpectedSets, self).__init__(TestSet)
         self.action_msg = action_msg
         self.issue_msg = issue_msg
 
@@ -291,20 +320,13 @@ class ExpectedSets(dict):
     def visitors(self):
         return self.keys()
 
-    def update(self, diff):
-        for visitors, change in diff.items():
-            for visitor in visitors.split():
-                cur = change.apply(self.get(visitor, None))
-                if not cur:
-                    self.pop(visitor, None)
-                else:
-                    self[visitor] = cur
-
     def set(self, visitors, text):
-        self.update({ visitors: PatchExact(text) })
+        for v in visitors.split():
+            self[v].set(text)
 
     def patch(self, visitors, text):
-        self.update({ visitors: PatchDiff(text) })
+        for v in visitors.split():
+            self[v].patch(text)
 
     def select_others(self, persons):
         other_visitors = set(persons.keys())
@@ -315,9 +337,9 @@ class ExpectedSets(dict):
     def combine(self, other):
         res = ExpectedSets(action_msg=self.action_msg, issue_msg=self.issue_msg)
         for k, v in self.items():
-            res[k] = v
+            res[k] = v.clone()
         for k, v in other.items():
-            res.setdefault(k, set()).update(v)
+            res[k].update(v)
         return res
 
     def assertEqual(self, testcase, visitor, got):

process/models.py

@@ -34,15 +34,28 @@ class ProcessVisitorPermissions(bmodels.PersonVisitorPermissions):
     def __init__(self, process, visitor):
         super(ProcessVisitorPermissions, self).__init__(process.person, visitor)
         self.process = process
+        self.process_frozen = self.process.frozen_by is not None
+        self.process_approved = self.process.approved_by is not None
+
+        if not self.process.closed and self.visitor is not None and not self.visitor.pending:
+            self.add("add_log")
 
         if self.visitor is None:
             pass
         elif self.visitor.is_admin:
             self.add("view_mbox")
+            if not self.process.closed:
+                if not self.process_frozen:
+                    self.add("proc_freeze")
+                elif self.process_approved:
+                    self.add("proc_unapprove")
+                else:
+                    self.update(("proc_unfreeze", "proc_approve"))
         elif self.visitor == self.person:
             self.add("view_mbox")
         elif self.visitor.is_active_am:
             self.add("view_mbox")
+        # TODO: advocates of this process can see the mailbox(?)
         #elif self.process.advocates.filter(pk=self.visitor.pk).exists():
         #    self.add("view_mbox")
 
@@ -55,12 +68,15 @@ class RequirementVisitorPermissions(ProcessVisitorPermissions):
         if self.visitor is None:
             pass
         elif self.visitor.is_admin:
-            self.add("edit_statements")
-        else:
+            if not self.process.closed:
+                self.update(("edit_statements", "req_approve", "req_unapprove"))
+        elif not self.process_frozen:
             if self.requirement.type == "intent":
                 if self.visitor == self.person: self.add("edit_statements")
+                if self.visitor.is_dd: self.update(("req_approve", "req_unapprove"))
             elif self.requirement.type == "sc_dmup":
                 if self.visitor == self.person: self.add("edit_statements")
+                if self.visitor.is_dd: self.update(("req_approve", "req_unapprove"))
             elif self.requirement.type == "advocate":
                 if self.process.applying_for == const.STATUS_DC_GA:
                     if self.visitor.status in (const.STATUS_DM, const.STATUS_DM_GA, const.STATUS_DD_NU, const.STATUS_DD_U):
@@ -77,11 +93,14 @@ class RequirementVisitorPermissions(ProcessVisitorPermissions):
                 elif self.process.applying_for == const.STATUS_DD_U:
                     if self.visitor.status in (const.STATUS_DD_NU, const.STATUS_DD_U):
                         self.add("edit_statements")
+                if self.visitor.is_dd: self.update(("req_approve", "req_unapprove"))
             elif self.requirement.type == "am_ok":
                 a = self.process.current_am_assignment
                 if a is not None:
                     if a.am.person == self.visitor:
                         self.add("edit_statements")
+                    elif self.visitor.is_active_am:
+                        self.update(("req_approve", "req_unapprove"))
 
 
 class ProcessManager(models.Manager):

process/tests/test_perms.py

@@ -8,7 +8,7 @@ from django.core.urlresolvers import reverse
 from django.utils.timezone import now
 from backend import const
 from backend import models as bmodels
-from backend.unittest import BaseFixtureMixin, PersonFixtureMixin, ExpectedSets, NamedObjects
+from backend.unittest import BaseFixtureMixin, PersonFixtureMixin, ExpectedSets, NamedObjects, TestSet
 import process.models as pmodels
 from .common import ProcessFixtureMixin
 
@@ -55,7 +55,7 @@ from .common import ProcessFixtureMixin
 
 class ProcExpected(object):
     def __init__(self):
-        self.advs = ExpectedSets("{visitor} advocating app", "{problem} target {mismatch}")
+        self.starts = TestSet()
         self.proc = ExpectedSets("{visitor} visiting app's process", "{problem} permissions {mismatch}")
         self.intent = ExpectedSets("{visitor} visiting app's intent requirement", "{problem} permissions {mismatch}")
         self.sc_dmup = ExpectedSets("{visitor} visiting app's sc_dmup requirement", "{problem} permissions {mismatch}")
@@ -139,12 +139,17 @@ class ProcExpected(object):
 class TestVisitApplicant(ProcessFixtureMixin, TestCase):
     def assertPerms(self, perms):
         # Check advocacy targets
-        for visitor in perms.advs.visitors:
-            visit_perms = self.processes.app.permissions_of(self.persons[visitor])
-            perms.advs.assertEqual(self, visitor, visit_perms.advocate_targets)
-        for visitor in perms.advs.select_others(self.persons):
-            visit_perms = self.processes.app.permissions_of(self.persons[visitor] if visitor else None)
-            perms.advs.assertEmpty(self, visitor, visit_perms.advocate_targets)
+        can_start = set(self.persons.app.possible_new_statuses)
+        if can_start != perms.starts:
+            extra = can_start - perms.starts
+            missing = perms.starts - can_start
+            msgs = []
+            if missing: msgs.append("missing: {}".format(", ".join(missing)))
+            if extra: msgs.append("extra: {}".format(", ".join(extra)))
+            self.fail("adv startable processes mismatch: " + "; ".join(msgs))
+
+        # If the process has not yet been created, we skip testing it
+        if "app" not in self.processes: return
 
         # Check process permissions
         for visitor in perms.proc.visitors:
@@ -152,7 +157,7 @@ class TestVisitApplicant(ProcessFixtureMixin, TestCase):
             perms.proc.assertEqual(self, visitor, visit_perms)
         for visitor in perms.proc.select_others(self.persons):
             visit_perms = self.processes.app.permissions_of(self.persons[visitor] if visitor else None)
-            perms.advs.assertEmpty(self, visitor, visit_perms)
+            perms.proc.assertEmpty(self, visitor, visit_perms)
 
         # Check requirements
         for req in ("intent", "sc_dmup", "advocate", "keycheck", "am_ok"):
@@ -232,6 +237,14 @@ class TestVisitApplicant(ProcessFixtureMixin, TestCase):
         self.processes.app.frozen_time = now()
         self.processes.app.save()
 
+    def _approve_process(self, visitor):
+        """
+        Set a process as approved by DAM
+        """
+        self.processes.app.approved_by = self.persons[visitor]
+        self.processes.app.approved_time = now()
+        self.processes.app.save()
+
     def _close_process(self):
         """
         Finalize a process
@@ -245,37 +258,56 @@ class TestVisitApplicant(ProcessFixtureMixin, TestCase):
         """
         Test all visit combinations for an applicant from dc to dc_ga, with a dm advocate
         """
-        # Start process
         self.persons.create("app", status=const.STATUS_DC)
+        expected = ProcExpected()
+        expected.starts.set("dc_ga dm dd_u dd_nu")
+        self.assertPerms(expected)
+
+        # Start process
         self.persons.create("adv", status=const.STATUS_DM)
         self.processes.create("app", person=self.persons.app, applying_for=const.STATUS_DC_GA)
-        expected = ProcExpected()
-        expected.advs.set("fd dam dd_nu dd_u", "dc_ga dm dd_u dd_nu")
-        expected.advs.set("adv dm dm_ga", "dc_ga")
-        expected.proc.set("fd dam app", "update_keycheck edit_bio edit_ldap view_person_audit_log view_mbox request_new_status")
+        expected.starts.patch("-dc_ga")
+        expected.proc.set("fd dam", "update_keycheck edit_bio edit_ldap view_person_audit_log view_mbox request_new_status proc_freeze")
+        expected.proc.set("app", "update_keycheck edit_bio edit_ldap view_person_audit_log view_mbox request_new_status")
         expected.proc.set("dd_nu dd_u", "view_person_audit_log update_keycheck")
-        expected.intent.set("fd dam app", "edit_statements")
-        expected.sc_dmup.set("fd dam app", "edit_statements")
-        expected.advocate.set("fd dam adv dd_nu dd_u dm dm_ga", "edit_statements")
+        expected.proc.patch("dc dc_ga dm dm_ga dd_nu dd_u dd_e dd_r fd dam app adv", "+add_log")
+        expected.intent.patch("fd dam app", "+edit_statements")
+        expected.intent.patch("fd dam dd_nu dd_u", "+req_approve +req_unapprove")
+        expected.sc_dmup.patch("fd dam app", "+edit_statements")
+        expected.sc_dmup.patch("fd dam dd_nu dd_u", "+req_approve +req_unapprove")
+        expected.advocate.patch("fd dam adv dd_nu dd_u dm dm_ga", "+edit_statements")
+        expected.advocate.patch("fd dam dd_nu dd_u", "+req_approve +req_unapprove")
         expected.keycheck = None
         expected.am_ok = None
         self.assertPerms(expected)
 
         # Freeze for review
         self._freeze_process("fd")
+        expected.proc.patch("fd dam", "-proc_freeze +proc_unfreeze +proc_approve")
         expected.proc.patch("app", "-edit_bio -edit_ldap")
         expected.intent.patch("app", "-edit_statements")
+        expected.intent.patch("dd_nu dd_u", "-req_approve -req_unapprove")
         expected.sc_dmup.patch("app", "-edit_statements")
+        expected.sc_dmup.patch("dd_nu dd_u", "-req_approve -req_unapprove")
         expected.advocate.patch("adv dd_nu dd_u dm dm_ga", "-edit_statements")
+        expected.advocate.patch("dd_nu dd_u", "-req_approve -req_unapprove")
         self.assertPerms(expected)
 
+        # Approve
+        self._approve_process("dam")
+        expected.proc.patch("fd dam", "-proc_unfreeze -proc_approve +proc_unapprove")
+
         # Finalize
-        self._close_process("fd")
-        expected.advs.patch("fd dam dd_nu dd_u adv dm dm_ga", "-dc_ga")
-        expected.proc.patch("fd dam", "-edit_ldap")
+        self._close_process()
+        expected.starts.patch("-dc_ga -dm +dm_ga")
+        expected.proc.patch("fd dam", "-edit_ldap -proc_unapprove")
+        expected.proc.patch("dc dc_ga dm dm_ga dd_nu dd_u dd_e dd_r fd dam app adv", "-add_log")
+        expected.intent.patch("fd dam", "-edit_statements -req_approve -req_unapprove")
+        expected.sc_dmup.patch("fd dam", "-edit_statements -req_approve -req_unapprove")
+        expected.advocate.patch("fd dam", "-edit_statements -req_approve -req_unapprove")
         self.assertPerms(expected)
 
-
+        # TODO: test log actions
         # TODO: intent with no statements
         # TODO: intent with a statement
         # TODO: intent approved