users: Fix check_valid_bot_config bug.

Currently `check_valid_bot_config` checks if `config_options` has a
falsy value to determine whether an integration is invalid or not. But,
what it actually does is check whether an exists in WEBHOOK_INTEGRATIONS
and it has at least one WebhookConfigOption.

It should instead, check if `config_options` is `None` to determine
whether an integration is invalid or not.
This commit is contained in:
PieterCK
2025-07-10 15:20:02 +07:00
committed by Tim Abbott
parent e633ba5c71
commit 6bf6249d56
2 changed files with 22 additions and 7 deletions

View File

@@ -134,7 +134,7 @@ def check_valid_bot_config(
option.name: option.validator for option in integration.config_options option.name: option.validator for option in integration.config_options
} }
break break
if not config_options: if config_options is None:
raise JsonableError( raise JsonableError(
_("Invalid integration '{integration_name}'.").format(integration_name=service_name) _("Invalid integration '{integration_name}'.").format(integration_name=service_name)
) )

View File

@@ -39,7 +39,7 @@ def _check_string(var_name: str, val: str) -> str | None:
return None return None
stripe_sample_config_options = [ test_sample_config_options = [
WebhookIntegration( WebhookIntegration(
"stripe", "stripe",
["financial"], ["financial"],
@@ -50,6 +50,7 @@ stripe_sample_config_options = [
) )
], ],
), ),
WebhookIntegration("helloworld", ["misc"], display_name="Hello World"),
] ]
@@ -2043,7 +2044,7 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
self.assertEqual(bot_owner_user_ids(new_bot), set()) self.assertEqual(bot_owner_user_ids(new_bot), set())
@patch("zerver.lib.integrations.WEBHOOK_INTEGRATIONS", stripe_sample_config_options) @patch("zerver.lib.integrations.WEBHOOK_INTEGRATIONS", test_sample_config_options)
def test_create_incoming_webhook_bot_with_service_name_and_with_keys(self) -> None: def test_create_incoming_webhook_bot_with_service_name_and_with_keys(self) -> None:
self.login("hamlet") self.login("hamlet")
bot_metadata = { bot_metadata = {
@@ -2060,7 +2061,21 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
config_data, {"integration_id": "stripe", "stripe_api_key": "sample-api-key"} config_data, {"integration_id": "stripe", "stripe_api_key": "sample-api-key"}
) )
@patch("zerver.lib.integrations.WEBHOOK_INTEGRATIONS", stripe_sample_config_options) @patch("zerver.lib.integrations.WEBHOOK_INTEGRATIONS", test_sample_config_options)
def test_create_incoming_webhook_bot_with_service_name_and_no_config_options(self) -> None:
self.login("hamlet")
bot_metadata = {
"full_name": "My Hello World Bot",
"short_name": "my-helloworld",
"bot_type": UserProfile.INCOMING_WEBHOOK_BOT,
"service_name": "helloworld",
}
self.create_bot(**bot_metadata)
new_bot = UserProfile.objects.get(full_name="My Hello World Bot")
config_data = get_bot_config(new_bot)
self.assertEqual(config_data, {"integration_id": "helloworld"})
@patch("zerver.lib.integrations.WEBHOOK_INTEGRATIONS", test_sample_config_options)
def test_create_incoming_webhook_bot_with_service_name_incorrect_keys(self) -> None: def test_create_incoming_webhook_bot_with_service_name_incorrect_keys(self) -> None:
self.login("hamlet") self.login("hamlet")
bot_metadata = { bot_metadata = {
@@ -2077,7 +2092,7 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
with self.assertRaises(UserProfile.DoesNotExist): with self.assertRaises(UserProfile.DoesNotExist):
UserProfile.objects.get(full_name="My Stripe Bot") UserProfile.objects.get(full_name="My Stripe Bot")
@patch("zerver.lib.integrations.WEBHOOK_INTEGRATIONS", stripe_sample_config_options) @patch("zerver.lib.integrations.WEBHOOK_INTEGRATIONS", test_sample_config_options)
def test_create_incoming_webhook_bot_with_service_name_without_keys(self) -> None: def test_create_incoming_webhook_bot_with_service_name_without_keys(self) -> None:
self.login("hamlet") self.login("hamlet")
bot_metadata = { bot_metadata = {
@@ -2093,7 +2108,7 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
with self.assertRaises(UserProfile.DoesNotExist): with self.assertRaises(UserProfile.DoesNotExist):
UserProfile.objects.get(full_name="My Stripe Bot") UserProfile.objects.get(full_name="My Stripe Bot")
@patch("zerver.lib.integrations.WEBHOOK_INTEGRATIONS", stripe_sample_config_options) @patch("zerver.lib.integrations.WEBHOOK_INTEGRATIONS", test_sample_config_options)
def test_create_incoming_webhook_bot_without_service_name(self) -> None: def test_create_incoming_webhook_bot_without_service_name(self) -> None:
self.login("hamlet") self.login("hamlet")
bot_metadata = { bot_metadata = {
@@ -2106,7 +2121,7 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
with self.assertRaises(ConfigError): with self.assertRaises(ConfigError):
get_bot_config(new_bot) get_bot_config(new_bot)
@patch("zerver.lib.integrations.WEBHOOK_INTEGRATIONS", stripe_sample_config_options) @patch("zerver.lib.integrations.WEBHOOK_INTEGRATIONS", test_sample_config_options)
def test_create_incoming_webhook_bot_with_incorrect_service_name(self) -> None: def test_create_incoming_webhook_bot_with_incorrect_service_name(self) -> None:
self.login("hamlet") self.login("hamlet")
bot_metadata = { bot_metadata = {