Merge pull request #2 from cde-ev/fix/implicit

cleanup: Fix logic for implicit subscriptions

subman/machine.py

@@ -99,16 +99,9 @@ class SubscriptionPolicy(enum.IntEnum):
     moderated_opt_in = 2
     #: User may not subscribe by themselves, but can be administratively subscribed.
     invitation_only = 3
-    #: Only implicit subscribers are allowed. If the user is neither implicit subscriber
-    #: nor has subscription override, their subscription will be removed.
-    implicits_only = 4
     #: User is not allowed to be subscribed except by subscription override.
     none = 5
 
-    def is_implicit(self) -> bool:
-        """Whether or not the user is only allowed to be subscribed implicitly."""
-        return self == self.implicits_only
-
     def is_none(self) -> bool:
         """Whether or not the user is not allowed to be subscribed."""
         return self == self.none

subman/subman.py

@@ -131,8 +131,6 @@ def apply_action(self,
         :param old_state: The current state of the relation between the affected user and
             the affected subscription object.
         :param allow_unsub: If this is not True, prevent the user from becoming unsubscribed.
-            We recommend only using the policies SubscriptionPolicy.implicits_only and
-            None for objects using this feature.
             Warning: This feature is not compatible with users with old_state in
             {SubscriptionState.unsubscribed, SubscriptionState.unsubscription_override}
             regarding the respective object.
@@ -185,8 +183,7 @@ def _apply_cleanup(self,
         if old_state.is_subscribed() and policy.is_none():
             # If user is not allowed as subscriber, remove them.
             return SubscriptionState.none
-        elif (old_state == SubscriptionState.implicit and not is_implied
-              and policy.is_implicit()):
+        elif old_state == SubscriptionState.implicit and not is_implied:
             # If user is implicit subscriber and not implied, remove them.
             # This conditional is only relevant if implicit subscribers are written.
             return SubscriptionState.none

tests/test_basics.py

@@ -4,7 +4,7 @@
 import itertools
 import unittest
 
-from subman import SubscriptionManager, SubscriptionState
+from subman import SubscriptionManager, SubscriptionPolicy, SubscriptionState
 
 
 class SubmanTest(unittest.TestCase):
@@ -53,6 +53,15 @@ def test_written_states(self) -> None:
                 self.assertEqual(subman.written_states,
                                  all_states.difference(unwritten))
 
+    def test_implicit_cleanup(self) -> None:
+        """Ensure implicits are marked as obsolet if they are no longer implied."""
+        subman = SubscriptionManager()
+        state = SubscriptionState.implicit
+        policies = {SubscriptionPolicy.none, SubscriptionPolicy.moderated_opt_in,
+                    SubscriptionPolicy.subscribable, SubscriptionPolicy.invitation_only}
+        for policy in policies:
+            self.assertTrue(subman.is_obsolete(policy, state, is_implied=False))
+
 
 if __name__ == "__main__":
     unittest.main()