From 2d043ffb8368f3c53dbef74635cabe3688009e8b Mon Sep 17 00:00:00 2001 From: Anders Kaseorg Date: Thu, 30 Jul 2026 13:25:03 -0700 Subject: [PATCH 01/14] ruff: Fix TRY201 Use `raise` without specifying exception name. Signed-off-by: Anders Kaseorg --- corporate/lib/stripe.py | 2 +- corporate/views/upgrade.py | 6 +++--- zerver/decorator.py | 6 +++--- zerver/lib/avatar.py | 4 ++-- zerver/lib/event_schema.py | 4 ++-- zerver/lib/push_notifications.py | 2 +- zerver/lib/send_email.py | 4 ++-- zerver/management/commands/send_password_reset_email.py | 2 +- zilencer/auth.py | 4 ++-- 9 files changed, 17 insertions(+), 17 deletions(-) diff --git a/corporate/lib/stripe.py b/corporate/lib/stripe.py index 258b8ca8692ac..120bfe9780430 100644 --- a/corporate/lib/stripe.py +++ b/corporate/lib/stripe.py @@ -1370,7 +1370,7 @@ def create_stripe_invoice_and_charge( if isinstance(e, stripe.CardError): raise StripeCardError("card error", e.user_message) else: # nocoverage - raise e + raise assert stripe_invoice.id is not None return stripe_invoice.id diff --git a/corporate/views/upgrade.py b/corporate/views/upgrade.py index a135275536b06..9de89bc57dfba 100644 --- a/corporate/views/upgrade.py +++ b/corporate/views/upgrade.py @@ -68,7 +68,7 @@ def upgrade( license_management, licenses, ) - raise e + raise except Exception: billing_logger.exception("Uncaught exception in billing:", stack_info=True) error_message = BillingError.CONTACT_SUPPORT.format(email=settings.ZULIP_ADMINISTRATOR) @@ -118,7 +118,7 @@ def remote_realm_upgrade( license_management, licenses, ) - raise e + raise except Exception: # nocoverage billing_logger.exception("Uncaught exception in billing:", stack_info=True) error_message = BillingError.CONTACT_SUPPORT.format(email=settings.ZULIP_ADMINISTRATOR) @@ -168,7 +168,7 @@ def remote_server_upgrade( license_management, licenses, ) - raise e + raise except Exception: # nocoverage billing_logger.exception("Uncaught exception in billing:", stack_info=True) error_message = BillingError.CONTACT_SUPPORT.format(email=settings.ZULIP_ADMINISTRATOR) diff --git a/zerver/decorator.py b/zerver/decorator.py index dd385b412131c..039e89d0199f3 100644 --- a/zerver/decorator.py +++ b/zerver/decorator.py @@ -412,7 +412,7 @@ def _wrapped_func_arguments( # the sender's fault, so tell the owner notify_bot_owner_about_invalid_json(user_profile, webhook_client_name) - raise err + raise # Store the event types registered for this webhook as an attribute, which can be access # later conveniently in zerver.lib.test_classes.WebhookTestCase. @@ -828,7 +828,7 @@ def _wrapped_func_arguments( return view_func(request, user_profile, *args, **kwargs) except Exception as err: if not webhook_client_name: - raise err + raise if not isinstance(err, JsonableError): # An unexpected exception of some form -- log it @@ -840,7 +840,7 @@ def _wrapped_func_arguments( err.webhook_name = webhook_client_name log_exception_to_webhook_logger(request, err) - raise err + raise return _wrapped_func_arguments diff --git a/zerver/lib/avatar.py b/zerver/lib/avatar.py index d518cd837fcd7..3e5bcd83b12dc 100644 --- a/zerver/lib/avatar.py +++ b/zerver/lib/avatar.py @@ -206,9 +206,9 @@ def generate_avatar_jdenticon(input: str, medium: bool) -> bytes: try: stdout = subprocess.check_output(command) return stdout - except subprocess.CalledProcessError as error: # nocoverage + except subprocess.CalledProcessError: # nocoverage logger.exception("Jdenticon generation failed for user_id: %s", input) - raise error + raise def generate_and_upload_jdenticon_avatar( diff --git a/zerver/lib/event_schema.py b/zerver/lib/event_schema.py index 9619dc2789f3b..8a1a62b8f5ece 100644 --- a/zerver/lib/event_schema.py +++ b/zerver/lib/event_schema.py @@ -149,7 +149,7 @@ def f(label: str, event: dict[str, object]) -> None: del event["id"] try: validate_with_model(event, base_model) - except Exception as e: # nocoverage + except Exception: # nocoverage print(f""" FAILURE: @@ -170,7 +170,7 @@ def f(label: str, event: dict[str, object]) -> None: """) PrettyPrinter(indent=4).pprint(event) - raise e + raise return f diff --git a/zerver/lib/push_notifications.py b/zerver/lib/push_notifications.py index 51d39c93c823b..1c8c278e7902c 100644 --- a/zerver/lib/push_notifications.py +++ b/zerver/lib/push_notifications.py @@ -1566,7 +1566,7 @@ def send_push_notifications( if is_test_notification: # Propagate the exception to the caller to notify the client # about the error while attempting to send test push notification. - raise e + raise return diff --git a/zerver/lib/send_email.py b/zerver/lib/send_email.py index 98aa223706c04..e3f013338ba39 100644 --- a/zerver/lib/send_email.py +++ b/zerver/lib/send_email.py @@ -474,9 +474,9 @@ def send_future_email( assert len(to_emails) == 1 email.address = parseaddr(to_emails[0])[1] email.save() - except Exception as e: + except Exception: email.delete() - raise e + raise def send_email_to_admins( diff --git a/zerver/management/commands/send_password_reset_email.py b/zerver/management/commands/send_password_reset_email.py index 1c2bcd19073f3..1ccd497fd0d15 100644 --- a/zerver/management/commands/send_password_reset_email.py +++ b/zerver/management/commands/send_password_reset_email.py @@ -45,7 +45,7 @@ def handle(self, *args: Any, **options: str) -> None: raise CommandError( "You have to pass -u/--users or -a/--all-users or --entire-server." ) - raise error + raise if options["only_never_logged_in"]: users = users.filter(tos_version=-1) diff --git a/zilencer/auth.py b/zilencer/auth.py index a0ccad76ead17..ab0cf5b69923c 100644 --- a/zilencer/auth.py +++ b/zilencer/auth.py @@ -95,9 +95,9 @@ def rate_limit_remote_server( try: RateLimitedRemoteZulipServer(remote_server, domain=domain).rate_limit_request(request) - except RateLimitedError as e: + except RateLimitedError: logger.warning("Remote server %s exceeded rate limits on domain %s", remote_server, domain) - raise e + raise def validate_remote_server( From cb8499df9583f514d63e3eca9e0ce43848388803 Mon Sep 17 00:00:00 2001 From: Anders Kaseorg Date: Thu, 30 Jul 2026 13:26:16 -0700 Subject: [PATCH 02/14] ruff: Fix RET501 Do not explicitly `return None` in function if it is the only possible return value. Signed-off-by: Anders Kaseorg --- corporate/lib/stripe.py | 2 +- zerver/lib/import_realm.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/corporate/lib/stripe.py b/corporate/lib/stripe.py index 120bfe9780430..18e57b509a732 100644 --- a/corporate/lib/stripe.py +++ b/corporate/lib/stripe.py @@ -4151,7 +4151,7 @@ def create_complimentary_access_plan( plan_tier = CustomerPlan.TIER_SELF_HOSTED_LEGACY if isinstance(self, RealmBillingSession): # nocoverage # TODO implement a complimentary access plan/tier for Zulip Cloud. - return None + return customer = self.update_or_create_customer() complimentary_access_plan = self.create_customer_plan( diff --git a/zerver/lib/import_realm.py b/zerver/lib/import_realm.py index cf6ee8a8f7605..42792f5bb57c2 100644 --- a/zerver/lib/import_realm.py +++ b/zerver/lib/import_realm.py @@ -1071,7 +1071,7 @@ def fix_subscriptions_is_user_active_column( def process_avatars(sanitized_record: SanitizedRecord) -> None: if not sanitized_record.safe_resolved_source_path.endswith(".original"): - return None + return user_profile = get_user_profile_by_id(sanitized_record.raw_record["user_profile_id"]) if settings.LOCAL_AVATARS_DIR is not None: avatar_path = user_avatar_base_path_from_ids( From 4b7ab48b400a3c4d6b174afd8aac4a1e73eddb27 Mon Sep 17 00:00:00 2001 From: Anders Kaseorg Date: Thu, 30 Jul 2026 14:09:51 -0700 Subject: [PATCH 03/14] ruff: Fix ASYNC230 Async functions should not open files with blocking methods. Signed-off-by: Anders Kaseorg --- zerver/lib/push_notifications.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/zerver/lib/push_notifications.py b/zerver/lib/push_notifications.py index 1c8c278e7902c..4bbc32d1f998d 100644 --- a/zerver/lib/push_notifications.py +++ b/zerver/lib/push_notifications.py @@ -185,6 +185,11 @@ def get_apns_context() -> APNsContext | None: if not has_apns_credentials(): # nocoverage return None + key: str | None = None + if settings.APNS_TOKEN_KEY_FILE: # nocoverage + with open(settings.APNS_TOKEN_KEY_FILE) as f: + key = f.read() + # NB if called concurrently, this will make excess connections. # That's a little sloppy, but harmless unless a server gets # hammered with a ton of these all at once after startup. @@ -200,10 +205,6 @@ async def err_func( pass # nocoverage async def make_apns() -> aioapns.APNs: - key: str | None = None - if settings.APNS_TOKEN_KEY_FILE: # nocoverage - with open(settings.APNS_TOKEN_KEY_FILE) as f: - key = f.read() return aioapns.APNs( client_cert=settings.APNS_CERT_FILE, key=key, From cd2aed28235d103314da24c3edfb9efb5a4ff712 Mon Sep 17 00:00:00 2001 From: Anders Kaseorg Date: Thu, 30 Jul 2026 14:28:04 -0700 Subject: [PATCH 04/14] ruff: Fix TRY401 Redundant exception object included in `logging.exception` call. Signed-off-by: Anders Kaseorg --- corporate/lib/stripe.py | 2 +- puppet/kandra/files/statuspage-pusher | 4 +-- puppet/zulip/files/postgresql/wal-g-exporter | 4 +-- zerver/actions/data_import.py | 2 +- zerver/forms.py | 4 +-- zerver/lib/cache.py | 8 ++--- zerver/lib/email_mirror_server.py | 4 +-- zerver/lib/send_email.py | 2 +- zerver/lib/thumbnail.py | 4 +-- zerver/lib/zulip_update_announcements.py | 4 +-- zerver/middleware.py | 4 +-- zerver/tests/test_email_mirror.py | 9 +++-- zerver/tests/test_send_email.py | 37 ++++++++++++-------- zerver/views/antispam.py | 4 +-- zerver/views/sentry.py | 4 +-- 15 files changed, 54 insertions(+), 42 deletions(-) diff --git a/corporate/lib/stripe.py b/corporate/lib/stripe.py index 18e57b509a732..d8fc75e312438 100644 --- a/corporate/lib/stripe.py +++ b/corporate/lib/stripe.py @@ -6012,7 +6012,7 @@ def invoice_plans_as_needed(event_time: datetime | None = None) -> None: stack_info=True, ) else: - billing_logger.exception(e, stack_info=True) # nocoverage + billing_logger.exception("Error while invoicing", stack_info=True) # nocoverage def is_realm_on_free_trial(realm: Realm) -> bool: diff --git a/puppet/kandra/files/statuspage-pusher b/puppet/kandra/files/statuspage-pusher index 2869a875f226a..3202fb063cc66 100755 --- a/puppet/kandra/files/statuspage-pusher +++ b/puppet/kandra/files/statuspage-pusher @@ -71,8 +71,8 @@ def main() -> None: for metric_id, query in metrics.items(): try: update_metric(metric_id, query, page_id, oauth_token) - except Exception as e: - logging.exception(e) + except Exception: + logging.exception("Error while updating metric") time.sleep(30) diff --git a/puppet/zulip/files/postgresql/wal-g-exporter b/puppet/zulip/files/postgresql/wal-g-exporter index 1aaaf5e5b783d..e394b121fce80 100755 --- a/puppet/zulip/files/postgresql/wal-g-exporter +++ b/puppet/zulip/files/postgresql/wal-g-exporter @@ -141,8 +141,8 @@ class WalGPrometheusServer(BaseHTTPRequestHandler): (t("finish_time") - t("start_time")) / timedelta(seconds=1), labels ) backup_ok(1) - except Exception as e: - logging.exception(e) + except Exception: + logging.exception("Error while getting backup information") finally: self.print_metrics() self.log_message( diff --git a/zerver/actions/data_import.py b/zerver/actions/data_import.py index 75a4ec93e41a2..7f1ef8157e5fe 100644 --- a/zerver/actions/data_import.py +++ b/zerver/actions/data_import.py @@ -189,7 +189,7 @@ def record_created_realm(new_realm: Realm) -> None: preregistration_realm.save() logger.info("(%s) All done!", string_id) except Exception as e: - logger.exception(e) + logger.exception("Error during Slack import") try: preregistration_realm.created_realm = None preregistration_realm.data_import_metadata["is_import_work_queued"] = False diff --git a/zerver/forms.py b/zerver/forms.py index 30b8c43823600..d2ef200ab3b52 100644 --- a/zerver/forms.py +++ b/zerver/forms.py @@ -405,8 +405,8 @@ def validate_captcha_payload(request: HttpRequest, captcha_payload: str) -> None raise forms.ValidationError(_("Validation failed, please try again.")) except forms.ValidationError: raise - except Exception as e: - logging.exception(e) + except Exception: + logging.exception("Error while validating altcha solution") raise forms.ValidationError(_("Validation failed, please try again.")) captcha_data = orjson.loads(base64.b64decode(captcha_payload)) diff --git a/zerver/lib/cache.py b/zerver/lib/cache.py index 6ab1b604d20a5..6d2ad85f207de 100644 --- a/zerver/lib/cache.py +++ b/zerver/lib/cache.py @@ -251,8 +251,8 @@ def cache_set( val = (val,) try: cache_backend.set(final_key, val, timeout=timeout) - except MemcachedException as e: - logger.exception(e) + except MemcachedException: + logger.exception("Error while storing to cache") remote_cache_stats_finish() @@ -306,8 +306,8 @@ def cache_set_many( remote_cache_stats_start() try: get_cache_backend(cache_name).set_many(items, timeout=timeout) - except MemcachedException as e: - logger.exception(e) + except MemcachedException: + logger.exception("Error while storing to cache") remote_cache_stats_finish() diff --git a/zerver/lib/email_mirror_server.py b/zerver/lib/email_mirror_server.py index 512d3465b45dc..763293f019423 100644 --- a/zerver/lib/email_mirror_server.py +++ b/zerver/lib/email_mirror_server.py @@ -64,8 +64,8 @@ def send_to_postmaster(msg: email.message.Message) -> None: e.smtp_error, stack_info=True, ) - except smtplib.SMTPException as e: - logger.exception("Error sending bounce email to %s: %s", mail.to, str(e), stack_info=True) + except smtplib.SMTPException: + logger.exception("Error sending bounce email to %s", mail.to, stack_info=True) class ZulipMessageHandler(MessageHandler): diff --git a/zerver/lib/send_email.py b/zerver/lib/send_email.py index e3f013338ba39..ee85b5a514d2d 100644 --- a/zerver/lib/send_email.py +++ b/zerver/lib/send_email.py @@ -352,7 +352,7 @@ def send_immediate_email( connection.close() raise EmailNotDeliveredError from e except (smtplib.SMTPException, OSError) as e: - logger.exception("Error sending %s email to %s: %s", template, mail.to, e, stack_info=True) + logger.exception("Error sending %s email to %s", template, mail.to, stack_info=True) connection.close() raise EmailNotDeliveredError from e diff --git a/zerver/lib/thumbnail.py b/zerver/lib/thumbnail.py index 67e651364204d..ff2867c7ac03a 100644 --- a/zerver/lib/thumbnail.py +++ b/zerver/lib/thumbnail.py @@ -194,8 +194,8 @@ def libvips_check_image( try: yield source_image - except pyvips.Error as e: # nocoverage - logging.exception(e) + except pyvips.Error: # nocoverage + logging.exception("Error while processing image") raise BadImageError(_("Image is corrupted or truncated")) diff --git a/zerver/lib/zulip_update_announcements.py b/zerver/lib/zulip_update_announcements.py index 97a90a78d73c2..4bb3c12282321 100644 --- a/zerver/lib/zulip_update_announcements.py +++ b/zerver/lib/zulip_update_announcements.py @@ -809,8 +809,8 @@ def send_zulip_update_announcements(skip_delay: bool, progress: bool = False) -> for i, realm in enumerate(realms, start=1): try: send_zulip_update_announcements_to_realm(realm, skip_delay) - except Exception as e: # nocoverage - logging.exception(e) + except Exception: # nocoverage + logging.exception("Error while sending update announcements") finally: if progress and i % 50 == 0: # nocoverage print(f"Processed {i}/{len(realms)} realms...") diff --git a/zerver/middleware.py b/zerver/middleware.py index 217108eed4d40..c23cac63cb7ce 100644 --- a/zerver/middleware.py +++ b/zerver/middleware.py @@ -299,8 +299,8 @@ def process_request(self, request: HttpRequest) -> None: try: request_notes.client_name, request_notes.client_version = parse_client(request) - except JsonableError as e: - logging.exception(e) + except JsonableError: + logging.exception("Error while parsing client from request") request_notes.client_name = "Unparsable" request_notes.client_version = None diff --git a/zerver/tests/test_email_mirror.py b/zerver/tests/test_email_mirror.py index cd2555b46fde9..efc0f3f35aade 100644 --- a/zerver/tests/test_email_mirror.py +++ b/zerver/tests/test_email_mirror.py @@ -2107,11 +2107,14 @@ def test_send_postmaster_failure(self) -> None: self.assertLogs("zerver.lib.email_mirror", "ERROR") as error_log, ): send_to_postmaster(email) - self.assert_length(error_log.output, 1) + self.assert_length(error_log.records, 1) + self.assertEqual(error_log.records[0].levelname, "ERROR") self.assertEqual( - error_log.output[0].splitlines()[0], - "ERROR:zerver.lib.email_mirror:Error sending bounce email to ['desdemona+admin@zulip.com']: moose", + error_log.records[0].getMessage(), + "Error sending bounce email to ['desdemona+admin@zulip.com']", ) + assert error_log.records[0].exc_info is not None + self.assertEqual(str(error_log.records[0].exc_info[1]), "moose") with ( mock.patch.object( diff --git a/zerver/tests/test_send_email.py b/zerver/tests/test_send_email.py index 6f1292d37b2a6..659ffbf90a644 100644 --- a/zerver/tests/test_send_email.py +++ b/zerver/tests/test_send_email.py @@ -411,18 +411,22 @@ def test_send_email_exceptions(self) -> None: self.assertEqual(mail.extra_headers["From"], f"{from_name} <{FromAddress.NOREPLY}>") # We test the cases that should raise an EmailNotDeliveredError - errors = { - f"Unknown error sending password_reset email to {mail.to}": [0], - f"Error sending password_reset email to {mail.to}": [SMTPException()], - f"Error sending password_reset email to {mail.to}: {{'{address}': (550, b'User unknown')}}": [ - SMTPRecipientsRefused(recipients={address: (550, b"User unknown")}) - ], - f"Error sending password_reset email to {mail.to} with error code 242: From field too long": [ - SMTPDataError(242, "From field too long.") - ], - } + errors = [ + (f"Unknown error sending password_reset email to {mail.to}", None, [0]), + (f"Error sending password_reset email to {mail.to}", "", [SMTPException()]), + ( + f"Error sending password_reset email to {mail.to}", + f"{{{address!r}: (550, b'User unknown')}}", + [SMTPRecipientsRefused(recipients={address: (550, b"User unknown")})], + ), + ( + f"Error sending password_reset email to {mail.to} with error code 242: From field too long.", + "(242, 'From field too long.')", + [SMTPDataError(242, "From field too long.")], + ), + ] - for message, side_effect in errors.items(): + for message, exc_message, side_effect in errors: with mock.patch.object(EmailBackend, "send_messages", side_effect=side_effect): with ( self.assertLogs(logger=logger) as info_log, @@ -436,11 +440,16 @@ def test_send_email_exceptions(self) -> None: language="en", ) self.assert_length(info_log.records, 2) + self.assertEqual(info_log.records[0].levelname, "INFO") + self.assertEqual( + info_log.records[0].getMessage(), f"Sending password_reset email to {mail.to}" + ) + self.assertEqual(info_log.records[1].levelname, "ERROR") + self.assertEqual(info_log.records[1].getMessage(), message) self.assertEqual( - info_log.output[0], - f"INFO:{logger.name}:Sending password_reset email to {mail.to}", + info_log.records[1].exc_info and str(info_log.records[1].exc_info[1]), + exc_message, ) - self.assertTrue(info_log.output[1].startswith(f"ERROR:zulip.send_email:{message}")) def test_send_email_config_error_logging(self) -> None: hamlet = self.example_user("hamlet") diff --git a/zerver/views/antispam.py b/zerver/views/antispam.py index aafc41123dae2..89c4117f70397 100644 --- a/zerver/views/antispam.py +++ b/zerver/views/antispam.py @@ -48,6 +48,6 @@ def get_challenge( (challenge.challenge, expires.timestamp()), ] return json_success(request, data=challenge.__dict__) - except Exception as e: # nocoverage - logging.exception(e) + except Exception: # nocoverage + logging.exception("Error while generating challenge") raise JsonableError(_("Failed to generate challenge")) diff --git a/zerver/views/sentry.py b/zerver/views/sentry.py index 2310ee28ce508..f711a44823044 100644 --- a/zerver/views/sentry.py +++ b/zerver/views/sentry.py @@ -87,9 +87,9 @@ def sentry_tunnel( sentry_request(url, updated_body) except CircuitBreakerError: logger.warning("Dropped a client exception due to circuit-breaking") - except RequestException as e: + except RequestException: # This logger has been configured, above, to not report to Sentry - logger.exception(e) + logger.exception("Error while reporting to Sentry") return HttpResponse(status=200) From 8d2ad9c5cadebbbd05b029dd3a59a2c96ce989c4 Mon Sep 17 00:00:00 2001 From: Anders Kaseorg Date: Thu, 30 Jul 2026 13:19:51 -0700 Subject: [PATCH 05/14] ruff: Extend the default rule selection. Signed-off-by: Anders Kaseorg --- pyproject.toml | 5 ++++- tools/run-dev | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 026a69fc99ea3..f3d8ab9e5842c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -463,7 +463,7 @@ target-version = "py310" [tool.ruff.lint] # See https://github.com/astral-sh/ruff#rules for error code definitions. -select = [ +extend-select = [ "ANN", # annotations "B", # bugbear "C4", # comprehensions @@ -503,6 +503,7 @@ ignore = [ "ANN401", # Dynamically typed expressions (typing.Any) are disallowed "B007", # Loop control variable not used within the loop body "B904", # Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling + "BLE001", # Do not catch blind exception "C408", # Unnecessary `dict` call (rewrite as a literal) "COM812", # Trailing comma missing "DJ001", # Avoid using `null=True` on string-based fields @@ -550,6 +551,8 @@ ignore = [ "TC002", # Move third-party import into a type-checking block "TC003", # Move standard library import into a type-checking block "TC006", # Add quotes to type expression in `typing.cast()` + "TRY002", # Create your own exception + "TRY004", # Prefer `TypeError` exception for invalid type ] [tool.ruff.lint.flake8-bandit] diff --git a/tools/run-dev b/tools/run-dev index 3b120c39c0d92..f8978ad72f8d1 100755 --- a/tools/run-dev +++ b/tools/run-dev @@ -477,7 +477,7 @@ async def serve() -> None: setup_routes(options.help_center_static_build, options.help_center_dev_server) - children.extend(subprocess.Popen(cmd) for cmd in server_processes()) + children.extend(subprocess.Popen(cmd) for cmd in server_processes()) # noqa: ASYNC220 session = aiohttp.ClientSession() runner = web.AppRunner(app, auto_decompress=False, handler_cancellation=True) From 80bc3a338b54f4c2e346b866e9f8b7e9101307b1 Mon Sep 17 00:00:00 2001 From: Anders Kaseorg Date: Thu, 30 Jul 2026 14:47:59 -0700 Subject: [PATCH 06/14] ruff: Fix INP001 File is part of an implicit namespace package. Signed-off-by: Anders Kaseorg --- docs/__init__.py | 0 pyproject.toml | 1 + tools/droplets/__init__.py | 0 tools/oneclickapps/__init__.py | 0 tools/setup/emoji/__init__.py | 0 5 files changed, 1 insertion(+) create mode 100644 docs/__init__.py create mode 100644 tools/droplets/__init__.py create mode 100644 tools/oneclickapps/__init__.py create mode 100644 tools/setup/emoji/__init__.py diff --git a/docs/__init__.py b/docs/__init__.py new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/pyproject.toml b/pyproject.toml index f3d8ab9e5842c..20f7ba56f4dda 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -477,6 +477,7 @@ extend-select = [ "FURB", # refurbishing "G", # logging format "I", # import sorting + "INP", # implicit namespace package "INT", # gettext "ISC", # string concatenation "LOG", # logging diff --git a/tools/droplets/__init__.py b/tools/droplets/__init__.py new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/tools/oneclickapps/__init__.py b/tools/oneclickapps/__init__.py new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/tools/setup/emoji/__init__.py b/tools/setup/emoji/__init__.py new file mode 100644 index 0000000000000..e69de29bb2d1d From 2dceabf8d35b16dcc783bafe15540c6f76290d0c Mon Sep 17 00:00:00 2001 From: Mateusz Mandera Date: Tue, 30 Jun 2026 00:10:04 +0800 Subject: [PATCH 07/14] slack: Pass convert_slack_threads=True in self-serve flow. We missed to set this flag in the original implementation of the self-serve flow - but there's no reason not to enable the conversion of threads. --- zerver/actions/data_import.py | 1 + zerver/tests/test_slack_importer.py | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/zerver/actions/data_import.py b/zerver/actions/data_import.py index 7f1ef8157e5fe..9093db6b3e973 100644 --- a/zerver/actions/data_import.py +++ b/zerver/actions/data_import.py @@ -112,6 +112,7 @@ def import_slack_data(event: dict[str, Any]) -> None: fh.name, output_dir, event["slack_access_token"], + convert_slack_threads=True, ) attachment_size = sum( os.path.getsize(os.path.join(dirpath, filename)) diff --git a/zerver/tests/test_slack_importer.py b/zerver/tests/test_slack_importer.py index 56a9317dcc4ec..07de67f3bb510 100644 --- a/zerver/tests/test_slack_importer.py +++ b/zerver/tests/test_slack_importer.py @@ -2899,7 +2899,7 @@ def fake_do_import_realm(*args: object, **kwargs: object) -> Realm: mock_save_attachment_contents.assert_called_once() mock_do_convert_zipfile.assert_called_once_with( - mock.ANY, mock.ANY, event["slack_access_token"] + mock.ANY, mock.ANY, event["slack_access_token"], convert_slack_threads=True ) mock_do_import_realm.assert_called_once_with( mock.ANY, prereg_realm.string_id, on_realm_created=mock.ANY @@ -3067,7 +3067,7 @@ def fake_do_import_realm(*args: object, **kwargs: object) -> Realm: mock_save_attachment_contents.assert_called_once() mock_do_convert_zipfile.assert_called_once_with( - mock.ANY, mock.ANY, event["slack_access_token"] + mock.ANY, mock.ANY, event["slack_access_token"], convert_slack_threads=True ) mock_do_import_realm.assert_called_once_with( mock.ANY, prereg_realm.string_id, on_realm_created=mock.ANY From 82f119d62c715824ab53014dab958ac2d8dbc6bd Mon Sep 17 00:00:00 2001 From: 0xthedance Date: Wed, 17 Jun 2026 18:21:47 +0100 Subject: [PATCH 08/14] compose_closed_ui: Restore label_text for stream/topic recipients. Commit 0cd5c18e71d66119d7136c43975d88026435849e refactored the reply-button label to use a template that consumes the structured fields (stream, topic_display_name, has_empty_string_topic) directly, and dropped the now-redundant label_text for stream/topic recipients. However, label_text is still consumed by the BigBlueButton and Nextcloud Talk call-creation paths in compose_call_ui.ts Restore the label_text field for stream/topic recipients using the same format the refactor removed. --- web/src/compose_closed_ui.ts | 7 ++++++- web/tests/compose_closed_ui.test.cjs | 28 ++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/web/src/compose_closed_ui.ts b/web/src/compose_closed_ui.ts index 5ad621268cf09..cf727fca89570 100644 --- a/web/src/compose_closed_ui.ts +++ b/web/src/compose_closed_ui.ts @@ -17,8 +17,12 @@ import * as stream_data from "./stream_data.ts"; import type {StreamSubscription} from "./sub_store.ts"; import * as util from "./util.ts"; +// `label_text` is a flat, pre-formatted string used by the call-creation +// paths in compose_call_ui.ts to build a meeting/room name. +// The reply-button template uses the structured `stream` / `topic_display_name` +// fields instead, so it can render the decorated channel icon. type RecipientLabel = { - label_text?: string; + label_text: string; has_empty_string_topic?: boolean; stream?: StreamSubscription; topic_display_name?: string; @@ -31,6 +35,7 @@ function get_stream_recipient_label(stream_id: number, topic: string): Recipient const topic_display_name = util.get_final_topic_display_name(topic); if (stream) { const recipient_label: RecipientLabel = { + label_text: `#${stream.name} > ${topic_display_name}`, has_empty_string_topic: topic === "", stream, topic_display_name, diff --git a/web/tests/compose_closed_ui.test.cjs b/web/tests/compose_closed_ui.test.cjs index 974fe6769ff2d..938d85aca39ba 100644 --- a/web/tests/compose_closed_ui.test.cjs +++ b/web/tests/compose_closed_ui.test.cjs @@ -251,6 +251,34 @@ run_test("test_non_message_list_input", ({mock_template}) => { assert.equal(label, "translated: Compose message"); }); +run_test("recipient_label_text_for_call_creation", () => { + recent_view_util.is_visible = () => true; + message_lists.current = undefined; + const stream = make_stream({ + subscribed: true, + name: "stream test", + stream_id: 11, + }); + stream_data.add_sub_for_tests(stream); + + assert.equal( + compose_closed_ui.get_recipient_label({ + stream_id: stream.stream_id, + topic: "topic test", + })?.label_text, + "#stream test > topic test", + ); + + // Empty topic falls back to the realm's empty-topic display name. + assert.equal( + compose_closed_ui.get_recipient_label({ + stream_id: stream.stream_id, + topic: "", + })?.label_text, + `#stream test > translated: ${REALM_EMPTY_TOPIC_DISPLAY_NAME}`, + ); +}); + run_test("update_reply_button_state", ({override, override_rewire}) => { const $compose_reply_wrapper = $("#legacy-closed-compose-box .compose-reply-button-wrapper"); const $reply_button = $(".compose_reply_button"); From 3ebd559786a1734fd9524ce9623b9d1e21f51215 Mon Sep 17 00:00:00 2001 From: Lauryn Menard Date: Tue, 28 Jul 2026 18:04:59 +0200 Subject: [PATCH 09/14] test-stripe: Add helper to check initial LicenseLedger for plan. --- corporate/tests/test_stripe.py | 99 ++++++++-------------------------- 1 file changed, 22 insertions(+), 77 deletions(-) diff --git a/corporate/tests/test_stripe.py b/corporate/tests/test_stripe.py index 4cf6ad1968a39..ba678a59731d6 100644 --- a/corporate/tests/test_stripe.py +++ b/corporate/tests/test_stripe.py @@ -102,6 +102,17 @@ class StripeTest(StripeTestCase): + def check_initial_ledger_entry( + self, plan: CustomerPlan, licenses_purchased: int + ) -> LicenseLedger: + return LicenseLedger.objects.get( + plan=plan, + is_renewal=True, + event_time=self.now, + licenses=licenses_purchased, + licenses_at_next_renewal=licenses_purchased, + ) + def test_catch_stripe_errors(self) -> None: @catch_stripe_errors def raise_invalid_request_error() -> None: @@ -225,13 +236,7 @@ def test_upgrade_by_card_to_plus_plan(self, *mocks: Mock) -> None: tier=CustomerPlan.TIER_CLOUD_PLUS, status=CustomerPlan.ACTIVE, ) - LicenseLedger.objects.get( - plan=plan, - is_renewal=True, - event_time=self.now, - licenses=licenses_purchased, - licenses_at_next_renewal=licenses_purchased, - ) + self.check_initial_ledger_entry(plan, licenses_purchased) # Check RealmAuditLog audit_log_entries = list( RealmAuditLog.objects.filter(acting_user=user) @@ -348,13 +353,7 @@ def test_upgrade_by_invoice_to_plus_plan(self, *mocks: Mock) -> None: tier=CustomerPlan.TIER_CLOUD_PLUS, status=CustomerPlan.ACTIVE, ) - LicenseLedger.objects.get( - plan=plan, - is_renewal=True, - event_time=self.now, - licenses=123, - licenses_at_next_renewal=123, - ) + self.check_initial_ledger_entry(plan, 123) # Check RealmAuditLog audit_log_entries = list( RealmAuditLog.objects.filter(acting_user=user) @@ -474,13 +473,7 @@ def test_upgrade_by_card(self, *mocks: Mock) -> None: tier=CustomerPlan.TIER_CLOUD_STANDARD, status=CustomerPlan.ACTIVE, ) - LicenseLedger.objects.get( - plan=plan, - is_renewal=True, - event_time=self.now, - licenses=self.seat_count, - licenses_at_next_renewal=self.seat_count, - ) + self.check_initial_ledger_entry(plan, self.seat_count) # Check RealmAuditLog audit_log_entries = list( RealmAuditLog.objects.filter(acting_user=user) @@ -608,13 +601,7 @@ def test_upgrade_by_invoice(self, *mocks: Mock) -> None: tier=CustomerPlan.TIER_CLOUD_STANDARD, status=CustomerPlan.ACTIVE, ) - LicenseLedger.objects.get( - plan=plan, - is_renewal=True, - event_time=self.now, - licenses=123, - licenses_at_next_renewal=123, - ) + self.check_initial_ledger_entry(plan, 123) # Check RealmAuditLog audit_log_entries = list( RealmAuditLog.objects.filter(acting_user=user) @@ -722,13 +709,7 @@ def test_free_trial_upgrade_by_card(self, *mocks: Mock) -> None: # For payment through card. charge_automatically=True, ) - LicenseLedger.objects.get( - plan=plan, - is_renewal=True, - event_time=self.now, - licenses=self.seat_count, - licenses_at_next_renewal=self.seat_count, - ) + self.check_initial_ledger_entry(plan, self.seat_count) audit_log_entries = list( RealmAuditLog.objects.filter(acting_user=user) .values_list("event_type", "event_time") @@ -940,13 +921,7 @@ def test_free_trial_upgrade_by_invoice(self, *mocks: Mock) -> None: charge_automatically=False, ) - LicenseLedger.objects.get( - plan=plan, - is_renewal=True, - event_time=self.now, - licenses=123, - licenses_at_next_renewal=123, - ) + self.check_initial_ledger_entry(plan, 123) audit_log_entries = list( RealmAuditLog.objects.filter(acting_user=user) .values_list("event_type", "event_time") @@ -1140,13 +1115,7 @@ def test_free_trial_upgrade_by_invoice_with_additional_users_after_payment( charge_automatically=False, ) - LicenseLedger.objects.get( - plan=plan, - is_renewal=True, - event_time=self.now, - licenses=123, - licenses_at_next_renewal=123, - ) + self.check_initial_ledger_entry(plan, 123) audit_log_entries = list( RealmAuditLog.objects.filter(acting_user=user) .values_list("event_type", "event_time") @@ -1333,13 +1302,7 @@ def test_free_trial_upgrade_by_invoice_customer_fails_to_pay(self, *mocks: Mock) charge_automatically=False, ) - LicenseLedger.objects.get( - plan=plan, - is_renewal=True, - event_time=self.now, - licenses=123, - licenses_at_next_renewal=123, - ) + self.check_initial_ledger_entry(plan, 123) audit_log_entries = list( RealmAuditLog.objects.filter(acting_user=user) .values_list("event_type", "event_time") @@ -2258,13 +2221,7 @@ def test_redirect_for_billing_page_downgrade_at_free_trial_end(self, *mocks: Moc # For payment through card. charge_automatically=True, ) - LicenseLedger.objects.get( - plan=plan, - is_renewal=True, - event_time=self.now, - licenses=self.seat_count, - licenses_at_next_renewal=self.seat_count, - ) + self.check_initial_ledger_entry(plan, self.seat_count) realm = get_realm("zulip") self.assertEqual(realm.plan_type, Realm.PLAN_TYPE_STANDARD) @@ -3291,13 +3248,7 @@ def test_switch_now_free_trial_from_monthly_to_annual(self, *mocks: Mock) -> Non status=CustomerPlan.FREE_TRIAL, charge_automatically=True, ) - ledger_entry = LicenseLedger.objects.get( - plan=new_plan, - is_renewal=True, - event_time=self.now, - licenses=self.seat_count, - licenses_at_next_renewal=self.seat_count, - ) + ledger_entry = self.check_initial_ledger_entry(new_plan, self.seat_count) self.assertEqual(new_plan.invoiced_through, ledger_entry) realm_audit_log = RealmAuditLog.objects.filter( @@ -3345,13 +3296,7 @@ def test_switch_now_free_trial_from_annual_to_monthly(self, *mocks: Mock) -> Non status=CustomerPlan.FREE_TRIAL, charge_automatically=True, ) - ledger_entry = LicenseLedger.objects.get( - plan=new_plan, - is_renewal=True, - event_time=self.now, - licenses=self.seat_count, - licenses_at_next_renewal=self.seat_count, - ) + ledger_entry = self.check_initial_ledger_entry(new_plan, self.seat_count) self.assertEqual(new_plan.invoiced_through, ledger_entry) realm_audit_log = RealmAuditLog.objects.filter( From bc2ef58a7d087f49b6895e920919f03c7a5e3ecb Mon Sep 17 00:00:00 2001 From: Lauryn Menard Date: Tue, 28 Jul 2026 19:26:38 +0200 Subject: [PATCH 10/14] test-stripe: Add helper to check latest ledger license counts. --- corporate/lib/test_stripe_class.py | 10 ++ corporate/tests/test_stripe.py | 209 ++++++++++------------------- 2 files changed, 79 insertions(+), 140 deletions(-) diff --git a/corporate/lib/test_stripe_class.py b/corporate/lib/test_stripe_class.py index e055f00855a6d..9859fa5c0fcbf 100644 --- a/corporate/lib/test_stripe_class.py +++ b/corporate/lib/test_stripe_class.py @@ -78,6 +78,7 @@ stripe_get_customer, ) from corporate.models.customers import Customer +from corporate.models.licenses import LicenseLedger from corporate.models.plans import CustomerPlan from corporate.models.stripe_state import Invoice from zerver.actions.users import do_deactivate_user @@ -1046,3 +1047,12 @@ def client_billing_patch(self, url_suffix: str, info: Mapping[str, Any] = {}) -> else: response = self.client_patch(url, info) return response + + def check_last_ledger_entry_license_counts( + self, plan: CustomerPlan, licenses: int, licenses_at_next_renewal: int + ) -> LicenseLedger: + ledger_entry = LicenseLedger.objects.filter(plan=plan).order_by("-id").first() + assert ledger_entry is not None + self.assertEqual(ledger_entry.licenses, licenses) + self.assertEqual(ledger_entry.licenses_at_next_renewal, licenses_at_next_renewal) + return ledger_entry diff --git a/corporate/tests/test_stripe.py b/corporate/tests/test_stripe.py index ba678a59731d6..d1cd1188e18d0 100644 --- a/corporate/tests/test_stripe.py +++ b/corporate/tests/test_stripe.py @@ -763,21 +763,11 @@ def test_free_trial_upgrade_by_card(self, *mocks: Mock) -> None: billing_session = RealmBillingSession(user=user, realm=realm) with patch("corporate.lib.stripe.get_latest_seat_count", return_value=12): billing_session.update_license_ledger_if_needed(self.now) - self.assertEqual( - LicenseLedger.objects.order_by("-id") - .values_list("licenses", "licenses_at_next_renewal") - .first(), - (12, 12), - ) + self.check_last_ledger_entry_license_counts(plan, 12, 12) with patch("corporate.lib.stripe.get_latest_seat_count", return_value=15): billing_session.update_license_ledger_if_needed(self.next_month) - self.assertEqual( - LicenseLedger.objects.order_by("-id") - .values_list("licenses", "licenses_at_next_renewal") - .first(), - (15, 15), - ) + self.check_last_ledger_entry_license_counts(plan, 15, 15) invoice_plans_as_needed(self.next_month) self.assertFalse(stripe.Invoice.list(customer=stripe_customer.id)) @@ -818,12 +808,8 @@ def test_free_trial_upgrade_by_card(self, *mocks: Mock) -> None: with patch("corporate.lib.stripe.get_latest_seat_count", return_value=19): billing_session.update_license_ledger_if_needed(add_months(free_trial_end_date, 10)) - self.assertEqual( - LicenseLedger.objects.order_by("-id") - .values_list("licenses", "licenses_at_next_renewal") - .first(), - (19, 19), - ) + self.check_last_ledger_entry_license_counts(plan, 19, 19) + # Fast forward next_invoice_date to 10 months from the free_trial_end_date plan.next_invoice_date = add_months(free_trial_end_date, 10) plan.save(update_fields=["next_invoice_date"]) @@ -1213,14 +1199,10 @@ def test_free_trial_upgrade_by_invoice_with_additional_users_after_payment( schedule=None, toggle_license_management=False, ) - self.assertEqual( - LicenseLedger.objects.filter(plan=plan).latest("id").licenses_at_next_renewal, 123 - ) + self.check_last_ledger_entry_license_counts(plan, 123, 123) with time_machine.travel(self.now, tick=False): self.billing_session.do_update_plan(update_plan_request) - self.assertEqual( - LicenseLedger.objects.filter(plan=plan).latest("id").licenses_at_next_renewal, 125 - ) + self.check_last_ledger_entry_license_counts(plan, 123, 125) invoice_plans_as_needed(free_trial_end_date) customer_plan.refresh_from_db() @@ -1429,10 +1411,9 @@ def test_upgrade_by_card_with_outdated_seat_count(self, *mocks: Mock) -> None: [item.amount for item in additional_license_invoice.lines], ) # Check LicenseLedger has the new amount - ledger_entry = LicenseLedger.objects.last() - assert ledger_entry is not None - self.assertEqual(ledger_entry.licenses, new_seat_count) - self.assertEqual(ledger_entry.licenses_at_next_renewal, new_seat_count) + plan = get_current_plan_by_customer(customer) + assert plan is not None + self.check_last_ledger_entry_license_counts(plan, new_seat_count, new_seat_count) @mock_stripe() def test_upgrade_by_card_with_outdated_lower_seat_count(self, *mocks: Mock) -> None: @@ -1474,10 +1455,9 @@ def test_upgrade_by_card_with_outdated_lower_seat_count(self, *mocks: Mock) -> N [item.amount for item in upgrade_invoice.lines], ) # Check LicenseLedger has the reduced license count at renewal - ledger_entry = LicenseLedger.objects.last() - assert ledger_entry is not None - self.assertEqual(ledger_entry.licenses, self.seat_count) - self.assertEqual(ledger_entry.licenses_at_next_renewal, new_seat_count) + plan = get_current_plan_by_customer(customer) + assert plan is not None + self.check_last_ledger_entry_license_counts(plan, self.seat_count, new_seat_count) # Check that we informed the support team about the potential billing error. from django.core.mail import outbox @@ -1539,10 +1519,11 @@ def test_upgrade_by_card_with_outdated_seat_count_and_minimum_for_plan_tier( [item.amount for item in upgrade_invoice.lines], ) # Check LicenseLedger has the minimum license count - ledger_entry = LicenseLedger.objects.last() - assert ledger_entry is not None - self.assertEqual(ledger_entry.licenses, minimum_for_plan_tier) - self.assertEqual(ledger_entry.licenses_at_next_renewal, minimum_for_plan_tier) + plan = get_current_plan_by_customer(customer) + assert plan is not None + self.check_last_ledger_entry_license_counts( + plan, minimum_for_plan_tier, minimum_for_plan_tier + ) @mock_stripe() def test_customer_minimum_licenses_for_plan(self, *mocks: Mock) -> None: @@ -1562,10 +1543,9 @@ def test_customer_minimum_licenses_for_plan(self, *mocks: Mock) -> None: assert customer is not None assert customer.stripe_customer_id is not None # Check LicenseLedger has the current seat count. - ledger_entry = LicenseLedger.objects.last() - assert ledger_entry is not None - self.assertEqual(ledger_entry.licenses, self.seat_count) - self.assertEqual(ledger_entry.licenses_at_next_renewal, self.seat_count) + plan = get_current_plan_by_customer(customer) + assert plan is not None + self.check_last_ledger_entry_license_counts(plan, self.seat_count, self.seat_count) # We manually set customer.minimum_licenses to the current seat count, # which is below the general Plus plan minimum licenses. @@ -2612,12 +2592,7 @@ def test_downgrade(self) -> None: billing_session = RealmBillingSession(user=user, realm=user.realm) with patch("corporate.lib.stripe.get_latest_seat_count", return_value=20): billing_session.update_license_ledger_if_needed(self.now) - self.assertEqual( - LicenseLedger.objects.order_by("-id") - .values_list("licenses", "licenses_at_next_renewal") - .first(), - (20, 20), - ) + self.check_last_ledger_entry_license_counts(plan, 20, 20) # Verify that we invoice them for the additional users mocked = self.setup_mocked_stripe(invoice_plans_as_needed, self.next_month) @@ -2632,12 +2607,8 @@ def test_downgrade(self) -> None: assert plan is not None self.assertEqual(get_realm("zulip").plan_type, Realm.PLAN_TYPE_LIMITED) self.assertEqual(plan.status, CustomerPlan.ENDED) - self.assertEqual( - LicenseLedger.objects.order_by("-id") - .values_list("licenses", "licenses_at_next_renewal") - .first(), - (20, 20), - ) + self.check_last_ledger_entry_license_counts(plan, 20, 20) + realm_audit_log = RealmAuditLog.objects.latest("id") self.assertEqual(realm_audit_log.event_type, AuditLogEventType.REALM_PLAN_TYPE_CHANGED) self.assertEqual(realm_audit_log.acting_user, None) @@ -2645,12 +2616,7 @@ def test_downgrade(self) -> None: # Verify that we don't write LicenseLedger rows once we've downgraded with patch("corporate.lib.stripe.get_latest_seat_count", return_value=40): billing_session.update_license_ledger_if_needed(self.next_year) - self.assertEqual( - LicenseLedger.objects.order_by("-id") - .values_list("licenses", "licenses_at_next_renewal") - .first(), - (20, 20), - ) + self.check_last_ledger_entry_license_counts(plan, 20, 20) # Verify that we call invoice_plan once more after cycle end but # don't invoice them for users added after the cycle end @@ -2721,12 +2687,7 @@ def test_switch_from_monthly_plan_to_annual_plan_for_automatic_license_managemen with patch("corporate.lib.stripe.get_latest_seat_count", return_value=20): billing_session.update_license_ledger_if_needed(self.now) self.assertEqual(LicenseLedger.objects.filter(plan=monthly_plan).count(), 2) - self.assertEqual( - LicenseLedger.objects.order_by("-id") - .values_list("licenses", "licenses_at_next_renewal") - .first(), - (20, 20), - ) + self.check_last_ledger_entry_license_counts(monthly_plan, 20, 20) with ( time_machine.travel(self.next_month, tick=False), @@ -3006,12 +2967,7 @@ def test_switch_from_annual_plan_to_monthly_plan_for_automatic_license_managemen with patch("corporate.lib.stripe.get_latest_seat_count", return_value=20): billing_session.update_license_ledger_if_needed(self.now) self.assertEqual(LicenseLedger.objects.filter(plan=annual_plan).count(), 2) - self.assertEqual( - LicenseLedger.objects.order_by("-id") - .values_list("licenses", "licenses_at_next_renewal") - .first(), - (20, 20), - ) + self.check_last_ledger_entry_license_counts(annual_plan, 20, 20) # Check that we don't switch to monthly plan at next invoice date (which is used to charge user for # additional licenses) but at the end of current billing cycle. @@ -3324,10 +3280,7 @@ def test_end_free_trial(self, *mocks: Mock) -> None: with patch("corporate.lib.stripe.get_latest_seat_count", return_value=21): billing_session.update_license_ledger_if_needed(self.now) - last_ledger_entry = LicenseLedger.objects.order_by("id").last() - assert last_ledger_entry is not None - self.assertEqual(last_ledger_entry.licenses, 21) - self.assertEqual(last_ledger_entry.licenses_at_next_renewal, 21) + last_ledger_entry = self.check_last_ledger_entry_license_counts(plan, 21, 21) self.login_user(user) @@ -3417,12 +3370,7 @@ def test_downgrade_at_end_of_free_trial(self, *mocks: Mock) -> None: # part of the cycle with patch("corporate.lib.stripe.get_latest_seat_count", return_value=20): billing_session.update_license_ledger_if_needed(self.now) - self.assertEqual( - LicenseLedger.objects.order_by("-id") - .values_list("licenses", "licenses_at_next_renewal") - .first(), - (20, 20), - ) + self.check_last_ledger_entry_license_counts(plan, 20, 20) # Verify that we don't invoice them for the additional users during free trial. mocked = self.setup_mocked_stripe(invoice_plans_as_needed, self.next_month) @@ -3438,22 +3386,12 @@ def test_downgrade_at_end_of_free_trial(self, *mocks: Mock) -> None: self.assertIsNone(plan.next_invoice_date) self.assertEqual(plan.status, CustomerPlan.ENDED) self.assertEqual(get_realm("zulip").plan_type, Realm.PLAN_TYPE_LIMITED) - self.assertEqual( - LicenseLedger.objects.order_by("-id") - .values_list("licenses", "licenses_at_next_renewal") - .first(), - (20, 20), - ) + self.check_last_ledger_entry_license_counts(plan, 20, 20) # Verify that we don't write LicenseLedger rows once we've downgraded with patch("corporate.lib.stripe.get_latest_seat_count", return_value=40): billing_session.update_license_ledger_if_needed(self.next_year) - self.assertEqual( - LicenseLedger.objects.order_by("-id") - .values_list("licenses", "licenses_at_next_renewal") - .first(), - (20, 20), - ) + self.check_last_ledger_entry_license_counts(plan, 20, 20) self.login_user(user) response = self.client_get("/billing/") @@ -3732,24 +3670,26 @@ def test_update_licenses_of_manual_plan_from_billing_page_exempt_from_license_nu self.login_user(user) customer = Customer.objects.get_or_create(realm=user.realm)[0] + reduced_seat_count = get_latest_seat_count(user.realm) - 2 customer.exempt_from_license_number_check = True customer.save() + paid_license_count = 100 with time_machine.travel(self.now, tick=False): - self.local_upgrade(100, False, CustomerPlan.BILLING_SCHEDULE_ANNUAL, True, False) + self.local_upgrade( + paid_license_count, False, CustomerPlan.BILLING_SCHEDULE_ANNUAL, True, False + ) with time_machine.travel(self.now, tick=False): result = self.client_billing_patch( "/billing/plan", - {"licenses_at_next_renewal": get_latest_seat_count(user.realm) - 2}, + {"licenses_at_next_renewal": reduced_seat_count}, ) self.assert_json_success(result) - latest_license_ledger = LicenseLedger.objects.last() - assert latest_license_ledger is not None - self.assertEqual( - latest_license_ledger.licenses_at_next_renewal, get_latest_seat_count(user.realm) - 2 - ) + plan = get_current_plan_by_customer(customer) + assert plan is not None + self.check_last_ledger_entry_license_counts(plan, paid_license_count, reduced_seat_count) def test_upgrade_exempt_from_license_number_check_realm_less_licenses_than_seat_count( self, @@ -3774,10 +3714,9 @@ def test_upgrade_exempt_from_license_number_check_realm_less_licenses_than_seat_ reduced_seat_count, False, CustomerPlan.BILLING_SCHEDULE_ANNUAL, True, False ) - latest_license_ledger = LicenseLedger.objects.last() - assert latest_license_ledger is not None - self.assertEqual(latest_license_ledger.licenses_at_next_renewal, reduced_seat_count) - self.assertEqual(latest_license_ledger.licenses, reduced_seat_count) + plan = get_current_plan_by_customer(customer) + assert plan is not None + self.check_last_ledger_entry_license_counts(plan, reduced_seat_count, reduced_seat_count) def test_update_licenses_of_automatic_plan_from_billing_page(self) -> None: user = self.example_user("hamlet") @@ -3899,10 +3838,7 @@ def test_deactivate_realm(self, mock_: Mock) -> None: with patch("corporate.lib.stripe.get_latest_seat_count", return_value=20): billing_session.update_license_ledger_if_needed(self.now) - last_ledger_entry = LicenseLedger.objects.order_by("id").last() - assert last_ledger_entry is not None - self.assertEqual(last_ledger_entry.licenses, 20) - self.assertEqual(last_ledger_entry.licenses_at_next_renewal, 20) + last_ledger_entry = self.check_last_ledger_entry_license_counts(plan, 20, 20) do_deactivate_realm( get_realm("zulip"), @@ -7033,23 +6969,21 @@ def test_upgrade_user_to_business_plan(self, *mocks: Mock) -> None: RemoteRealmAuditLog.objects.count(), min_licenses + 10 - realm_user_count + audit_log_count, ) - latest_ledger = LicenseLedger.objects.last() - assert latest_ledger is not None - self.assertEqual(latest_ledger.licenses, min_licenses + 10) + paid_license_count = min_licenses + 10 + self.check_last_ledger_entry_license_counts(plan, paid_license_count, paid_license_count) with time_machine.travel(self.now + timedelta(days=3), tick=False): response = self.client_get( f"{self.billing_session.billing_base_url}/billing/", subdomain="selfhosting" ) - self.assertEqual(latest_ledger.licenses, 35) for substring in [ "Zulip Business", "Number of licenses", - f"{latest_ledger.licenses}", + f"{paid_license_count}", "January 2, 2013", "Your plan will automatically renew on", - f"${80 * latest_ledger.licenses:,.2f}", + f"${80 * paid_license_count:,.2f}", "Visa ending in 4242", "Update card", ]: @@ -7261,23 +7195,23 @@ def test_upgrade_user_to_basic_plan_free_trial(self, *mocks: Mock) -> None: RemoteRealmAuditLog.objects.count(), min_licenses + 10 - realm_user_count + audit_log_count, ) - latest_ledger = LicenseLedger.objects.last() - assert latest_ledger is not None - self.assertEqual(latest_ledger.licenses, min_licenses + 10) + paid_license_count = min_licenses + 10 + self.check_last_ledger_entry_license_counts( + plan, paid_license_count, paid_license_count + ) with time_machine.travel(self.now + timedelta(days=3), tick=False): response = self.client_get( f"{self.billing_session.billing_base_url}/billing/", subdomain="selfhosting" ) - self.assertEqual(latest_ledger.licenses, min_licenses + 10) for substring in [ "Zulip Basic", "Number of licenses", - f"{latest_ledger.licenses}", + f"{paid_license_count}", "February 1, 2012", "Your plan will automatically renew on", - f"${3.5 * latest_ledger.licenses - flat_discount // 100 * 1:,.2f}", + f"${3.5 * paid_license_count - flat_discount // 100 * 1:,.2f}", "Visa ending in 4242", "Update card", ]: @@ -7469,23 +7403,21 @@ def test_upgrade_remote_realm_user_to_monthly_basic_plan(self, *mocks: Mock) -> RemoteRealmAuditLog.objects.count(), min_licenses + 10 - realm_user_count + audit_log_count, ) - latest_ledger = LicenseLedger.objects.last() - assert latest_ledger is not None - self.assertEqual(latest_ledger.licenses, min_licenses + 10) + paid_license_count = min_licenses + 10 + self.check_last_ledger_entry_license_counts(plan, paid_license_count, paid_license_count) with time_machine.travel(self.now + timedelta(days=3), tick=False): response = self.client_get( f"{self.billing_session.billing_base_url}/billing/", subdomain="selfhosting" ) - self.assertEqual(latest_ledger.licenses, min_licenses + 10) for substring in [ "Zulip Basic", "Number of licenses", - f"{latest_ledger.licenses}", + f"{paid_license_count}", "February 2, 2012", "Your plan will automatically renew on", - f"${3.5 * latest_ledger.licenses - flat_discount // 100 * 1:,.2f}", + f"${3.5 * paid_license_count - flat_discount // 100 * 1:,.2f}", "Visa ending in 4242", "Update card", ]: @@ -9013,9 +8945,8 @@ def test_non_sponsorship_billing(self, *mocks: Mock) -> None: RemoteRealmAuditLog.objects.count(), audit_log_count + 8, ) - latest_ledger = LicenseLedger.objects.last() - assert latest_ledger is not None - self.assertEqual(latest_ledger.licenses, server_user_count + 8) + paid_license_count = server_user_count + 8 + self.check_last_ledger_entry_license_counts(plan, paid_license_count, paid_license_count) # Login again result = self.execute_remote_billing_authentication_flow( @@ -9449,23 +9380,23 @@ def test_upgrade_user_to_basic_plan_free_trial_remote_server(self, *mocks: Mock) RemoteRealmAuditLog.objects.count(), audit_log_count + 10, ) - latest_ledger = LicenseLedger.objects.last() - assert latest_ledger is not None - self.assertEqual(latest_ledger.licenses, 28) + paid_license_count = realm_user_count + 10 + self.check_last_ledger_entry_license_counts( + plan, paid_license_count, paid_license_count + ) with time_machine.travel(self.now + timedelta(days=3), tick=False): response = self.client_get( f"{self.billing_session.billing_base_url}/billing/", subdomain="selfhosting" ) - self.assertEqual(latest_ledger.licenses, 28) for substring in [ "Zulip Basic", "Number of licenses", - f"{latest_ledger.licenses}", + f"{paid_license_count}", "February 1, 2012", "Your plan will automatically renew on", - f"${3.5 * latest_ledger.licenses - flat_discount // 100 * 1:,.2f}", + f"${3.5 * paid_license_count - flat_discount // 100 * 1:,.2f}", "Visa ending in 4242", "Update card", ]: @@ -9659,23 +9590,21 @@ def test_upgrade_server_user_to_monthly_basic_plan(self, *mocks: Mock) -> None: RemoteRealmAuditLog.objects.count(), audit_log_count + 10, ) - latest_ledger = LicenseLedger.objects.last() - assert latest_ledger is not None - self.assertEqual(latest_ledger.licenses, 28) + paid_license_count = server_user_count + 10 + self.check_last_ledger_entry_license_counts(plan, paid_license_count, paid_license_count) with time_machine.travel(self.now + timedelta(days=3), tick=False): response = self.client_get( f"{self.billing_session.billing_base_url}/billing/", subdomain="selfhosting" ) - self.assertEqual(latest_ledger.licenses, 28) for substring in [ "Zulip Basic", "Number of licenses", - f"{latest_ledger.licenses}", + f"{paid_license_count}", "February 2, 2012", "Your plan will automatically renew on", - f"${3.5 * latest_ledger.licenses - flat_discount // 100 * 1:,.2f}", + f"${3.5 * paid_license_count - flat_discount // 100 * 1:,.2f}", "Visa ending in 4242", "Update card", ]: From a794e6a2331f4a6982ff5e31cd51342e2b957039 Mon Sep 17 00:00:00 2001 From: Lauryn Menard Date: Tue, 28 Jul 2026 20:58:58 +0200 Subject: [PATCH 11/14] test-stripe: Get initial plan once in switch annual/monthly tests. When testing for the logging output was improved in these stripe billing tests, duplicate calls to get the CustomerPlan object were added to multiple tests. Removing that duplication simplifies these tests and removes any confusion about whether the different plan variables are the same or different plans. Also removes any confusion via variable names that the logging output contains Stripe.customer IDs, when they in fact contain the IDs of our database's Customer objects. --- corporate/tests/test_stripe.py | 127 ++++++++++++++------------------- 1 file changed, 54 insertions(+), 73 deletions(-) diff --git a/corporate/tests/test_stripe.py b/corporate/tests/test_stripe.py index d1cd1188e18d0..a2d8a12d67a8a 100644 --- a/corporate/tests/test_stripe.py +++ b/corporate/tests/test_stripe.py @@ -2549,7 +2549,9 @@ def test_downgrade(self) -> None: self.local_upgrade( self.seat_count, True, CustomerPlan.BILLING_SCHEDULE_ANNUAL, True, False ) - plan = get_current_plan_by_realm(user.realm) + customer = get_customer_by_realm(user.realm) + assert customer is not None + plan = get_current_plan_by_customer(customer) assert plan is not None self.assertEqual(plan.licenses(), self.seat_count) self.assertEqual(plan.licenses_at_next_renewal(), self.seat_count) @@ -2561,10 +2563,7 @@ def test_downgrade(self) -> None: "/billing/plan", {"status": CustomerPlan.DOWNGRADE_AT_END_OF_CYCLE}, ) - stripe_customer_id = Customer.objects.get(realm=user.realm).id - new_plan = get_current_plan_by_realm(user.realm) - assert new_plan is not None - expected_log = f"INFO:corporate.stripe:Change plan status: Customer.id: {stripe_customer_id}, CustomerPlan.id: {new_plan.id}, status: {CustomerPlan.DOWNGRADE_AT_END_OF_CYCLE}" + expected_log = f"INFO:corporate.stripe:Change plan status: Customer.id: {customer.id}, CustomerPlan.id: {plan.id}, status: {CustomerPlan.DOWNGRADE_AT_END_OF_CYCLE}" self.assertEqual(m.output[0], expected_log) self.assert_json_success(response) plan.refresh_from_db() @@ -2603,8 +2602,7 @@ def test_downgrade(self) -> None: # Check that we downgrade properly if the cycle is over with patch("corporate.lib.stripe.get_latest_seat_count", return_value=30): billing_session.update_license_ledger_if_needed(self.next_year) - plan = CustomerPlan.objects.first() - assert plan is not None + plan.refresh_from_db() self.assertEqual(get_realm("zulip").plan_type, Realm.PLAN_TYPE_LIMITED) self.assertEqual(plan.status, CustomerPlan.ENDED) self.check_last_ledger_entry_license_counts(plan, 20, 20) @@ -2620,8 +2618,7 @@ def test_downgrade(self) -> None: # Verify that we call invoice_plan once more after cycle end but # don't invoice them for users added after the cycle end - plan = CustomerPlan.objects.first() - assert plan is not None + plan.refresh_from_db() self.assertIsNotNone(plan.next_invoice_date) mocked = self.setup_mocked_stripe( @@ -2632,8 +2629,7 @@ def test_downgrade(self) -> None: mocked["Invoice"].create.assert_not_called() # Check that we updated next_invoice_date in invoice_plan - plan = CustomerPlan.objects.first() - assert plan is not None + plan.refresh_from_db() self.assertIsNone(plan.next_invoice_date) # Check that we don't call invoice_plan after that final call @@ -2655,15 +2651,13 @@ def test_switch_from_monthly_plan_to_annual_plan_for_automatic_license_managemen self.login_user(user) self.add_card_and_upgrade(user, schedule="monthly") - monthly_plan = get_current_plan_by_realm(user.realm) + customer = get_customer_by_realm(user.realm) + assert customer is not None + monthly_plan = get_current_plan_by_customer(customer) assert monthly_plan is not None self.assertEqual(monthly_plan.automanage_licenses, True) self.assertEqual(monthly_plan.billing_schedule, CustomerPlan.BILLING_SCHEDULE_MONTHLY) - stripe_customer_id = Customer.objects.get(realm=user.realm).id - new_plan = get_current_plan_by_realm(user.realm) - assert new_plan is not None - with ( self.assertLogs("corporate.stripe", "INFO") as m, time_machine.travel(self.now, tick=False), @@ -2672,7 +2666,7 @@ def test_switch_from_monthly_plan_to_annual_plan_for_automatic_license_managemen "/billing/plan", {"status": CustomerPlan.SWITCH_TO_ANNUAL_AT_END_OF_CYCLE}, ) - expected_log = f"INFO:corporate.stripe:Change plan status: Customer.id: {stripe_customer_id}, CustomerPlan.id: {new_plan.id}, status: {CustomerPlan.SWITCH_TO_ANNUAL_AT_END_OF_CYCLE}" + expected_log = f"INFO:corporate.stripe:Change plan status: Customer.id: {customer.id}, CustomerPlan.id: {monthly_plan.id}, status: {CustomerPlan.SWITCH_TO_ANNUAL_AT_END_OF_CYCLE}" self.assertEqual(m.output[0], expected_log) self.assert_json_success(response) monthly_plan.refresh_from_db() @@ -2832,13 +2826,12 @@ def test_switch_from_monthly_plan_to_annual_plan_for_manual_license_management( self.add_card_and_upgrade( user, schedule="monthly", license_management="manual", licenses=num_licenses ) - monthly_plan = get_current_plan_by_realm(user.realm) + customer = get_customer_by_realm(user.realm) + assert customer is not None + monthly_plan = get_current_plan_by_customer(customer) assert monthly_plan is not None self.assertEqual(monthly_plan.automanage_licenses, False) self.assertEqual(monthly_plan.billing_schedule, CustomerPlan.BILLING_SCHEDULE_MONTHLY) - stripe_customer_id = Customer.objects.get(realm=user.realm).id - new_plan = get_current_plan_by_realm(user.realm) - assert new_plan is not None with ( self.assertLogs("corporate.stripe", "INFO") as m, time_machine.travel(self.now, tick=False), @@ -2849,7 +2842,7 @@ def test_switch_from_monthly_plan_to_annual_plan_for_manual_license_management( ) self.assertEqual( m.output[0], - f"INFO:corporate.stripe:Change plan status: Customer.id: {stripe_customer_id}, CustomerPlan.id: {new_plan.id}, status: {CustomerPlan.SWITCH_TO_ANNUAL_AT_END_OF_CYCLE}", + f"INFO:corporate.stripe:Change plan status: Customer.id: {customer.id}, CustomerPlan.id: {monthly_plan.id}, status: {CustomerPlan.SWITCH_TO_ANNUAL_AT_END_OF_CYCLE}", ) self.assert_json_success(response) monthly_plan.refresh_from_db() @@ -2934,15 +2927,13 @@ def test_switch_from_annual_plan_to_monthly_plan_for_automatic_license_managemen user = self.example_user("hamlet") self.login_user(user) self.add_card_and_upgrade(user, schedule="annual") - annual_plan = get_current_plan_by_realm(user.realm) + customer = get_customer_by_realm(user.realm) + assert customer is not None + annual_plan = get_current_plan_by_customer(customer) assert annual_plan is not None self.assertEqual(annual_plan.automanage_licenses, True) self.assertEqual(annual_plan.billing_schedule, CustomerPlan.BILLING_SCHEDULE_ANNUAL) - stripe_customer_id = Customer.objects.get(realm=user.realm).id - new_plan = get_current_plan_by_realm(user.realm) - assert new_plan is not None - assert self.now is not None with ( self.assertLogs("corporate.stripe", "INFO") as m, @@ -2952,7 +2943,7 @@ def test_switch_from_annual_plan_to_monthly_plan_for_automatic_license_managemen "/billing/plan", {"status": CustomerPlan.SWITCH_TO_MONTHLY_AT_END_OF_CYCLE}, ) - expected_log = f"INFO:corporate.stripe:Change plan status: Customer.id: {stripe_customer_id}, CustomerPlan.id: {new_plan.id}, status: {CustomerPlan.SWITCH_TO_MONTHLY_AT_END_OF_CYCLE}" + expected_log = f"INFO:corporate.stripe:Change plan status: Customer.id: {customer.id}, CustomerPlan.id: {annual_plan.id}, status: {CustomerPlan.SWITCH_TO_MONTHLY_AT_END_OF_CYCLE}" self.assertEqual(m.output[0], expected_log) self.assert_json_success(response) annual_plan.refresh_from_db() @@ -3099,14 +3090,14 @@ def test_reupgrade_after_plan_status_changed_to_downgrade_at_end_of_cycle(self) "/billing/plan", {"status": CustomerPlan.DOWNGRADE_AT_END_OF_CYCLE}, ) - stripe_customer_id = Customer.objects.get(realm=user.realm).id - new_plan = get_current_plan_by_realm(user.realm) - assert new_plan is not None - expected_log = f"INFO:corporate.stripe:Change plan status: Customer.id: {stripe_customer_id}, CustomerPlan.id: {new_plan.id}, status: {CustomerPlan.DOWNGRADE_AT_END_OF_CYCLE}" + customer = get_customer_by_realm(user.realm) + assert customer is not None + plan = get_current_plan_by_customer(customer) + assert plan is not None + expected_log = f"INFO:corporate.stripe:Change plan status: Customer.id: {customer.id}, CustomerPlan.id: {plan.id}, status: {CustomerPlan.DOWNGRADE_AT_END_OF_CYCLE}" self.assertEqual(m.output[0], expected_log) self.assert_json_success(response) - plan = CustomerPlan.objects.first() - assert plan is not None + plan.refresh_from_db() self.assertEqual(plan.status, CustomerPlan.DOWNGRADE_AT_END_OF_CYCLE) with ( self.assertLogs("corporate.stripe", "INFO") as m, @@ -3116,11 +3107,10 @@ def test_reupgrade_after_plan_status_changed_to_downgrade_at_end_of_cycle(self) "/billing/plan", {"status": CustomerPlan.ACTIVE}, ) - expected_log = f"INFO:corporate.stripe:Change plan status: Customer.id: {stripe_customer_id}, CustomerPlan.id: {new_plan.id}, status: {CustomerPlan.ACTIVE}" + expected_log = f"INFO:corporate.stripe:Change plan status: Customer.id: {customer.id}, CustomerPlan.id: {plan.id}, status: {CustomerPlan.ACTIVE}" self.assertEqual(m.output[0], expected_log) self.assert_json_success(response) - plan = CustomerPlan.objects.first() - assert plan is not None + plan.refresh_from_db() self.assertEqual(plan.status, CustomerPlan.ACTIVE) @patch("stripe.Invoice.create") @@ -3138,28 +3128,26 @@ def test_downgrade_during_invoicing(self, *mocks: Mock) -> None: self.local_upgrade( self.seat_count, True, CustomerPlan.BILLING_SCHEDULE_ANNUAL, True, False ) + customer = get_customer_by_realm(user.realm) + assert customer is not None + plan = get_current_plan_by_customer(customer) + assert plan is not None with self.assertLogs("corporate.stripe", "INFO") as m: - stripe_customer_id = Customer.objects.get(realm=user.realm).id - new_plan = get_current_plan_by_realm(user.realm) - assert new_plan is not None with time_machine.travel(self.now, tick=False): self.client_billing_patch( "/billing/plan", {"status": CustomerPlan.DOWNGRADE_AT_END_OF_CYCLE}, ) - expected_log = f"INFO:corporate.stripe:Change plan status: Customer.id: {stripe_customer_id}, CustomerPlan.id: {new_plan.id}, status: {CustomerPlan.DOWNGRADE_AT_END_OF_CYCLE}" + expected_log = f"INFO:corporate.stripe:Change plan status: Customer.id: {customer.id}, CustomerPlan.id: {plan.id}, status: {CustomerPlan.DOWNGRADE_AT_END_OF_CYCLE}" self.assertEqual(m.output[0], expected_log) - - plan = CustomerPlan.objects.first() - assert plan is not None + plan.refresh_from_db() self.assertIsNotNone(plan.next_invoice_date) self.assertEqual(plan.status, CustomerPlan.DOWNGRADE_AT_END_OF_CYCLE) # Fast forward the next_invoice_date to next year. plan.next_invoice_date = self.next_year plan.save(update_fields=["next_invoice_date"]) invoice_plans_as_needed(self.next_year) - plan = CustomerPlan.objects.first() - assert plan is not None + plan.refresh_from_db() self.assertIsNone(plan.next_invoice_date) self.assertEqual(plan.status, CustomerPlan.ENDED) @@ -3320,7 +3308,9 @@ def test_downgrade_at_end_of_free_trial(self, *mocks: Mock) -> None: with self.settings(CLOUD_FREE_TRIAL_DAYS=60): with time_machine.travel(self.now, tick=False): self.add_card_and_upgrade(user, schedule="annual") - plan = get_current_plan_by_realm(user.realm) + customer = get_customer_by_realm(user.realm) + assert customer is not None + plan = get_current_plan_by_customer(customer) assert plan is not None self.assertEqual(plan.next_invoice_date, free_trial_end_date) self.assertEqual(get_realm("zulip").plan_type, Realm.PLAN_TYPE_STANDARD) @@ -3337,10 +3327,7 @@ def test_downgrade_at_end_of_free_trial(self, *mocks: Mock) -> None: "/billing/plan", {"status": CustomerPlan.DOWNGRADE_AT_END_OF_FREE_TRIAL}, ) - stripe_customer_id = Customer.objects.get(realm=user.realm).id - new_plan = get_current_plan_by_realm(user.realm) - assert new_plan is not None - expected_log = f"INFO:corporate.stripe:Change plan status: Customer.id: {stripe_customer_id}, CustomerPlan.id: {new_plan.id}, status: {CustomerPlan.DOWNGRADE_AT_END_OF_FREE_TRIAL}" + expected_log = f"INFO:corporate.stripe:Change plan status: Customer.id: {customer.id}, CustomerPlan.id: {plan.id}, status: {CustomerPlan.DOWNGRADE_AT_END_OF_FREE_TRIAL}" self.assertEqual(m.output[0], expected_log) self.assert_json_success(response) plan.refresh_from_db() @@ -3381,8 +3368,7 @@ def test_downgrade_at_end_of_free_trial(self, *mocks: Mock) -> None: # Check that we downgrade properly if the cycle is over with patch("corporate.lib.stripe.get_latest_seat_count", return_value=30): billing_session.update_license_ledger_if_needed(free_trial_end_date) - plan = CustomerPlan.objects.first() - assert plan is not None + plan.refresh_from_db() self.assertIsNone(plan.next_invoice_date) self.assertEqual(plan.status, CustomerPlan.ENDED) self.assertEqual(get_realm("zulip").plan_type, Realm.PLAN_TYPE_LIMITED) @@ -3417,7 +3403,9 @@ def test_cancel_downgrade_at_end_of_free_trial(self, *mocks: Mock) -> None: with self.settings(CLOUD_FREE_TRIAL_DAYS=60): with time_machine.travel(self.now, tick=False): self.add_card_and_upgrade(user, schedule="annual") - plan = get_current_plan_by_realm(user.realm) + customer = get_customer_by_realm(user.realm) + assert customer is not None + plan = get_current_plan_by_customer(customer) assert plan is not None self.assertEqual(plan.next_invoice_date, free_trial_end_date) self.assertEqual(get_realm("zulip").plan_type, Realm.PLAN_TYPE_STANDARD) @@ -3434,10 +3422,7 @@ def test_cancel_downgrade_at_end_of_free_trial(self, *mocks: Mock) -> None: "/billing/plan", {"status": CustomerPlan.DOWNGRADE_AT_END_OF_FREE_TRIAL}, ) - stripe_customer_id = Customer.objects.get(realm=user.realm).id - new_plan = get_current_plan_by_realm(user.realm) - assert new_plan is not None - expected_log = f"INFO:corporate.stripe:Change plan status: Customer.id: {stripe_customer_id}, CustomerPlan.id: {new_plan.id}, status: {CustomerPlan.DOWNGRADE_AT_END_OF_FREE_TRIAL}" + expected_log = f"INFO:corporate.stripe:Change plan status: Customer.id: {customer.id}, CustomerPlan.id: {plan.id}, status: {CustomerPlan.DOWNGRADE_AT_END_OF_FREE_TRIAL}" self.assertEqual(m.output[0], expected_log) self.assert_json_success(response) plan.refresh_from_db() @@ -3456,10 +3441,7 @@ def test_cancel_downgrade_at_end_of_free_trial(self, *mocks: Mock) -> None: "/billing/plan", {"status": CustomerPlan.FREE_TRIAL}, ) - stripe_customer_id = Customer.objects.get(realm=user.realm).id - new_plan = get_current_plan_by_realm(user.realm) - assert new_plan is not None - expected_log = f"INFO:corporate.stripe:Change plan status: Customer.id: {stripe_customer_id}, CustomerPlan.id: {new_plan.id}, status: {CustomerPlan.FREE_TRIAL}" + expected_log = f"INFO:corporate.stripe:Change plan status: Customer.id: {customer.id}, CustomerPlan.id: {plan.id}, status: {CustomerPlan.FREE_TRIAL}" self.assertEqual(m.output[0], expected_log) self.assert_json_success(response) plan.refresh_from_db() @@ -3484,10 +3466,11 @@ def test_reupgrade_by_billing_admin_after_downgrade(self) -> None: "/billing/plan", {"status": CustomerPlan.DOWNGRADE_AT_END_OF_CYCLE}, ) - stripe_customer_id = Customer.objects.get(realm=user.realm).id - new_plan = get_current_plan_by_realm(user.realm) - assert new_plan is not None - expected_log = f"INFO:corporate.stripe:Change plan status: Customer.id: {stripe_customer_id}, CustomerPlan.id: {new_plan.id}, status: {CustomerPlan.DOWNGRADE_AT_END_OF_CYCLE}" + customer = get_customer_by_realm(user.realm) + assert customer is not None + plan = get_current_plan_by_customer(customer) + assert plan is not None + expected_log = f"INFO:corporate.stripe:Change plan status: Customer.id: {customer.id}, CustomerPlan.id: {plan.id}, status: {CustomerPlan.DOWNGRADE_AT_END_OF_CYCLE}" self.assertEqual(m.output[0], expected_log) with ( @@ -3507,8 +3490,8 @@ def test_reupgrade_by_billing_admin_after_downgrade(self) -> None: ) # Fast forward the next_invoice_date to next year. - new_plan.next_invoice_date = self.next_year - new_plan.save(update_fields=["next_invoice_date"]) + plan.next_invoice_date = self.next_year + plan.save(update_fields=["next_invoice_date"]) invoice_plans_as_needed(self.next_year) with time_machine.travel(self.next_year, tick=False): @@ -8964,13 +8947,11 @@ def test_non_sponsorship_billing(self, *mocks: Mock) -> None: "/billing/plan", {"status": CustomerPlan.DOWNGRADE_AT_END_OF_CYCLE}, ) - customer = Customer.objects.get(remote_server=self.remote_server) - new_plan = get_current_plan_by_customer(customer) - assert new_plan is not None - expected_log = f"INFO:corporate.stripe:Change plan status: Customer.id: {customer.id}, CustomerPlan.id: {new_plan.id}, status: {CustomerPlan.DOWNGRADE_AT_END_OF_CYCLE}" + expected_log = f"INFO:corporate.stripe:Change plan status: Customer.id: {customer.id}, CustomerPlan.id: {plan.id}, status: {CustomerPlan.DOWNGRADE_AT_END_OF_CYCLE}" self.assertEqual(m.output[0], expected_log) self.assert_json_success(response) - self.assertEqual(new_plan.licenses_at_next_renewal(), None) + plan.refresh_from_db() + self.assertEqual(plan.licenses_at_next_renewal(), None) @responses.activate def test_request_sponsorship(self) -> None: From 03307e08f5990b4d62c6779aa27051d5cc7e256a Mon Sep 17 00:00:00 2001 From: Srinandha Murugesan Date: Fri, 31 Jul 2026 13:53:22 +0000 Subject: [PATCH 12/14] github: Support signature verification using bot_config. Update WebhookTestCase to inject test secrets and build signature headers automatically. Enable signature validation in GitHub webhooks to reject invalid payloads early, and simplify test setup for future integrations. Co-authored-by: Srinandha Murugesan Co-authored-by: Isaiah Marte Co-authored-by: Akshaj Katkuri Co-authored-by: Jason Zheng --- zerver/lib/test_classes.py | 46 +++++++++++++++++++++++- zerver/lib/webhooks/common.py | 52 +++++++++++++++++++++++----- zerver/tests/test_webhooks_common.py | 52 ++++++++++++++++++++++------ zerver/webhooks/github/tests.py | 48 +++++++++++++++++++++++++ zerver/webhooks/github/view.py | 3 ++ 5 files changed, 180 insertions(+), 21 deletions(-) diff --git a/zerver/lib/test_classes.py b/zerver/lib/test_classes.py index 6f3c061f8644e..2f57f637caa28 100644 --- a/zerver/lib/test_classes.py +++ b/zerver/lib/test_classes.py @@ -1,5 +1,7 @@ import asyncio import base64 +import hashlib +import hmac import os import re import shutil @@ -35,6 +37,7 @@ from django.test.testcases import SerializeMixin from django.urls import resolve from django.utils import translation +from django.utils.encoding import force_bytes from django.utils.module_loading import import_string from django.utils.timezone import now as timezone_now from fakeldap import MockLDAP @@ -54,6 +57,7 @@ from zerver.actions.user_settings import do_change_full_name, do_change_user_setting from zerver.actions.users import do_change_user_role from zerver.decorator import do_two_factor_login +from zerver.lib.bot_config import set_bot_config from zerver.lib.cache import bounce_key_prefix_for_testing from zerver.lib.email_notifications import MissedMessageData, handle_missedmessage_emails from zerver.lib.initial_password import initial_password @@ -2544,6 +2548,8 @@ class WebhookTestCase(ZulipTestCase): DEFAULT_URL_TEMPLATE: str = ( "/api/v1/external/{webhook_dir_name}?stream={stream}&api_key={api_key}" ) + WEBHOOK_SIGNATURE_HEADER: str | None = None + WEBHOOK_TEST_SECRET: str | None = None def get_webhook_dir_name(self) -> str: module_parts = self.__module__.split(".") @@ -2619,6 +2625,18 @@ def api_channel_message( **extra, ) + def get_webhook_signature(self, raw_payload: bytes) -> str | None: + """ + Generate the signature header value for a given payload. + Override this method in child classes if the integration uses different signature format. + """ + secret = getattr(self, "WEBHOOK_TEST_SECRET", None) + if secret is None: + return None # nocoverage + + # Default implementation matches the current GitHub standard format + return "sha256=" + hmac.new(force_bytes(secret), raw_payload, hashlib.sha256).hexdigest() + def check_webhook( self, fixture_name: str, @@ -2650,16 +2668,37 @@ def check_webhook( """ self.subscribe(self.test_user, self.channel_name) + url = getattr(self, "url", None) + if url is None: + url = self.build_webhook_url() # nocoverage + + webhook_secret = getattr(self, "WEBHOOK_TEST_SECRET", None) + if webhook_secret is not None: + set_bot_config(self.test_user, "webhook_secret", webhook_secret) + payload = self.get_payload(fixture_name) if content_type is not None: extra["content_type"] = content_type + + signature_header_name = getattr(self, "WEBHOOK_SIGNATURE_HEADER", None) + if signature_header_name is not None: + try: + raw_payload = self.get_body(fixture_name) + except FileNotFoundError: # nocoverage + raw_payload = "" + + signature_value = self.get_webhook_signature(force_bytes(raw_payload)) + if signature_value is not None: + django_header = "HTTP_" + signature_header_name.upper().replace("-", "_") + extra[django_header] = signature_value + headers = call_fixture_to_headers(self.webhook_dir_name, fixture_name) headers = standardize_headers(headers) extra.update(headers) try: msg = self.send_webhook_payload( self.test_user, - self.url, + url, payload, **extra, ) @@ -2715,6 +2754,11 @@ def send_and_test_private_message( Most webhooks send to streams, and you will want to look at check_webhook. """ + + webhook_secret = getattr(self, "WEBHOOK_TEST_SECRET", None) + if webhook_secret is not None: + set_bot_config(self.test_user, "webhook_secret", webhook_secret) # nocoverage + payload = self.get_payload(fixture_name) extra["content_type"] = content_type diff --git a/zerver/lib/webhooks/common.py b/zerver/lib/webhooks/common.py index ac2d8ba7e7a16..649575dc85535 100644 --- a/zerver/lib/webhooks/common.py +++ b/zerver/lib/webhooks/common.py @@ -25,6 +25,7 @@ check_send_stream_message_by_id, send_rate_limited_pm_notification_to_bot_owner, ) +from zerver.lib.bot_config import ConfigError, get_bot_config from zerver.lib.exceptions import ( AnomalousWebhookPayloadError, ErrorCode, @@ -320,8 +321,45 @@ def parse_multipart_string(body: str) -> dict[str, str]: return data +def validate_webhook_delivery( + request: HttpRequest, signature_header_name: str, algorithm: str = "sha256" +) -> None: + assert request.user.is_authenticated + user_profile = request.user + assert isinstance(user_profile, UserProfile) + + try: + config = get_bot_config(user_profile) + webhook_secret = config.get("webhook_secret", "") + except ConfigError: + webhook_secret = "" + + if not webhook_secret: + return + + signature_header = request.headers.get(signature_header_name, "") + signature = signature_header.split("=")[-1] if "=" in signature_header else signature_header + + payload = request.body.decode("utf-8") + + try: + validate_webhook_signature( + payload=payload, + signature=signature, + secret=webhook_secret, + algorithm=algorithm, + ) + except JsonableError: + raise + except Exception as err: # nocoverage + raise JsonableError(str(err)) + + def validate_webhook_signature( - request: HttpRequest, payload: str, signature: str, algorithm: str = "sha256" + payload: str, + signature: str, + secret: str, + algorithm: str = "sha256", ) -> None: if not settings.VERIFY_WEBHOOK_SIGNATURES: # nocoverage return @@ -331,14 +369,10 @@ def validate_webhook_signature( _("The algorithm '{algorithm}' is not supported.").format(algorithm=algorithm) ) - webhook_secret: str | None = request.GET.get("webhook_secret") - if webhook_secret is None: - raise JsonableError( - _( - "The webhook secret is missing. Please set the webhook_secret while generating the URL." - ) - ) - webhook_secret_bytes = force_bytes(webhook_secret) + if not secret: + raise JsonableError(_("Webhook secret is not configured for this bot.")) + + webhook_secret_bytes = force_bytes(secret) payload_bytes = force_bytes(payload) signed_payload = hmac.new( diff --git a/zerver/tests/test_webhooks_common.py b/zerver/tests/test_webhooks_common.py index 02db0bdd1ce09..48ea22a889aaf 100644 --- a/zerver/tests/test_webhooks_common.py +++ b/zerver/tests/test_webhooks_common.py @@ -14,6 +14,7 @@ from zerver.actions.custom_profile_fields import try_add_realm_custom_profile_field from zerver.actions.streams import do_rename_stream from zerver.decorator import webhook_view +from zerver.lib.bot_config import set_bot_config from zerver.lib.exceptions import InvalidJSONError, JsonableError from zerver.lib.request import RequestNotes from zerver.lib.send_email import FromAddress @@ -29,6 +30,7 @@ get_service_api_data, guess_zulip_user_from_external_account, standardize_headers, + validate_webhook_delivery, validate_webhook_signature, ) from zerver.models import Client, CustomProfileField, Message, UserProfile @@ -152,18 +154,14 @@ def test_standardize_headers(self) -> None: @override_settings(VERIFY_WEBHOOK_SIGNATURES=True) def test_validate_webhook_signature(self) -> None: - request = HostRequestMock() - request.GET = QueryDict("", mutable=True) - - # Valid signature webhook_secret = "test_secret" payload = '{"key": "value"}' signature = hmac.new( force_bytes(webhook_secret), force_bytes(payload), hashlib.sha256 ).hexdigest() - request.GET.update({"webhook_secret": webhook_secret}) - validate_webhook_signature(request, payload, signature) + # Valid signature + validate_webhook_signature(payload, signature, webhook_secret) # Invalid signature invalid_signature = "invalid_signature" @@ -171,15 +169,47 @@ def test_validate_webhook_signature(self) -> None: JsonableError, "Webhook signature verification failed.", ): - validate_webhook_signature(request, payload, invalid_signature) + validate_webhook_signature(payload, invalid_signature, webhook_secret) - # No webhook_secret parameter - request.GET.clear() + # Missing or empty secret with self.assertRaisesRegex( JsonableError, - "The webhook secret is missing. Please set the webhook_secret while generating the URL.", + "Webhook secret is not configured for this bot.", ): - validate_webhook_signature(request, payload, signature) + validate_webhook_signature(payload, signature, secret="") + + @override_settings(VERIFY_WEBHOOK_SIGNATURES=True) + def test_validate_webhook_delivery(self) -> None: + webhook_bot = get_user("webhook-bot@zulip.com", get_realm("zulip")) + webhook_secret = "test_secret" + payload = '{"key": "value"}' + signature = hmac.new( + force_bytes(webhook_secret), force_bytes(payload), hashlib.sha256 + ).hexdigest() + + set_bot_config(webhook_bot, "webhook_secret", webhook_secret) + request = HostRequestMock(meta_data={"HTTP_X_HUB_SIGNATURE_256": f"sha256={signature}"}) + request.user = webhook_bot + request.GET = QueryDict("", mutable=True) + request._body = force_bytes(payload) + + # Valid signature + validate_webhook_delivery(request, "X_HUB_Signature_256") + + # Invalid signature + request.META["HTTP_X_HUB_SIGNATURE_256"] = "sha256=invalid_signature" + del request.headers + with self.assertRaisesRegex( + JsonableError, + "Webhook signature verification failed.", + ): + validate_webhook_delivery(request, "X_HUB_Signature_256") + + # No webhook_secret configured for this bot skips validation + set_bot_config(webhook_bot, "webhook_secret", "") + request.META["HTTP_X_HUB_SIGNATURE_256"] = f"sha256={signature}" + del request.headers + validate_webhook_delivery(request, "X_HUB_Signature_256") def test_check_send_webhook_message_returns_id(self) -> None: webhook_bot = get_user("webhook-bot@zulip.com", get_realm("zulip")) diff --git a/zerver/webhooks/github/tests.py b/zerver/webhooks/github/tests.py index 90c563f0e2db2..5effc6bda0f07 100644 --- a/zerver/webhooks/github/tests.py +++ b/zerver/webhooks/github/tests.py @@ -1,7 +1,9 @@ from unittest.mock import patch import orjson +from django.test import override_settings +from zerver.lib.bot_config import set_bot_config from zerver.lib.message import truncate_topic from zerver.lib.test_classes import WebhookTestCase from zerver.lib.webhooks.git import COMMITS_LIMIT @@ -22,6 +24,9 @@ class GitHubWebhookTest(WebhookTestCase): + WEBHOOK_SIGNATURE_HEADER = "X_HUB_Signature_256" + WEBHOOK_TEST_SECRET = "testingthis" + def test_ping_event(self) -> None: expected_message = "GitHub webhook has been successfully configured by TomaszKolek." self.check_webhook("ping", TOPIC_REPO, expected_message) @@ -851,6 +856,49 @@ def test_issue_comment_silent_mention_with_multiple_matches(self) -> None: expected_message = "baxterthehacker [commented](https://github.com/baxterthehacker/public-repo/issues/2#issuecomment-99262140) on [issue #2](https://github.com/baxterthehacker/public-repo/issues/2):\n\n``` quote\nYou are totally right! I'll get this fixed right away.\n```" self.check_webhook("issue_comment", TOPIC_ISSUE, expected_message) + def test_github_webhook_bad_signature(self) -> None: + with override_settings(VERIFY_WEBHOOK_SIGNATURES=True): + url = self.build_webhook_url() + set_bot_config(self.test_user, "webhook_secret", self.WEBHOOK_TEST_SECRET) + + result = self.client_post( + url, + self.get_payload("ping"), + content_type="application/json", + HTTP_X_HUB_SIGNATURE_256="sha256=completely_invalid_hash_value", + ) + self.assert_json_error(result, "Webhook signature verification failed.") + + def test_github_webhook_signature_disabled_skips_validation(self) -> None: + """Verifies that when VERIFY_WEBHOOK_SIGNATURES is explicitly disabled, + requests pass through even if the signature value is completely bogus. + """ + with override_settings(VERIFY_WEBHOOK_SIGNATURES=False): + expected_message = "GitHub webhook has been successfully configured by TomaszKolek." + self.check_webhook( + "ping", + TOPIC_REPO, + expected_message, + HTTP_X_HUB_SIGNATURE_256="sha256=invalid_hash", + ) + + def test_github_webhook_valid_signature_success(self) -> None: + """Verifies that a mathematically correct HMAC signature passes + cleanly when verification enforcement is active.""" + expected_message = "GitHub webhook has been successfully configured by TomaszKolek." + + with override_settings(VERIFY_WEBHOOK_SIGNATURES=True): + self.check_webhook("ping", TOPIC_REPO, expected_message) + + def test_github_webhook_missing_secret(self) -> None: + """Verifies that if no webhook secret is configured for the bot, + the request is processed normally without requiring signature verification.""" + + with override_settings(VERIFY_WEBHOOK_SIGNATURES=True): + set_bot_config(self.test_user, "webhook_secret", "") + expected_message = "GitHub webhook has been successfully configured by TomaszKolek." + self.check_webhook("ping", TOPIC_REPO, expected_message) + class GitHubSponsorsHookTests(WebhookTestCase): URL_TEMPLATE = "/api/v1/external/githubsponsors?stream={stream}&api_key={api_key}" diff --git a/zerver/webhooks/github/view.py b/zerver/webhooks/github/view.py index c1a13b5ad0daf..b276df37fd713 100644 --- a/zerver/webhooks/github/view.py +++ b/zerver/webhooks/github/view.py @@ -22,6 +22,7 @@ get_event_header, get_setup_webhook_message, guess_zulip_user_from_external_account, + validate_webhook_delivery, ) from zerver.lib.webhooks.git import ( CONTENT_MESSAGE_TEMPLATE, @@ -1215,6 +1216,8 @@ def api_github_webhook( directly to the X-GitHub-Event header's event, but we sometimes refine it based on the payload. """ + validate_webhook_delivery(request, "X_HUB_Signature_256", "sha256") + header_event = get_event_header(request, "X-GitHub-Event", "GitHub") # Ignore events from private repositories if the URL option is set From 58e722d50b96b9071ef81e8c91ea4047d6eedb34 Mon Sep 17 00:00:00 2001 From: Srinandha Murugesan Date: Fri, 31 Jul 2026 14:00:16 +0000 Subject: [PATCH 13/14] settings: Allow setting and clearing webhook secrets in bot UI. Add UI for adding a webhook secret in the add bot form and modifying or deleting it in the manage bot form. This allows for secure storage of secrets in BotConfigData over URL parameters and lets users modify it as needed. Co-authored-by: Srinandha Murugesan Co-authored-by: Isaiah Marte Co-authored-by: Akshaj Katkuri Co-authored-by: Jason Zheng --- web/src/bot_type_values.ts | 1 + web/src/settings_bots.ts | 40 ++++++++--- web/src/user_profile.ts | 42 +++++++++-- web/templates/settings/add_new_bot_form.hbs | 7 ++ web/templates/settings/edit_bot_form.hbs | 21 +++++- zerver/tests/test_bots.py | 79 +++++++++++++++++++++ 6 files changed, 174 insertions(+), 16 deletions(-) diff --git a/web/src/bot_type_values.ts b/web/src/bot_type_values.ts index a93e47a8ad333..5b4e302ed6c72 100644 --- a/web/src/bot_type_values.ts +++ b/web/src/bot_type_values.ts @@ -4,5 +4,6 @@ export const INCOMING_WEBHOOK_BOT_TYPE_INT = 2; export const OUTGOING_WEBHOOK_BOT_TYPE_INT = 3; // String forms used as HTML form values. +export const INCOMING_WEBHOOK_BOT_TYPE = "2"; export const OUTGOING_WEBHOOK_BOT_TYPE = "3"; export const EMBEDDED_BOT_TYPE = "4"; diff --git a/web/src/settings_bots.ts b/web/src/settings_bots.ts index 1a496fc991ea4..0acf9b405a9ae 100644 --- a/web/src/settings_bots.ts +++ b/web/src/settings_bots.ts @@ -13,6 +13,7 @@ import * as bot_helper from "./bot_helper.ts"; import { EMBEDDED_BOT_TYPE, GENERIC_BOT_TYPE, + INCOMING_WEBHOOK_BOT_TYPE, INCOMING_WEBHOOK_BOT_TYPE_INT, OUTGOING_WEBHOOK_BOT_TYPE, OUTGOING_WEBHOOK_BOT_TYPE_INT, @@ -298,6 +299,20 @@ export function add_a_new_bot(): void { formData.append("interface_type", interface_type); break; } + case INCOMING_WEBHOOK_BOT_TYPE: { + const config_data: Record = {}; + $("#webhook_secret_inputbox input").each(function () { + const key = $(this).attr("name")!; + const raw_val = $(this).val(); + if (typeof raw_val === "string" && raw_val.trim() !== "") { + config_data[key] = raw_val.trim(); + } + }); + if (Object.keys(config_data).length > 0) { + formData.append("config_data", JSON.stringify(config_data)); + } + break; + } case EMBEDDED_BOT_TYPE: { formData.append("service_name", service_name); const config_data: Record = {}; @@ -336,7 +351,7 @@ export function add_a_new_bot(): void { } function set_up_form_fields(): void { - $("#create_bot_type").val(INCOMING_WEBHOOK_BOT_TYPE_INT); + $("#create_bot_type").val(INCOMING_WEBHOOK_BOT_TYPE).trigger("change"); $("#payload_url_inputbox").hide(); $("#create_payload_url").val(""); $("#service_name_list").hide(); @@ -362,7 +377,13 @@ export function add_a_new_bot(): void { $("#payload_url_inputbox").hide(); $("#create_payload_url").removeClass("required"); + + $("#webhook_secret_inputbox").hide(); switch (bot_type) { + case INCOMING_WEBHOOK_BOT_TYPE: { + $("#webhook_secret_inputbox").show(); + break; + } case OUTGOING_WEBHOOK_BOT_TYPE: { $("#payload_url_inputbox").show(); $("#create_payload_url").addClass("required"); @@ -377,7 +398,7 @@ export function add_a_new_bot(): void { } } }); - + $("#create_bot_type").val(INCOMING_WEBHOOK_BOT_TYPE).trigger("change"); $("#select_service_name").on("change", () => { $("#config_inputbox").children().hide(); const selected_bot = $( @@ -742,11 +763,12 @@ function set_up_bot_handlers($container: JQuery): void { add_a_new_bot(); }); - $container.find(".download-botserverrc-file").on("click", function () { + $container.find(".download-botserverrc-file").on("click", (e) => { + const currentTarget = e.currentTarget; void (async () => { let content = ""; - buttons.show_button_loading_indicator($(this)); - $(this).prop("disabled", true); + buttons.show_button_loading_indicator($(currentTarget)); + $(currentTarget).prop("disabled", true); for (const bot of bot_data.get_all_bots_for_current_user()) { if (bot.is_active && bot.bot_type === OUTGOING_WEBHOOK_BOT_TYPE_INT) { const bot_token = bot_helper.get_outgoing_webhook_token(bot.user_id); @@ -755,15 +777,15 @@ function set_up_bot_handlers($container: JQuery): void { $("#admin-your-bots-list .bot-list-error"), ); if (!api_key) { - buttons.hide_button_loading_indicator($(this)); - $(this).prop("disabled", false); + buttons.hide_button_loading_indicator($(currentTarget)); + $(currentTarget).prop("disabled", false); return; } content += generate_botserverrc_content(bot.email, api_key, bot_token); } } - buttons.hide_button_loading_indicator($(this)); - $(this).prop("disabled", false); + buttons.hide_button_loading_indicator($(currentTarget)); + $(currentTarget).prop("disabled", false); $container .find(".hidden-botserverrc-download") diff --git a/web/src/user_profile.ts b/web/src/user_profile.ts index 2722d938aed79..2afb40df808ea 100644 --- a/web/src/user_profile.ts +++ b/web/src/user_profile.ts @@ -1,6 +1,5 @@ import ClipboardJS from "clipboard"; import {parseISO} from "date-fns"; -import {parseOneAddress} from "email-addresses"; import {$} from "jquery"; import _ from "lodash"; import assert from "minimalistic-assert"; @@ -24,6 +23,7 @@ import * as bot_data from "./bot_data.ts"; import * as bot_helper from "./bot_helper.ts"; import { EMBEDDED_BOT_TYPE, + INCOMING_WEBHOOK_BOT_TYPE, INCOMING_WEBHOOK_BOT_TYPE_INT, OUTGOING_WEBHOOK_BOT_TYPE, } from "./bot_type_values.ts"; @@ -851,11 +851,7 @@ export function show_edit_bot_info_modal(user_id: number, $container: JQuery): v assert(bot.is_bot); // Extract short_name from email (format: {short_name}-bot@domain) - const parsed_address = parseOneAddress(bot.email); - assert(parsed_address?.type === "mailbox"); - const short_name = parsed_address.local.endsWith("-bot") - ? parsed_address.local.slice(0, -"-bot".length) - : parsed_address.local; + const short_name = bot.email.split("@", 1)[0]!.slice(0, -4); const modal_content_html = render_edit_bot_form({ user_id, is_active, @@ -879,7 +875,32 @@ export function show_edit_bot_info_modal(user_id: number, $container: JQuery): v const bot_type = bot.bot_type.toString(); const services = bot_data.get_services(bot.user_id); const service = services?.[0]; + let is_delete_requested = false; edit_bot_post_render(); + + $("#bot-edit-form").on("click", "#clear_webhook_secret_button", (e) => { + e.preventDefault(); + is_delete_requested = true; + + // Clear the input value and set visual feedback + const $secret_input = $("#edit_webhook_secret"); + $secret_input.val(""); + $secret_input.attr( + "placeholder", + $t({defaultMessage: "Secret will be deleted when saved."}), + ); + + // Notify form handler that a change was made so the save button is enabled + $("#user-profile-modal .dialog_submit_button").prop("disabled", false); + }); + + // If the user types anything manually into the input, reset the clear flag + $("#bot-edit-form").on("input", "#edit_webhook_secret", function () { + if ($(this).val() !== "") { + $(this).data("clear-secret", false); + } + }); + original_values = get_current_values($("#bot-edit-form")); $("#bot-edit-form").on("input", "input, select, button", (e) => { e.preventDefault(); @@ -935,6 +956,14 @@ export function show_edit_bot_info_modal(user_id: number, $container: JQuery): v config_data[$(this).attr("name")!] = $(this).val()!; }); formData.append("config_data", JSON.stringify(config_data)); + } else if (bot_type === INCOMING_WEBHOOK_BOT_TYPE) { + const webhook_secret = $("#edit_webhook_secret").val()?.trim(); + if (is_delete_requested) { + formData.append("config_data", JSON.stringify({webhook_secret: ""})); + is_delete_requested = false; + } else if (webhook_secret) { + formData.append("config_data", JSON.stringify({webhook_secret})); + } } const files = util.the( @@ -957,6 +986,7 @@ export function show_edit_bot_info_modal(user_id: number, $container: JQuery): v contentType: false, success() { $("#bot-edit-form-error").hide(); + $("#edit-webhook-secret").val(""); avatar_widget.clear(); hide_button_spinner($submit_button); original_values = get_current_values($("#bot-edit-form")); diff --git a/web/templates/settings/add_new_bot_form.hbs b/web/templates/settings/add_new_bot_form.hbs index 14c89e4fd2b15..b5dadbff1346a 100644 --- a/web/templates/settings/add_new_bot_form.hbs +++ b/web/templates/settings/add_new_bot_form.hbs @@ -53,6 +53,13 @@
+
+
+ + +
+
+
{{#each realm_embedded_bots}} {{#each (object_entries config) as |entry|}} diff --git a/web/templates/settings/edit_bot_form.hbs b/web/templates/settings/edit_bot_form.hbs index 4d71e501168b8..9b460fc21eb4c 100644 --- a/web/templates/settings/edit_bot_form.hbs +++ b/web/templates/settings/edit_bot_form.hbs @@ -39,8 +39,15 @@ widget_name="edit_bot_owner" label=(t 'Owner')}} -
+
+ + {{#if is_incoming_webhook_bot}} +
+ +
+ {{/if}} +
{{!-- Shows the current avatar --}} @@ -109,6 +116,18 @@ }}
{{/if}} + + {{#if is_incoming_webhook_bot}} +
+ {{> ../components/action_button + label=(t "Delete secret") + variant="subtle" + intent="danger" + id="clear_webhook_secret_button" + }} +
+ {{/if}} +
{{#if is_active}} {{> ../components/action_button diff --git a/zerver/tests/test_bots.py b/zerver/tests/test_bots.py index 63769fa63f3a6..a2fda496db7e8 100644 --- a/zerver/tests/test_bots.py +++ b/zerver/tests/test_bots.py @@ -2307,6 +2307,85 @@ def test_create_incoming_webhook_bot_with_incorrect_service_name(self) -> None: with self.assertRaises(UserProfile.DoesNotExist): UserProfile.objects.get(full_name="My Stripe Bot") + def test_create_incoming_webhook_bot_with_secret(self) -> None: + self.login("hamlet") + self.assert_num_bots_equal(0) + self.create_bot( + bot_type=UserProfile.INCOMING_WEBHOOK_BOT, + config_data=orjson.dumps({"webhook_secret": "test-secret-key-123"}).decode(), + ) + self.assert_num_bots_equal(1) + + new_bot = UserProfile.objects.get(full_name="The Bot of Hamlet") + config_data = get_bot_config(new_bot) + self.assertEqual(config_data["webhook_secret"], "test-secret-key-123") + + def test_create_incoming_webhook_bot_without_secret(self) -> None: + self.login("hamlet") + self.assert_num_bots_equal(0) + self.create_bot(bot_type=UserProfile.INCOMING_WEBHOOK_BOT) + self.assert_num_bots_equal(1) + + new_bot = UserProfile.objects.get(full_name="The Bot of Hamlet") + + with self.assertRaisesMessage(ConfigError, "No config data available."): + get_bot_config(new_bot) + + def test_patch_incoming_webhook_bot_add_secret(self) -> None: + self.login("hamlet") + self.create_bot(bot_type=UserProfile.INCOMING_WEBHOOK_BOT) + + bot_email = "hambot-bot@zulip.testserver" + bot_id = self.get_bot_user(bot_email).id + + bot_update = { + "config_data": orjson.dumps({"webhook_secret": "test-secret-key-123"}).decode() + } + result = self.client_patch(f"/json/bots/{bot_id}", bot_update) + self.assert_json_success(result) + + bot = self.get_bot_user(bot_email) + config_data = get_bot_config(bot) + self.assertEqual(config_data["webhook_secret"], "test-secret-key-123") + + def test_patch_incoming_webhook_bot_update_secret(self) -> None: + self.login("hamlet") + self.create_bot( + bot_type=UserProfile.INCOMING_WEBHOOK_BOT, + config_data=orjson.dumps({"webhook_secret": "old-test-secret-123"}).decode(), + ) + + bot_email = "hambot-bot@zulip.testserver" + bot_id = self.get_bot_user(bot_email).id + + bot_update = { + "config_data": orjson.dumps({"webhook_secret": "new-test-secret-123"}).decode() + } + result = self.client_patch(f"/json/bots/{bot_id}", bot_update) + self.assert_json_success(result) + + bot = self.get_bot_user(bot_email) + config_data = get_bot_config(bot) + self.assertEqual(config_data["webhook_secret"], "new-test-secret-123") + + def test_patch_incoming_webhook_bot_clear_secret(self) -> None: + self.login("hamlet") + self.create_bot( + bot_type=UserProfile.INCOMING_WEBHOOK_BOT, + config_data=orjson.dumps({"webhook_secret": "test-secret-key-123"}).decode(), + ) + + bot_email = "hambot-bot@zulip.testserver" + bot_id = self.get_bot_user(bot_email).id + + bot_update = {"config_data": orjson.dumps({"webhook_secret": ""}).decode()} + result = self.client_patch(f"/json/bots/{bot_id}", bot_update) + self.assert_json_success(result) + + bot = self.get_bot_user(bot_email) + config_data = get_bot_config(bot) + self.assertEqual(config_data["webhook_secret"], "") + def test_get_bot_api_key(self) -> None: self.login("hamlet") self.create_bot() From a3f9305c6b0cdb75473de30d8309965fc69aa24e Mon Sep 17 00:00:00 2001 From: Srinandha Murugesan Date: Fri, 31 Jul 2026 15:27:05 +0000 Subject: [PATCH 14/14] integrations: Add signature recalculation to dev panel. Add a UI field and backend endpoint to the integrations dev panel to type a secret and dynamically calculate signature headers for fixture payloads. This allows developers to test signature verification in webhook endpoints directly through fixture mock requests. Co-authored-by: Srinandha Murugesan Co-authored-by: Isaiah Marte Co-authored-by: Akshaj Katkuri Co-authored-by: Jason Zheng --- package.json | 1 + pnpm-lock.yaml | 3 + .../development/integrations_dev_panel.html | 4 + web/src/portico/integrations_dev_panel.ts | 78 ++++++++- web/styles/portico/integrations_dev_panel.css | 3 +- zerver/tests/test_integrations_dev_panel.py | 154 ++++++++++++++++++ zerver/views/development/integrations.py | 67 +++++++- zerver/views/documentation.py | 2 +- zproject/dev_urls.py | 5 + 9 files changed, 313 insertions(+), 4 deletions(-) diff --git a/package.json b/package.json index 5bf45a9ac01cd..ee438272692d0 100644 --- a/package.json +++ b/package.json @@ -114,6 +114,7 @@ "@types/convert-source-map": "^2.0.3", "@types/css-tree": "^2.3.11", "@types/eslint-config-prettier": "^6.11.3", + "@types/estree": "^1.0.9", "@types/gtag.js": "^0.0.20", "@types/is-url": "^1.2.32", "@types/jquery": "^4.0.0", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 6db1dfeca5512..bdf017bcc8e29 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -355,6 +355,9 @@ importers: '@types/eslint-config-prettier': specifier: ^6.11.3 version: 6.11.3 + '@types/estree': + specifier: ^1.0.9 + version: 1.0.9 '@types/gtag.js': specifier: ^0.0.20 version: 0.0.20 diff --git a/templates/zerver/development/integrations_dev_panel.html b/templates/zerver/development/integrations_dev_panel.html index b948daf30bbaf..2f9e5f6e076d8 100644 --- a/templates/zerver/development/integrations_dev_panel.html +++ b/templates/zerver/development/integrations_dev_panel.html @@ -69,6 +69,10 @@
+
+ + +

diff --git a/web/src/portico/integrations_dev_panel.ts b/web/src/portico/integrations_dev_panel.ts index 240c7a5e4cea1..56ec628158bb3 100644 --- a/web/src/portico/integrations_dev_panel.ts +++ b/web/src/portico/integrations_dev_panel.ts @@ -49,6 +49,8 @@ const integrations_api_response_schema = z.object({ type ServerResponse = z.infer; +let last_computed_header_key: string | null = null; // Tracks the current signature header for auto-clearing when switching integrations + const loaded_fixtures = new Map(); const url_base = "/api/v1/external/"; @@ -231,11 +233,83 @@ function update_url(): void { params.set("topic", topic_name); } } + const webhook_secret = $("input#webhook_secret").val()!; const url = `${url_base}${integration_name}?${params.toString()}`; url_field!.value = url; + + sync_signature_headers(integration_name, webhook_secret); } +} - return; +function sync_signature_headers(integration_name: string, webhook_secret: string): void { + const $custom_headers_field = $("textarea#custom_http_headers"); + const current_headers_raw = $custom_headers_field.val()?.toString().trim() ?? ""; + + let headers_object: Record = {}; + if (current_headers_raw !== "") { + try { + headers_object = z + .record(z.string(), z.string()) + .parse(JSON.parse(current_headers_raw)); + } catch { + headers_object = {}; + } + } + + if (last_computed_header_key && Object.hasOwn(headers_object, last_computed_header_key)) { + Reflect.deleteProperty(headers_object, last_computed_header_key); + } + + if (webhook_secret.trim() === "") { + last_computed_header_key = null; + if (Object.keys(headers_object).length === 0) { + $custom_headers_field.val("{}"); + } else { + $custom_headers_field.val(JSON.stringify(headers_object, null, 4)); + } + return; + } + + const raw_payload = $("textarea#fixture_body").val() ?? ""; + let cleaned_payload: string; + + try { + cleaned_payload = JSON.stringify(JSON.parse(raw_payload)); + } catch { + cleaned_payload = raw_payload.trim(); + } + + channel.post({ + url: "/devtools/integrations/recalculate_signature", + data: JSON.stringify({ + secret: webhook_secret, + payload: cleaned_payload, + integration_name, + }), + success(raw_data: unknown) { + const data = z + .object({ + supported: z.optional(z.boolean()), + clear_signature: z.optional(z.boolean()), + header_key: z.string(), + signature: z.string(), + }) + .parse(raw_data); + + if (!data.supported || data.clear_signature) { + last_computed_header_key = null; + if (Object.keys(headers_object).length === 0) { + $custom_headers_field.val("{}"); + } else { + $custom_headers_field.val(JSON.stringify(headers_object, null, 4)); + } + } else { + headers_object[data.header_key] = data.signature; + last_computed_header_key = data.header_key; + $custom_headers_field.val(JSON.stringify(headers_object, null, 4)); + } + }, + }); } // API callers: These methods handle communicating with the Python backend API. @@ -440,4 +514,6 @@ $(() => { $("#stream_name").on("change", update_url); $("#topic_name").on("change", update_url); + + $("#webhook_secret").on("change", update_url); }); diff --git a/web/styles/portico/integrations_dev_panel.css b/web/styles/portico/integrations_dev_panel.css index fdf156382f823..31394093e93a6 100644 --- a/web/styles/portico/integrations_dev_panel.css +++ b/web/styles/portico/integrations_dev_panel.css @@ -105,7 +105,8 @@ } #stream_name, -#topic_name { +#topic_name, +#webhook_secret { width: 206px; } diff --git a/zerver/tests/test_integrations_dev_panel.py b/zerver/tests/test_integrations_dev_panel.py index ad46910f00f3a..7188affc10bb2 100644 --- a/zerver/tests/test_integrations_dev_panel.py +++ b/zerver/tests/test_integrations_dev_panel.py @@ -1,3 +1,5 @@ +import hashlib +import hmac from unittest.mock import MagicMock, patch import orjson @@ -339,3 +341,155 @@ def test_send_all_webhook_fixture_messages_for_missing_fixtures( } self.assertEqual(response.status_code, 404) self.assertEqual(orjson.loads(response.content), expected_response) + + def test_recalculate_signature_method_not_allowed(self) -> None: + target_url = "/devtools/integrations/recalculate_signature" + # The endpoint expects a POST request. GET should fail with 405. + response = self.client_get(target_url) + self.assertEqual(response.status_code, 405) + self.assertEqual(orjson.loads(response.content), {"error": "Method not allowed"}) + + def test_recalculate_signature_unsupported_integration(self) -> None: + target_url = "/devtools/integrations/recalculate_signature" + data = { + "secret": "my_secret", + "payload": '{"event": "ping"}', + "integration_name": "unsupported_platform", + } + response = self.client_post(target_url, data, content_type="application/json") + self.assertEqual(response.status_code, 200) + + expected_response = { + "supported": False, + "msg": "No signature rules configured for this platform.", + } + self.assertEqual(orjson.loads(response.content), expected_response) + + def test_recalculate_signature_empty_secret_triggers_clear(self) -> None: + target_url = "/devtools/integrations/recalculate_signature" + data = { + "secret": "", + "payload": '{"event": "ping"}', + "integration_name": "github", + } + response = self.client_post(target_url, data, content_type="application/json") + self.assertEqual(response.status_code, 200) + + expected_response = {"supported": True, "clear_signature": True} + self.assertEqual(orjson.loads(response.content), expected_response) + + def test_recalculate_signature_success_with_json_payload(self) -> None: + target_url = "/devtools/integrations/recalculate_signature" + secret = "github_webhook_secret" + + payload = '{\n "zen": "Non-blocking is better than blocking."\n}' + + data = { + "secret": secret, + "payload": payload, + "integration_name": "github ", # Tests trimming behavior + } + + # Manually compute the expected HMAC hash of minified JSON + minified_payload_bytes = orjson.dumps(orjson.loads(payload)) + expected_hash = hmac.new( + secret.encode(), minified_payload_bytes, hashlib.sha256 + ).hexdigest() + + response = self.client_post(target_url, data, content_type="application/json") + self.assertEqual(response.status_code, 200) + + expected_response = { + "supported": True, + "clear_signature": False, + "header_key": "X_HUB_SIGNATURE_256", + "signature": f"sha256={expected_hash}", + } + self.assertEqual(orjson.loads(response.content), expected_response) + + def test_recalculate_signature_success_with_non_json_payload(self) -> None: + target_url = "/devtools/integrations/recalculate_signature" + secret = "github_webhook_secret" + payload = "plain-text-payload-string" + + data = { + "secret": secret, + "payload": payload, + "integration_name": "GITHUB", + } + + # Falls back to plain text bytes computation upon JSON extraction failure + expected_hash = hmac.new(secret.encode(), payload.encode(), hashlib.sha256).hexdigest() + + response = self.client_post(target_url, data, content_type="application/json") + self.assertEqual(response.status_code, 200) + + expected_response = { + "supported": True, + "clear_signature": False, + "header_key": "X_HUB_SIGNATURE_256", + "signature": f"sha256={expected_hash}", + } + self.assertEqual(orjson.loads(response.content), expected_response) + + def test_recalculate_signature_exception_handling(self) -> None: + target_url = "/devtools/integrations/recalculate_signature" + + # Sending a malformed request context (e.g. string payload instead of valid json object) + # to force the parsing logic down the general exception handling path. + response = self.client_post( + target_url, "invalid_json_body", content_type="application/json" + ) + self.assertEqual(response.status_code, 400) + + response_data = orjson.loads(response.content) + self.assertIn("error", response_data) + + def test_sync_signature_headers_endpoint_success(self) -> None: + """Tests the backend counterpart of sync_signature_headers for a valid integration.""" + target_url = "/devtools/integrations/recalculate_signature" + data = { + "secret": "my_webhook_secret", + "payload": '{"event": "ping"}', + "integration_name": "github", + } + + response = self.client_post(target_url, data, content_type="application/json") + self.assertEqual(response.status_code, 200) + + response_data = orjson.loads(response.content) + self.assertTrue(response_data["supported"]) + self.assertFalse(response_data["clear_signature"]) + self.assertEqual(response_data["header_key"], "X_HUB_SIGNATURE_256") + self.assertTrue(response_data["signature"].startswith("sha256=")) + + def test_sync_signature_headers_endpoint_empty_secret(self) -> None: + """Tests that passing an empty secret returns clear_signature=True to clear the UI instantly.""" + target_url = "/devtools/integrations/recalculate_signature" + data = { + "secret": "", + "payload": '{"event": "ping"}', + "integration_name": "github", + } + + response = self.client_post(target_url, data, content_type="application/json") + self.assertEqual(response.status_code, 200) + + response_data = orjson.loads(response.content) + self.assertTrue(response_data["supported"]) + self.assertTrue(response_data["clear_signature"]) + + def test_sync_signature_headers_endpoint_unsupported(self) -> None: + """Tests that an unregistered integration name returns supported=False to drop headers.""" + target_url = "/devtools/integrations/recalculate_signature" + data = { + "secret": "secret", + "payload": "{}", + "integration_name": "some_random_platform", + } + + response = self.client_post(target_url, data, content_type="application/json") + self.assertEqual(response.status_code, 200) + + response_data = orjson.loads(response.content) + self.assertFalse(response_data["supported"]) diff --git a/zerver/views/development/integrations.py b/zerver/views/development/integrations.py index a3ec1ce71221c..6c524999d13f7 100644 --- a/zerver/views/development/integrations.py +++ b/zerver/views/development/integrations.py @@ -1,12 +1,17 @@ +import hashlib +import hmac import os +from collections.abc import Callable from contextlib import suppress from typing import TYPE_CHECKING, Any import orjson -from django.http import HttpRequest, HttpResponse +from django.http import HttpRequest, HttpResponse, JsonResponse from django.http.response import HttpResponseBase from django.shortcuts import render from django.test import Client +from django.utils.encoding import force_bytes +from django.views.decorators.csrf import csrf_exempt from pydantic import Json from zerver.lib.exceptions import JsonableError, ResourceNotFoundError @@ -156,3 +161,63 @@ def send_all_webhook_fixture_messages( } ) return json_success(request, data={"responses": responses}) + + +def format_github_signature(secret_bytes: bytes, payload_bytes: bytes) -> tuple[str, str]: + """Formats signature header following X-Hub-Signature-256 standard.""" + signed_payload = hmac.new(secret_bytes, payload_bytes, hashlib.sha256).hexdigest() + return "X_HUB_SIGNATURE_256", f"sha256={signed_payload}" + + +SIGNATURE_REGISTRY: dict[str, Callable[[bytes, bytes], tuple[str, str]]] = { + "github": format_github_signature +} + + +@csrf_exempt +def recalculate_signature(request: HttpRequest) -> JsonResponse: + """ + Unified endpoint invoked by the frontend UI dev panel to dynamically compute + and format signature header blocks based on the integration. + """ + if request.method != "POST": + return JsonResponse({"error": "Method not allowed"}, status=405) + + try: + data = orjson.loads(request.body) + secret = data.get("secret", "") + payload_string = data.get("payload", "") + integration_name = data.get("integration_name", "").lower().strip() + + # Check if the integration has signature management registered + if integration_name not in SIGNATURE_REGISTRY: + return JsonResponse( + {"supported": False, "msg": "No signature rules configured for this platform."} + ) + + if not secret: + return JsonResponse({"supported": True, "clear_signature": True}) + + # Normalize and minify JSON formats for crypto verification stability + try: + payload_bytes = orjson.dumps(orjson.loads(payload_string)) + except Exception: + payload_bytes = force_bytes(payload_string) + + webhook_secret_bytes = force_bytes(secret) + + # Execute the registered structural format strategy + formatter = SIGNATURE_REGISTRY[integration_name] + header_key, header_value = formatter(webhook_secret_bytes, payload_bytes) + + return JsonResponse( + { + "supported": True, + "clear_signature": False, + "header_key": header_key, + "signature": header_value, + } + ) + + except Exception: + return JsonResponse({"error": "Invalid request payload."}, status=400) diff --git a/zerver/views/documentation.py b/zerver/views/documentation.py index fe64df3046c28..52ba609167e86 100644 --- a/zerver/views/documentation.py +++ b/zerver/views/documentation.py @@ -427,7 +427,7 @@ def get_categories_for_integration(integration: Integration) -> list[tuple[str, for display_name in integration.categories: slug = display_to_slug.get(display_name) assert slug is not None - result.append((slug, display_name)) + result.append((slug, str(display_name))) return result diff --git a/zproject/dev_urls.py b/zproject/dev_urls.py index c57717f9fe2a1..d637ca8e497c5 100644 --- a/zproject/dev_urls.py +++ b/zproject/dev_urls.py @@ -24,6 +24,7 @@ check_send_webhook_fixture_message, dev_panel, get_fixtures, + recalculate_signature, send_all_webhook_fixture_messages, ) from zerver.views.development.registration import ( @@ -98,6 +99,10 @@ "devtools/integrations/send_all_webhook_fixture_messages", send_all_webhook_fixture_messages ), path("devtools/integrations//fixtures", get_fixtures), + path( + "devtools/integrations/recalculate_signature", + recalculate_signature, + ), path("config-error/", config_error, name="config_error"), # Special endpoint to remove all the server-side caches. path("flush_caches", remove_caches),