Fix four known bugs in demo plugin - #215
Conversation
- Add null guards for team and channel name in rhs_view.jsx — missing values were causing a webapp crash in the pop-out tab - Add trigger_id to openInteractiveDialog to prevent the dialog from being lost in Redux - Add unique cancel/confirm handling in handleDialog2 with distinct messages for each outcome - Add data-testid to left sidebar header hook status element Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe changes update dialog confirmation and cancellation messages, add a plugin dialog trigger identifier, expose a sidebar hook status test identifier, and guard RHS popout and team navigation paths when context is unavailable. ChangesInteraction updates
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions 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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
server/http_hooks.go (1)
237-245: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd regression tests for both dialog outcomes.
The new
request.Cancelledbranch changes the post content. Add handler tests for confirmation and cancellation. Also cover the relative callback suffix.🤖 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 `@server/http_hooks.go` around lines 237 - 245, Add regression coverage for the dialog handler around the message construction using request.Cancelled: verify both confirmation and cancellation posts, and assert each includes the expected relative callback suffix. Reuse the existing handler test setup and validate the created post content for both outcomes.
🤖 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 `@webapp/src/components/right_hand_sidebar/rhs_view.jsx`:
- Around line 56-57: Update the anchor rendering in the RHS view so it is only
rendered when team?.name exists, preventing the href text from displaying an
undefined team route; otherwise render the appropriate non-link fallback while
preserving the current navigation behavior for valid teams.
- Around line 15-20: Update the pop-out button disabled conditions and the
popoutRhsPlugin guard in the RHS view component to require popoutSupported,
team?.name, and channel?.name. Ensure both buttons are disabled whenever either
context name is unavailable, matching the API invocation checks.
---
Nitpick comments:
In `@server/http_hooks.go`:
- Around line 237-245: Add regression coverage for the dialog handler around the
message construction using request.Cancelled: verify both confirmation and
cancellation posts, and assert each includes the expected relative callback
suffix. Reuse the existing handler test setup and validate the created post
content for both outcomes.
🪄 Autofix
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 Plus
Run ID: c7bae1df-7622-4c93-b243-b3ccc6bd685a
📒 Files selected for processing (4)
server/http_hooks.gowebapp/src/components/left_sidebar_header/left_sidebar_header.jsxwebapp/src/components/right_hand_sidebar/rhs_view.jsxwebapp/src/plugin.jsx
|
@saturninoabril this PR resolves 4 bugs identified while adding Playwright coverage. Once this is in place E2E test in the monorepo can be used to guard against regression. |
|
Thanks @DHaussermann, lgtm. |
Fixed 4 minor bugs with Demo plugin that were found while adding Playwright test coverage.
data-testidto left sidebar header hook status element