Skip to content

Exclude comment deletion event from subscription events#198

Merged
avasconcelos114 merged 9 commits intomattermost:masterfrom
avasconcelos114:feature/exclude_remove_comment_events
Jan 15, 2026
Merged

Exclude comment deletion event from subscription events#198
avasconcelos114 merged 9 commits intomattermost:masterfrom
avasconcelos114:feature/exclude_remove_comment_events

Conversation

@avasconcelos114
Copy link
Contributor

@avasconcelos114 avasconcelos114 commented Oct 31, 2025

Summary

This PR reworks the displaying of webhook events so that the events shown are divided by <v9.0 and v9.0+, and are filtered by and responded by the server, instead of relying on constants in the webapp (though they still exist as a fallback value in case the API request fails)

In this PR, the comment deletion webhook event will now only appear if the Confluence plugin is configured to be used with a server of version below 9

It also adds validation on the save and edit subscription flows so that only supported events are accepted

Ticket Link

Fixes #195

QA Notes

Preparation

  • Have a Confluence Server instance running Confluence v9+

Test Steps

  1. Install the Confluence Plugin in a fresh instance
  2. Run /confluence install server and follow the wizard instructions to configure the plugin
  3. Run /confluence connect to connect accounts
  4. Run /confluence subscribe to open the modal to create a subscription to a channel
  5. Confirm that the Comment remove event does not appear as a selectable webhook event

- Currently divided into Confluence <9 and v9+, but can be further divided to 10+ if the need ever arises
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors webhook event handling to dynamically serve supported events based on Confluence server version (pre-v9.0 vs. v9.0+). The comment deletion event is now excluded for v9.0+ servers, with event filtering handled server-side rather than relying solely on webapp constants.

Key changes:

  • Added a new /config API endpoint that returns supported events based on server version
  • Implemented server-side validation to reject subscriptions with unsupported events
  • Updated the subscription modal to fetch and display only supported events dynamically

Reviewed Changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
webapp/src/components/subscription_modal/subscription_modal.jsx Loads plugin config on mount to fetch supported events and filters subscription events based on server configuration
webapp/src/components/subscription_modal/subscription_modal.test.jsx Added test coverage for plugin config loading and mock implementation
webapp/src/components/subscription_modal/index.js Connected getPluginConfig action to the subscription modal component
webapp/src/client/client.js Added client method to fetch plugin configuration from the new /config endpoint
webapp/src/actions/subscription_modal.js Implemented getPluginConfig action to retrieve plugin configuration
webapp/src/actions/index.js Exported the new getPluginConfig action
server/serializer/channel_subscription.go Added version-specific event lists, validation logic, and helper functions for event management
server/save_subscription.go Added event validation against server version before saving subscriptions
server/edit_subscription.go Added event validation against server version before editing subscriptions
server/plugin_config.go Created new endpoint handler to serve supported events based on server version
server/controller.go Registered the new /config endpoint in the controller
README.md Fixed typo in documentation

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Member

@wiggin77 wiggin77 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @avasconcelos114 I have a question re auth but overall it looks good.

- Changed error return to fit coding standards
@avasconcelos114
Copy link
Contributor Author

@wiggin77 Thanks for the review! I've switched the isAuthenticated to true (and also matched the formatting of the error as pointed out in Copilot)

@mattermost-build
Copy link
Contributor

This PR has been automatically labelled "stale" because it hasn't had recent activity.
A core team member will check in on the status of the PR to help with questions.
Thank you for your contribution!

@avasconcelos114 avasconcelos114 requested a review from a team as a code owner November 28, 2025 09:34
@avasconcelos114
Copy link
Contributor Author

Synced with master branch to make sure it can be ready for a merge, let me know if anything else is needed! 🙌

Copy link
Contributor

@nevyangelova nevyangelova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @avasconcelos114 LGTM!

@wiggin77 wiggin77 self-requested a review January 6, 2026 16:56
Copy link
Member

@wiggin77 wiggin77 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great progress @avasconcelos114 I have a couple of questions that I think may be bugs.

@avasconcelos114
Copy link
Contributor Author

Thanks for the review @wiggin77! Let me know what you think of the latest batch of changes 🙌

Copy link
Member

@wiggin77 wiggin77 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@avasconcelos114 avasconcelos114 requested a review from ogi-m January 13, 2026 11:21
@avasconcelos114 avasconcelos114 added the 3: QA Review Requires review by a QA tester label Jan 13, 2026
@ogi-m ogi-m added the Setup Cloud Test Server Setup a test server using Mattermost Cloud label Jan 14, 2026
@ogi-m
Copy link

ogi-m commented Jan 14, 2026

@avasconcelos114 "Comment Remove" still appears for me and is auto-selected by default on the subscription modal:
Screenshot 2026-01-14 at 19 39 20

image

@mm-cloud-bot
Copy link

Plugin Spinwick PR #198 🎉

Test server created!

Access here: https://confluence-pr-198-5yt6u.test.mattermost.cloud

Plugin Version Artifact
confluence 76ede3d Download

⚠️ Plugin Installation Issue

The test server was created successfully, but there was an issue installing or enabling the plugin automatically:

  • Install Error: failed to wait for S3 artifact: timed out waiting for S3 artifact

You can manually install the plugin:

  1. Download the plugin artifact from the link above
  2. Upload it to your test server at z4a9b5zjxpgcpjgtzosw7cxe5c
  3. Enable it in System Console > Plugins

Future commits will still attempt to automatically update the plugin.

Installation ID: z4a9b5zjxpgcpjgtzosw7cxe5c
Logs: Click here

Credentials: Posted securely in this Mattermost channel - Look for PR #198

@avasconcelos114
Copy link
Contributor Author

@ogi-m Ahh great catch, there was an issue where the list of supported events stored in the client would reset when the modal is closed. I changed things so that the list of events supported that the server returns is preserved as it should

@mm-cloud-bot
Copy link

Plugin test server update attempted, but encountered an issue:

⚠️ Install Error: failed to wait for S3 artifact: timed out waiting for S3 artifact

The test server is still available. You can manually download and install the updated plugin using the artifact link below.

Updated with git commit 2ba4d35368457fa5ae7fa40030cf69ebbbf410a5

Plugin Version Artifact
confluence 2ba4d35 Download

@mm-cloud-bot
Copy link

Plugin test server update attempted, but encountered an issue:

⚠️ Install Error: failed to wait for S3 artifact: timed out waiting for S3 artifact

The test server is still available. You can manually download and install the updated plugin using the artifact link below.

Updated with git commit 459e02ded2539f81874a1d301f66e6abe8247d9d

Plugin Version Artifact
confluence 459e02d Download

Copy link

@ogi-m ogi-m left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @avasconcelos114, all good now!

@ogi-m ogi-m added 4: Reviews Complete All reviewers have approved the pull request and removed 3: QA Review Requires review by a QA tester Setup Cloud Test Server Setup a test server using Mattermost Cloud labels Jan 14, 2026
@mm-cloud-bot
Copy link

Test server destroyed

@avasconcelos114 avasconcelos114 merged commit e0adae9 into mattermost:master Jan 15, 2026
6 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4: Reviews Complete All reviewers have approved the pull request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enhancement : Exclude comment deletion event from subscription events

6 participants