Improve slack guidelines. #1507
Conversation
ahmedxgouda
commented
May 21, 2025
ahmedxgouda
commented
- Improved the docs with some steps.
- Added Django prefix to workspace token environment variable.
Summary by CodeRabbit
Summary by CodeRabbit
WalkthroughThe changes update the Slack bot token environment variable naming convention in the backend code and reorganize the contributing guide for setting up NestBot development. The documentation now provides clearer, step-by-step instructions for Slack workspace and app setup, including updated environment variable formats and a new Slack data sync step. Changes
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ 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:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
|
github-actions
bot
added
docs
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
CONTRIBUTING.md (2)
344-347: Use a consistent callout style for warnings
The “❗” bullets stand out, but consider using Markdown admonitions (e.g.,> **Warning:** …) to improve consistency and accessibility.
377-386: Clarify workspace ID casing
When pulling the ID from the Slack URL, note that it must be uppercase in the env var name. Recommend adding a line like:Use the uppercase workspace ID when setting
DJANGO_SLACK_BOT_TOKEN_<WORKSPACE_ID>.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
CONTRIBUTING.md(2 hunks)backend/apps/slack/models/workspace.py(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Run frontend unit tests
- GitHub Check: Run backend tests
- GitHub Check: Run frontend e2e tests
- GitHub Check: CodeQL (python)
- GitHub Check: CodeQL (javascript-typescript)
🔇 Additional comments (1)
CONTRIBUTING.md (1)
402-405: Approve Slack data sync step
The new “Sync the slack data” step andmake slack-sync-datacommand align well with the code updates and enhance the onboarding flow.
| @@ -29,4 +29,4 @@ def bot_token(self) -> str: | |||
| str: The bot token for the workspace. | |||
|
|
|||
| """ | |||
| return os.getenv(f"SLACK_BOT_TOKEN_{self.slack_workspace_id.upper()}", "") | |||
| return os.getenv(f"DJANGO_SLACK_BOT_TOKEN_{self.slack_workspace_id.upper()}", "") | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ensure fallback for global bot token
The documentation instructs users to also set DJANGO_SLACK_BOT_TOKEN without a workspace suffix, but this property only reads the suffixed variable. You should either update the docs to drop the global var or add a fallback in code.
Apply this diff to fall back to the generic token when the workspace‐specific one isn’t set:
@@ class Workspace(TimestampedModel):
@property
def bot_token(self) -> str:
"""Get bot token for the workspace."""
- return os.getenv(f"DJANGO_SLACK_BOT_TOKEN_{self.slack_workspace_id.upper()}", "")
+ return (
+ os.getenv(f"DJANGO_SLACK_BOT_TOKEN_{self.slack_workspace_id.upper()}")
+ or os.getenv("DJANGO_SLACK_BOT_TOKEN", "")
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return os.getenv(f"DJANGO_SLACK_BOT_TOKEN_{self.slack_workspace_id.upper()}", "") | |
| @property | |
| def bot_token(self) -> str: | |
| """Get bot token for the workspace.""" | |
| return ( | |
| os.getenv(f"DJANGO_SLACK_BOT_TOKEN_{self.slack_workspace_id.upper()}") | |
| or os.getenv("DJANGO_SLACK_BOT_TOKEN", "") | |
| ) |
🤖 Prompt for AI Agents
In backend/apps/slack/models/workspace.py at line 32, the code only retrieves
the workspace-specific Slack bot token environment variable and does not fall
back to the global DJANGO_SLACK_BOT_TOKEN if the specific one is not set. Modify
the return statement to first attempt fetching the workspace-specific token, and
if it is empty, then return the global DJANGO_SLACK_BOT_TOKEN environment
variable as a fallback.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
backend/tests/slack/models/workspace_test.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.11.9)
backend/tests/slack/models/workspace_test.py
11-11: Line too long (104 > 99)
(E501)
🪛 GitHub Actions: Run CI/CD
backend/tests/slack/models/workspace_test.py
[error] 11-11: Ruff E501: Line too long (104 > 99) at line 11.
[error] 11-13: Ruff format hook modified the file to fix formatting issues related to line length and code style.
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: CodeQL (python)
- GitHub Check: CodeQL (javascript-typescript)
|
