diff --git a/zerver/lib/test_classes.py b/zerver/lib/test_classes.py index c401a5655a..753f3301be 100644 --- a/zerver/lib/test_classes.py +++ b/zerver/lib/test_classes.py @@ -756,11 +756,14 @@ class ZulipTestCase(TestCase): # Subscribe to a stream by making an API request def common_subscribe_to_streams(self, user: UserProfile, streams: Iterable[str], extra_post_data: Dict[str, Any]={}, invite_only: bool=False, + allow_fail: bool=False, **kwargs: Any) -> HttpResponse: post_data = {'subscriptions': ujson.dumps([{"name": stream} for stream in streams]), 'invite_only': ujson.dumps(invite_only)} post_data.update(extra_post_data) result = self.api_post(user, "/api/v1/users/me/subscriptions", post_data, **kwargs) + if not allow_fail: + self.assert_json_success(result) return result def check_user_subscribed_only_to_streams(self, user_name: str, diff --git a/zerver/tests/test_decorators.py b/zerver/tests/test_decorators.py index bbc82ccc3b..e91a308434 100644 --- a/zerver/tests/test_decorators.py +++ b/zerver/tests/test_decorators.py @@ -1537,7 +1537,7 @@ class TestAuthenticatedJsonPostViewDecorator(ZulipTestCase): def _do_test(self, user: UserProfile) -> HttpResponse: stream_name = "stream name" - self.common_subscribe_to_streams(user, [stream_name]) + self.common_subscribe_to_streams(user, [stream_name], allow_fail=True) data = {"password": initial_password(user.email), "stream": stream_name} return self.client_post('/json/subscriptions/exists', data) @@ -1665,7 +1665,7 @@ class TestRequireDecorators(ZulipTestCase): def test_require_non_guest_user_decorator(self) -> None: guest_user = self.example_user('polonius') self.login_user(guest_user) - result = self.common_subscribe_to_streams(guest_user, ["Denmark"]) + result = self.common_subscribe_to_streams(guest_user, ["Denmark"], allow_fail=True) self.assert_json_error(result, "Not allowed for guest users") outgoing_webhook_bot = self.example_user('outgoing_webhook_bot') diff --git a/zerver/tests/test_subs.py b/zerver/tests/test_subs.py index 9b85512e15..c7161551c0 100644 --- a/zerver/tests/test_subs.py +++ b/zerver/tests/test_subs.py @@ -1294,7 +1294,7 @@ class StreamAdminTest(ZulipTestCase): # Cannot create stream because not an admin. stream_name = ['admins_only'] - result = self.common_subscribe_to_streams(user_profile, stream_name) + result = self.common_subscribe_to_streams(user_profile, stream_name, allow_fail=True) self.assert_json_error(result, 'User cannot create streams.') # Make current user an admin. @@ -1302,8 +1302,7 @@ class StreamAdminTest(ZulipTestCase): # Can successfully create stream as user is now an admin. stream_name = ['admins_only'] - result = self.common_subscribe_to_streams(user_profile, stream_name) - self.assert_json_success(result) + self.common_subscribe_to_streams(user_profile, stream_name) # Allow users older than the waiting period to create streams. do_set_realm_property(user_profile.realm, 'create_stream_policy', @@ -1311,8 +1310,7 @@ class StreamAdminTest(ZulipTestCase): # Can successfully create stream despite being under waiting period because user is admin. stream_name = ['waiting_period_as_admin'] - result = self.common_subscribe_to_streams(user_profile, stream_name) - self.assert_json_success(result) + self.common_subscribe_to_streams(user_profile, stream_name) # Make current user no longer an admin. do_change_user_role(user_profile, UserProfile.ROLE_MEMBER) @@ -1320,7 +1318,7 @@ class StreamAdminTest(ZulipTestCase): # Cannot create stream because user is not an admin and is not older than the waiting # period. stream_name = ['waiting_period'] - result = self.common_subscribe_to_streams(user_profile, stream_name) + result = self.common_subscribe_to_streams(user_profile, stream_name, allow_fail=True) self.assert_json_error(result, 'User cannot create streams.') # Make user account 11 days old.. @@ -1329,8 +1327,7 @@ class StreamAdminTest(ZulipTestCase): # Can successfully create stream now that account is old enough. stream_name = ['waiting_period'] - result = self.common_subscribe_to_streams(user_profile, stream_name) - self.assert_json_success(result) + self.common_subscribe_to_streams(user_profile, stream_name) def test_invite_to_stream_by_invite_period_threshold(self) -> None: """ @@ -1354,15 +1351,19 @@ class StreamAdminTest(ZulipTestCase): # Hamlet creates a stream as an admin.. stream_name = ['waitingperiodtest'] - result = self.common_subscribe_to_streams(hamlet_user, stream_name) - self.assert_json_success(result) + self.common_subscribe_to_streams(hamlet_user, stream_name) # Can only invite users to stream if their account is ten days old.. do_change_user_role(hamlet_user, UserProfile.ROLE_MEMBER) do_set_realm_property(hamlet_user.realm, 'waiting_period_threshold', 10) # Attempt and fail to invite Cordelia to the stream.. - result = self.common_subscribe_to_streams(hamlet_user, stream_name, {"principals": ujson.dumps([cordelia_user_id])}) + result = self.common_subscribe_to_streams( + hamlet_user, + stream_name, + {"principals": ujson.dumps([cordelia_user_id])}, + allow_fail=True, + ) self.assert_json_error(result, "Your account is too new to modify other users' subscriptions.") @@ -1370,8 +1371,7 @@ class StreamAdminTest(ZulipTestCase): do_set_realm_property(hamlet_user.realm, 'waiting_period_threshold', 0) # Attempt and succeed to invite Cordelia to the stream.. - result = self.common_subscribe_to_streams(hamlet_user, stream_name, {"principals": ujson.dumps([cordelia_user_id])}) - self.assert_json_success(result) + self.common_subscribe_to_streams(hamlet_user, stream_name, {"principals": ujson.dumps([cordelia_user_id])}) # Set threshold to 20 days.. do_set_realm_property(hamlet_user.realm, 'waiting_period_threshold', 20) @@ -1382,8 +1382,7 @@ class StreamAdminTest(ZulipTestCase): self.unsubscribe(cordelia_user, stream_name[0]) # Attempt and succeed to invite Aaron to the stream.. - result = self.common_subscribe_to_streams(hamlet_user, stream_name, {"principals": ujson.dumps([cordelia_user_id])}) - self.assert_json_success(result) + self.common_subscribe_to_streams(hamlet_user, stream_name, {"principals": ujson.dumps([cordelia_user_id])}) def test_remove_already_not_subbed(self) -> None: """ @@ -2305,7 +2304,6 @@ class SubscriptionAPITest(ZulipTestCase): """ result = self.common_subscribe_to_streams(self.test_user, subscriptions, other_params, invite_only=invite_only) - self.assert_json_success(result) json = result.json() self.assertEqual(sorted(subscribed), sorted(json["subscribed"][email])) self.assertEqual(sorted(already_subscribed), sorted(json["already_subscribed"][email])) @@ -2367,7 +2365,7 @@ class SubscriptionAPITest(ZulipTestCase): current_stream = self.get_streams(invitee)[0] invite_streams = self.make_random_stream_names([current_stream])[:1] - result = self.common_subscribe_to_streams( + self.common_subscribe_to_streams( invitee, invite_streams, extra_post_data={ @@ -2375,7 +2373,6 @@ class SubscriptionAPITest(ZulipTestCase): 'principals': ujson.dumps([self.user_profile.id]), }, ) - self.assert_json_success(result) def test_successful_subscriptions_notifies_stream(self) -> None: """ @@ -2395,7 +2392,7 @@ class SubscriptionAPITest(ZulipTestCase): # picked up cache.cache_delete(cache.user_profile_by_email_cache_key(invitee.email)) - result = self.common_subscribe_to_streams( + self.common_subscribe_to_streams( invitee, invite_streams, extra_post_data=dict( @@ -2403,7 +2400,6 @@ class SubscriptionAPITest(ZulipTestCase): principals= ujson.dumps([self.user_profile.id]), ), ) - self.assert_json_success(result) msg = self.get_second_to_last_message() self.assertEqual(msg.recipient.type, Recipient.STREAM) @@ -2432,7 +2428,7 @@ class SubscriptionAPITest(ZulipTestCase): # picked up cache.cache_delete(cache.user_profile_by_email_cache_key(user.email)) - result = self.common_subscribe_to_streams( + self.common_subscribe_to_streams( user, invite_streams, extra_post_data=dict( @@ -2440,7 +2436,6 @@ class SubscriptionAPITest(ZulipTestCase): ), subdomain="testrealm", ) - self.assert_json_success(result) msg = self.get_second_to_last_message() self.assertEqual(msg.recipient.type, Recipient.STREAM) @@ -2462,7 +2457,7 @@ class SubscriptionAPITest(ZulipTestCase): self.test_realm.save() invite_streams = ['strange ) \\ test'] - result = self.common_subscribe_to_streams( + self.common_subscribe_to_streams( invitee, invite_streams, extra_post_data={ @@ -2470,7 +2465,6 @@ class SubscriptionAPITest(ZulipTestCase): 'principals': ujson.dumps([self.user_profile.id]), }, ) - self.assert_json_success(result) msg = self.get_second_to_last_message() self.assertEqual(msg.sender_id, self.notification_bot().id) @@ -2492,7 +2486,7 @@ class SubscriptionAPITest(ZulipTestCase): """ # character limit is 60 characters long_stream_name = "a" * 61 - result = self.common_subscribe_to_streams(self.test_user, [long_stream_name]) + result = self.common_subscribe_to_streams(self.test_user, [long_stream_name], allow_fail=True) self.assert_json_error(result, "Stream name too long (limit: 60 characters).") @@ -2502,23 +2496,21 @@ class SubscriptionAPITest(ZulipTestCase): null characters should return a JSON error. """ stream_name = "abc\000" - result = self.common_subscribe_to_streams(self.test_user, [stream_name]) + result = self.common_subscribe_to_streams(self.test_user, [stream_name], allow_fail=True) self.assert_json_error(result, f"Stream name '{stream_name}' contains NULL (0x00) characters.") def test_user_settings_for_adding_streams(self) -> None: with mock.patch('zerver.models.UserProfile.can_create_streams', return_value=False): - result = self.common_subscribe_to_streams(self.test_user, ['stream1']) + result = self.common_subscribe_to_streams(self.test_user, ['stream1'], allow_fail=True) self.assert_json_error(result, 'User cannot create streams.') with mock.patch('zerver.models.UserProfile.can_create_streams', return_value=True): - result = self.common_subscribe_to_streams(self.test_user, ['stream2']) - self.assert_json_success(result) + self.common_subscribe_to_streams(self.test_user, ['stream2']) # User should still be able to subscribe to an existing stream with mock.patch('zerver.models.UserProfile.can_create_streams', return_value=False): - result = self.common_subscribe_to_streams(self.test_user, ['stream2']) - self.assert_json_success(result) + self.common_subscribe_to_streams(self.test_user, ['stream2']) def test_can_create_streams(self) -> None: othello = self.example_user('othello') @@ -2555,30 +2547,36 @@ class SubscriptionAPITest(ZulipTestCase): do_set_realm_property(realm, "invite_to_stream_policy", Realm.POLICY_ADMINS_ONLY) result = self.common_subscribe_to_streams( - self.test_user, ['stream1'], {"principals": ujson.dumps([invitee_user_id])}) + self.test_user, + ['stream1'], + {"principals": ujson.dumps([invitee_user_id])}, + allow_fail=True, + ) self.assert_json_error( result, "Only administrators can modify other users' subscriptions.") do_set_realm_property(realm, "invite_to_stream_policy", Realm.POLICY_MEMBERS_ONLY) - result = self.common_subscribe_to_streams( + self.common_subscribe_to_streams( self.test_user, ['stream2'], {"principals": ujson.dumps([ self.test_user.id, invitee_user_id])}) - self.assert_json_success(result) self.unsubscribe(user_profile, "stream2") do_set_realm_property(realm, "invite_to_stream_policy", Realm.POLICY_FULL_MEMBERS_ONLY) do_set_realm_property(realm, "waiting_period_threshold", 100000) result = self.common_subscribe_to_streams( - self.test_user, ['stream2'], {"principals": ujson.dumps([invitee_user_id])}) + self.test_user, + ['stream2'], + {"principals": ujson.dumps([invitee_user_id])}, + allow_fail=True, + ) self.assert_json_error( result, "Your account is too new to modify other users' subscriptions.") do_set_realm_property(realm, "waiting_period_threshold", 0) - result = self.common_subscribe_to_streams( + self.common_subscribe_to_streams( self.test_user, ['stream2'], {"principals": ujson.dumps([invitee_user_id])}) - self.assert_json_success(result) def test_can_subscribe_other_users(self) -> None: """ @@ -2611,7 +2609,7 @@ class SubscriptionAPITest(ZulipTestCase): """ # currently, the only invalid name is the empty string invalid_stream_name = "" - result = self.common_subscribe_to_streams(self.test_user, [invalid_stream_name]) + result = self.common_subscribe_to_streams(self.test_user, [invalid_stream_name], allow_fail=True) self.assert_json_error(result, f"Invalid stream name '{invalid_stream_name}'") @@ -2837,7 +2835,7 @@ class SubscriptionAPITest(ZulipTestCase): def test_guest_user_subscribe(self) -> None: """Guest users cannot subscribe themselves to anything""" guest_user = self.example_user("polonius") - result = self.common_subscribe_to_streams(guest_user, ["Denmark"]) + result = self.common_subscribe_to_streams(guest_user, ["Denmark"], allow_fail=True) self.assert_json_error(result, "Not allowed for guest users") # Verify the internal checks also block guest users. @@ -2857,7 +2855,7 @@ class SubscriptionAPITest(ZulipTestCase): list_to_streams(streams_raw, guest_user) stream = self.make_stream('private_stream', invite_only=True) - result = self.common_subscribe_to_streams(guest_user, ["private_stream"]) + result = self.common_subscribe_to_streams(guest_user, ["private_stream"], allow_fail=True) self.assert_json_error(result, "Not allowed for guest users") self.assertEqual(filter_stream_authorization(guest_user, [stream]), ([], [stream])) @@ -2994,6 +2992,7 @@ class SubscriptionAPITest(ZulipTestCase): stream_names, dict(principals=ujson.dumps([mit_user.id])), subdomain="zephyr", + allow_fail=True, ) # Make sure Zephyr mirroring realms such as MIT do not get # any tornado subscription events @@ -3049,11 +3048,10 @@ class SubscriptionAPITest(ZulipTestCase): post_data = dict( principals=ujson.dumps([target_profile.id]), ) - result = self.common_subscribe_to_streams(self.test_user, "Verona", post_data) - self.assert_json_success(result) + self.common_subscribe_to_streams(self.test_user, "Verona", post_data) do_deactivate_user(target_profile) - result = self.common_subscribe_to_streams(self.test_user, "Denmark", post_data) + result = self.common_subscribe_to_streams(self.test_user, "Denmark", post_data, allow_fail=True) self.assert_json_error( result, f"User not authorized to execute queries on behalf of '{target_profile.id}'", @@ -3088,7 +3086,8 @@ class SubscriptionAPITest(ZulipTestCase): with self.assertRaises(UserProfile.DoesNotExist): get_user(invalid_principal, invalid_principal_realm) result = self.common_subscribe_to_streams(self.test_user, self.streams, - {"principals": ujson.dumps([invalid_principal])}) + {"principals": ujson.dumps([invalid_principal])}, + allow_fail=True) self.assert_json_error( result, f"User not authorized to execute queries on behalf of '{invalid_principal}'", @@ -3101,7 +3100,8 @@ class SubscriptionAPITest(ZulipTestCase): with self.assertRaises(UserProfile.DoesNotExist): get_user_profile_by_id_in_realm(invalid_principal, invalid_principal_realm) result = self.common_subscribe_to_streams(self.test_user, self.streams, - {"principals": ujson.dumps([invalid_principal])}) + {"principals": ujson.dumps([invalid_principal])}, + allow_fail=True) self.assert_json_error( result, f"User not authorized to execute queries on behalf of '{invalid_principal}'", @@ -3118,7 +3118,8 @@ class SubscriptionAPITest(ZulipTestCase): # verify that principal exists (thus, the reason for the error is the cross-realming) self.assertIsInstance(profile, UserProfile) result = self.common_subscribe_to_streams(self.test_user, self.streams, - {"principals": ujson.dumps([principal])}) + {"principals": ujson.dumps([principal])}, + allow_fail=True) self.assert_json_error( result, f"User not authorized to execute queries on behalf of '{principal}'", @@ -3242,8 +3243,7 @@ class SubscriptionAPITest(ZulipTestCase): """ stream_name = "new_public_stream" cordelia = self.example_user('cordelia') - result = self.common_subscribe_to_streams(cordelia, [stream_name], - invite_only=False) + self.common_subscribe_to_streams(cordelia, [stream_name], invite_only=False) result = self.client_post("/json/subscriptions/exists", {"stream": stream_name, "autosubscribe": "false"}) self.assert_json_success(result) @@ -3262,8 +3262,7 @@ class SubscriptionAPITest(ZulipTestCase): """ stream_name = "Saxony" cordelia = self.example_user('cordelia') - result = self.common_subscribe_to_streams(cordelia, [stream_name], - invite_only=True) + self.common_subscribe_to_streams(cordelia, [stream_name], invite_only=True) stream = get_stream(stream_name, self.test_realm) result = self.client_post("/json/subscriptions/exists", @@ -3674,10 +3673,8 @@ class InviteOnlyStreamTest(ZulipTestCase): user = self.example_user('hamlet') self.login_user(user) - result1 = self.common_subscribe_to_streams(user, ["Saxony"], invite_only=True) - self.assert_json_success(result1) - result2 = self.common_subscribe_to_streams(user, ["Normandy"], invite_only=False) - self.assert_json_success(result2) + self.common_subscribe_to_streams(user, ["Saxony"], invite_only=True) + self.common_subscribe_to_streams(user, ["Normandy"], invite_only=False) result = self.api_get(user, "/api/v1/users/me/subscriptions") self.assert_json_success(result) self.assertIn("subscriptions", result.json()) @@ -3696,7 +3693,6 @@ class InviteOnlyStreamTest(ZulipTestCase): stream_name = "Saxony" result = self.common_subscribe_to_streams(hamlet, [stream_name], invite_only=True) - self.assert_json_success(result) json = result.json() self.assertEqual(json["subscribed"], {hamlet.email: [stream_name]}) @@ -3704,14 +3700,13 @@ class InviteOnlyStreamTest(ZulipTestCase): # Subscribing oneself to an invite-only stream is not allowed self.login_user(othello) - result = self.common_subscribe_to_streams(othello, [stream_name]) + result = self.common_subscribe_to_streams(othello, [stream_name], allow_fail=True) self.assert_json_error(result, 'Unable to access stream (Saxony).') # authorization_errors_fatal=False works self.login_user(othello) result = self.common_subscribe_to_streams(othello, [stream_name], extra_post_data={'authorization_errors_fatal': ujson.dumps(False)}) - self.assert_json_success(result) json = result.json() self.assertEqual(json["unauthorized"], [stream_name]) self.assertEqual(json["subscribed"], {}) @@ -3722,7 +3717,6 @@ class InviteOnlyStreamTest(ZulipTestCase): result = self.common_subscribe_to_streams( hamlet, [stream_name], extra_post_data={'principals': ujson.dumps([othello.id])}) - self.assert_json_success(result) json = result.json() self.assertEqual(json["subscribed"], {othello.email: [stream_name]}) self.assertEqual(json["already_subscribed"], {}) @@ -3805,13 +3799,11 @@ class GetSubscribersTest(ZulipTestCase): self.example_user("othello").id, self.example_user("cordelia").id, ] - ret = self.common_subscribe_to_streams( + self.common_subscribe_to_streams( self.user_profile, streams, dict(principals=ujson.dumps(users_to_subscribe))) - self.assert_json_success(ret) - msg = ''' @**King Hamlet** subscribed you to the following streams: @@ -3830,21 +3822,19 @@ class GetSubscribersTest(ZulipTestCase): self.assert_user_got_subscription_notification(msg) # Subscribe ourself first. - ret = self.common_subscribe_to_streams( + self.common_subscribe_to_streams( self.user_profile, ["stream_invite_only_1"], dict(principals=ujson.dumps([self.user_profile.id])), invite_only=True) - self.assert_json_success(ret) # Now add in other users, and this should trigger messages # to notify the user. - ret = self.common_subscribe_to_streams( + self.common_subscribe_to_streams( self.user_profile, ["stream_invite_only_1"], dict(principals=ujson.dumps(users_to_subscribe)), invite_only=True) - self.assert_json_success(ret) msg = ''' @**King Hamlet** subscribed you to the stream #**stream_invite_only_1**. @@ -3889,23 +3879,21 @@ class GetSubscribersTest(ZulipTestCase): for stream_name in public_streams: self.make_stream(stream_name, realm=realm) - ret = self.common_subscribe_to_streams( + self.common_subscribe_to_streams( self.user_profile, public_streams, dict(principals=ujson.dumps(users_to_subscribe)), ) - self.assert_json_success(ret) create_public_streams() def create_private_streams() -> None: - ret = self.common_subscribe_to_streams( + self.common_subscribe_to_streams( self.user_profile, private_streams, dict(principals=ujson.dumps(users_to_subscribe)), invite_only=True, ) - self.assert_json_success(ret) create_private_streams() @@ -4024,13 +4012,12 @@ class GetSubscribersTest(ZulipTestCase): stream = self.subscribe(mit_user_profile, "mit_stream") self.assertTrue(stream.is_in_zephyr_realm) - ret = self.common_subscribe_to_streams( + self.common_subscribe_to_streams( mit_user_profile, ["mit_invite_only"], dict(principals=ujson.dumps(users_to_subscribe)), invite_only=True, subdomain="zephyr") - self.assert_json_success(ret) with queries_captured() as queries: subscribed_streams, _ = gather_subscriptions(