paulmataruso/zulip
1
0
Fork 0
mirror of https://github.com/zulip/zulip.git synced 2025-11-03 13:33:24 +00:00
Code Issues Packages Projects Releases Wiki Activity

users: Always pass delivery_email in user's own object.

Browse Source 
This commit changes the code to always pass delivery_email
field in the user's own object in 'realm_users'.

This commit also fixes the events sent by notify_created_user.
In the "realm_user/add" event sent when creating the user,
the delivery_email field was set according to the access
for the created user itself as the created user was passed as
acting_user to format_user_row. But now since we have changed
the code to always allow the user themselves to have access
to the email, this bug was caught in tests and we fix the person
object in the event to have delivery_email field based on whether
the user receiving the event has access to email or not.
This commit is contained in:
Sahil Batra
2021-12-11 12:47:57 +05:30
committed by Tim Abbott
parent aa98b39429
commit 9a6886f630
7 changed files with 62 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files

31
zerver/actions/create_user.py
View File

@@ -26,7 +26,12 @@ from zerver.lib.stream_subscription import bulk_get_subscriber_peer_info
from zerver.lib.streams import get_signups_stream
from zerver.lib.user_counts import realm_user_count, realm_user_count_by_role
from zerver.lib.user_groups import get_system_user_group_for_user
from zerver.lib.users import format_user_row, get_api_key, user_profile_to_user_row
from zerver.lib.users import (
can_access_delivery_email,
format_user_row,
get_api_key,
user_profile_to_user_row,
)
from zerver.models import (
DefaultStreamGroup,
Message,
@@ -40,7 +45,6 @@ from zerver.models import (
UserGroupMembership,
UserMessage,
UserProfile,
active_user_ids,
bot_owner_user_ids,
get_realm,
get_system_bot,
@@ -292,8 +296,27 @@ def notify_created_user(user_profile: UserProfile) -> None:
# later event.
custom_profile_field_data={},
)
event: Dict[str, Any] = dict(type="realm_user", op="add", person=person)
send_event(user_profile.realm, event, active_user_ids(user_profile.realm_id))
active_users = user_profile.realm.get_active_users()
user_ids_with_real_email_access = []
user_ids_without_real_email_access = []
for user in active_users:
if can_access_delivery_email(
user, user_profile.id, user_profile.realm.email_address_visibility
):
user_ids_with_real_email_access.append(user.id)
else:
user_ids_without_real_email_access.append(user.id)
if user_ids_with_real_email_access:
person["delivery_email"] = user_profile.delivery_email
event: Dict[str, Any] = dict(type="realm_user", op="add", person=person)
send_event(user_profile.realm, event, user_ids_with_real_email_access)
if user_ids_without_real_email_access:
del person["delivery_email"]
event = dict(type="realm_user", op="add", person=person)
send_event(user_profile.realm, event, user_ids_without_real_email_access)
def created_bot_event(user_profile: UserProfile) -> Dict[str, Any]:

9
zerver/lib/users.py
View File

@@ -394,7 +394,12 @@ def validate_user_custom_profile_data(
raise JsonableError(error.message)
def can_access_delivery_email(user_profile: UserProfile, email_address_visibility: int) -> bool:
def can_access_delivery_email(
user_profile: UserProfile, target_user_id: int, email_address_visibility: int
) -> bool:
if target_user_id == user_profile.id:
return True
if email_address_visibility == Realm.EMAIL_ADDRESS_VISIBILITY_ADMINS:
return user_profile.is_realm_admin
@@ -477,7 +482,7 @@ def format_user_row(
)
if acting_user is not None and can_access_delivery_email(
acting_user, realm.email_address_visibility
acting_user, row["id"], realm.email_address_visibility
):
result["delivery_email"] = row["delivery_email"]

8
zerver/tests/test_bots.py
View File

@@ -164,7 +164,7 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
self.login("hamlet")
self.assert_num_bots_equal(0)
events: List[Mapping[str, Any]] = []
with self.tornado_redirected_to_list(events, expected_num_events=4):
with self.tornado_redirected_to_list(events, expected_num_events=5):
result = self.create_bot()
self.assert_num_bots_equal(1)
@@ -330,7 +330,7 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
self.login_user(user)
self.assert_num_bots_equal(0)
events: List[Mapping[str, Any]] = []
with self.tornado_redirected_to_list(events, expected_num_events=4):
with self.tornado_redirected_to_list(events, expected_num_events=5):
result = self.create_bot()
self.assert_num_bots_equal(1)
@@ -422,7 +422,7 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
self.assert_num_bots_equal(0)
events: List[Mapping[str, Any]] = []
with self.tornado_redirected_to_list(events, expected_num_events=4):
with self.tornado_redirected_to_list(events, expected_num_events=5):
result = self.create_bot(default_sending_stream="Denmark")
self.assert_num_bots_equal(1)
self.assertEqual(result["default_sending_stream"], "Denmark")
@@ -496,7 +496,7 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
self.assert_num_bots_equal(0)
events: List[Mapping[str, Any]] = []
with self.tornado_redirected_to_list(events, expected_num_events=4):
with self.tornado_redirected_to_list(events, expected_num_events=5):
result = self.create_bot(default_events_register_stream="Denmark")
self.assert_num_bots_equal(1)
self.assertEqual(result["default_events_register_stream"], "Denmark")

16
zerver/tests/test_event_system.py
View File

@@ -459,7 +459,10 @@ class FetchInitialStateDataTest(ZulipTestCase):
result = fetch_initial_state_data(user_profile)
for key, value in result["raw_users"].items():
self.assertNotIn("delivery_email", value)
if key == user_profile.id:
self.assertEqual(value["delivery_email"], user_profile.delivery_email)
else:
self.assertNotIn("delivery_email", value)
do_set_realm_property(
user_profile.realm,
@@ -470,7 +473,10 @@ class FetchInitialStateDataTest(ZulipTestCase):
result = fetch_initial_state_data(user_profile)
for key, value in result["raw_users"].items():
self.assertNotIn("delivery_email", value)
if key == user_profile.id:
self.assertEqual(value["delivery_email"], user_profile.delivery_email)
else:
self.assertNotIn("delivery_email", value)
def test_delivery_email_presence_for_admins(self) -> None:
user_profile = self.example_user("iago")
@@ -483,8 +489,12 @@ class FetchInitialStateDataTest(ZulipTestCase):
acting_user=None,
)
result = fetch_initial_state_data(user_profile)
for key, value in result["raw_users"].items():
self.assertNotIn("delivery_email", value)
if key == user_profile.id:
self.assertEqual(value["delivery_email"], user_profile.delivery_email)
else:
self.assertNotIn("delivery_email", value)
do_set_realm_property(
user_profile.realm,

3
zerver/tests/test_home.py
View File

@@ -661,6 +661,7 @@ class HomeTest(ZulipTestCase):
avatar_version=cross_realm_email_gateway_bot.avatar_version,
bot_owner_id=None,
bot_type=1,
delivery_email=cross_realm_email_gateway_bot.delivery_email,
email=cross_realm_email_gateway_bot.email,
user_id=cross_realm_email_gateway_bot.id,
full_name=cross_realm_email_gateway_bot.full_name,
@@ -677,6 +678,7 @@ class HomeTest(ZulipTestCase):
avatar_version=cross_realm_notification_bot.avatar_version,
bot_owner_id=None,
bot_type=1,
delivery_email=cross_realm_notification_bot.delivery_email,
email=cross_realm_notification_bot.email,
user_id=cross_realm_notification_bot.id,
full_name=cross_realm_notification_bot.full_name,
@@ -693,6 +695,7 @@ class HomeTest(ZulipTestCase):
avatar_version=cross_realm_welcome_bot.avatar_version,
bot_owner_id=None,
bot_type=1,
delivery_email=cross_realm_welcome_bot.delivery_email,
email=cross_realm_welcome_bot.email,
user_id=cross_realm_welcome_bot.id,
full_name=cross_realm_welcome_bot.full_name,

4
zerver/tests/test_signup.py
View File

@@ -875,13 +875,13 @@ class LoginTest(ZulipTestCase):
with self.captureOnCommitCallbacks(execute=True):
self.register(self.nonreg_email("test"), "test")
# Ensure the number of queries we make is not O(streams)
self.assert_length(queries, 95)
self.assert_length(queries, 96)
# We can probably avoid a couple cache hits here, but there doesn't
# seem to be any O(N) behavior. Some of the cache hits are related
# to sending messages, such as getting the welcome bot, looking up
# the alert words for a realm, etc.
self.assert_length(cache_tries, 23)
self.assert_length(cache_tries, 22)
user_profile = self.nonreg_user("test")
self.assert_logged_in_user_id(user_profile.id)

12
zerver/tests/test_users.py
View File

@@ -316,7 +316,7 @@ class PermissionTest(ZulipTestCase):
hamlet = find_dict(members, "user_id", user.id)
self.assertEqual(hamlet["email"], user.email)
self.assertIsNone(hamlet["avatar_url"])
self.assertNotIn("delivery_email", hamlet)
self.assertEqual(hamlet["delivery_email"], user.delivery_email)
# Also verify the /events code path. This is a bit hacky, but
# we need to verify client_gravatar is not being overridden.
@@ -346,7 +346,7 @@ class PermissionTest(ZulipTestCase):
# `delivery_email`; otherwise, we won't be able to serve the
# user's Gravatar.
self.assertEqual(hamlet["avatar_url"], get_gravatar_url(user.delivery_email, 1))
self.assertNotIn("delivery_email", hamlet)
self.assertEqual(hamlet["delivery_email"], user.delivery_email)
# Also verify the /events code path. This is a bit hacky, but
# basically we want to verify client_gravatar is being
@@ -809,7 +809,7 @@ class QueryCountTest(ZulipTestCase):
with queries_captured() as queries:
with cache_tries_captured() as cache_tries:
with self.tornado_redirected_to_list(events, expected_num_events=10):
with self.tornado_redirected_to_list(events, expected_num_events=11):
fred = do_create_user(
email="fred@zulip.com",
password="password",
@@ -819,9 +819,9 @@ class QueryCountTest(ZulipTestCase):
acting_user=None,
)
self.assert_length(queries, 90)
self.assert_length(cache_tries, 29)
self.assert_length(queries, 91)
self.assert_length(cache_tries, 28)
peer_add_events = [event for event in events if event["event"].get("op") == "peer_add"]
notifications = set()
@@ -2008,7 +2008,7 @@ class GetProfileTest(ZulipTestCase):
self.assertFalse(result["is_owner"])
self.assertFalse(result["is_guest"])
self.assertEqual(result["role"], UserProfile.ROLE_MEMBER)
self.assertFalse("delivery_email" in result)
self.assertEqual(result["delivery_email"], hamlet.delivery_email)
self.login("iago")
result = orjson.loads(self.client_get("/json/users/me").content)
self.assertEqual(result["email"], iago.email)