MM-68540: Stop pushing sidebar data over cluster WS; refetch via REST

[nang2049]

Summary

This change makes sendRefreshEvent ship just {user_id}, and the webapp's handleRefresh dispatches the existing refetch action instead of consuming the embedded payload.
Net wire payload drops from ~250 KB to ~50 bytes.

Verified on test server https://github-test.test.mattermost.cloud/test-github/ that the new payload is minimal:

{
    "event": "custom_github_refresh",
    "data": {
        "user_id": "h47wpohqsbdwxrxnydxf6t8t8e"
    },
    "broadcast": {
        "omit_users": null,
        "user_id": "h47wpohqsbdwxrxnydxf6t8t8e",
        "channel_id": "",
        "team_id": "",
        "connection_id": "",
        "omit_connection_id": ""
    },
    "seq": 8
}

Ticket Link

https://mattermost.atlassian.net/browse/MM-68540

Change Impact: 🟡 Medium

Reasoning: The PR modifies a critical user-facing sidebar refresh mechanism that is triggered by multiple webhook events across the system. While the changes successfully reduce wire payload from ~250KB to ~50 bytes, the conversion from synchronous embedded-data delivery to asynchronous REST-based fetching introduces a new failure mode where REST timeouts or errors could silently leave sidebar content stale—a regression from the previous guaranteed-delivery semantics.

Regression Risk: The handler parameter change from (msg) => {} to () => {} is safe in isolation, but the underlying shift from receiving data over WebSocket to fetching it via REST removes built-in resilience. If getSidebarContent() REST calls fail, encounter timeouts, or experience rate-limiting, users will see no sidebar updates—whereas the old embedded-payload approach guaranteed delivery within the WebSocket frame. The connected state check provides some mitigation but doesn't handle REST-specific failures. The function is called frequently from 6+ webhook event handlers, so any issues will affect multiple user workflows.

QA Recommendation: Moderate-to-full manual QA is recommended. Test scenarios should include: sidebar refresh under poor network conditions, REST endpoint slowness/timeouts, server errors during REST calls, rapid consecutive refresh triggers (webhook storms), and recovery after transient failures. Verify sidebar doesn't appear frozen or stale if REST calls fail. All webhook event types that trigger sendRefreshEvent should be manually tested. The localized nature of the change allows for targeted testing rather than full regression, but the importance of the sidebar feature warrants comprehensive QA coverage. Adding unit tests for error paths in handleRefresh and sendRefreshEvent would significantly reduce ongoing risk.

Generated by CodeRabbitAI

[coderabbitai]

📝 Walkthrough

Walkthrough

The refresh event flow was simplified by shifting responsibility from server to client. The server now sends a minimal refresh event containing only user_id instead of computing and broadcasting sidebar content, while the client conditionally fetches sidebar data upon receiving the event.

Changes

Refresh Event Flow Simplification

Layer / File(s)	Summary
Server Event Payload server/plugin/plugin.go	sendRefreshEvent reduced to verify user connection via getGitHubUserInfo and publish wsEventRefresh with minimal payload {"user_id": userID}. Removed sidebar computation, JSON marshaling imports, logger import, and SidebarContent.toMap() helper.
Client Event Handler webapp/src/websocket/index.js	handleRefresh callback signature changed from (msg) handler that extracted msg.data to zero-argument callback that conditionally calls getSidebarContent() when plugin connection is active.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

---

🐰 A cleaner path, the rabbit hops with glee—
Server sends a whisper, client fetches free,
Less burden on the wire, more grace to be,
The refresh event dances, now light as can be! 🌿

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: removing sidebar data transmission over WebSocket and switching to REST-based refetching, which aligns with the actual modifications across both server and webapp components.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch MM-68540-github-refresh-payload

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

• Generate code and open pull requests
• Plan features and break down work
• Investigate incidents and troubleshoot customer tickets together
• Automate recurring tasks and respond to alerts with triggers
• Summarize progress and report instantly

Built for teams:

• Shared memory across your entire org—no repeating context
• Per-thread sandboxes to safely plan and execute work
• Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[lieut-data]

Do we even need to emit this at all, given the receiving client knows to fetch their own updated data?

[nang2049]

Thank you for your question, its still needed becase it's the only signal the client has that a webhook arrived. Without it the sidebar wouldn't update until reconnect or a manual refresh.

[lieut-data]

Sorry, the websocket is expected, but telling the user their own user_id is not.

[nang2049]

You're right followup PR to drop it #1004

[avasconcelos114]

LGTM!