From 7b42c802b1ccc67b923801950ada0f9240a07c2c Mon Sep 17 00:00:00 2001 From: Sahil Batra Date: Thu, 25 May 2023 19:36:13 +0530 Subject: [PATCH] invites: Add include_realm_default_subscriptions parameter. This commit adds include_realm_default_subscriptions parameter to the invite endpoints and the corresponding field in PreregistrationUser and MultiuseInvite objects. This field will be used to subscribe the new users to the default streams at the time of account creation and not to the streams that were default when sending the invite. --- analytics/tests/test_counts.py | 4 + api_docs/changelog.md | 6 + corporate/tests/test_support_views.py | 1 + zerver/actions/create_user.py | 9 +- zerver/actions/invites.py | 14 +- ...de_realm_default_subscriptions_and_more.py | 25 +++ ...ault_subscriptions_for_existing_objects.py | 111 ++++++++++ ...e_subscribe_to_default_streams_and_more.py | 22 ++ zerver/models/prereg_users.py | 3 + zerver/openapi/zulip.yaml | 37 +++- zerver/tests/test_auth_backends.py | 13 +- zerver/tests/test_events.py | 6 + zerver/tests/test_invite.py | 199 ++++++++++++++++-- zerver/tests/test_users.py | 25 ++- zerver/views/auth.py | 7 + zerver/views/invite.py | 34 ++- zerver/views/registration.py | 9 + 17 files changed, 461 insertions(+), 64 deletions(-) create mode 100644 zerver/migrations/0521_multiuseinvite_include_realm_default_subscriptions_and_more.py create mode 100644 zerver/migrations/0522_set_include_realm_default_subscriptions_for_existing_objects.py create mode 100644 zerver/migrations/0523_alter_multiuseinvite_subscribe_to_default_streams_and_more.py diff --git a/analytics/tests/test_counts.py b/analytics/tests/test_counts.py index db35dcd9b0..463aad8997 100644 --- a/analytics/tests/test_counts.py +++ b/analytics/tests/test_counts.py @@ -1724,6 +1724,7 @@ class TestLoggingCountStats(AnalyticsTestCase): user, ["user1@domain.tld", "user2@domain.tld"], [stream], + include_realm_default_subscriptions=False, invite_expires_in_minutes=invite_expires_in_minutes, ) assertInviteCountEquals(2) @@ -1735,6 +1736,7 @@ class TestLoggingCountStats(AnalyticsTestCase): user, ["user1@domain.tld", "user2@domain.tld"], [stream], + include_realm_default_subscriptions=False, invite_expires_in_minutes=invite_expires_in_minutes, ) assertInviteCountEquals(4) @@ -1745,6 +1747,7 @@ class TestLoggingCountStats(AnalyticsTestCase): user, ["user3@domain.tld", "malformed"], [stream], + include_realm_default_subscriptions=False, invite_expires_in_minutes=invite_expires_in_minutes, ) assertInviteCountEquals(4) @@ -1755,6 +1758,7 @@ class TestLoggingCountStats(AnalyticsTestCase): user, ["first@domain.tld", "user4@domain.tld"], [stream], + include_realm_default_subscriptions=False, invite_expires_in_minutes=invite_expires_in_minutes, ) self.assert_length(skipped, 1) diff --git a/api_docs/changelog.md b/api_docs/changelog.md index 8fd75d43b5..5b334a141b 100644 --- a/api_docs/changelog.md +++ b/api_docs/changelog.md @@ -26,6 +26,12 @@ format used by the Zulip server that they are interacting with. /invites/multiuse`](/api/create-invite-link): Newly created user is now subscribed to the default streams in the organization even when the user sending the invite is not allowed to subscribe others. +* [`POST /invites`](/api/send-invites), [`POST + /invites/multiuse`](/api/create-invite-link): Added + `include_realm_default_subscriptions` parameter to indicate whether to + subscribe the invited user to default streams when creating the new + user account instead of passing the stream IDs of default streams + in `stream_ids` parameter. **Feature level 260**: diff --git a/corporate/tests/test_support_views.py b/corporate/tests/test_support_views.py index 12f8595cc0..f0737c6d1e 100644 --- a/corporate/tests/test_support_views.py +++ b/corporate/tests/test_support_views.py @@ -967,6 +967,7 @@ class TestSupportEndpoint(ZulipTestCase): self.example_user("hamlet"), invited_as=1, invite_expires_in_minutes=invite_expires_in_minutes, + include_realm_default_subscriptions=False, ) result = query_result_from_before("zulip", 2) check_multiuse_invite_link_query_result(result) diff --git a/zerver/actions/create_user.py b/zerver/actions/create_user.py index 91e132dc1e..6bb975f4fc 100644 --- a/zerver/actions/create_user.py +++ b/zerver/actions/create_user.py @@ -139,18 +139,15 @@ def set_up_streams_for_new_human_user( streams = [] acting_user = None - user_was_invited = prereg_user is not None and ( - prereg_user.referred_by is not None or prereg_user.multiuse_invite is not None - ) - if add_initial_stream_subscriptions: # If the Preregistration object didn't explicitly list some streams (it # happens when user directly signs up without any invitation), we add the # default streams for the realm. Note that we are fine with "slim" Stream # objects for calling bulk_add_subscriptions and add_new_user_history, # which we verify in StreamSetupTest tests that check query counts. - if len(streams) == 0 and not user_was_invited: - streams = get_slim_realm_default_streams(realm.id) + if prereg_user is None or prereg_user.include_realm_default_subscriptions: + default_streams = get_slim_realm_default_streams(realm.id) + streams = list(set(streams) | set(default_streams)) for default_stream_group in default_stream_groups: default_stream_group_streams = default_stream_group.streams.all() diff --git a/zerver/actions/invites.py b/zerver/actions/invites.py index 26d79f3556..df493560be 100644 --- a/zerver/actions/invites.py +++ b/zerver/actions/invites.py @@ -176,6 +176,7 @@ def do_invite_users( streams: Collection[Stream], *, invite_expires_in_minutes: Optional[int], + include_realm_default_subscriptions: bool, invite_as: int = PreregistrationUser.INVITE_AS["MEMBER"], ) -> List[Tuple[str, str, bool]]: num_invites = len(invitee_emails) @@ -258,7 +259,11 @@ def do_invite_users( for email in validated_emails: # The logged in user is the referrer. prereg_user = PreregistrationUser( - email=email, referred_by=user_profile, invited_as=invite_as, realm=realm + email=email, + referred_by=user_profile, + invited_as=invite_as, + realm=realm, + include_realm_default_subscriptions=include_realm_default_subscriptions, ) prereg_user.save() stream_ids = [stream.id for stream in streams] @@ -355,10 +360,15 @@ def do_create_multiuse_invite_link( referred_by: UserProfile, invited_as: int, invite_expires_in_minutes: Optional[int], + include_realm_default_subscriptions: bool, streams: Sequence[Stream] = [], ) -> str: realm = referred_by.realm - invite = MultiuseInvite.objects.create(realm=realm, referred_by=referred_by) + invite = MultiuseInvite.objects.create( + realm=realm, + referred_by=referred_by, + include_realm_default_subscriptions=include_realm_default_subscriptions, + ) if streams: invite.streams.set(streams) invite.invited_as = invited_as diff --git a/zerver/migrations/0521_multiuseinvite_include_realm_default_subscriptions_and_more.py b/zerver/migrations/0521_multiuseinvite_include_realm_default_subscriptions_and_more.py new file mode 100644 index 0000000000..c4d43ed4ae --- /dev/null +++ b/zerver/migrations/0521_multiuseinvite_include_realm_default_subscriptions_and_more.py @@ -0,0 +1,25 @@ +# Generated by Django 4.2.10 on 2024-03-20 16:40 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ( + "zerver", + "0520_attachment_zerver_attachment_realm_create_time", + ), + ] + + operations = [ + migrations.AddField( + model_name="multiuseinvite", + name="include_realm_default_subscriptions", + field=models.BooleanField(null=True), + ), + migrations.AddField( + model_name="preregistrationuser", + name="include_realm_default_subscriptions", + field=models.BooleanField(null=True), + ), + ] diff --git a/zerver/migrations/0522_set_include_realm_default_subscriptions_for_existing_objects.py b/zerver/migrations/0522_set_include_realm_default_subscriptions_for_existing_objects.py new file mode 100644 index 0000000000..006be1bcac --- /dev/null +++ b/zerver/migrations/0522_set_include_realm_default_subscriptions_for_existing_objects.py @@ -0,0 +1,111 @@ +# Generated by Django 5.0.5 on 2024-04-30 09:22 + +from django.db import migrations, transaction +from django.db.backends.base.schema import BaseDatabaseSchemaEditor +from django.db.migrations.state import StateApps +from django.db.models import Max, Min + + +def set_include_realm_default_subscriptions_for_existing_prereg_objects( + apps: StateApps, schema_editor: BaseDatabaseSchemaEditor +) -> None: + """Here, we set the value of include_realm_default_subscriptions for + existing PreregistrationUser objects such that its value will + match what would have happened prior to this migration. There are 3 cases: + + - User was not invited, and is being created by the API, LDAP + auth, or some other process that does not generate a + preregistration user object. This requires no migration. + + - User is using an invitation. In that case, both the new and + previous code-level logic will use the streams on the invitation + object. So the migration should use False for these. + + - User was not invited, but has a prereg_user created by the + account registration flow. In this case, for new objects, + create_preregistration_user create one with the Django models + default of include_realm_default_subscription=True. + + If the preregistration object has streams specified, usually + from being copied from an invitation for the user, the previous + logic would ignore default streams; as a result, existing + objects with referred_by=None, multiuse_invite=None, + streams=None should be migrated to + include_realm_default_subscription=True. + """ + PreregistrationUser = apps.get_model("zerver", "PreregistrationUser") + + BATCH_SIZE = 1000 + max_id = PreregistrationUser.objects.filter(include_realm_default_subscriptions=None).aggregate( + Max("id") + )["id__max"] + + if max_id is None: + # Do nothing if there are no PreregistrationUser on the server. + return + + lower_bound = PreregistrationUser.objects.filter( + include_realm_default_subscriptions=None + ).aggregate(Min("id"))["id__min"] + while lower_bound <= max_id: + upper_bound = lower_bound + BATCH_SIZE - 1 + print(f"Processing batch {lower_bound} to {upper_bound} for PreregistrationUser") + + with transaction.atomic(): + PreregistrationUser.objects.filter( + id__range=(lower_bound, upper_bound), + include_realm_default_subscriptions=None, + referred_by=None, + multiuse_invite=None, + streams=None, + ).update(include_realm_default_subscriptions=True) + PreregistrationUser.objects.filter( + id__range=(lower_bound, upper_bound), include_realm_default_subscriptions=None + ).update(include_realm_default_subscriptions=False) + + +def set_include_realm_default_subscriptions_for_existing_multiuse_invites( + apps: StateApps, schema_editor: BaseDatabaseSchemaEditor +) -> None: + MultiuseInvite = apps.get_model("zerver", "MultiuseInvite") + + BATCH_SIZE = 1000 + max_id = MultiuseInvite.objects.filter(include_realm_default_subscriptions=None).aggregate( + Max("id") + )["id__max"] + + if max_id is None: + # Do nothing if there are no MultiuseInvite on the server. + return + + lower_bound = MultiuseInvite.objects.filter(include_realm_default_subscriptions=None).aggregate( + Min("id") + )["id__min"] + while lower_bound <= max_id: + upper_bound = lower_bound + BATCH_SIZE - 1 + print(f"Processing batch {lower_bound} to {upper_bound} for MultiuseInvite") + + MultiuseInvite.objects.filter( + id__range=(lower_bound, upper_bound), include_realm_default_subscriptions=None + ).update(include_realm_default_subscriptions=False) + + +class Migration(migrations.Migration): + atomic = False + + dependencies = [ + ("zerver", "0521_multiuseinvite_include_realm_default_subscriptions_and_more"), + ] + + operations = [ + migrations.RunPython( + set_include_realm_default_subscriptions_for_existing_prereg_objects, + elidable=True, + reverse_code=migrations.RunPython.noop, + ), + migrations.RunPython( + set_include_realm_default_subscriptions_for_existing_multiuse_invites, + elidable=True, + reverse_code=migrations.RunPython.noop, + ), + ] diff --git a/zerver/migrations/0523_alter_multiuseinvite_subscribe_to_default_streams_and_more.py b/zerver/migrations/0523_alter_multiuseinvite_subscribe_to_default_streams_and_more.py new file mode 100644 index 0000000000..0f2d286121 --- /dev/null +++ b/zerver/migrations/0523_alter_multiuseinvite_subscribe_to_default_streams_and_more.py @@ -0,0 +1,22 @@ +# Generated by Django 5.0.5 on 2024-04-30 09:48 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("zerver", "0522_set_include_realm_default_subscriptions_for_existing_objects"), + ] + + operations = [ + migrations.AlterField( + model_name="multiuseinvite", + name="include_realm_default_subscriptions", + field=models.BooleanField(default=True), + ), + migrations.AlterField( + model_name="preregistrationuser", + name="include_realm_default_subscriptions", + field=models.BooleanField(default=True), + ), + ] diff --git a/zerver/models/prereg_users.py b/zerver/models/prereg_users.py index 7715d2a04c..b6880e4fb3 100644 --- a/zerver/models/prereg_users.py +++ b/zerver/models/prereg_users.py @@ -103,6 +103,8 @@ class PreregistrationUser(models.Model): UserProfile, null=True, related_name="+", on_delete=models.SET_NULL ) + include_realm_default_subscriptions = models.BooleanField(default=True) + class Meta: indexes = [ models.Index(Upper("email"), name="upper_preregistration_email_idx"), @@ -130,6 +132,7 @@ class MultiuseInvite(models.Model): streams = models.ManyToManyField("zerver.Stream") realm = models.ForeignKey(Realm, on_delete=CASCADE) invited_as = models.PositiveSmallIntegerField(default=PreregistrationUser.INVITE_AS["MEMBER"]) + include_realm_default_subscriptions = models.BooleanField(default=True) # status for tracking whether the invite has been revoked. # If revoked, set to confirmation.settings.STATUS_REVOKED. diff --git a/zerver/openapi/zulip.yaml b/zerver/openapi/zulip.yaml index 753f98a0d8..8621c56830 100644 --- a/zerver/openapi/zulip.yaml +++ b/zerver/openapi/zulip.yaml @@ -12160,14 +12160,14 @@ paths: $ref: "#/components/schemas/InviteRoleParameter" stream_ids: description: | - A list containing the [IDs of the streams](/api/get-stream-id) that the - newly created user will be automatically subscribed to if the invitation - is accepted. If the list is empty, then the new user will not be - subscribed to any streams. + A list containing the [IDs of the streams](/api/get-stream-id) that + the newly created user will be automatically subscribed to if the + invitation is accepted, in addition to default streams configured via + the `include_realm_default_subscriptions` setting. - If the user is not allowed to subscribe others, then this list should - be empty but the newly created user will still be subscribed to default - streams in the organization. + This parameter must be the empty list if the current user has the + unlikely configuration of being able to generate invitations while + lacking permission to subscribe other users to streams. **Changes**: Prior to Zulip 9.0 (feature level 261), if the user was not allowed to subscribe others to stream, the newly created user was not @@ -12179,6 +12179,16 @@ paths: items: type: integer example: [1, 10] + include_realm_default_subscriptions: + description: | + Boolean indicating that the newly created user should be subscribed + to the [default streams][default-streams] for the organization. + + **Changes**: New in Zulip 9.0 (feature level 261). + + [default-streams]: /help/set-default-streams-for-new-users + type: boolean + example: false required: - invitee_emails - stream_ids @@ -12315,7 +12325,8 @@ paths: If the user is not allowed to subscribe others, then this list should be empty but the newly created user will still be subscribed to default - streams in the organization. + streams in the organization depending on the `include_realm_default_subscriptions` + value. **Changes**: Prior to Zulip 9.0 (feature level 261), if the user was not allowed to subscribe others to stream, the newly created user was not @@ -12325,6 +12336,16 @@ paths: type: integer default: [] example: [1, 10] + include_realm_default_subscriptions: + description: | + Boolean indicating that the newly created user should be subscribed + to the [default streams][default-streams] for the organization. + + **Changes**: New in Zulip 9.0 (feature level 261). + + [default-streams]: /help/set-default-streams-for-new-users + type: boolean + example: false encoding: invite_expires_in_minutes: contentType: application/json diff --git a/zerver/tests/test_auth_backends.py b/zerver/tests/test_auth_backends.py index 7066ffb19f..3e83d72033 100644 --- a/zerver/tests/test_auth_backends.py +++ b/zerver/tests/test_auth_backends.py @@ -1542,7 +1542,13 @@ class SocialAuthBase(DesktopFlowTestingLib, ZulipTestCase, ABC): iago = self.example_user("iago") with self.captureOnCommitCallbacks(execute=True): - do_invite_users(iago, [email], [], invite_expires_in_minutes=2 * 24 * 60) + do_invite_users( + iago, + [email], + [], + include_realm_default_subscriptions=True, + invite_expires_in_minutes=2 * 24 * 60, + ) account_data_dict = self.get_account_data_dict(email=email, name=name) result = self.social_auth_test( @@ -1889,6 +1895,7 @@ class SocialAuthBase(DesktopFlowTestingLib, ZulipTestCase, ABC): iago, [email], [], + include_realm_default_subscriptions=False, invite_expires_in_minutes=invite_expires_in_minutes, invite_as=PreregistrationUser.INVITE_AS["REALM_ADMIN"], ) @@ -4702,7 +4709,9 @@ class GoogleAuthBackendTest(SocialAuthBase): # Now confirm an invitation link works referrer = self.example_user("hamlet") - multiuse_obj = MultiuseInvite.objects.create(realm=realm, referred_by=referrer) + multiuse_obj = MultiuseInvite.objects.create( + realm=realm, referred_by=referrer, include_realm_default_subscriptions=False + ) multiuse_obj.streams.set(streams) validity_in_minutes = 2 * 24 * 60 create_confirmation_link( diff --git a/zerver/tests/test_events.py b/zerver/tests/test_events.py index 5987475f93..60ce4bc525 100644 --- a/zerver/tests/test_events.py +++ b/zerver/tests/test_events.py @@ -1115,6 +1115,7 @@ class NormalActionsTest(BaseAction): self.user_profile, ["foo@zulip.com"], streams, + include_realm_default_subscriptions=False, invite_expires_in_minutes=invite_expires_in_minutes, ) check_invites_changed("events[0]", events[0]) @@ -1132,6 +1133,7 @@ class NormalActionsTest(BaseAction): self.user_profile, PreregistrationUser.INVITE_AS["MEMBER"], invite_expires_in_minutes, + False, streams, ) check_invites_changed("events[0]", events[0]) @@ -1145,6 +1147,7 @@ class NormalActionsTest(BaseAction): user_profile, ["foo@zulip.com"], [], + include_realm_default_subscriptions=False, invite_expires_in_minutes=invite_expires_in_minutes, ) @@ -1167,6 +1170,7 @@ class NormalActionsTest(BaseAction): self.user_profile, ["foo@zulip.com"], streams, + include_realm_default_subscriptions=False, invite_expires_in_minutes=invite_expires_in_minutes, ) prereg_users = PreregistrationUser.objects.filter( @@ -1188,6 +1192,7 @@ class NormalActionsTest(BaseAction): self.user_profile, PreregistrationUser.INVITE_AS["MEMBER"], invite_expires_in_minutes, + False, streams, ) @@ -1211,6 +1216,7 @@ class NormalActionsTest(BaseAction): self.user_profile, ["foo@zulip.com"], streams, + include_realm_default_subscriptions=False, invite_expires_in_minutes=invite_expires_in_minutes, ) prereg_user = PreregistrationUser.objects.get(email="foo@zulip.com") diff --git a/zerver/tests/test_invite.py b/zerver/tests/test_invite.py index cc9d0581c4..9e041ee0c0 100644 --- a/zerver/tests/test_invite.py +++ b/zerver/tests/test_invite.py @@ -29,7 +29,7 @@ from zerver.actions.create_user import ( process_new_human_user, set_up_streams_for_new_human_user, ) -from zerver.actions.default_streams import do_add_default_stream +from zerver.actions.default_streams import do_add_default_stream, do_remove_default_stream from zerver.actions.invites import ( do_create_multiuse_invite_link, do_get_invites_controlled_by_user, @@ -137,6 +137,7 @@ class StreamSetupTest(ZulipTestCase): admin, [new_user_email], streams, + include_realm_default_subscriptions=False, invite_expires_in_minutes=1000, ) @@ -177,6 +178,7 @@ class InviteUserBase(ZulipTestCase): invite_expires_in_minutes: Optional[int] = INVITATION_LINK_VALIDITY_MINUTES, body: str = "", invite_as: int = PreregistrationUser.INVITE_AS["MEMBER"], + include_realm_default_subscriptions: bool = False, realm: Optional[Realm] = None, ) -> "TestHttpResponse": """ @@ -201,6 +203,9 @@ class InviteUserBase(ZulipTestCase): "invite_expires_in_minutes": invite_expires_in, "stream_ids": orjson.dumps(stream_ids).decode(), "invite_as": invite_as, + "include_realm_default_subscriptions": orjson.dumps( + include_realm_default_subscriptions + ).decode(), }, subdomain=realm.string_id if realm else "zulip", ) @@ -795,14 +800,45 @@ class InviteUserTest(InviteUserBase): self.assert_json_success(self.invite(invitee, [])) self.assertTrue(find_key_by_email(invitee)) - default_streams = get_default_streams_for_realm_as_dicts(realm.id) + default_streams = get_slim_realm_default_streams(realm.id) self.assert_length(default_streams, 3) self.submit_reg_form_for_user(invitee, "password") - # If no streams are provided, user is not subscribed to - # default streams as well. + # If no streams are provided, user is not subscribed to default + # streams as well if include_realm_default_subscriptions is False. self.check_user_subscribed_only_to_streams("bob", []) + verona = get_stream("Verona", realm) + sandbox = get_stream("sandbox", realm) + zulip = get_stream("Zulip", realm) + default_streams = get_slim_realm_default_streams(realm.id) + self.assert_length(default_streams, 3) + self.assertCountEqual(default_streams, [verona, sandbox, zulip]) + + # Check that user is subscribed to the streams that were set as default + # at the time of account creation and not at the time of inviting them. + invitee = self.nonreg_email("test") + self.assert_json_success(self.invite(invitee, [], include_realm_default_subscriptions=True)) + self.assertTrue(find_key_by_email(invitee)) + + denmark = get_stream("Denmark", realm) + do_add_default_stream(denmark) + do_remove_default_stream(verona) + self.submit_reg_form_for_user(invitee, "password") + self.check_user_subscribed_only_to_streams("test", [denmark, sandbox, zulip]) + + default_streams = get_slim_realm_default_streams(realm.id) + self.assertCountEqual(default_streams, [denmark, sandbox, zulip]) + invitee = self.nonreg_email("test1") + self.assert_json_success( + self.invite(invitee, [verona.name], include_realm_default_subscriptions=True) + ) + self.assertTrue(find_key_by_email(invitee)) + # Check that the user is subscribed to both default streams and stream + # passed in streams list. + self.submit_reg_form_for_user(invitee, "password") + self.check_user_subscribed_only_to_streams("test1", [denmark, sandbox, verona, zulip]) + def test_can_invite_others_to_realm(self) -> None: def validation_func(user_profile: UserProfile) -> bool: return user_profile.can_invite_users_by_email() @@ -1303,7 +1339,7 @@ so we didn't send them an invitation. We did send invitations to everyone else!" # User will be subscribed to default streams even when the # referrer does not have permission to subscribe others. - result = self.invite(invitee, []) + result = self.invite(invitee, [], include_realm_default_subscriptions=True) self.assert_json_success(result) self.check_sent_emails([invitee]) @@ -1314,6 +1350,19 @@ so we didn't send them an invitation. We did send invitations to everyone else!" self.check_user_subscribed_only_to_streams("alice", default_streams) mail.outbox.pop() + invitee = self.nonreg_email("newguy") + # The invited user will not be subscribed to default streams as well. + result = self.invite(invitee, [], include_realm_default_subscriptions=False) + self.assert_json_success(result) + self.check_sent_emails([invitee]) + + self.submit_reg_form_for_user(invitee, "password") + default_streams = get_slim_realm_default_streams(realm.id) + self.assert_length(default_streams, 3) + + self.check_user_subscribed_only_to_streams("newguy", []) + mail.outbox.pop() + self.login("iago") invitee = self.nonreg_email("bob") result = self.invite(invitee, ["Denmark", "Scotland"]) @@ -1332,14 +1381,14 @@ so we didn't send them an invitation. We did send invitations to everyone else!" mail.outbox.pop() invitee = self.nonreg_email("test1") - # User will not be subscribed to any streams, because the streams - # list is empty when referrer has permission to subscribe others. - result = self.invite(invitee, []) + result = self.invite(invitee, [], include_realm_default_subscriptions=True) self.assert_json_success(result) self.check_sent_emails([invitee]) self.submit_reg_form_for_user(invitee, "password") - self.check_user_subscribed_only_to_streams("test1", []) + self.check_user_subscribed_only_to_streams( + "test1", get_slim_realm_default_streams(realm.id) + ) def test_invitation_reminder_email(self) -> None: # All users belong to zulip realm @@ -1512,6 +1561,7 @@ so we didn't send them an invitation. We did send invitations to everyone else!" self.user_profile, ["foo@zulip.com"], streams, + include_realm_default_subscriptions=False, invite_expires_in_minutes=invite_expires_in_minutes, ) prereg_user = PreregistrationUser.objects.get(email="foo@zulip.com") @@ -1520,12 +1570,14 @@ so we didn't send them an invitation. We did send invitations to everyone else!" self.user_profile, ["foo@zulip.com"], streams, + include_realm_default_subscriptions=False, invite_expires_in_minutes=invite_expires_in_minutes, ) do_invite_users( self.user_profile, ["foo@zulip.com"], streams, + include_realm_default_subscriptions=False, invite_expires_in_minutes=invite_expires_in_minutes, ) @@ -1537,6 +1589,7 @@ so we didn't send them an invitation. We did send invitations to everyone else!" lear_user, ["foo@zulip.com"], [], + include_realm_default_subscriptions=True, invite_expires_in_minutes=invite_expires_in_minutes, ) @@ -1748,37 +1801,49 @@ class InvitationsTestCase(InviteUserBase): user_profile, ["TestOne@zulip.com"], streams, + include_realm_default_subscriptions=False, invite_expires_in_minutes=invite_expires_in_minutes, ) do_invite_users( user_profile, ["TestTwo@zulip.com"], streams, + include_realm_default_subscriptions=False, invite_expires_in_minutes=invite_expires_in_minutes, ) do_invite_users( hamlet, ["TestThree@zulip.com"], streams, + include_realm_default_subscriptions=False, invite_expires_in_minutes=invite_expires_in_minutes, ) do_invite_users( othello, ["TestFour@zulip.com"], streams, + include_realm_default_subscriptions=False, invite_expires_in_minutes=invite_expires_in_minutes, ) do_invite_users( self.mit_user("sipbtest"), ["TestOne@mit.edu"], [], + include_realm_default_subscriptions=False, invite_expires_in_minutes=invite_expires_in_minutes, ) + do_create_multiuse_invite_link( - user_profile, PreregistrationUser.INVITE_AS["MEMBER"], invite_expires_in_minutes + user_profile, + PreregistrationUser.INVITE_AS["MEMBER"], + invite_expires_in_minutes, + include_realm_default_subscriptions=False, ) do_create_multiuse_invite_link( - hamlet, PreregistrationUser.INVITE_AS["MEMBER"], invite_expires_in_minutes + hamlet, + PreregistrationUser.INVITE_AS["MEMBER"], + invite_expires_in_minutes, + include_realm_default_subscriptions=False, ) self.assert_length(do_get_invites_controlled_by_user(user_profile), 6) self.assert_length(do_get_invites_controlled_by_user(hamlet), 2) @@ -1808,6 +1873,7 @@ class InvitationsTestCase(InviteUserBase): user_profile, ["TestOne@zulip.com"], streams, + include_realm_default_subscriptions=False, invite_expires_in_minutes=invite_expires_in_minutes, ) @@ -1818,10 +1884,14 @@ class InvitationsTestCase(InviteUserBase): user_profile, ["TestTwo@zulip.com"], streams, + include_realm_default_subscriptions=False, invite_expires_in_minutes=invite_expires_in_minutes, ) do_create_multiuse_invite_link( - othello, PreregistrationUser.INVITE_AS["MEMBER"], invite_expires_in_minutes + othello, + PreregistrationUser.INVITE_AS["MEMBER"], + invite_expires_in_minutes, + include_realm_default_subscriptions=False, ) prereg_user_three = PreregistrationUser( @@ -1835,7 +1905,10 @@ class InvitationsTestCase(InviteUserBase): ) do_create_multiuse_invite_link( - hamlet, PreregistrationUser.INVITE_AS["MEMBER"], invite_expires_in_minutes + hamlet, + PreregistrationUser.INVITE_AS["MEMBER"], + invite_expires_in_minutes, + include_realm_default_subscriptions=False, ) result = self.client_get("/json/invites") @@ -1865,19 +1938,27 @@ class InvitationsTestCase(InviteUserBase): user_profile, ["TestOne@zulip.com"], streams, + include_realm_default_subscriptions=False, invite_expires_in_minutes=None, ) do_invite_users( user_profile, ["TestTwo@zulip.com"], streams, + include_realm_default_subscriptions=False, invite_expires_in_minutes=100 * 24 * 60, ) do_create_multiuse_invite_link( - user_profile, PreregistrationUser.INVITE_AS["MEMBER"], None + user_profile, + PreregistrationUser.INVITE_AS["MEMBER"], + None, + include_realm_default_subscriptions=False, ) do_create_multiuse_invite_link( - user_profile, PreregistrationUser.INVITE_AS["MEMBER"], 100 + user_profile, + PreregistrationUser.INVITE_AS["MEMBER"], + 100, + include_realm_default_subscriptions=False, ) result = self.client_get("/json/invites") @@ -2311,9 +2392,16 @@ class MultiuseInviteTest(ZulipTestCase): self.realm.save() def generate_multiuse_invite_link( - self, streams: Optional[List[Stream]] = None, date_sent: Optional[datetime] = None + self, + streams: Optional[List[Stream]] = None, + date_sent: Optional[datetime] = None, + include_realm_default_subscriptions: bool = False, ) -> str: - invite = MultiuseInvite(realm=self.realm, referred_by=self.example_user("iago")) + invite = MultiuseInvite( + realm=self.realm, + referred_by=self.example_user("iago"), + include_realm_default_subscriptions=include_realm_default_subscriptions, + ) invite.save() if streams is not None: @@ -2411,9 +2499,13 @@ class MultiuseInviteTest(ZulipTestCase): name1 = "newuser" name2 = "bob" name3 = "alice" + name4 = "test" + name5 = "test1" email1 = self.nonreg_email(name1) email2 = self.nonreg_email(name2) email3 = self.nonreg_email(name3) + email4 = self.nonreg_email(name4) + email5 = self.nonreg_email(name5) stream_names = ["Rome", "Scotland", "Venice"] streams = [get_stream(stream_name, self.realm) for stream_name in stream_names] @@ -2428,11 +2520,32 @@ class MultiuseInviteTest(ZulipTestCase): self.check_user_subscribed_only_to_streams(name2, streams) streams = [] - invite_link = self.generate_multiuse_invite_link(streams=streams) + invite_link = self.generate_multiuse_invite_link( + streams=streams, include_realm_default_subscriptions=True + ) self.check_user_able_to_register(email3, invite_link) - # User is not subscribed to default streams as well. - self.assert_length(get_default_streams_for_realm_as_dicts(self.realm.id), 3) - self.check_user_subscribed_only_to_streams(name3, []) + self.assert_length(get_slim_realm_default_streams(self.realm.id), 3) + self.check_user_subscribed_only_to_streams( + name3, get_slim_realm_default_streams(self.realm.id) + ) + + invite_link = self.generate_multiuse_invite_link( + streams=streams, include_realm_default_subscriptions=False + ) + self.check_user_able_to_register(email4, invite_link) + self.check_user_subscribed_only_to_streams(name4, []) + + default_streams = get_slim_realm_default_streams(self.realm.id) + self.assert_length(default_streams, 3) + rome = get_stream("Rome", self.realm) + invite_link = self.generate_multiuse_invite_link( + streams=[rome], include_realm_default_subscriptions=True + ) + self.check_user_able_to_register(email5, invite_link) + rome = get_stream("Rome", self.realm) + self.check_user_subscribed_only_to_streams( + name5, [rome, default_streams[0], default_streams[1], default_streams[2]] + ) def test_multiuse_link_different_realms(self) -> None: """ @@ -2490,11 +2603,54 @@ class MultiuseInviteTest(ZulipTestCase): self.login("iago") stream_ids = [] + default_streams = get_slim_realm_default_streams(self.realm.id) + verona = get_stream("Verona", self.realm) + sandbox = get_stream("sandbox", self.realm) + zulip = get_stream("Zulip", self.realm) + self.assertCountEqual(default_streams, [verona, sandbox, zulip]) + + # Check that user is subscribed to the streams that were set as default + # at the time of account creation and not at the time of inviting them. result = self.client_post( "/json/invites/multiuse", { "stream_ids": orjson.dumps(stream_ids).decode(), "invite_expires_in_minutes": 2 * 24 * 60, + "include_realm_default_subscriptions": orjson.dumps(True).decode(), + }, + ) + invite_link = self.assert_json_success(result)["invite_link"] + + denmark = get_stream("Denmark", self.realm) + do_add_default_stream(denmark) + do_remove_default_stream(verona) + self.check_user_able_to_register(self.nonreg_email("test1"), invite_link) + self.check_user_subscribed_only_to_streams("test1", [denmark, sandbox, zulip]) + + stream_ids = [verona.id] + self.login("iago") + result = self.client_post( + "/json/invites/multiuse", + { + "stream_ids": orjson.dumps(stream_ids).decode(), + "invite_expires_in_minutes": 2 * 24 * 60, + "include_realm_default_subscriptions": orjson.dumps(True).decode(), + }, + ) + invite_link = self.assert_json_success(result)["invite_link"] + self.check_user_able_to_register(self.nonreg_email("newguy"), invite_link) + default_streams = get_slim_realm_default_streams(self.realm.id) + self.assertCountEqual(default_streams, [denmark, sandbox, zulip]) + self.check_user_subscribed_only_to_streams("newguy", [denmark, sandbox, verona, zulip]) + + self.login("iago") + stream_ids = [] + result = self.client_post( + "/json/invites/multiuse", + { + "stream_ids": orjson.dumps(stream_ids).decode(), + "invite_expires_in_minutes": 2 * 24 * 60, + "include_realm_default_subscriptions": orjson.dumps(False).decode(), }, ) invite_link = self.assert_json_success(result)["invite_link"] @@ -2535,6 +2691,7 @@ class MultiuseInviteTest(ZulipTestCase): { "stream_ids": orjson.dumps([]).decode(), "invite_expires_in_minutes": 2 * 24 * 60, + "include_realm_default_subscriptions": orjson.dumps(True).decode(), }, ) invite_link = self.assert_json_success(result)["invite_link"] diff --git a/zerver/tests/test_users.py b/zerver/tests/test_users.py index a0d79e3beb..fefecc3f67 100644 --- a/zerver/tests/test_users.py +++ b/zerver/tests/test_users.py @@ -902,6 +902,7 @@ class QueryCountTest(ZulipTestCase): user_profile=self.example_user("hamlet"), invitee_emails=["fred@zulip.com"], streams=streams, + include_realm_default_subscriptions=False, invite_expires_in_minutes=invite_expires_in_minutes, ) @@ -1707,6 +1708,7 @@ class ActivateTest(ZulipTestCase): iago, ["new1@zulip.com", "new2@zulip.com"], [], + include_realm_default_subscriptions=False, invite_expires_in_minutes=invite_expires_in_minutes, invite_as=PreregistrationUser.INVITE_AS["REALM_ADMIN"], ) @@ -1714,6 +1716,7 @@ class ActivateTest(ZulipTestCase): desdemona, ["new3@zulip.com", "new4@zulip.com"], [], + include_realm_default_subscriptions=False, invite_expires_in_minutes=invite_expires_in_minutes, invite_as=PreregistrationUser.INVITE_AS["REALM_ADMIN"], ) @@ -1722,6 +1725,7 @@ class ActivateTest(ZulipTestCase): iago, ["new5@zulip.com"], [], + include_realm_default_subscriptions=False, invite_expires_in_minutes=None, invite_as=PreregistrationUser.INVITE_AS["REALM_ADMIN"], ) @@ -1729,22 +1733,35 @@ class ActivateTest(ZulipTestCase): desdemona, ["new6@zulip.com"], [], + include_realm_default_subscriptions=False, invite_expires_in_minutes=None, invite_as=PreregistrationUser.INVITE_AS["REALM_ADMIN"], ) iago_multiuse_key = do_create_multiuse_invite_link( - iago, PreregistrationUser.INVITE_AS["MEMBER"], invite_expires_in_minutes + iago, + PreregistrationUser.INVITE_AS["MEMBER"], + invite_expires_in_minutes, + include_realm_default_subscriptions=False, ).split("/")[-2] desdemona_multiuse_key = do_create_multiuse_invite_link( - desdemona, PreregistrationUser.INVITE_AS["MEMBER"], invite_expires_in_minutes + desdemona, + PreregistrationUser.INVITE_AS["MEMBER"], + invite_expires_in_minutes, + include_realm_default_subscriptions=False, ).split("/")[-2] iago_never_expire_multiuse_key = do_create_multiuse_invite_link( - iago, PreregistrationUser.INVITE_AS["MEMBER"], None + iago, + PreregistrationUser.INVITE_AS["MEMBER"], + None, + include_realm_default_subscriptions=False, ).split("/")[-2] desdemona_never_expire_multiuse_key = do_create_multiuse_invite_link( - desdemona, PreregistrationUser.INVITE_AS["MEMBER"], None + desdemona, + PreregistrationUser.INVITE_AS["MEMBER"], + None, + include_realm_default_subscriptions=False, ).split("/")[-2] self.assertEqual( diff --git a/zerver/views/auth.py b/zerver/views/auth.py index e9cffe568c..e56b360538 100644 --- a/zerver/views/auth.py +++ b/zerver/views/auth.py @@ -272,19 +272,26 @@ def maybe_send_to_registration( ) streams_to_subscribe = None + include_realm_default_subscriptions = None if multiuse_obj is not None: # If the user came here explicitly via a multiuse invite link, then # we use the defaults implied by the invite. streams_to_subscribe = list(multiuse_obj.streams.all()) + include_realm_default_subscriptions = multiuse_obj.include_realm_default_subscriptions elif existing_prereg_user: # Otherwise, the user is doing this signup not via any invite link, # but we can use the pre-existing PreregistrationUser for these values # since it tells how they were intended to be, when the user was invited. streams_to_subscribe = list(existing_prereg_user.streams.all()) + include_realm_default_subscriptions = ( + existing_prereg_user.include_realm_default_subscriptions + ) invited_as = existing_prereg_user.invited_as if streams_to_subscribe: prereg_user.streams.set(streams_to_subscribe) + if include_realm_default_subscriptions is not None: + prereg_user.include_realm_default_subscriptions = include_realm_default_subscriptions prereg_user.invited_as = invited_as prereg_user.multiuse_invite = multiuse_obj prereg_user.save() diff --git a/zerver/views/invite.py b/zerver/views/invite.py index 03fb45f6f2..a5542c83cc 100644 --- a/zerver/views/invite.py +++ b/zerver/views/invite.py @@ -16,12 +16,11 @@ from zerver.actions.invites import ( do_send_user_invite_email, ) from zerver.decorator import require_member_or_admin -from zerver.lib.default_streams import get_slim_realm_default_streams from zerver.lib.exceptions import InvitationError, JsonableError, OrganizationOwnerRequiredError from zerver.lib.request import REQ, has_request_variables from zerver.lib.response import json_success from zerver.lib.streams import access_stream_by_id -from zerver.lib.validator import check_int, check_int_in, check_list, check_none_or +from zerver.lib.validator import check_bool, check_int, check_int_in, check_list, check_none_or from zerver.models import MultiuseInvite, PreregistrationUser, Stream, UserProfile # Convert INVITATION_LINK_VALIDITY_DAYS into minutes. @@ -60,6 +59,7 @@ def invite_users_backend( default=PreregistrationUser.INVITE_AS["MEMBER"], ), stream_ids: List[int] = REQ(json_validator=check_list(check_int)), + include_realm_default_subscriptions: bool = REQ(json_validator=check_bool, default=False), ) -> HttpResponse: if not user_profile.can_invite_users_by_email(): # Guest users case will not be handled here as it will @@ -92,21 +92,15 @@ def invite_users_backend( ) streams.append(stream) - if not user_profile.can_subscribe_other_users(): - if len(streams): - raise JsonableError( - _("You do not have permission to subscribe other users to channels.") - ) - - # We would subscribe the invited user to default streams even when the user - # inviting them does not have permission to subscribe others. - streams = get_slim_realm_default_streams(user_profile.realm_id) + if len(streams) and not user_profile.can_subscribe_other_users(): + raise JsonableError(_("You do not have permission to subscribe other users to channels.")) skipped = do_invite_users( user_profile, invitee_emails, streams, invite_expires_in_minutes=invite_expires_in_minutes, + include_realm_default_subscriptions=include_realm_default_subscriptions, invite_as=invite_as, ) @@ -221,6 +215,7 @@ def generate_multiuse_invite_backend( default=PreregistrationUser.INVITE_AS["MEMBER"], ), stream_ids: Sequence[int] = REQ(json_validator=check_list(check_int), default=[]), + include_realm_default_subscriptions: bool = REQ(json_validator=check_bool, default=False), ) -> HttpResponse: if not user_profile.can_create_multiuse_invite_to_realm(): # Guest users case will not be handled here as it will @@ -248,17 +243,14 @@ def generate_multiuse_invite_backend( ) streams.append(stream) - if not user_profile.can_subscribe_other_users(): - if len(streams) != 0: - raise JsonableError( - _("You do not have permission to subscribe other users to channels.") - ) - - # We would subscribe the invited user to default streams even when the user - # inviting them does not have permission to subscribe others. - streams = get_slim_realm_default_streams(user_profile.realm_id) + if len(streams) and not user_profile.can_subscribe_other_users(): + raise JsonableError(_("You do not have permission to subscribe other users to channels.")) invite_link = do_create_multiuse_invite_link( - user_profile, invite_as, invite_expires_in_minutes, streams + user_profile, + invite_as, + invite_expires_in_minutes, + include_realm_default_subscriptions, + streams, ) return json_success(request, data={"invite_link": invite_link}) diff --git a/zerver/views/registration.py b/zerver/views/registration.py index 1d870a4bf6..8296f951e9 100644 --- a/zerver/views/registration.py +++ b/zerver/views/registration.py @@ -767,6 +767,7 @@ def prepare_activation_url( realm: Optional[Realm], streams: Optional[Iterable[Stream]] = None, invited_as: Optional[int] = None, + include_realm_default_subscriptions: Optional[bool] = None, multiuse_invite: Optional[MultiuseInvite] = None, ) -> str: """ @@ -780,6 +781,11 @@ def prepare_activation_url( if invited_as is not None: prereg_user.invited_as = invited_as + + if include_realm_default_subscriptions is not None: + prereg_user.include_realm_default_subscriptions = include_realm_default_subscriptions + + if invited_as is not None or include_realm_default_subscriptions is not None: prereg_user.save() confirmation_type = Confirmation.USER_REGISTRATION @@ -1016,6 +1022,7 @@ def accounts_home( from_multiuse_invite = False streams_to_subscribe = None invited_as = None + include_realm_default_subscriptions = None if multiuse_object: # multiuse_object's realm should have been validated by the caller, @@ -1026,6 +1033,7 @@ def accounts_home( streams_to_subscribe = multiuse_object.streams.all() from_multiuse_invite = True invited_as = multiuse_object.invited_as + include_realm_default_subscriptions = multiuse_object.include_realm_default_subscriptions if request.method == "POST": form = HomepageForm( @@ -1059,6 +1067,7 @@ def accounts_home( realm=realm, streams=streams_to_subscribe, invited_as=invited_as, + include_realm_default_subscriptions=include_realm_default_subscriptions, multiuse_invite=multiuse_object, ) try: