Skip to content
This repository has been archived by the owner on Dec 10, 2024. It is now read-only.

Update Slack integration #2042

Merged
merged 3 commits into from
Nov 4, 2024
Merged

Update Slack integration #2042

merged 3 commits into from
Nov 4, 2024

Conversation

svanharmelen
Copy link
Member

Fixes #2039

PushEvents bool `json:"push_events"`
TagPushEvents bool `json:"tag_push_events"`
VulnerabilityEvents bool `json:"vulnerability_events"`
WikiPageEvents bool `json:"wiki_page_events"`

Copy link
Contributor

@jgangemi jgangemi Oct 17, 2024

Choose a reason for hiding this comment

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

this should include this as well:

        LabelsToBeNotified         string `json:"labels_to_be_notified,omitempty"`
	LabelsToBeNotifiedBehavior string `json:"labels_to_be_notified_behavior,omitempty"`
	PushChannel                string `json:"push_channel,omitempty"`
	IssueChannel               string `json:"issue_channel,omitempty"`
	ConfidentialIssueChannel   string `json:"confidential_issue_channel,omitempty"`
	MergeRequestChannel        string `json:"merge_request_channel,omitempty"`
	NoteChannel                string `json:"note_channel,omitempty"`
	ConfidentialNoteChannel    string `json:"confidential_note_channel,omitempty"`
	TagPushChannel             string `json:"tag_push_channel,omitempty"`
	PipelineChannel            string `json:"pipeline_channel,omitempty"`
	WikiPageChannel            string `json:"wiki_page_channel,omitempty"`
	DeploymentChannel          string `json:"deployment_channel,omitempty"`
	IncidentChannel            string `json:"incident_channel,omitempty"`
	VulnerabilityChannel       string `json:"vulnerability_channel,omitempty"`
	AlertChannel               string `json:"alert_channel,omitempty"`

@jgangemi
Copy link
Contributor

aside from the missing fields i mentioned, this appears to be working.

is it possible to change the SetSlackApplication to return the SlackApplicationProperties struct w/ the response instead of needing to get another get call?

@svanharmelen
Copy link
Member Author

Nice catch, added them to the struct 👍🏻 And no, the API doesn't return the properties on the PUT call, so it needs another GET call is you want the updated/saved properties from the server...

@jgangemi
Copy link
Contributor

it does - all the json output from our discussion in #2040 was from the PUT request.

@jgangemi
Copy link
Contributor

settings := new(SlackApplication)
req, err := s.client.NewRequest(http.MethodPut, u, opt, options)
if err != nil {
	return nil, err
}

resp, err := s.client.Do(req, &settings)
fmt.Println("-- SetSlackApplication --")
fmt.Printf("Inheritted: %t\n", settings.Inherited)
fmt.Println("-- SetSlackApplication --")
go run slack-test.go
--- GET ---
Service Id: 128662996
Inherited: false
Channel: 
Alert Events: false
Alert Channel: 
-- SetSlackApplication --
Inheritted: true
-- SetSlackApplication --
--- GET ---
Service Id: 128662996
Inherited: true
Channel: 
Alert Events: false
Alert Channel: 

@svanharmelen
Copy link
Member Author

svanharmelen commented Oct 20, 2024

I guess this should fix it, but the tests now fail 😏 Yet I'm out of time to spend on this for now, so will have another look next weekend or so...

@jgangemi
Copy link
Contributor

if you don't have time for this right now i can take a look at fixing the tests, i'd like to see this get merged b/c i'd like to start work on the terraform resource for it.

@jgangemi jgangemi mentioned this pull request Nov 3, 2024
@svanharmelen svanharmelen merged commit e53b7a1 into main Nov 4, 2024
6 checks passed
@svanharmelen svanharmelen deleted the fix/slack branch November 4, 2024 11:01
@svanharmelen
Copy link
Member Author

Thanks you very much for the help @jgangemi!

# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix support for the GitLab Slack App
2 participants