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: