8000 Events migration to jinja template by Dishant1804 · Pull Request #1437 · OWASP/Nest · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Dishant1804
Copy link
Collaborator

Resolves #1420

@Dishant1804 Dishant1804 requested a review from arkid15r as a code owner April 29, 2025 07:15
Copy link
Contributor
coderabbitai bot commented Apr 29, 2025

Summary by CodeRabbit

  • Refactor
    • Slack event handlers for app home opened, team join, and channel join events were refactored from function-based to class-based implementations for improved modularity and maintainability.
    • Event handling logic now uses reusable templates for welcome and onboarding messages.
  • New Features
    • Introduced dynamic, templated welcome and onboarding messages for new users and contributors in Slack.
  • Bug Fixes
    • Improved error handling and message delivery reliability in Slack event responses.
  • Tests
    • Updated and enhanced test suites to align with new class-based event handlers and improved message content validation.
  • Documentation
    • Added and updated message templates for Slack events, providing clearer and more structured onboarding communication.

Summary by CodeRabbit

  • New Features

    • Introduced improved, template-driven welcome and onboarding messages for Slack events such as app home opened, team join, and channel join (contribute and GSoC channels).
    • Added support for dynamic, personalized Slack messages using Jinja templates.
  • Refactor

    • Migrated Slack event handlers from function-based to class-based structure for better modularity and maintainability.
    • Centralized event handling logic and error management into a reusable base class for Slack events.
  • Tests

    • Updated and refactored test suites to align with the new class-based event handler architecture, improving test clarity and coverage.

Walkthrough

This pull request refactors several Slack event handlers from function-based to class-based implementations by introducing a new EventBase class for standardized event handling and template rendering. The handlers for events such as app_home_opened, member_joined_channel (for both contribute and gsoc channels), and team_join now use Jinja templates for message content, replacing hardcoded message blocks. New template files are added for each event. Corresponding test files are updated to accommodate the new class-based handlers and template-driven output, ensuring alignment with the updated architecture.

Changes

Files/Paths Change Summary
backend/apps/slack/events/app_home_opened.py,
member_joined_channel/contribute.py,
member_joined_channel/gsoc.py,
team_join.py
Refactored event handlers from function-based to class-based, inheriting from EventBase, with logic moved to handle_event methods and registration via class instantiation. Switched to template-based message rendering.
backend/apps/slack/events/event.py Introduced new EventBase class for standardized Slack event handling, registration, template rendering, error handling, and helper utilities.
backend/apps/slack/templates/events/apphomeopened.jinja,
contribute.jinja,
gsoc.jinja,
teamjoin.jinja
Added new Jinja template files for Slack event messages, replacing hardcoded message blocks with dynamic, context-driven content.
backend/tests/apps/slack/events/app_home_opened_test.py,
team_join_test.py,
user_joined_channel/contribute_test.py,
user_joined_channel/gsoc_test.py
Refactored tests to use class-based handlers, updated mocks and assertions for template-driven outputs, and improved validation of Slack message content.

Assessment against linked issues

Objective Addressed Explanation
Migrate Slack event handlers to use the template system for message rendering (#1420)
Structure templates according to the Slack app sub-modules (#1420)
Refactor event handlers to be compatible with the new template-based architecture (#1420)
Update tests to reflect handler refactoring and template-driven outputs (#1420)

Possibly related PRs

Suggested labels

backend

Suggested reviewers

  • arkid15r

📜 Recent review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ed18cb1 and 1d5418c.

📒 Files selected for processing (2)
  • 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)
🚧 Files skipped from review as they are similar to previous changes (2)
  • backend/tests/apps/slack/events/user_joined_channel/gsoc_test.py
  • backend/tests/apps/slack/events/user_joined_channel/contribute_test.py
⏰ Context from checks skipped due to timeout of 90000ms (5)
  • GitHub Check: Run Code Scan
  • GitHub Check: Run backend tests
  • GitHub Check: Run frontend unit tests
  • GitHub Check: CodeQL (python)
  • GitHub Check: CodeQL (javascript-typescript)
✨ Finishing Touches
  • 📝 Generate Docstrings

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor
@coderabbitai coderabbitai bot left a 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 just str | 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.fixture
backend/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 level

These 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 blocks

The 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 blocks

Similar 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 recomputing get_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 used

Either 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-effects

Instantiating 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

📥 Commits

Reviewing files that changed from the base of the PR and between 60d5b5d and d208e62.

📒 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 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)

🪛 GitHub Actions: Run CI/CD

[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/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 to TestTeamJoinHandler 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 the open_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 new EventBase 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 architecture

The import change reflects the architectural shift from function-based to class-based event handlers.


8-8: Class rename improves test structure

Renaming the test class from TestSlackHandler to TestAppHomeOpened makes the test focus clearer and aligns with the class-based handler approach.


15-15: Method name update improves clarity

Renaming the test method from test_handler_app_home_opened to test_handle_event better aligns with the class-based method being tested.


30-43: Good implementation of the class instantiation and mocking approach

The test correctly instantiates the AppHomeOpened class and mocks its get_render_blocks method, allowing for clean testing of the handler's logic without needing to test the template rendering.


62-66: Thorough context validation

The 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 update

The updated docstring explicitly mentions templates, which helps clarify the implementation approach.


11-11: Good import for the new base class

The 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 matchers

The class is well-defined with appropriate event type and matcher logic to handle only events for the contribute channel.


58-59: Clean registration approach

The 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 approach

The imports have been updated correctly to reflect the architectural changes.


18-18: Good channel ID normalization

Normalizing the channel ID by removing the leading "#" ensures consistency with the expected format in the new matcher logic.


46-46: New mock for URL generation

Adding the get_absolute_url mock supports the URL generation used in the templated message blocks.


63-64: Clean class instantiation and handler invocation

The test correctly instantiates the Contribute class and calls its handler method, aligning with the new class-based approach.


71-76: Thorough ephemeral message verification

The test now includes detailed assertions for the ephemeral message, which improves test coverage.


80-90: Improved message content verification

The 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 test

The 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 update

The updated docstring explicitly mentions templates, which helps clarify the implementation approach.


26-26: Good import for the new base class

The import of EventBase aligns with the architectural refactoring to use a common base class for event handlers.


32-35: Well-structured class definition

The class is well-defined with appropriate event type and docstrings.


41-45: Good conversation handling with error logging

The implementation includes proper error handling for failed conversations, including warning logs.


47-65: Comprehensive context dictionary

The context dictionary includes all necessary channel IDs and messages for the template, which is well-organized.


73-81: Comprehensive error handling

The implementation includes thorough error handling for Slack API errors and general exceptions, with appropriate logging and re-raising when needed.


84-85: Clean registration approach

The 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" with OWASP_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 3

Length of output: 9573


Review suggestion not needed – matcher is correct
The OWASP_GSOC_CHANNEL_ID constant is defined with a leading “#” ("#CFJLZNFN1"), and events supply the bare ID (e.g. "CFJLZNFN1"). Prefixing "#" on event["channel"] correctly produces "#CFJLZNFN1" to match the constant. Existing tests (which create events via OWASP_GSOC_CHANNEL_ID.replace("#", "")) confirm this behavior. No change required.

Likely an incorrect or invalid review comment.

Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Migrate NestBot events to use templates
1 participant
0