diff --git a/server/events/event_parser.go b/server/events/event_parser.go index 1042ef759..2de7728f9 100644 --- a/server/events/event_parser.go +++ b/server/events/event_parser.go @@ -253,7 +253,6 @@ type EventParsing interface { // baseRepo is the repo that the pull request will be merged into. // user is the pull request author. // pullNum is the number of the pull request that triggered the webhook. - // *** Add tests and handle linking multiple pull requests to a work item. ParseAzureDevopsWorkItemCommentedEvent(comment *azuredevops.WorkItem, baseURL *string) (pullRefs []PullRef, err error) // ParseAzureDevopsPull parses the response from the Azure Devops API endpoint (not @@ -835,23 +834,19 @@ type PullRef struct { // pullNumStr is retrieved from a URI that resembles: // vstfs:///Git/PullRequestId/a7573007-bbb3-4341-b726-0c4148a07853%2f3411ebc1-d5aa-464f-9615-0b527bc66719%2f22 // where the pull request number is 22, following the %2f near tne end -// Must set a default error value that will be returned if there are no valid -// pull request relations in the event func (e *EventParser) ParseAzureDevopsWorkItemCommentedEvent(comment *azuredevops.WorkItem, baseURL *string) (pullRefs []PullRef, err error) { - defaultErrorStr := "no valid pull request relations in event payload" - err = errors.New(defaultErrorStr) for _, relation := range comment.Relations { ref := &PullRef{} if uri := relation.GetURL(); strings.Contains(uri, "vstfs:///Git/PullRequestId/") { var parsed *url.URL parsed, err = url.Parse(uri) if err != nil { - return pullRefs, err + continue } pullNumStr := strings.Split(parsed.Path, "/")[5] ref.PullNum, err = strconv.Atoi(pullNumStr) if err != nil { - return pullRefs, err + continue } // Retrieve the linked pull request to get baseRepo and user @@ -863,7 +858,7 @@ func (e *EventParser) ParseAzureDevopsWorkItemCommentedEvent(comment *azuredevop httpClient.Timeout = 10 * time.Second client, err := azuredevops.NewClient(httpClient) if err != nil { - return pullRefs, err + continue } if baseURL != nil { if !strings.HasSuffix(*baseURL, "/") { @@ -871,20 +866,20 @@ func (e *EventParser) ParseAzureDevopsWorkItemCommentedEvent(comment *azuredevop } parsed, err = url.Parse(*baseURL) if err != nil { - return pullRefs, err + continue } client.BaseURL = *parsed } opts := azuredevops.PullRequestListOptions{} pr, _, err := client.PullRequests.Get(context.Background(), e.AzureDevopsOrg, e.AzureDevopsProject, ref.PullNum, &opts) if err != nil { - return pullRefs, err + continue } createdBy := pr.GetCreatedBy() ref.User = models.User{Username: createdBy.GetUniqueName()} ref.BaseRepo, err = e.ParseAzureDevopsRepo(pr.GetRepository()) if err != nil { - return pullRefs, err + continue } pullRefs = append(pullRefs, *ref) } diff --git a/server/events/event_parser_test.go b/server/events/event_parser_test.go index 5f20a6268..f4de93cbd 100644 --- a/server/events/event_parser_test.go +++ b/server/events/event_parser_test.go @@ -1139,19 +1139,19 @@ func TestParseAzureDevopsWorkItemCommentedEvent(t *testing.T) { resource := testEvent.Resource.(azuredevops.WorkItem) resource.Relations = nil _, err := parser.ParseAzureDevopsWorkItemCommentedEvent(&resource, nil) - ErrEquals(t, "no valid pull request relations in event payload", err) + Ok(t, err) testEvent = deepcopy.Copy(event).(azuredevops.Event) resource = testEvent.Resource.(azuredevops.WorkItem) resource.Relations[0] = nil _, err = parser.ParseAzureDevopsWorkItemCommentedEvent(&resource, nil) - ErrEquals(t, "no valid pull request relations in event payload", err) + Ok(t, err) testEvent = deepcopy.Copy(event).(azuredevops.Event) resource = testEvent.Resource.(azuredevops.WorkItem) - resource.Relations[0].URL = azuredevops.String("") + resource.Relations[0].URL = azuredevops.String("vstfs:///Git/PullRequestId/a7573007-bbb3-4341-b726-0c4148a07853%2f3411ebc1-d5aa-464f-9615-0b527bc66719%2fwat") _, err = parser.ParseAzureDevopsWorkItemCommentedEvent(&resource, nil) - ErrEquals(t, "no valid pull request relations in event payload", err) + ErrEquals(t, "strconv.Atoi: parsing \"wat\": invalid syntax", err) // this should be successful // This parse function requires a mock HTTP server to return the ADPull fixture diff --git a/server/events_controller.go b/server/events_controller.go index a70f747cd..001006d57 100644 --- a/server/events_controller.go +++ b/server/events_controller.go @@ -226,11 +226,11 @@ func (e *EventsController) handleAzureDevopsPost(w http.ResponseWriter, r *http. e.Logger.Debug("request valid") azuredevopsReqID := "Request-Id=" + r.Header.Get("Request-Id") - event, _ := azuredevops.ParseWebHook(payload) - /*event, ok := webhook.(*azuredevops.Event) - if !ok { - e.respond(w, logging.Debug, http.StatusBadRequest, "Error unmarshaling webhook payload %s", azuredevopsReqID) - }*/ + event, err := azuredevops.ParseWebHook(payload) + if err != nil { + e.respond(w, logging.Error, http.StatusBadRequest, "Failed parsing webhook: %v %s", err, azuredevopsReqID) + return + } switch event.PayloadType { case azuredevops.WorkItemCommentedEvent: e.Logger.Debug("handling as comment event") @@ -239,7 +239,7 @@ func (e *EventsController) handleAzureDevopsPost(w http.ResponseWriter, r *http. e.Logger.Debug("handling as pull request event") e.HandleAzureDevopsPullRequestEvent(w, event, azuredevopsReqID) default: - e.respond(w, logging.Debug, http.StatusOK, "Ignoring unsupported event %s", azuredevopsReqID) + e.respond(w, logging.Debug, http.StatusOK, "Ignoring unsupported event: %v %s", event.PayloadType, azuredevopsReqID) } } @@ -477,15 +477,22 @@ func (e *EventsController) HandleAzureDevopsCommentEvent(w http.ResponseWriter, } strippedComment := bluemonday.StrictPolicy().SanitizeBytes([]byte(*comment)) - if len(workItem.Relations) == 0 { + pullRefs, err := e.Parser.ParseAzureDevopsWorkItemCommentedEvent(workItem, nil) + + if err != nil && len(pullRefs) == 0 { + e.respond(w, logging.Error, http.StatusBadRequest, "Failed parsing event: %v %s", err, azuredevopsReqID) + return + } + + if len(pullRefs) == 0 { e.respond(w, logging.Debug, http.StatusOK, "Ignoring comment event since no pull request is linked to work item; Request-Id = %s", azuredevopsReqID) return } - pullRefs, err := e.Parser.ParseAzureDevopsWorkItemCommentedEvent(workItem, nil) - if err != nil { - e.respond(w, logging.Error, http.StatusBadRequest, "Failed parsing event: %v %s", err, azuredevopsReqID) - return + if err != nil && len(pullRefs) > 0 { + e.respond(w, logging.Warn, http.StatusOK, "Failed parsing one of the linked pull requests: %v %s", err, azuredevopsReqID) + } else { + e.respond(w, logging.Info, http.StatusOK, "Successfully parsed workitem.commented event; Request-Id = %s", azuredevopsReqID) } for _, ref := range pullRefs {