-
-
Notifications
You must be signed in to change notification settings - Fork 94
Events migration to jinja template #1437
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Summary by CodeRabbit
Summary by CodeRabbit
WalkthroughThis pull request refactors several Slack event handlers from function-based to class-based implementations by introducing a new Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (5)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (20)
backend/apps/slack/templates/events/teamjoin.jinja (1)
1-44
: Fix trailing whitespace and missing newline at end of file.The pre-commit hooks have identified trailing whitespace issues and a missing end-of-file newline, which should be addressed.
We're here to support your journey in making software security visible and strengthening the security of the software we all depend on. Have questions or need help? Don't hesitate to ask -- this community thrives on collaboration! {{ SECTION_BREAK }} -{{ FEEDBACK_CHANNEL_MESSAGE }} +{{ FEEDBACK_CHANNEL_MESSAGE }} +🧰 Tools
🪛 GitHub Actions: Run CI/CD
[error] Pre-commit hook 'end-of-file-fixer' failed and fixed missing end-of-file newlines.
[error] Pre-commit hook 'trailing-whitespace' failed and fixed trailing whitespace issues.
backend/apps/slack/events/event.py (5)
3-5
: Add future annotations import to simplify typing.Adding this import would allow simplifying
Optional[str]
to juststr | None
when using Python 3.10+.import logging +from __future__ import annotations from typing import Optional
73-75
: Fix docstring style and exception handling.The docstring should use imperative mood, and string literals shouldn't be used directly in exceptions.
def handle_event(self, event, client): - """Main event handling logic to be implemented by subclasses.""" - raise NotImplementedError("Subclasses must implement handle_event") + """Implement main event handling logic in subclasses.""" + error_msg = "Subclasses must implement handle_event" + raise NotImplementedError(error_msg)🧰 Tools
🪛 Ruff (0.8.2)
74-74: First line of docstring should be in imperative mood: "Main event handling logic to be implemented by subclasses."
(D401)
75-75: Exception must not use a string literal, assign to variable first
Assign to variable; remove string literal
(EM101)
77-79
: Remove blank line after docstring.Remove the blank line that appears after the function docstring.
def handle_error(self, event, client, error): """Handle errors during event processing.""" - try:
🧰 Tools
🪛 Ruff (0.8.2)
78-78: No blank lines allowed after function docstring (found 1)
Remove blank line(s) after function docstring
(D202)
🪛 GitHub Actions: Run CI/CD
[error] 77-77: Ruff EM101: Exception must not use a string literal, assign to variable first.
92-94
: Fix docstring style for open_conversation method.The docstring should use imperative mood.
def open_conversation(self, client, user_id): - """Helper to open DM conversation.""" + """Open DM conversation with a user.""" try:🧰 Tools
🪛 Ruff (0.8.2)
93-93: First line of docstring should be in imperative mood: "Helper to open DM conversation."
(D401)
🪛 GitHub Actions: Run CI/CD
[error] 94-94: Ruff D401: First line of docstring should be in imperative mood: "Helper to open DM conversation."
134-135
: Add newline at end of file.Add a trailing newline at the end of the file to fix the W292 linting error.
def get_template_file_name(self): """Get template file name (if using templates).""" - return f"events/{self.__class__.__name__.lower()}.jinja" + return f"events/{self.__class__.__name__.lower()}.jinja" +🧰 Tools
🪛 Ruff (0.8.2)
135-135: No newline at end of file
Add trailing newline
(W292)
backend/apps/slack/templates/events/apphomeopened.jinja (1)
1-22
: Fix trailing whitespace and missing newline at end of file.The pre-commit hooks have identified trailing whitespace issues and a missing end-of-file newline, which should be addressed.
{{ TAB }}• /sponsors{{ NL }} {{ TAB }}• /staff{{ NL }} -{{ TAB }}• /users{{ NL }} +{{ TAB }}• /users{{ NL }} +🧰 Tools
🪛 GitHub Actions: Run CI/CD
[error] Pre-commit hook 'end-of-file-fixer' failed and fixed missing end-of-file newlines.
[error] Pre-commit hook 'trailing-whitespace' failed and fixed trailing whitespace issues.
backend/apps/slack/templates/events/gsoc.jinja (2)
1-16
: Fix trailing whitespace and missing newline at end of file.The pre-commit hooks have identified trailing whitespace issues and a missing end-of-file newline, which should be addressed.
{{ NL }} -{{ FEEDBACK_CHANNEL_MESSAGE }} +{{ FEEDBACK_CHANNEL_MESSAGE }} +🧰 Tools
🪛 GitHub Actions: Run CI/CD
[error] Pre-commit hook 'end-of-file-fixer' failed and fixed missing end-of-file newlines.
[error] Pre-commit hook 'trailing-whitespace' failed and fixed trailing whitespace issues.
6-6
: Consider breaking long line for better readability.This line is quite long. Consider breaking it into multiple lines for better maintainability.
-• Join the <#{{ channel_id }}|{{ channel_name }}> and #contribute channels if you haven't done it yet for suggestions and tips on how to get started. +• Join the <#{{ channel_id }}|{{ channel_name }}> and #contribute channels + if you haven't done it yet for suggestions and tips on how to get started.🧰 Tools
🪛 GitHub Actions: Run CI/CD
[error] Pre-commit hook 'end-of-file-fixer' failed and fixed missing end-of-file newlines.
[error] Pre-commit hook 'trailing-whitespace' failed and fixed trailing whitespace issues.
backend/apps/slack/templates/events/contribute.jinja (2)
2-2
: Fix trailing whitespace.There's trailing whitespace on this line.
-We're happy to have you here as part of the OWASP community! +We're happy to have you here as part of the OWASP community!🧰 Tools
🪛 GitHub Actions: Run CI/CD
[error] Pre-commit hook 'end-of-file-fixer' failed and fixed missing end-of-file newlines.
[error] Pre-commit hook 'trailing-whitespace' failed and fixed trailing whitespace issues.
1-25
: Consider adding a template header with variable documentation.For improved maintainability, consider adding a comment block at the top of the template explaining the purpose and available variables.
+{# + Welcome template for new contributors joining the channel + Variables: + - user_id: Slack user ID + - contribute_channel_id: Contribute channel ID + - active_projects_count: Number of active OWASP projects + - open_issues_count: Number of open issues + - nest_bot_name: Name of the Nest bot + - contribute_url: URL to the OWASP contribution page + - NL: Newline character + - SECTION_BREAK: Section break formatting + - FEEDBACK_CHANNEL_MESSAGE: Standard feedback channel message +#} + Hello <@{{ user_id }}> and welcome to <{{ contribute_channel_id }}> channel!{{ NL }}🧰 Tools
🪛 GitHub Actions: Run CI/CD
[error] Pre-commit hook 'end-of-file-fixer' failed and fixed missing end-of-file newlines.
[error] Pre-commit hook 'trailing-whitespace' failed and fixed trailing whitespace issues.
backend/tests/apps/slack/events/team_join_test.py (1)
8-44
: Consider adding a test for event_type attribute.To ensure complete coverage of the new class-based handler, consider adding a test that verifies the
event_type
attribute is correctly set.class TestTeamJoinHandler: + def test_event_type(self): + handler = TeamJoin() + assert handler.event_type == "team_join" + @pytest.fixturebackend/tests/apps/slack/events/user_joined_channel/gsoc_test.py (1)
12-88
: Consider adding dedicated test for handle_event method.While the handler method is being tested, consider adding a dedicated test for the
handle_event
method separate from the handler to ensure thorough coverage.def test_matcher(self, channel_id, expected_result): gsoc = Gsoc() assert gsoc.matchers[0]({"channel": channel_id}) == expected_result + def test_handle_event(self): + mock_client = MagicMock() + mock_event = {"user": "U123456", "channel": OWASP_GSOC_CHANNEL_ID.replace("#", "")} + + gsoc = Gsoc() + gsoc.handle_event(event=mock_event, client=mock_client) + + # Verify that the ephemeral message is sent + mock_client.chat_postEphemeral.assert_called_once() + # Verify that a conversation is opened + mock_client.conversations_open.assert_called_once() + # Verify that a message is posted + mock_client.chat_postMessage.assert_called_once()backend/apps/slack/events/app_home_opened.py (1)
17-18
: Add clarification for TAB variable.The purpose of the TAB variable in the context might not be immediately obvious to future developers.
context = { "user_id": user_id, - "TAB": " ", # 4 spaces for indentation + "TAB": " ", # 4 spaces for indentation in code blocks or pre-formatted text }backend/apps/slack/events/member_joined_channel/contribute.py (2)
27-29
: Consider moving imports to module levelThese imports are inside the method, which is an uncommon pattern and could affect performance if the method is called frequently.
from apps.slack.utils import get_text +from apps.github.models.issue import Issue +from apps.owasp.models.project import Project class Contribute(EventBase): """Slack contribute channel join event handler."""
51-55
: Optimize by reusing rendered blocksThe
get_render_blocks(context)
method is called twice, which might be inefficient if the template rendering is expensive.+ blocks = self.get_render_blocks(context) client.chat_postMessage( - blocks=self.get_render_blocks(context), + blocks=blocks, channel=conv["channel"]["id"], - text=get_text(self.get_render_blocks(context)), + text=get_text(blocks), )backend/apps/slack/events/team_join.py (1)
67-71
: Consider reusing rendered blocksSimilar to the contribute.py file,
get_render_blocks(context)
is called twice, which might be inefficient.+ blocks = self.get_render_blocks(context) client.chat_postMessage( - blocks=self.get_render_blocks(context), + blocks=blocks, channel=conv["channel"]["id"], - text=get_text(self.get_render_blocks(context)), + text=get_text(blocks), )backend/apps/slack/events/member_joined_channel/gsoc.py (3)
44-48
: Avoid recomputingget_render_blocks
get_render_blocks()
is pure; calling it twice wastes cycles and may diverge if it becomes non-deterministic later.- client.chat_postMessage( - blocks=self.get_render_blocks(context), - channel=conv["channel"]["id"], - text=get_text(self.get_render_blocks(context)), - ) + blocks = self.get_render_blocks(context) + client.chat_postMessage( + blocks=blocks, + channel=conv["channel"]["id"], + text=get_text(blocks), + )
3-11
: Logger imported but never usedEither log useful breadcrumbs (e.g. match results, API failures) or remove the unused import to keep the module tidy.
51-52
: Module-level registration causes import-time side-effectsInstantiating and registering the handler on import can surprise test runners and makes DI harder. Prefer:
def register_handlers(): if SlackConfig.app: Gsoc().register()and call
register_handlers()
from the app’s startup hook (SlackConfig.ready()
or similar).
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
backend/apps/slack/events/app_home_opened.py
(1 hunks)backend/apps/slack/events/event.py
(1 hunks)backend/apps/slack/events/member_joined_channel/contribute.py
(1 hunks)backend/apps/slack/events/member_joined_channel/gsoc.py
(1 hunks)backend/apps/slack/events/team_join.py
(2 hunks)backend/apps/slack/templates/events/apphomeopened.jinja
(1 hunks)backend/apps/slack/templates/events/contribute.jinja
(1 hunks)backend/apps/slack/templates/events/gsoc.jinja
(1 hunks)backend/apps/slack/templates/events/teamjoin.jinja
(1 hunks)backend/tests/apps/slack/events/app_home_opened_test.py
(3 hunks)backend/tests/apps/slack/events/team_join_test.py
(1 hunks)backend/tests/apps/slack/events/user_joined_channel/contribute_test.py
(3 hunks)backend/tests/apps/slack/events/user_joined_channel/gsoc_test.py
(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
backend/apps/slack/events/event.py (6)
backend/apps/slack/blocks.py (1)
markdown
(14-27)backend/apps/slack/apps.py (1)
SlackConfig
(10-21)backend/apps/slack/events/app_home_opened.py (1)
handle_event
(13-23)backend/apps/slack/events/member_joined_channel/gsoc.py (1)
handle_event
(24-48)backend/apps/slack/events/team_join.py (1)
handle_event
(37-81)backend/apps/slack/events/member_joined_channel/contribute.py (1)
handle_event
(25-55)
🪛 GitHub Actions: Run CI/CD
backend/apps/slack/templates/events/apphomeopened.jinja
[error] Pre-commit hook 'end-of-file-fixer' failed and fixed missing end-of-file newlines.
[error] Pre-commit hook 'trailing-whitespace' failed and fixed trailing whitespace issues.
backend/apps/slack/templates/events/gsoc.jinja
[error] Pre-commit hook 'end-of-file-fixer' failed and fixed missing end-of-file newlines.
[error] Pre-commit hook 'trailing-whitespace' failed and fixed trailing whitespace issues.
backend/apps/slack/templates/events/contribute.jinja
[error] Pre-commit hook 'end-of-file-fixer' failed and fixed missing end-of-file newlines.
[error] Pre-commit hook 'trailing-whitespace' failed and fixed trailing whitespace issues.
backend/apps/slack/events/event.py
[error] 24-24: Ruff FA100: Add from __future__ import annotations
to simplify typing.Optional
.
[error] 76-76: Ruff D401: First line of docstring should be in imperative mood: "Main event handling logic to be implemented by subclasses."
[error] 77-77: Ruff EM101: Exception must not use a string literal, assign to variable first.
[error] 94-94: Ruff D401: First line of docstring should be in imperative mood: "Helper to open DM conversation."
backend/apps/slack/templates/events/teamjoin.jinja
[error] Pre-commit hook 'end-of-file-fixer' failed and fixed missing end-of-file newlines.
[error] Pre-commit hook 'trailing-whitespace' failed and fixed trailing whitespace issues.
🪛 Ruff (0.8.2)
backend/apps/slack/events/event.py
23-23: Add from __future__ import annotations
to simplify typing.Optional
(FA100)
51-51: Missing blank line after last section ("Args")
Add blank line after "Args"
(D413)
74-74: First line of docstring should be in imperative mood: "Main event handling logic to be implemented by subclasses."
(D401)
75-75: Exception must not use a string literal, assign to variable first
Assign to variable; remove string literal
(EM101)
78-78: No blank lines allowed after function docstring (found 1)
Remove blank line(s) after function docstring
(D202)
93-93: First line of docstring should be in imperative mood: "Helper to open DM conversation."
(D401)
135-135: No newline at end of file
Add trailing newline
(W292)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: CodeQL (python)
- GitHub Check: CodeQL (javascript-typescript)
🔇 Additional comments (48)
backend/apps/slack/templates/events/teamjoin.jinja (1)
1-44
: Great welcome template for new team members!The template is well-structured with clear sections introducing new users to OWASP resources and channels. The use of Slack markdown and proper formatting will make this message engaging and user-friendly.
🧰 Tools
🪛 GitHub Actions: Run CI/CD
[error] Pre-commit hook 'end-of-file-fixer' failed and fixed missing end-of-file newlines.
[error] Pre-commit hook 'trailing-whitespace' failed and fixed trailing whitespace issues.
backend/apps/slack/events/event.py (1)
1-135
: Excellent base class design for event handling!The
EventBase
class provides a robust foundation for Slack event handling with good error management, template rendering, and event registration capabilities.🧰 Tools
🪛 Ruff (0.8.2)
23-23: Add
from __future__ import annotations
to simplifytyping.Optional
(FA100)
51-51: Missing blank line after last section ("Args")
Add blank line after "Args"
(D413)
74-74: First line of docstring should be in imperative mood: "Main event handling logic to be implemented by subclasses."
(D401)
75-75: Exception must not use a string literal, assign to variable first
Assign to variable; remove string literal
(EM101)
78-78: No blank lines allowed after function docstring (found 1)
Remove blank line(s) after function docstring
(D202)
93-93: First line of docstring should be in imperative mood: "Helper to open DM conversation."
(D401)
135-135: No newline at end of file
Add trailing newline
(W292)
🪛 GitHub Actions: Run CI/CD
[error] 24-24: Ruff FA100: Add
from __future__ import annotations
to simplifytyping.Optional
.
[error] 76-76: Ruff D401: First line of docstring should be in imperative mood: "Main event handling logic to be implemented by subclasses."
[error] 77-77: Ruff EM101: Exception must not use a string literal, assign to variable first.
[error] 94-94: Ruff D401: First line of docstring should be in imperative mood: "Helper to open DM conversation."
backend/apps/slack/templates/events/apphomeopened.jinja (1)
1-22
: Well-structured welcome message with command list!The template provides a clear introduction to the OWASP Slack Community and lists available slash commands in an organized, easy-to-read format.
🧰 Tools
🪛 GitHub Actions: Run CI/CD
[error] Pre-commit hook 'end-of-file-fixer' failed and fixed missing end-of-file newlines.
[error] Pre-commit hook 'trailing-whitespace' failed and fixed trailing whitespace issues.
backend/apps/slack/templates/events/gsoc.jinja (1)
1-16
: Well-designed GSoC welcome template!The template provides a friendly welcome and clear guidance for new participants in the GSoC channel, with actionable steps to get started.
🧰 Tools
🪛 GitHub Actions: Run CI/CD
[error] Pre-commit hook 'end-of-file-fixer' failed and fixed missing end-of-file newlines.
[error] Pre-commit hook 'trailing-whitespace' failed and fixed trailing whitespace issues.
backend/tests/apps/slack/events/team_join_test.py (5)
5-5
: Import update for new class-based handler.Correctly updated the import to use the new
TeamJoin
class instead of the previous function-based handler.
8-8
: Class name updated to match new handler pattern.Good rename from
TestSlackHandler
toTestTeamJoinHandler
which better represents the specific handler being tested.
15-23
: Test method updated to use class-based handler.The test properly instantiates the new
TeamJoin
class and tests its behavior by mocking theopen_conversation
method.
24-25
: Good assertion of handler method calls.Correctly asserts that the handler opens a conversation with the right client and user ID.
30-30
: Improved test coverage for text content.Added assertion for the "text" field in addition to the blocks content.
backend/tests/apps/slack/events/user_joined_channel/gsoc_test.py (8)
6-9
: Updated imports for new class-based implementation.Good update to the imports, now including the
Gsoc
class, constants, and utility functions needed for the tests.
15-15
: Channel ID normalization for matcher compatibility.Correctly updated the mock event to use a normalized channel ID (removing the "#" prefix) which aligns with the new matcher function expectations.
29-33
: Simplified message assertions for templated content.Wisely simplified the expected message assertions to use substrings rather than exact messages, making the tests more resilient to template changes.
47-48
: Updated test to use class-based handler.Now correctly instantiates the
Gsoc
class and calls its handler method, aligned with the new implementation.
55-60
: Added ephemeral message assertions.Good addition of assertions for the ephemeral messages sent with the GSOC milestones information.
66-76
: Improved block text extraction for assertions.Well-implemented logic to extract and combine text from section blocks, allowing for more flexible assertions against the template-generated content.
81-81
: Updated channel ID format in test parameters.Properly updated test parameters to use normalized channel ID (without "#" prefix) consistent with the new implementation.
85-87
: Updated matcher test for class-based implementation.Now correctly tests the first matcher function of the
Gsoc
instance rather than a standalone function.backend/apps/slack/events/app_home_opened.py (7)
1-1
: Improved docstring with template reference.Updated module docstring to mention the use of templates which is more accurate for the refactored implementation.
4-5
: Updated imports for new class-based implementation.Correctly added imports for
get_header
and the newEventBase
class which is essential for the refactored implementation.
8-11
: Well-structured class definition with clear docstring.Good class declaration with appropriate docstring and correct inheritance from
EventBase
.
13-19
: Well-implemented handle_event method with context preparation.The method properly extracts the user ID from the event and prepares a context dictionary for template rendering.
21-21
: Elegant view construction with reusable components.The home view is well-constructed by combining header blocks with template-rendered blocks using the unpacking operator.
23-23
: Simplified client interaction without duplicated error handling.Leveraging error handling from the base class allows for cleaner client interaction without redundant try-except blocks.
27-27
: Updated registration approach using class instance.Correctly updated the registration to instantiate the class and call its register method, aligning with the new architecture.
backend/tests/apps/slack/events/app_home_opened_test.py (5)
5-5
: Clean import update that aligns with the new architectureThe import change reflects the architectural shift from function-based to class-based event handlers.
8-8
: Class rename improves test structureRenaming the test class from
TestSlackHandler
toTestAppHomeOpened
makes the test focus clearer and aligns with the class-based handler approach.
15-15
: Method name update improves clarityRenaming the test method from
test_handler_app_home_opened
totest_handle_event
better aligns with the class-based method being tested.
30-43
: Good implementation of the class instantiation and mocking approachThe test correctly instantiates the
AppHomeOpened
class and mocks itsget_render_blocks
method, allowing for clean testing of the handler's logic without needing to test the template rendering.
62-66
: Thorough context validationThe test verifies that
get_render_blocks
is called with the correct context, which is a good practice for ensuring the template will receive the right data.backend/apps/slack/events/member_joined_channel/contribute.py (4)
1-1
: Clear docstring updateThe updated docstring explicitly mentions templates, which helps clarify the implementation approach.
11-11
: Good import for the new base classThe import of
EventBase
aligns with the architectural refactoring to use a common base class for event handlers.
15-23
: Well-structured class definition with clear matchersThe class is well-defined with appropriate event type and matcher logic to handle only events for the contribute channel.
58-59
: Clean registration approachThe registration approach using class instantiation and the
register()
method is consistent with the new architecture and improves maintainability.backend/tests/apps/slack/events/user_joined_channel/contribute_test.py (7)
11-12
: Appropriate imports for the new class-based approachThe imports have been updated correctly to reflect the architectural changes.
18-18
: Good channel ID normalizationNormalizing the channel ID by removing the leading "#" ensures consistency with the expected format in the new matcher logic.
46-46
: New mock for URL generationAdding the
get_absolute_url
mock supports the URL generation used in the templated message blocks.
63-64
: Clean class instantiation and handler invocationThe test correctly instantiates the
Contribute
class and calls its handler method, aligning with the new class-based approach.
71-76
: Thorough ephemeral message verificationThe test now includes detailed assertions for the ephemeral message, which improves test coverage.
80-90
: Improved message content verificationThe new approach of extracting and concatenating text from all "section" blocks before asserting expected substrings is more robust than simple string checks.
103-104
: Simplified matcher testThe test for channel matching now directly uses the class's matcher function, which simplifies the test and makes it more maintainable.
backend/apps/slack/events/team_join.py (7)
1-1
: Clear docstring updateThe updated docstring explicitly mentions templates, which helps clarify the implementation approach.
26-26
: Good import for the new base classThe import of
EventBase
aligns with the architectural refactoring to use a common base class for event handlers.
32-35
: Well-structured class definitionThe class is well-defined with appropriate event type and docstrings.
41-45
: Good conversation handling with error loggingThe implementation includes proper error handling for failed conversations, including warning logs.
47-65
: Comprehensive context dictionaryThe context dictionary includes all necessary channel IDs and messages for the template, which is well-organized.
73-81
: Comprehensive error handlingThe implementation includes thorough error handling for Slack API errors and general exceptions, with appropriate logging and re-raising when needed.
84-85
: Clean registration approachThe registration approach using class instantiation and the
register()
method is consistent with the new architecture and improves maintainability.backend/apps/slack/events/member_joined_channel/gsoc.py (1)
21-23< 8000 /code>:
❌ Incorrect review comment
Channel matcher likely never matches –
#
prefix mismatches Slack’s channel ID
member_joined_channel
events provide the raw channel ID (e.g."C05ABCDE"
).
By prepending"#"
you compare"#C05ABCDE"
withOWASP_GSOC_CHANNEL_ID
, which in most code bases is the bare ID. This will silently prevent the handler from ever executing.- self.matchers = [lambda event: f"#{event['channel']}" == OWASP_GSOC_CHANNEL_ID] + # Compare the raw ID; strip any leading “#” in the constant for safety. + self.matchers = [ + lambda event: event["channel"] == OWASP_GSOC_CHANNEL_ID.lstrip("#") + ]
I’ll locate the definition of
OWASP_GSOC_CHANNEL_ID
to confirm whether it includes a leading “#”.
🏁 Script executed:
#!/bin/bash # Search for the constant and show surrounding context rg -n "OWASP_GSOC_CHANNEL_ID" -C 3Length of output: 9573
Review suggestion not needed – matcher is correct
TheOWASP_GSOC_CHANNEL_ID
constant is defined with a leading “#” ("#CFJLZNFN1"
), and events supply the bare ID (e.g."CFJLZNFN1"
). Prefixing"#"
onevent["channel"]
correctly produces"#CFJLZNFN1"
to match the constant. Existing tests (which create events viaOWASP_GSOC_CHANNEL_ID.replace("#", "")
) confirm this behavior. No change required.Likely an incorrect or invalid review comment.
|
Resolves #1420