Skip to content

[MM-99] Added a note to create subscription for "pipeline" event after running a pipeline#433

Merged
mickmister merged 2 commits intomattermost:masterfrom
Brightscout:MM-341
Feb 8, 2024
Merged

[MM-99] Added a note to create subscription for "pipeline" event after running a pipeline#433
mickmister merged 2 commits intomattermost:masterfrom
Brightscout:MM-341

Conversation

@raghavaggarwal2308
Copy link
Copy Markdown

Summary

Added a note to create subscription for "pipeline" event after running a pipeline

Screenshot:

image

What to test?

  • Getting a note to create a subscription for pipeline event if not present already.

Steps to reproduce:

  1. Connect your GitLab account.
  2. Run the pipeline of the desired project using the command below.
    /gitlab pipelines run <org/repo> <branch-name>
  3. Make sure there was no subscription present for the project mentioned in the above command.

Ticket Link

Fixes #341

@codecov
Copy link
Copy Markdown

codecov bot commented Jan 8, 2024

Codecov Report

Attention: 14 lines in your changes are missing coverage. Please review.

Comparison is base (98fdf4f) 33.40% compared to head (2898af3) 33.28%.
Report is 8 commits behind head on master.

Files Patch % Lines
server/command.go 0.00% 14 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #433      +/-   ##
==========================================
- Coverage   33.40%   33.28%   -0.12%     
==========================================
  Files          22       22              
  Lines        3979     3993      +14     
==========================================
  Hits         1329     1329              
- Misses       2519     2533      +14     
  Partials      131      131              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Copy Markdown
Contributor

@mickmister mickmister left a comment

Choose a reason for hiding this comment

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

LGTM, just one suggestion

foundPipelineSubscription := false
subs, err := p.GetSubscriptionsByChannel(channelID)
if err != nil {
p.API.LogWarn("Failed to get subscriptions for the channel", "ChannelID", channelID, "Error", err.Error())
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

0/5 we should use the pluginapi package when possible

Suggested change
p.API.LogWarn("Failed to get subscriptions for the channel", "ChannelID", channelID, "Error", err.Error())
p.client.Log.Warn("Failed to get subscriptions for the channel", "channel_id", channelID, "error", err.Error())

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@mickmister Updated

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

0/5 we should use the pluginapi package when possible

@mickmister Are you referring to this https://pkg.go.dev/github.com/mattermost/mattermost-plugin-api package?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Co-authored-by: Michael Kochell <6913320+mickmister@users.noreply.github.com>
@hanzei hanzei added the 3: QA Review Requires review by a QA tester label Feb 6, 2024
Copy link
Copy Markdown

@AayushChaudhary0001 AayushChaudhary0001 left a comment

Choose a reason for hiding this comment

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

This PR has been tested for the following scenario:-

  • Note added for running pipeline of the subscription is not already created.

This PR was working fine for the above condition, LGTM. Approved.

@mickmister mickmister merged commit d89afc9 into mattermost:master Feb 8, 2024
@mickmister mickmister added 4: Reviews Complete All reviewers have approved the pull request and removed 3: QA Review Requires review by a QA tester labels Feb 8, 2024
@raghavaggarwal2308 raghavaggarwal2308 deleted the MM-341 branch February 9, 2024 07:15
@ayusht2810 ayusht2810 mentioned this pull request Mar 22, 2024
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.

Triggering a pipeline does not automatically create a subscription to pipeline events

5 participants