[ENG-895] feat: update Slack App documentation - #59
[ENG-895] feat: update Slack App documentation#59miguelangaranocurrents wants to merge 13 commits into
Conversation
Co-authored-by: miguelangaranocurrents <miguelangaranocurrents@users.noreply.github.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughSlack documentation now covers Slack-based Fix with AI entry points, expanded Slack App setup and notification configuration, lifecycle administration, data access, and migration from legacy webhooks. ChangesSlack documentation
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@resources/integrations/slack/slack-app.md`:
- Around line 2-31: Update the nearby Slack Enterprise Grid administrator
warning in the Slack App documentation to say the app may be installed by an Org
Owner or Org Admin, matching the permissions table and preserving the rest of
the warning.
In `@resources/integrations/slack/slack-webhook.md`:
- Around line 81-89: Update the “Migrate to the Slack App” instructions,
specifically step 3, to explicitly map the legacy “Events (Optional)”
settings—including Run Start, Run Finish, Run Timeout, and Run Canceled—to each
destination, preserving the existing failed-run, lifecycle-event, branch, and
tag guidance.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9328ae7f-1a72-430c-8944-e5410e71b70c
📒 Files selected for processing (6)
ai/overview.mdguides/currents-actions/lifecycle-notifications.mdresources/data-privacy/access-to-customer-data.mdresources/integrations/slack/README.mdresources/integrations/slack/slack-app.mdresources/integrations/slack/slack-webhook.md
Co-authored-by: miguelangaranocurrents <miguelangaranocurrents@users.noreply.github.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
resources/integrations/slack/slack-app.md (1)
46-49: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winScope
account_inactiverecovery by deployment type.The checklist covers both single-workspace and Enterprise Grid installations, but Line 49 requires an Org Owner/Admin for every recovery. Single workspaces should reference the Workspace Owner/Admin from Line 47; reserve Org Owner/Admin for Enterprise Grid.
Suggested clarification
-4. If the app was already installed and notifications stopped working with an `account_inactive` error, the integration should be **disconnected** and reinstalled by an Org Owner/Admin. Reinstalling generates a new, valid token. +4. If the app was already installed and notifications stopped working with an `account_inactive` error, disconnect and reinstall it using the required administrator level: **Workspace Owner/Admin** for a single workspace or **Org Owner/Admin** for Enterprise Grid. Reinstalling generates a new, valid token.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@resources/integrations/slack/slack-app.md` around lines 46 - 49, Update the account_inactive recovery step in the Slack installation checklist to distinguish deployment types: require a Workspace Owner/Admin for single-workspace reinstallations and an Org Owner/Admin for Enterprise Grid reinstallations. Preserve the existing disconnect-and-reinstall flow and clarify that the required role depends on the deployment type.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@resources/integrations/slack/slack-app.md`:
- Around line 46-49: Update the account_inactive recovery step in the Slack
installation checklist to distinguish deployment types: require a Workspace
Owner/Admin for single-workspace reinstallations and an Org Owner/Admin for
Enterprise Grid reinstallations. Preserve the existing disconnect-and-reinstall
flow and clarify that the required role depends on the deployment type.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e71a2d4a-6893-433c-88bc-96116bc07096
📒 Files selected for processing (2)
resources/integrations/slack/README.mdresources/integrations/slack/slack-app.md
Co-authored-by: miguelangaranocurrents <miguelangaranocurrents@users.noreply.github.com>
Co-authored-by: miguelangaranocurrents <miguelangaranocurrents@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@resources/integrations/slack/slack-webhook.md`:
- Around line 87-94: The migration checklist must preserve the legacy “Single
Notification for All Groups” setting by mapping it to the Slack App equivalent
or explicitly documenting that it cannot be migrated. Clarify under “Events
(Optional)” that a blank selection means all legacy events are enabled,
including “Run Start,” and specify how that event should be handled despite
having no Slack App equivalent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a9dfe8cc-93f0-4fbf-a90d-bd338f406537
📒 Files selected for processing (2)
resources/integrations/slack/slack-app.mdresources/integrations/slack/slack-webhook.md
🚧 Files skipped from review as they are similar to previous changes (1)
- resources/integrations/slack/slack-app.md
Co-authored-by: miguelangaranocurrents <miguelangaranocurrents@users.noreply.github.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
guides/currents-actions/lifecycle-notifications.md (1)
26-26: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDocument the concrete Slack roles required for each workspace type.
Saying that the required role “depends” on single-workspace versus Enterprise Grid leaves administrators unsure whom to involve. Name the required role for each topology, or link directly to the installation section that specifies it.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@guides/currents-actions/lifecycle-notifications.md` at line 26, Update the Slack authorization guidance in the lifecycle notifications documentation to name the concrete Slack role required for single-workspace installations and for Enterprise Grid, or link directly to the existing installation section that specifies both roles. Keep the Currents administrator and member permissions unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@guides/currents-actions/lifecycle-notifications.md`:
- Line 26: Update the Slack authorization guidance in the lifecycle
notifications documentation to name the concrete Slack role required for
single-workspace installations and for Enterprise Grid, or link directly to the
existing installation section that specifies both roles. Keep the Currents
administrator and member permissions unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 86116e09-f694-41a6-86c1-120b3858b017
📒 Files selected for processing (1)
guides/currents-actions/lifecycle-notifications.md
Co-authored-by: miguelangaranocurrents <miguelangaranocurrents@users.noreply.github.com>
Co-authored-by: miguelangaranocurrents <miguelangaranocurrents@users.noreply.github.com>
Co-authored-by: miguelangaranocurrents <miguelangaranocurrents@users.noreply.github.com>
Co-authored-by: miguelangaranocurrents <miguelangaranocurrents@users.noreply.github.com>
Co-authored-by: miguelangaranocurrents <miguelangaranocurrents@users.noreply.github.com>
Co-authored-by: miguelangaranocurrents <miguelangaranocurrents@users.noreply.github.com>
Co-authored-by: miguelangaranocurrents <miguelangaranocurrents@users.noreply.github.com>
Co-authored-by: miguelangaranocurrents <miguelangaranocurrents@users.noreply.github.com>
| ## Requirements and permissions | ||
|
|
||
| Before installing the Currents Slack App, the installation must be performed by a user with the correct administrator permissions in Slack. The required role depends on how the Slack account is structured: | ||
| Currents and Slack permissions are separate: |
There was a problem hiding this comment.
The heading here is now ## Requirements and permissions, but the FAQ's account_inactive recovery text still links See [Requirements](#requirements), so the anchor no longer resolves and the administrator-role guidance table becomes unreachable — should we update the link to #requirements-and-permissions (confirm exact GitBook slug), or rename the heading back to Requirements?
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
resources/integrations/slack/slack-app.md, the section heading around lines 19-21 was
renamed to `## Requirements and permissions`, changing its GitBook-generated anchor away
from `#requirements`. The FAQ/recovery instructions for `account_inactive` (around line
~384-389) still contain `See [Requirements](#requirements)`, which now points to a
non-existent anchor. Update that link's fragment to match the new heading's anchor
(likely `#requirements-and-permissions`, but confirm the exact GitBook slug), or
alternatively rename the heading back to `Requirements` to preserve the old anchor.
After the change, verify by previewing the GitBook page that clicking the link navigates
correctly to the Requirements section.
User description
Summary
main(including Vitest docs and CI setup updates)Out of scope (per review)
main)Verification
origin/maininto this branchgit diff --checkScreenshots
Current product screenshots for newly documented UI states are not in the repository; existing Slack assets received alt text and captions only.
Summary by CodeRabbit
Generated description
Below is a concise technical summary of the changes proposed in this PR:
Expand the Slack App documentation to clarify installation roles, notification configuration, destination/channel handling, recovery states, and data-access boundaries for Currents and Slack administrators. Add a Slack-based
Fix with AIpath and update the AI overview so failed-test notifications can be handed off to agent tools directly from Slack.Fix with AIentry point and document how failed-test messages can launch or copy prompts for AI coding tools.Modified files (3)
Latest Contributors(2)
Modified files (4)
Latest Contributors(2)