user_topics: Introduce visibility policy enum subclass in models.py.

This commit gives more readable code than using the `VISIBILITY_POLICY`
prefix.
This commit does not alter the database schema.
This commit is contained in:
Abhijeet Prasad Bodas 2023-03-12 20:49:42 +05:30 committed by Tim Abbott
parent 4d5e12961a
commit ba0dd70904
13 changed files with 98 additions and 77 deletions

View File

@ -786,7 +786,7 @@ def do_update_message(
muting_user,
stream_being_edited,
orig_topic_name,
visibility_policy=UserTopic.VISIBILITY_POLICY_INHERIT,
visibility_policy=UserTopic.VisibilityPolicy.INHERIT,
)
else:
# Otherwise, we move the muted topic record for the
@ -796,9 +796,9 @@ def do_update_message(
muting_user,
stream_being_edited,
orig_topic_name,
visibility_policy=UserTopic.VISIBILITY_POLICY_INHERIT,
visibility_policy=UserTopic.VisibilityPolicy.INHERIT,
# do_set_user_topic_visibility_policy with visibility_policy
# set to UserTopic.MUTED will send an updated muted topic
# set to UserTopic.VisibilityPolicy.MUTED will send an updated muted topic
# event, which contains the full set of muted
# topics, just after this.
skip_muted_topics_event=True,
@ -808,7 +808,7 @@ def do_update_message(
muting_user,
new_stream if new_stream is not None else stream_being_edited,
topic_name if topic_name is not None else orig_topic_name,
visibility_policy=UserTopic.MUTED,
visibility_policy=UserTopic.VisibilityPolicy.MUTED,
ignore_duplicate=True,
)

View File

@ -248,7 +248,7 @@ def get_recipient_info(
if user_allows_notifications_in_StreamTopic(
row["is_muted"],
user_id_to_visibility_policy.get(
row["user_profile_id"], UserTopic.VISIBILITY_POLICY_INHERIT
row["user_profile_id"], UserTopic.VisibilityPolicy.INHERIT
),
row[setting],
row["user_profile_" + setting],

View File

@ -1372,7 +1372,7 @@ def apply_event(
state["user_status"] = user_status
elif event["type"] == "user_topic":
if event["visibility_policy"] == UserTopic.VISIBILITY_POLICY_INHERIT:
if event["visibility_policy"] == UserTopic.VisibilityPolicy.INHERIT:
user_topics_state = state["user_topics"]
for i in range(len(user_topics_state)):
if (

View File

@ -184,10 +184,10 @@ def user_allows_notifications_in_StreamTopic(
Captures the hierarchy of notification settings, where visibility policy is considered first,
followed by stream-specific settings, and the global-setting in the UserProfile is the fallback.
"""
if stream_is_muted and visibility_policy != UserTopic.UNMUTED:
if stream_is_muted and visibility_policy != UserTopic.VisibilityPolicy.UNMUTED:
return False
if visibility_policy == UserTopic.MUTED:
if visibility_policy == UserTopic.VisibilityPolicy.MUTED:
return False
if stream_specific_setting is not None:

View File

@ -68,7 +68,7 @@ def get_topic_mutes(
user_profile=user_profile,
include_deactivated=include_deactivated,
include_stream_name=True,
visibility_policy=UserTopic.MUTED,
visibility_policy=UserTopic.VisibilityPolicy.MUTED,
)
return [
@ -88,7 +88,7 @@ def set_topic_mutes(
UserTopic.objects.filter(
user_profile=user_profile,
visibility_policy=UserTopic.MUTED,
visibility_policy=UserTopic.VisibilityPolicy.MUTED,
).delete()
if date_muted is None:
@ -103,7 +103,7 @@ def set_topic_mutes(
stream_id=stream.id,
recipient_id=recipient_id,
topic_name=topic_name,
visibility_policy=UserTopic.MUTED,
visibility_policy=UserTopic.VisibilityPolicy.MUTED,
last_updated=date_muted,
)
@ -119,7 +119,7 @@ def set_user_topic_visibility_policy_in_database(
last_updated: Optional[datetime.datetime] = None,
ignore_duplicate: bool = False,
) -> None:
if visibility_policy == UserTopic.VISIBILITY_POLICY_INHERIT:
if visibility_policy == UserTopic.VisibilityPolicy.INHERIT:
try:
# Will throw UserTopic.DoesNotExist if the user doesn't
# already have a visibility policy for this topic.
@ -174,7 +174,7 @@ def topic_is_muted(user_profile: UserProfile, stream_id: int, topic_name: str) -
user_profile=user_profile,
stream_id=stream_id,
topic_name__iexact=topic_name,
visibility_policy=UserTopic.MUTED,
visibility_policy=UserTopic.VisibilityPolicy.MUTED,
).exists()
return is_muted
@ -187,7 +187,7 @@ def exclude_topic_mutes(
# never filtered from the query in this method.
query = UserTopic.objects.filter(
user_profile=user_profile,
visibility_policy=UserTopic.MUTED,
visibility_policy=UserTopic.VisibilityPolicy.MUTED,
)
if stream_id is not None:
@ -220,7 +220,7 @@ def exclude_topic_mutes(
def build_topic_mute_checker(user_profile: UserProfile) -> Callable[[int, str], bool]:
rows = UserTopic.objects.filter(
user_profile=user_profile, visibility_policy=UserTopic.MUTED
user_profile=user_profile, visibility_policy=UserTopic.VisibilityPolicy.MUTED
).values(
"recipient_id",
"topic_name",
@ -242,7 +242,7 @@ def get_users_muting_topic(stream_id: int, topic_name: str) -> QuerySet[UserProf
return UserProfile.objects.select_related("realm").filter(
id__in=UserTopic.objects.filter(
stream_id=stream_id,
visibility_policy=UserTopic.MUTED,
visibility_policy=UserTopic.VisibilityPolicy.MUTED,
topic_name__iexact=topic_name,
).values("user_profile_id")
)

View File

@ -2651,33 +2651,29 @@ class UserTopic(models.Model):
default=datetime.datetime(2020, 1, 1, 0, 0, tzinfo=datetime.timezone.utc)
)
# Implicitly, if a UserTopic does not exist, the (user, topic)
# pair should have normal behavior for that (user, stream) pair.
class VisibilityPolicy(models.IntegerChoices):
# A normal muted topic. No notifications and unreads hidden.
MUTED = 1, "Muted topic"
# We use this in our code to represent the condition in the comment above.
VISIBILITY_POLICY_INHERIT = 0
# This topic will behave like an unmuted topic in an unmuted stream even if it
# belongs to a muted stream.
UNMUTED = 2, "Unmuted topic in muted stream"
# A normal muted topic. No notifications and unreads hidden.
MUTED = 1
# This topic will behave like `UNMUTED`, plus some additional
# display and/or notifications priority that is TBD and likely to
# be configurable; see #6027. Not yet implemented.
FOLLOWED = 3, "Followed topic"
# This topic will behave like an unmuted topic in an unmuted stream even if it
# belongs to a muted stream.
UNMUTED = 2
# Implicitly, if a UserTopic does not exist, the (user, topic)
# pair should have normal behavior for that (user, stream) pair.
# This topic will behave like `UNMUTED`, plus some additional
# display and/or notifications priority that is TBD and likely to
# be configurable; see #6027. Not yet implemented.
FOLLOWED = 3
# We use this in our code to represent the condition in the comment above.
INHERIT = 0, "User's default policy for the stream."
visibility_policy_choices = (
(MUTED, "Muted topic"),
(UNMUTED, "Unmuted topic in muted stream"),
(FOLLOWED, "Followed topic"),
(VISIBILITY_POLICY_INHERIT, "User's default policy for the stream."),
visibility_policy = models.SmallIntegerField(
choices=VisibilityPolicy.choices, default=VisibilityPolicy.MUTED
)
visibility_policy = models.SmallIntegerField(choices=visibility_policy_choices, default=MUTED)
class Meta:
constraints = [
models.UniqueConstraint(

View File

@ -338,7 +338,7 @@ class MissedMessageHookTest(ZulipTestCase):
self.user_profile,
get_stream("Denmark", self.user_profile.realm),
"mutingtest",
visibility_policy=UserTopic.MUTED,
visibility_policy=UserTopic.VisibilityPolicy.MUTED,
)
msg_id = self.send_stream_message(
self.iago, "Denmark", topic_name="mutingtest", content="@**all** what's up?"
@ -539,7 +539,7 @@ class MissedMessageHookTest(ZulipTestCase):
self.user_profile,
get_stream("Denmark", self.user_profile.realm),
"mutingtest",
visibility_policy=UserTopic.MUTED,
visibility_policy=UserTopic.VisibilityPolicy.MUTED,
)
msg_id = self.send_stream_message(
self.iago, "Denmark", topic_name="mutingtest", content="what's up everyone?"
@ -597,7 +597,7 @@ class MissedMessageHookTest(ZulipTestCase):
self.user_profile,
get_stream("Denmark", self.user_profile.realm),
"mutingtest",
visibility_policy=UserTopic.MUTED,
visibility_policy=UserTopic.VisibilityPolicy.MUTED,
)
msg_id = self.send_stream_message(
self.iago,
@ -642,7 +642,7 @@ class MissedMessageHookTest(ZulipTestCase):
self.user_profile,
get_stream("Denmark", self.user_profile.realm),
"unmutingtest",
visibility_policy=UserTopic.UNMUTED,
visibility_policy=UserTopic.VisibilityPolicy.UNMUTED,
)
msg_id = self.send_stream_message(
self.iago,
@ -677,7 +677,7 @@ class MissedMessageHookTest(ZulipTestCase):
self.user_profile,
get_stream("Denmark", self.user_profile.realm),
"unmutingtest",
visibility_policy=UserTopic.UNMUTED,
visibility_policy=UserTopic.VisibilityPolicy.UNMUTED,
)
msg_id = self.send_stream_message(
self.iago,
@ -710,7 +710,7 @@ class MissedMessageHookTest(ZulipTestCase):
self.user_profile,
get_stream("Denmark", self.user_profile.realm),
"unmutingtest",
visibility_policy=UserTopic.UNMUTED,
visibility_policy=UserTopic.VisibilityPolicy.UNMUTED,
)
msg_id = self.send_stream_message(
self.iago,
@ -740,7 +740,7 @@ class MissedMessageHookTest(ZulipTestCase):
self.user_profile,
get_stream("Denmark", self.user_profile.realm),
"unmutingtest",
visibility_policy=UserTopic.UNMUTED,
visibility_policy=UserTopic.VisibilityPolicy.UNMUTED,
)
msg_id = self.send_stream_message(
self.iago,

View File

@ -1429,7 +1429,10 @@ class NormalActionsTest(BaseAction):
stream = get_stream("Denmark", self.user_profile.realm)
events = self.verify_action(
lambda: do_set_user_topic_visibility_policy(
self.user_profile, stream, "topic", visibility_policy=UserTopic.MUTED
self.user_profile,
stream,
"topic",
visibility_policy=UserTopic.VisibilityPolicy.MUTED,
),
num_events=2,
)
@ -1441,7 +1444,7 @@ class NormalActionsTest(BaseAction):
self.user_profile,
stream,
"topic",
visibility_policy=UserTopic.VISIBILITY_POLICY_INHERIT,
visibility_policy=UserTopic.VisibilityPolicy.INHERIT,
),
num_events=2,
)
@ -1450,7 +1453,10 @@ class NormalActionsTest(BaseAction):
events = self.verify_action(
lambda: do_set_user_topic_visibility_policy(
self.user_profile, stream, "topic", visibility_policy=UserTopic.MUTED
self.user_profile,
stream,
"topic",
visibility_policy=UserTopic.VisibilityPolicy.MUTED,
),
event_types=["muted_topics", "user_topic"],
)
@ -1460,7 +1466,10 @@ class NormalActionsTest(BaseAction):
stream = get_stream("Denmark", self.user_profile.realm)
events = self.verify_action(
lambda: do_set_user_topic_visibility_policy(
self.user_profile, stream, "topic", visibility_policy=UserTopic.UNMUTED
self.user_profile,
stream,
"topic",
visibility_policy=UserTopic.VisibilityPolicy.UNMUTED,
),
num_events=2,
)

View File

@ -776,7 +776,7 @@ class RealmImportExportTest(ExportFile):
sample_user,
stream,
"Verona2",
visibility_policy=UserTopic.MUTED,
visibility_policy=UserTopic.VisibilityPolicy.MUTED,
)
# data to test import of muted users
@ -1121,7 +1121,7 @@ class RealmImportExportTest(ExportFile):
def get_muted_topics(r: Realm) -> Set[str]:
user_profile_id = get_user_id(r, "King Hamlet")
muted_topics = UserTopic.objects.filter(
user_profile_id=user_profile_id, visibility_policy=UserTopic.MUTED
user_profile_id=user_profile_id, visibility_policy=UserTopic.VisibilityPolicy.MUTED
)
topic_names = {muted_topic.topic_name for muted_topic in muted_topics}
return topic_names
@ -1765,17 +1765,17 @@ class SingleUserExportTest(ExportFile):
self.assertEqual(rec["status_text"], "on vacation")
do_set_user_topic_visibility_policy(
cordelia, scotland, "bagpipe music", visibility_policy=UserTopic.MUTED
cordelia, scotland, "bagpipe music", visibility_policy=UserTopic.VisibilityPolicy.MUTED
)
do_set_user_topic_visibility_policy(
othello, scotland, "nessie", visibility_policy=UserTopic.MUTED
othello, scotland, "nessie", visibility_policy=UserTopic.VisibilityPolicy.MUTED
)
@checker
def zerver_usertopic(records: List[Record]) -> None:
rec = records[-1]
self.assertEqual(rec["topic_name"], "bagpipe music")
self.assertEqual(rec["visibility_policy"], UserTopic.MUTED)
self.assertEqual(rec["visibility_policy"], UserTopic.VisibilityPolicy.MUTED)
"""
For some tables we don't bother with super realistic test data

View File

@ -476,7 +476,7 @@ class FixUnreadTests(ZulipTestCase):
user,
stream,
topic_name,
visibility_policy=UserTopic.MUTED,
visibility_policy=UserTopic.VisibilityPolicy.MUTED,
)
def force_unsubscribe(stream_name: str) -> None:
@ -715,7 +715,7 @@ class GetUnreadMsgsTest(ZulipTestCase):
user_profile,
stream,
topic_name,
visibility_policy=UserTopic.MUTED,
visibility_policy=UserTopic.VisibilityPolicy.MUTED,
)
def test_raw_unread_stream(self) -> None:

View File

@ -28,7 +28,7 @@ class MutedTopicsTests(ZulipTestCase):
user,
stream,
"Verona3",
visibility_policy=UserTopic.MUTED,
visibility_policy=UserTopic.VisibilityPolicy.MUTED,
last_updated=datetime(2020, 1, 1, tzinfo=timezone.utc),
)
@ -51,7 +51,9 @@ class MutedTopicsTests(ZulipTestCase):
topic_name=topic_name,
)
user_ids = stream_topic_target.user_ids_with_visibility_policy(UserTopic.MUTED)
user_ids = stream_topic_target.user_ids_with_visibility_policy(
UserTopic.VisibilityPolicy.MUTED
)
self.assertEqual(user_ids, set())
def mute_topic_for_user(user: UserProfile) -> None:
@ -59,23 +61,27 @@ class MutedTopicsTests(ZulipTestCase):
user,
stream,
"test TOPIC",
visibility_policy=UserTopic.MUTED,
visibility_policy=UserTopic.VisibilityPolicy.MUTED,
last_updated=date_muted,
)
mute_topic_for_user(hamlet)
user_ids = stream_topic_target.user_ids_with_visibility_policy(UserTopic.MUTED)
user_ids = stream_topic_target.user_ids_with_visibility_policy(
UserTopic.VisibilityPolicy.MUTED
)
self.assertEqual(user_ids, {hamlet.id})
hamlet_date_muted = UserTopic.objects.filter(
user_profile=hamlet, visibility_policy=UserTopic.MUTED
user_profile=hamlet, visibility_policy=UserTopic.VisibilityPolicy.MUTED
)[0].last_updated
self.assertEqual(hamlet_date_muted, date_muted)
mute_topic_for_user(cordelia)
user_ids = stream_topic_target.user_ids_with_visibility_policy(UserTopic.MUTED)
user_ids = stream_topic_target.user_ids_with_visibility_policy(
UserTopic.VisibilityPolicy.MUTED
)
self.assertEqual(user_ids, {hamlet.id, cordelia.id})
cordelia_date_muted = UserTopic.objects.filter(
user_profile=cordelia, visibility_policy=UserTopic.MUTED
user_profile=cordelia, visibility_policy=UserTopic.VisibilityPolicy.MUTED
)[0].last_updated
self.assertEqual(cordelia_date_muted, date_muted)
@ -108,7 +114,7 @@ class MutedTopicsTests(ZulipTestCase):
user,
stream,
"Verona3",
visibility_policy=UserTopic.VISIBILITY_POLICY_INHERIT,
visibility_policy=UserTopic.VisibilityPolicy.INHERIT,
)
assert stream.recipient is not None
@ -139,7 +145,7 @@ class MutedTopicsTests(ZulipTestCase):
user,
stream,
"Verona3",
visibility_policy=UserTopic.MUTED,
visibility_policy=UserTopic.VisibilityPolicy.MUTED,
last_updated=datetime(2020, 1, 1, tzinfo=timezone.utc),
)
self.assertIn((stream.name, "Verona3", mock_date_muted), get_topic_mutes(user))
@ -157,7 +163,11 @@ class MutedTopicsTests(ZulipTestCase):
stream = get_stream("Verona", realm)
do_set_user_topic_visibility_policy(
user, stream, "Verona3", visibility_policy=UserTopic.MUTED, last_updated=timezone_now()
user,
stream,
"Verona3",
visibility_policy=UserTopic.VisibilityPolicy.MUTED,
last_updated=timezone_now(),
)
url = "/api/v1/users/me/subscriptions/muted_topics"
@ -222,7 +232,9 @@ class UnmutedTopicsTests(ZulipTestCase):
topic_name=topic_name,
)
user_ids = stream_topic_target.user_ids_with_visibility_policy(UserTopic.UNMUTED)
user_ids = stream_topic_target.user_ids_with_visibility_policy(
UserTopic.VisibilityPolicy.UNMUTED
)
self.assertEqual(user_ids, set())
def set_topic_visibility_for_user(user: UserProfile, visibility_policy: int) -> None:
@ -234,19 +246,23 @@ class UnmutedTopicsTests(ZulipTestCase):
last_updated=date_unmuted,
)
set_topic_visibility_for_user(hamlet, UserTopic.UNMUTED)
set_topic_visibility_for_user(cordelia, UserTopic.MUTED)
user_ids = stream_topic_target.user_ids_with_visibility_policy(UserTopic.UNMUTED)
set_topic_visibility_for_user(hamlet, UserTopic.VisibilityPolicy.UNMUTED)
set_topic_visibility_for_user(cordelia, UserTopic.VisibilityPolicy.MUTED)
user_ids = stream_topic_target.user_ids_with_visibility_policy(
UserTopic.VisibilityPolicy.UNMUTED
)
self.assertEqual(user_ids, {hamlet.id})
hamlet_date_unmuted = UserTopic.objects.filter(
user_profile=hamlet, visibility_policy=UserTopic.UNMUTED
user_profile=hamlet, visibility_policy=UserTopic.VisibilityPolicy.UNMUTED
)[0].last_updated
self.assertEqual(hamlet_date_unmuted, date_unmuted)
set_topic_visibility_for_user(cordelia, UserTopic.UNMUTED)
user_ids = stream_topic_target.user_ids_with_visibility_policy(UserTopic.UNMUTED)
set_topic_visibility_for_user(cordelia, UserTopic.VisibilityPolicy.UNMUTED)
user_ids = stream_topic_target.user_ids_with_visibility_policy(
UserTopic.VisibilityPolicy.UNMUTED
)
self.assertEqual(user_ids, {hamlet.id, cordelia.id})
cordelia_date_unmuted = UserTopic.objects.filter(
user_profile=cordelia, visibility_policy=UserTopic.UNMUTED
user_profile=cordelia, visibility_policy=UserTopic.VisibilityPolicy.UNMUTED
)[0].last_updated
self.assertEqual(cordelia_date_unmuted, date_unmuted)

View File

@ -1850,7 +1850,7 @@ class RecipientInfoTest(ZulipTestCase):
hamlet,
stream,
topic_name,
visibility_policy=UserTopic.UNMUTED,
visibility_policy=UserTopic.VisibilityPolicy.UNMUTED,
)
info = get_recipient_info(
@ -1865,7 +1865,7 @@ class RecipientInfoTest(ZulipTestCase):
sub.is_muted = False
sub.save()
do_set_user_topic_visibility_policy(
hamlet, stream, topic_name, visibility_policy=UserTopic.VISIBILITY_POLICY_INHERIT
hamlet, stream, topic_name, visibility_policy=UserTopic.VisibilityPolicy.INHERIT
)
# Now have Hamlet mute the topic to omit him from stream_push_user_ids.
@ -1873,7 +1873,7 @@ class RecipientInfoTest(ZulipTestCase):
hamlet,
stream,
topic_name,
visibility_policy=UserTopic.MUTED,
visibility_policy=UserTopic.VisibilityPolicy.MUTED,
)
info = get_recipient_info(

View File

@ -36,7 +36,7 @@ def mute_topic(
user_profile,
stream,
topic_name,
visibility_policy=UserTopic.MUTED,
visibility_policy=UserTopic.VisibilityPolicy.MUTED,
last_updated=date_muted,
)
@ -56,7 +56,7 @@ def unmute_topic(
stream = access_stream_for_unmute_topic_by_id(user_profile, stream_id, error)
do_set_user_topic_visibility_policy(
user_profile, stream, topic_name, visibility_policy=UserTopic.VISIBILITY_POLICY_INHERIT
user_profile, stream, topic_name, visibility_policy=UserTopic.VisibilityPolicy.INHERIT
)