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

Add support for Incident Custom Fields and sanitize Orchestration PUT request #139

Merged

Conversation

swbradshaw
Copy link
Contributor

@swbradshaw swbradshaw commented Sep 21, 2023

This PR adds support for Incident Custom Fields on the Event Orchestration path. This feature will be released in January 2024. In addition it fixes the API call for updating the Event Orchestration.

Changes:

  • Added new EventOrchestrationPathIncidentCustomFieldUpdate struct which is an ID/Value pair of the custom field
  • Added this new property to existing EventOrchestrationPathRuleActions struct
  • Updated test in pagerduty/event_orchestration_path_test.go for new action type
  • In the process of testing Getting/Saving a Service Orchestration, I determined there was a bug in this client where it was sending null values for arrays. This is not allowed by the API and results in an HTTP 400 Bad Request. I've added functions sanitizeOrchestrationPath and sanitizeActions to replace null values with empty arrays.

The following is a test client I wrote to test this library. The code below simply gets the service orchestration and then updates it with the same object from the get request. Without the sanitize change, this library throws an error.

func main() {
	client, err := pagerduty.NewClient(&pagerduty.Config{
		Token: os.Getenv("PAGERDUTY_TOKEN")})
	if err != nil {
		panic(err)
	}

	servicesResp, _, err := client.Services.List(&pagerduty.ListServicesOptions{})
	if err != nil {
		panic(err)
	}
	serviceId := servicesResp.Services[0].ID
	
	servicePathResp, _, err := client.EventOrchestrationPaths.Get(serviceId, "service")
	if err != nil {
		panic(err)
	}
	
	updatePayload, _, err := client.EventOrchestrationPaths.Update(serviceId, "service", servicePathResp)
	if err != nil {
		panic(err)
	}

}

Copy link
Collaborator

@imjaroiswebdev imjaroiswebdev left a comment

Choose a reason for hiding this comment

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

Thank you for adding this very needed fix @swbradshaw, however before moving on, there are a few comments which need to be addressed to continue with the merge. So, please if You find We need to discuss this further don't hesitate in reaching me out. ✌🏽

pagerduty/event_orchestration_path.go Outdated Show resolved Hide resolved
pagerduty/event_orchestration_path_test.go Outdated Show resolved Hide resolved
pagerduty/event_orchestration_path_test.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@imjaroiswebdev imjaroiswebdev left a comment

Choose a reason for hiding this comment

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

LGTM! 👍🏽

@imjaroiswebdev imjaroiswebdev marked this pull request as ready for review February 26, 2024 19:16
@imjaroiswebdev imjaroiswebdev merged commit bfc8dce into heimweh:master Feb 26, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants