Merge pull request #1038 from ninesinc/andrewring/close-drafts

Release locks when closing draft PRs

server/events/event_parser.go

@@ -418,9 +418,13 @@ func (e *EventParser) ParseGithubPullEvent(pullEvent *github.PullRequestEvent) (
 	}
 
 	if pullEvent.GetPullRequest().GetDraft() {
-		// if the PR is in draft state we don't care about the action type
-		// we can set the type to Other and ignore the PR
-		pullEventType = models.OtherPullEvent
+		// if the PR is in draft state we do not initiate actions proactively however,
+		// we must still clean up locks in the event of a user initiated plan
+		if pullEvent.GetAction() == "closed" {
+			pullEventType = models.ClosedPullEvent
+		} else {
+			pullEventType = models.OtherPullEvent
+		}
 	} else {
 		switch pullEvent.GetAction() {
 		case "opened":

server/events/event_parser_test.go

@@ -160,74 +160,83 @@ func TestParseGithubPullEvent(t *testing.T) {
 	Equals(t, models.User{Username: "user"}, actUser)
 }
 
-func TestParseGithubPullEventFromDraft(t *testing.T) {
-	// verify that draft PRs are treated as 'other' events
-	testEvent := deepcopy.Copy(PullEvent).(github.PullRequestEvent)
-	draftPR := true
-	testEvent.PullRequest.Draft = &draftPR
-	_, evType, _, _, _, err := parser.ParseGithubPullEvent(&testEvent)
-	Ok(t, err)
-	Equals(t, models.OtherPullEvent, evType)
-}
-
 func TestParseGithubPullEvent_EventType(t *testing.T) {
 	cases := []struct {
-		action string
-		exp    models.PullRequestEventType
+		action   string
+		exp      models.PullRequestEventType
+		draftExp models.PullRequestEventType
 	}{
 		{
-			action: "synchronize",
-			exp:    models.UpdatedPullEvent,
+			action:   "synchronize",
+			exp:      models.UpdatedPullEvent,
+			draftExp: models.OtherPullEvent,
 		},
 		{
-			action: "unassigned",
-			exp:    models.OtherPullEvent,
+			action:   "unassigned",
+			exp:      models.OtherPullEvent,
+			draftExp: models.OtherPullEvent,
 		},
 		{
-			action: "review_requested",
-			exp:    models.OtherPullEvent,
+			action:   "review_requested",
+			exp:      models.OtherPullEvent,
+			draftExp: models.OtherPullEvent,
 		},
 		{
-			action: "review_request_removed",
-			exp:    models.OtherPullEvent,
+			action:   "review_request_removed",
+			exp:      models.OtherPullEvent,
+			draftExp: models.OtherPullEvent,
 		},
 		{
-			action: "labeled",
-			exp:    models.OtherPullEvent,
+			action:   "labeled",
+			exp:      models.OtherPullEvent,
+			draftExp: models.OtherPullEvent,
 		},
 		{
-			action: "unlabeled",
-			exp:    models.OtherPullEvent,
+			action:   "unlabeled",
+			exp:      models.OtherPullEvent,
+			draftExp: models.OtherPullEvent,
 		},
 		{
-			action: "opened",
-			exp:    models.OpenedPullEvent,
+			action:   "opened",
+			exp:      models.OpenedPullEvent,
+			draftExp: models.OtherPullEvent,
 		},
 		{
-			action: "edited",
-			exp:    models.OtherPullEvent,
+			action:   "edited",
+			exp:      models.OtherPullEvent,
+			draftExp: models.OtherPullEvent,
 		},
 		{
-			action: "closed",
-			exp:    models.ClosedPullEvent,
+			action:   "closed",
+			exp:      models.ClosedPullEvent,
+			draftExp: models.ClosedPullEvent,
 		},
 		{
-			action: "reopened",
-			exp:    models.OtherPullEvent,
+			action:   "reopened",
+			exp:      models.OtherPullEvent,
+			draftExp: models.OtherPullEvent,
 		},
 		{
-			action: "ready_for_review",
-			exp:    models.OpenedPullEvent,
+			action:   "ready_for_review",
+			exp:      models.OpenedPullEvent,
+			draftExp: models.OtherPullEvent,
 		},
 	}
 
 	for _, c := range cases {
 		t.Run(c.action, func(t *testing.T) {
+			// Test normal parsing
 			event := deepcopy.Copy(PullEvent).(github.PullRequestEvent)
 			event.Action = &c.action
 			_, actType, _, _, _, err := parser.ParseGithubPullEvent(&event)
 			Ok(t, err)
 			Equals(t, c.exp, actType)
+			// Test draft parsing
+			draftPR := true
+			event.PullRequest.Draft = &draftPR
+			_, draftEvType, _, _, _, err := parser.ParseGithubPullEvent(&event)
+			Ok(t, err)
+			Equals(t, c.draftExp, draftEvType)
 		})
 	}
 }