MM-67256 Adding support for MR assigned/unassigned messages in subscriptions by avasconcelos114 · Pull Request #651 · mattermost/mattermost-plugin-gitlab

avasconcelos114 · 2026-03-04T13:08:25Z

Summary

This PR adds the handling necessary for users to subscribe to merge request assigned/unassigned events

Ticket Link

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

Summary by CodeRabbit

Release Notes

New Features
- Merge request assignment and unassignment notifications are now available as subscribable events.
- Users can receive separate notifications when assigned to or unassigned from merge requests.
- Channels can be configured to receive notifications for merge request assignment changes.
Documentation
- Updated documentation to include the new merge request assignment notification event.

avasconcelos114 · 2026-03-04T13:17:52Z

@coderabbitai please review this PR

coderabbitai · 2026-03-04T13:18:09Z

@avasconcelos114 Sure! I'll review this PR right away.

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai · 2026-03-04T13:20:12Z

📝 Walkthrough

Walkthrough

This pull request introduces a new notification feature for merge request assignment and unassignment events. The implementation includes documentation, CLI support, a feature flag with accessor, webhook event handling logic with channel filtering, and comprehensive test coverage across multiple files.

Changes

Cohort / File(s)	Summary
Documentation & CLI `README.md`, `server/command.go`	Added new `merge_request_assigns` event to documentation and updated subscription command help text and autocomplete data to include the new feature.
Feature Flag & Subscription Logic `server/subscription/subscription.go`	Introduced new `merge_request_assigns` feature flag to the known features set and added public accessor method `MergeRequestAssigns()` for feature checking.
Webhook Event Handling `server/webhook/merge_request.go`	Replaced single "assignees updated" handling with separate paths for `newlyAssigned` and `newlyUnassigned` events. Added logic to accumulate assignment messages and dispatch them to channels based on the new `merge_request_assigns` feature flag. Integrated per-user assignment message processing and channel filtering.
Channel Filtering Utility `server/webhook/webhook.go`	Added new unexported helper function `filterChannelsByFeature()` that filters subscribed channels based on feature flag predicates and event label matching, with error handling for label retrieval.
Test Fixtures `server/webhook/merge_request_fixture_test.go`	Added two new test fixtures: `RootUnassignUserMergeRequest` (for unassignment scenarios) and `RootAssignMergeRequestWithChannel` (for assignment scenarios with channel details).
Test Cases `server/webhook/merge_request_test.go`	Added comprehensive test coverage for assignment/unassignment flows, including scenarios with channel subscriptions, feature flag combinations, and repository path variations.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~30 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 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 (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately reflects the main change: adding support for merge request assigned/unassigned messages in subscriptions, which is the primary focus across all modified files.
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.}

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

coderabbitai

🧹 Nitpick comments (1)

server/webhook/webhook.go (1)
152-174: Inconsistent error handling for label retrieval compared to anyEventLabelInSubs.

When sub.Labels() returns an error, this function appends a warning but still includes the channel in the result (execution continues to line 171). However, anyEventLabelInSubs (lines 176-186) returns false on error, effectively excluding the channel.

This inconsistency could lead to unexpected behaviour where channels with malformed label subscriptions are included by filterChannelsByFeature but excluded by anyEventLabelInSubs.

Consider adding a continue after appending the warning to match the existing behaviour:
♻️ Proposed fix for consistent error handling
 		labels, err := sub.Labels()
 		if err != nil {
 			warnings = append(warnings, err.Error())
+			continue
 		} else if len(labels) > 0 && !containsAnyLabel(eventLabels, labels) {
 			continue
 		}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/webhook/webhook.go` around lines 152 - 174, filterChannelsByFeature
currently appends a warning when sub.Labels() errors but continues and includes
sub.ChannelID, which is inconsistent with anyEventLabelInSubs that excludes
channels on label errors; update filterChannelsByFeature (the loop in
filterChannelsByFeature calling sub.Labels()) to continue the loop immediately
after appending the warning (i.e., skip adding sub.ChannelID on error) so
behavior matches anyEventLabelInSubs and malformed label subscriptions are
excluded.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@server/webhook/webhook.go`:
- Around line 152-174: filterChannelsByFeature currently appends a warning when
sub.Labels() errors but continues and includes sub.ChannelID, which is
inconsistent with anyEventLabelInSubs that excludes channels on label errors;
update filterChannelsByFeature (the loop in filterChannelsByFeature calling
sub.Labels()) to continue the loop immediately after appending the warning
(i.e., skip adding sub.ChannelID on error) so behavior matches
anyEventLabelInSubs and malformed label subscriptions are excluded.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 24104402-39dc-452a-945e-fb39df791837

📥 Commits

Reviewing files that changed from the base of the PR and between f364310 and b3d6a15.

📒 Files selected for processing (7)

README.md
server/command.go
server/subscription/subscription.go
server/webhook/merge_request.go
server/webhook/merge_request_fixture_test.go
server/webhook/merge_request_test.go
server/webhook/webhook.go

marianunez · 2026-03-04T20:47:06Z

server/webhook/webhook.go

+		if err != nil {
+			warnings = append(warnings, err.Error())
+		} else if len(labels) > 0 && !containsAnyLabel(eventLabels, labels) {
+			continue
+		}
+
+		channels = append(channels, sub.ChannelID)


It seems that when there is an error we store the warning message but still add the channel to the filtered list. Is that intended?

Ah this is a bit of pre-existing logic that was ported over to be applied here too, the original snippet takes a pretty lenient approach where it virtually treats this error as human error in the part of the user (in https://github.com/mattermost/mattermost-plugin-gitlab/blob/master/server/webhook/issue.go#L110) and still appends the channel ID to the list (the error happens if a user formats the label as label:bug instead of label:"bug")

We could change our approach to be stricter though that would likely mean changing the pre-existing behavior of the issue handler too (or we could add the validation that #603 alludes to be missing where it would prevent malformed or inexistent labels from being accepted by subscriptions to begin with -- though this would be out of scope for this PR)

marianunez · 2026-03-04T21:52:28Z

server/webhook/merge_request.go

+	if len(assignMessages) > 0 {
+		toChannels, ws := filterChannelsByFeature(subs, event.Labels, func(sub *subscription.Subscription) bool {


Will existing channel subscriptions start to get the notifications for assigned/unassiged?

Yes, from what I could understand the merges subscription pertains to anything related to merge requests, so these events will automatically kick-in for subscriptions made at that level, and the new merge_request_assigns is a more granular event added in case users would like to individually select these events to be included in a subscription

avasconcelos114 added 5 commits March 4, 2026 14:25

Adding support for MR assign/unassign subscriptions

78d05f0

Moving redundant logic into own function

323e77c

Fixing shadowing issues

d36fcbd

Added new event to autocomplete and help messages

556d0ed

Moving location of merge_request_assigns in help message

b3d6a15

avasconcelos114 self-assigned this Mar 4, 2026

avasconcelos114 requested a review from a team as a code owner March 4, 2026 13:08

avasconcelos114 added 2: Dev Review Requires review by a core committer 3: QA Review Requires review by a QA tester labels Mar 4, 2026

coderabbitai bot reviewed Mar 4, 2026

View reviewed changes

coderabbitai bot approved these changes Mar 4, 2026

View reviewed changes

marianunez reviewed Mar 4, 2026

View reviewed changes

avasconcelos114 requested a review from marianunez March 6, 2026 19:05

marianunez approved these changes Mar 6, 2026

View reviewed changes

avasconcelos114 requested a review from ogi-m March 9, 2026 10:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MM-67256 Adding support for MR assigned/unassigned messages in subscriptions#651

MM-67256 Adding support for MR assigned/unassigned messages in subscriptions#651
avasconcelos114 wants to merge 5 commits intomasterfrom
MM-67256

avasconcelos114 commented Mar 4, 2026 •

edited

Loading

Uh oh!

avasconcelos114 commented Mar 4, 2026

Uh oh!

coderabbitai bot commented Mar 4, 2026

Uh oh!

coderabbitai bot commented Mar 4, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

❌ Failed checks (1 warning)

Uh oh!

coderabbitai bot left a comment

Uh oh!

marianunez Mar 4, 2026

Uh oh!

avasconcelos114 Mar 5, 2026

Uh oh!

marianunez Mar 4, 2026

Uh oh!

avasconcelos114 Mar 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		if len(assignMessages) > 0 {
		toChannels, ws := filterChannelsByFeature(subs, event.Labels, func(sub *subscription.Subscription) bool {

Conversation

avasconcelos114 commented Mar 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Ticket Link

Summary by CodeRabbit

Release Notes

Uh oh!

avasconcelos114 commented Mar 4, 2026

Uh oh!

coderabbitai bot commented Mar 4, 2026

Uh oh!

coderabbitai bot commented Mar 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

❌ Failed checks (1 warning)

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

marianunez Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

avasconcelos114 Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

marianunez Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

avasconcelos114 Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

avasconcelos114 commented Mar 4, 2026 •

edited

Loading

coderabbitai bot commented Mar 4, 2026 •

edited

Loading