Merge pull request #2741 from lucianjon/drone-2685-process-action

Add support for Action conditions in pull request events

service/hook/parser/parse.go

@@ -246,7 +246,7 @@ func (p *parser) Parse(req *http.Request, secretFunc func(string) string) (*core
 		hook = &core.Hook{
 			Trigger:      core.TriggerHook, // core.TriggerHook,
 			Event:        core.EventPullRequest,
-			Action:       core.ActionCreate,
+			Action:       v.Action.String(),
 			Link:         v.PullRequest.Link,
 			Timestamp:    v.PullRequest.Created.Unix(),
 			Title:        v.PullRequest.Title,
@@ -263,9 +263,6 @@ func (p *parser) Parse(req *http.Request, secretFunc func(string) string) (*core
 			AuthorAvatar: v.PullRequest.Author.Avatar,
 			Sender:       v.Sender.Login,
 		}
-		if v.Action != scm.ActionSync {
-			hook.Action = core.ActionSync
-		}
 		// HACK this is a workaround for github. The pull
 		// request title is populated, but not the message.
 		if hook.Message == "" {

trigger/trigger.go

@@ -260,7 +260,7 @@ func (t *triggerer) Trigger(ctx context.Context, repo *core.Repository, base *co
 			logger = logger.WithField("pipeline", pipeline.Name)
 			logger.Infoln("trigger: skipping pipeline, does not match event")
 		} else if skipAction(pipeline, base.Action) {
-			logger = logger.WithField("pipeline", pipeline.Name)
+			logger = logger.WithField("pipeline", pipeline.Name).WithField("action", base.Action)
 			logger.Infoln("trigger: skipping pipeline, does not match action")
 		} else if skipRef(pipeline, base.Ref) {
 			logger = logger.WithField("pipeline", pipeline.Name)

trigger/trigger_test.go

@@ -271,6 +271,35 @@ func TestTrigger_SkipEvent(t *testing.T) {
 	}
 }
 
+// this test verifies that no build should be scheduled if the
+// hook action does not match the actions defined in the yaml.
+func TestTrigger_SkipAction(t *testing.T) {
+	controller := gomock.NewController(t)
+	defer controller.Finish()
+
+	mockUsers := mock.NewMockUserStore(controller)
+	mockUsers.EXPECT().Find(noContext, dummyRepo.UserID).Return(dummyUser, nil)
+
+	mockConfigService := mock.NewMockConfigService(controller)
+	mockConfigService.EXPECT().Find(gomock.Any(), gomock.Any()).Return(dummyYamlSkipAction, nil)
+
+	triggerer := New(
+		mockConfigService,
+		nil,
+		nil,
+		nil,
+		nil,
+		nil,
+		mockUsers,
+		nil,
+	)
+
+	_, err := triggerer.Trigger(noContext, dummyRepo, dummyHook)
+	if err != nil {
+		t.Errorf("Expect build silenty skipped if action does not match")
+	}
+}
+
 // this test verifies that if the system cannot increment the
 // build number, the function must exit with error and must not
 // schedule a new build.
@@ -407,6 +436,7 @@ var (
 		AuthorEmail:  "octocat@hello-world.com",
 		AuthorAvatar: "https://avatars3.githubusercontent.com/u/583231",
 		Sender:       "octocat",
+		Action:       "opened",
 	}
 
 	dummyBuild = &core.Build{
@@ -427,6 +457,7 @@ var (
 		AuthorEmail:  "octocat@hello-world.com",
 		AuthorAvatar: "https://avatars3.githubusercontent.com/u/583231",
 		Sender:       "octocat",
+		Action:       "opened",
 	}
 
 	dummyRepo = &core.Repository{
@@ -482,11 +513,30 @@ var (
 	}
 
 	dummyYamlSkipBranch = &core.Config{
-		Data: "kind: pipeline\ntrigger: { branch: { exclude: master } }",
+		Data: `
+kind: pipeline
+trigger:
+  branch:
+    exclude:
+    - master`,
 	}
 
 	dummyYamlSkipEvent = &core.Config{
-		Data: "kind: pipeline\ntrigger: { event: { exclude: push } }",
+		Data: `
+kind: pipeline
+trigger:
+  event:
+    exclude:
+    - push`,
+	}
+
+	dummyYamlSkipAction = &core.Config{
+		Data: `
+kind: pipeline
+trigger:
+  action:
+    exclude:
+    - opened`,
 	}
 
 	ignoreBuildFields = cmpopts.IgnoreFields(core.Build{},