MM-69657 Added guards to prevent cases of replies sent on GM/DMs

[avasconcelos114]

Summary

This PR adds guards that handle an edgecase where a plugin running v4.6.0 or lower can create a DM/GM subscription, and continue to receive replies on an issue even if they had previously disconnected their accounts

Ticket Link

Fixes https://mattermost.atlassian.net/browse/MM-69657

QA Notes

Setup

1. Install v4.6.0 of the plugin, configure it against your Jira test instance, and connect as user A with /jira connect.
2. Open the DM channel with the Jira bot and run /jira subscribe edit. Create a subscription on your test project for the Created and Comment Created events. On v4.6.0 this is allowed.
3. Confirm it exists: /jira subscribe list should show a Group and Direct Messages section containing the subscription.
4. Create a Jira ticket in the test project and confirm the notification post lands in the bot DM. Do not skip this — it stores the root post ID that later comments thread onto, which is the specific symptom in the report.

Test 1: Disconnect removes the DM subscription

1. Upgrade the plugin to your MM-69657 build. Confirm user A is still connected (/jira instance list / /jira connect shows connected).
2. Add another comment to the ticket created and confirm the threaded reply still arrives in the DM (to confirm that the subscription is live)
3. Run /jira disconnect.
4. Comment again on that same ticket.
5. Expected: no new post in the DM, and specifically no threaded reply under the earlier notification. /jira subscribe list no longer shows the Group and Direct Messages section. Before the fix, the threaded reply would still appear.

Test 2: Delivery guard and self-heal

1. Roll back to v4.6.0 and recreate the DM subscription as in Setup
2. Still on v4.6.0, run /jira disconnect
3. Upgrade to PR build (without reconnecting)
4. Comment on the ticket from step 1
5. Expected: no post appears

Change Impact: 🟠 Medium

Reasoning: The changes span subscription cleanup and webhook delivery across multiple server modules. They affect account disconnection and user-facing message delivery, but include broad test coverage.

Regression Risk: Medium. Delivery guards and subscription removal change behavior for DM and GM channels. Failure handling and legacy subscription paths still require validation.

** QA Recommendation:** Perform manual QA for disconnect behavior, legacy subscriptions, threaded replies, connected members, and connection lookup failures. Skipping manual QA carries moderate risk.

Generated by CodeRabbitAI

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 4ada0420-71e2-4829-8401-33fdbde55430

📥 Commits

Reviewing files that changed from the base of the PR and between 047e895 and a6aead8.

📒 Files selected for processing (3)

• server/subscribe.go
• server/subscribe_test.go
• server/webhook_worker_test.go

🚧 Files skipped from review as they are similar to previous changes (3)

• server/webhook_worker_test.go
• server/subscribe_test.go
• server/subscribe.go

---

📝 Walkthrough

Walkthrough

The change adds connected-member checks for DM and GM webhook delivery, improves subscription discovery when channel indexes drift, and expands disconnect cleanup to preserve subscriptions while another member remains connected.

Changes

Subscription delivery lifecycle

Layer / File(s)	Summary
Connection and subscription checks server/subscribe.go, server/command_test.go, server/subscribe_test.go	The server detects connected non-bot members, scans ByID for subscriptions, and handles missing connection records with kvstore.ErrNotFound. Tests cover DM/GM membership and channel index drift.
Disconnect cleanup server/subscribe.go, server/subscribe_test.go	Disconnect cleanup scans subscribed DM and GM channels. It removes subscriptions only when no connected member remains and skips channels that do not contain the disconnected user.
Webhook delivery guards server/webhook_http.go, server/webhook_http_test.go, server/webhook_worker.go, server/webhook_worker_test.go	HTTP and worker webhook paths skip delivery when no connected member exists. The worker removes orphaned subscriptions and continues delivery when connection lookup fails or the channel is not a DM/GM.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant WebhookRequest
  participant WebhookWorker
  participant ChannelAPI
  participant UserStore
  participant SubscriptionStore
  WebhookRequest->>ChannelAPI: resolve target channel
  WebhookWorker->>ChannelAPI: check channel members
  ChannelAPI->>UserStore: load member connections
  UserStore-->>ChannelAPI: connection lookup result
  WebhookWorker->>SubscriptionStore: remove orphaned subscription
  WebhookWorker-->>WebhookRequest: post notification when allowed

Loading

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 18.18% 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
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the issue and the main change: adding guards to prevent replies on GM/DM channels.
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.

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch MM-69657

---

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

[coderabbitai]

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 `@server/subscribe_test.go`:
- Around line 1799-1809: Assert that the expected KVSetWithOptions calls occur
in all three positive test cases: server/subscribe_test.go lines 1799-1809 and
1993-2004, and server/webhook_worker_test.go lines 83-95. Add api.AssertCalled
or api.AssertExpectations after the relevant operations, preserving the existing
argument matchers and test behavior.

🪄 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: Repository UI (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 638079a5-67c3-4f08-a090-4ab4c8a9e3a2

📥 Commits

Reviewing files that changed from the base of the PR and between 40d4d49 and 047e895.

📒 Files selected for processing (7)

• server/command_test.go
• server/subscribe.go
• server/subscribe_test.go
• server/webhook_http.go
• server/webhook_http_test.go
• server/webhook_worker.go
• server/webhook_worker_test.go

[nang2049]

LGTM! :)