Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MM-387]: Added support to subscribe to release and deployment #515

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

Kshitij-Katiyar
Copy link
Contributor

@Kshitij-Katiyar Kshitij-Katiyar commented Aug 7, 2024

Description

Added support to subscribe to release and deployment.

Screenshots

release7AugGitlab
image (2)

Issue link

Fixes #387

What to Test

  • create a subscription with release and deployment features and test the expected notifications that are received.

server/command.go Outdated Show resolved Hide resolved
server/command.go Outdated Show resolved Hide resolved
@@ -74,6 +74,8 @@ func (g *gitlab) NewGroupHook(ctx context.Context, user *UserInfo, token *oauth2
JobEvents: &webhookOptions.JobEvents,
PipelineEvents: &webhookOptions.PipelineEvents,
WikiPageEvents: &webhookOptions.WikiPageEvents,
DeploymentEvents: &webhookOptions.DeploymentEvents,
ReleasesEvents: &webhookOptions.ReleaseEvents,
Copy link
Member

Choose a reason for hiding this comment

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

This is called ReleaseEvents everywhere else.

Copy link
Contributor Author

@Kshitij-Katiyar Kshitij-Katiyar Aug 8, 2024

Choose a reason for hiding this comment

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

@wiggin77 the key ReleasesEvents is used in AddGroupHookOptions struct from the github.com/xanzy/go-gitlab package. It is predefined as ReleasesEvents there.

Copy link
Member

Choose a reason for hiding this comment

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

Can we not at least be consistent within our own code?

@@ -111,6 +113,8 @@ func (g *gitlab) NewProjectHook(ctx context.Context, user *UserInfo, token *oaut
JobEvents: &webhookOptions.JobEvents,
PipelineEvents: &webhookOptions.PipelineEvents,
WikiPageEvents: &webhookOptions.WikiPageEvents,
DeploymentEvents: &webhookOptions.DeploymentEvents,
ReleasesEvents: &webhookOptions.ReleaseEvents,
Copy link
Member

Choose a reason for hiding this comment

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

Should be "ReleaseEvents"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@@ -52,6 +52,14 @@ func (fakeWebhookHandler) HandleJobs(_ context.Context, _ *gitlabLib.JobEvent) (
return nil, nil
}

func (fakeWebhookHandler) HandleDeployment(_ context.Context, _ *gitlabLib.DeploymentEvent) ([]*webhook.HandleWebhook, error) {
Copy link
Member

Choose a reason for hiding this comment

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

Are there are any unit tests for these?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@wiggin77, Added them, Please have a look

@wiggin77 wiggin77 added 2: Dev Review Requires review by a core committer 3: QA Review Requires review by a QA tester labels Aug 7, 2024
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 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2: Dev Review Requires review by a core committer 3: QA Review Requires review by a QA tester
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Trigger types available in Gitlab not mapped to the plugin?
2 participants