Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cleanup: Fix logic for implicit subscriptions #2

Merged
merged 3 commits into from
Nov 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 0 additions & 7 deletions subman/machine.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 1 addition & 4 deletions subman/subman.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down
11 changes: 10 additions & 1 deletion tests/test_basics.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import itertools
import unittest

from subman import SubscriptionManager, SubscriptionState
from subman import SubscriptionManager, SubscriptionPolicy, SubscriptionState


class SubmanTest(unittest.TestCase):
Expand Down Expand Up @@ -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()