diff --git a/templates/zerver/api/incoming-webhooks-walkthrough.md b/templates/zerver/api/incoming-webhooks-walkthrough.md index 477e38cf1c..f7eedac85b 100644 --- a/templates/zerver/api/incoming-webhooks-walkthrough.md +++ b/templates/zerver/api/incoming-webhooks-walkthrough.md @@ -52,16 +52,18 @@ put key details like the event type in a separate HTTP header test Zulip's handling of that integration, you will need to record which HTTP headers are used with each fixture you capture. -Since this integration-dependent, Zulip offers a simple API for doing -this, which is probably best explained by looking at the example for -GitHub: `zerver/webhooks/github/headers.py`; basically, as part of -writing your integration, you'll write a special function at that path -that maps the filename of the fixture to the set of HTTP headers to -use. Most such functions will use the same strategy as the GitHub -integration: encoding the third party variable header data (usually -just an event type) in the fixture filename, in such a case, you -won't need to explicitly write such a special function and can instead -just use the same helper method that the GitHub integration uses. +Since this is integration-dependent, Zulip offers a simple API for +doing this, which is probably best explained by looking at the example +for GitHub: `zerver/webhooks/github/view.py`; basically, as part of +writing your integration, you'll write a special function in your +view.py file that maps the filename of the fixture to the set of HTTP +headers to use. This function must be named "fixture_to_headers". Most +integrations will use the same strategy as the GitHub integration: +encoding the third party variable header data (usually just an event +type) in the fixture filename, in such a case, you won't need to +explicitly write the logic for such a special function again, +instead you can just use the same helper method that the GitHub +integration uses. ## Step 1: Initialize your webhook python package diff --git a/zerver/lib/webhooks/common.py b/zerver/lib/webhooks/common.py index 15d58559f5..17aeff8a68 100644 --- a/zerver/lib/webhooks/common.py +++ b/zerver/lib/webhooks/common.py @@ -146,26 +146,16 @@ def get_fixture_http_headers(integration_name: str, function from the target integration module to determine what set of HTTP headers goes with the given test fixture. """ - - headers_module_name = "zerver.webhooks.{integration_name}.headers".format( - integration_name=integration_name) + view_module_name = "zerver.webhooks.{integration_name}.view".format( + integration_name=integration_name + ) try: - # Try to import the headers.py file. If it does not exist, - # then that means that the integration does not have any - # special http headers registered. - # # TODO: We may want to migrate to a more explicit registration # strategy for this behavior rather than a try/except import. - headers_module = importlib.import_module(headers_module_name) - except ImportError: + view_module = importlib.import_module(view_module_name) + fixture_to_headers = view_module.fixture_to_headers # type: ignore # we do extra exception handling in case it does not exist below. + except (ImportError, AttributeError): return {} - try: - fixture_to_headers = headers_module.fixture_to_headers # type: ignore # we do extra exception handling in case it does not exist below. - except AttributeError as e: - msg = ". The {module_name} module must contain a fixture_to_headers method.".format( - module_name=headers_module_name - ) - raise AttributeError(str(e) + msg) return fixture_to_headers(fixture_name) diff --git a/zerver/tests/test_webhooks_common.py b/zerver/tests/test_webhooks_common.py index d73b133233..188d83419a 100644 --- a/zerver/tests/test_webhooks_common.py +++ b/zerver/tests/test_webhooks_common.py @@ -109,12 +109,18 @@ class WebhooksCommonTestCase(ZulipTestCase): self.assertEqual(headers, {}) @patch("zerver.lib.webhooks.common.importlib.import_module") - def test_get_fixture_http_headers_for_error_handling(self, import_module_mock: MagicMock) -> None: + def test_get_fixture_http_headers_with_no_fixtures_to_headers_function( + self, + import_module_mock: MagicMock + ) -> None: + fake_module = SimpleNamespace() import_module_mock.return_value = fake_module - with self.assertRaises(AttributeError): - get_fixture_http_headers("some_integration", "simple_fixture") + self.assertEqual( + get_fixture_http_headers("some_integration", "simple_fixture"), + {} + ) def test_standardize_headers(self) -> None: self.assertEqual(standardize_headers({}), {}) diff --git a/zerver/webhooks/github/headers.py b/zerver/webhooks/github/headers.py deleted file mode 100644 index 01bd560e69..0000000000 --- a/zerver/webhooks/github/headers.py +++ /dev/null @@ -1,4 +0,0 @@ -from zerver.lib.webhooks.common import get_http_headers_from_filename - -key = "HTTP_X_GITHUB_EVENT" -fixture_to_headers = get_http_headers_from_filename(key) diff --git a/zerver/webhooks/github/view.py b/zerver/webhooks/github/view.py index 1476573c30..6f287fb352 100644 --- a/zerver/webhooks/github/view.py +++ b/zerver/webhooks/github/view.py @@ -16,6 +16,10 @@ from zerver.lib.webhooks.git import CONTENT_MESSAGE_TEMPLATE, \ get_pull_request_event_message, get_push_commits_event_message, \ get_push_tag_event_message, get_setup_webhook_message from zerver.models import UserProfile +from zerver.lib.webhooks.common import \ + get_http_headers_from_filename + +fixture_to_headers = get_http_headers_from_filename("HTTP_X_GITHUB_EVENT") class UnknownEventType(Exception): pass