Merge pull request #1207 from getfider/delete-post-webhook-fix

Fix: No webhook message when post deleted with no comment.

app/handlers/apiv1/post.go

@@ -173,10 +173,7 @@ func DeletePost() web.HandlerFunc {
 			return c.Failure(err)
 		}
 
-		if action.Text != "" {
-			// Only send notification if user wrote a comment.
-			c.Enqueue(tasks.NotifyAboutDeletedPost(action.Post))
-		}
+		c.Enqueue(tasks.NotifyAboutDeletedPost(action.Post, action.Text != ""))
 
 		return c.Ok(web.Map{})
 	}

app/tasks/delete_post.go

@@ -15,18 +15,41 @@ import (
 	"github.com/getfider/fider/app/pkg/worker"
 )
 
-//NotifyAboutDeletedPost sends a notification (web and email) to subscribers of the post that has been deleted
-func NotifyAboutDeletedPost(post *entity.Post) worker.Task {
+// NotifyAboutDeletedPost sends a notification (web and email) to subscribers of the post that has been deleted
+func NotifyAboutDeletedPost(post *entity.Post, deleteCommentAdded bool) worker.Task {
 	return describe("Notify about deleted post", func(c *worker.Context) error {
 
+		tenant := c.Tenant()
+		baseURL, logoURL := web.BaseURL(c), web.LogoURL(c)
+		author := c.User()
+		title := fmt.Sprintf("**%s** deleted **%s**", author.Name, post.Title)
+
+		// Webhook
+		webhookProps := webhook.Props{}
+		webhookProps.SetPost(post, "post", baseURL, true, true)
+		webhookProps.SetUser(author, "author")
+		webhookProps.SetTenant(tenant, "tenant", baseURL, logoURL)
+
+		err := bus.Dispatch(c, &cmd.TriggerWebhooks{
+			Type:  enum.WebhookDeletePost,
+			Props: webhookProps,
+		})
+		if err != nil {
+			return c.Failure(err)
+		}
+
+		// If no comment was added, we can stop here
+		// (I'm not sure about the rational of this business rule, but this is how it currently works)
+		if !deleteCommentAdded {
+			return nil
+		}
+
 		// Web notification
 		users, err := getActiveSubscribers(c, post, enum.NotificationChannelWeb, enum.NotificationEventChangeStatus)
 		if err != nil {
 			return c.Failure(err)
 		}
 
-		author := c.User()
-		title := fmt.Sprintf("**%s** deleted **%s**", author.Name, post.Title)
 		for _, user := range users {
 			if user.ID != author.ID {
 				err = bus.Dispatch(c, &cmd.AddNewNotification{
@@ -53,9 +76,6 @@ func NotifyAboutDeletedPost(post *entity.Post) worker.Task {
 			}
 		}
 
-		tenant := c.Tenant()
-		baseURL, logoURL := web.BaseURL(c), web.LogoURL(c)
-
 		props := dto.Props{
 			"title":    post.Title,
 			"siteName": tenant.Name,
@@ -71,19 +91,6 @@ func NotifyAboutDeletedPost(post *entity.Post) worker.Task {
 			Props:        props,
 		})
 
-		webhookProps := webhook.Props{}
-		webhookProps.SetPost(post, "post", baseURL, true, true)
-		webhookProps.SetUser(author, "author")
-		webhookProps.SetTenant(tenant, "tenant", baseURL, logoURL)
-
-		err = bus.Dispatch(c, &cmd.TriggerWebhooks{
-			Type:  enum.WebhookDeletePost,
-			Props: webhookProps,
-		})
-		if err != nil {
-			return c.Failure(err)
-		}
-
 		return nil
 	})
 }

app/tasks/delete_post_test.go

@@ -60,7 +60,7 @@ func TestNotifyAboutDeletePostTask(t *testing.T) {
 		},
 	}
 
-	task := tasks.NotifyAboutDeletedPost(post)
+	task := tasks.NotifyAboutDeletedPost(post, true)
 
 	err := worker.
 		OnTenant(mock.DemoTenant).
@@ -127,3 +127,88 @@ func TestNotifyAboutDeletePostTask(t *testing.T) {
 		"tenant_url":                 "http://domain.com",
 	})
 }
+
+func TestNotifyAboutDeletePostTask_NoComment(t *testing.T) {
+	RegisterT(t)
+	bus.Init(emailmock.Service{})
+
+	var addNewNotification *cmd.AddNewNotification
+	bus.AddHandler(func(ctx context.Context, c *cmd.AddNewNotification) error {
+		addNewNotification = c
+		return nil
+	})
+
+	bus.AddHandler(func(ctx context.Context, q *query.GetActiveSubscribers) error {
+		q.Result = []*entity.User{
+			mock.AryaStark,
+		}
+		return nil
+	})
+
+	var triggerWebhooks *cmd.TriggerWebhooks
+	bus.AddHandler(func(ctx context.Context, c *cmd.TriggerWebhooks) error {
+		triggerWebhooks = c
+		return nil
+	})
+
+	worker := mock.NewWorker()
+	post := &entity.Post{
+		ID:          1,
+		Number:      1,
+		Title:       "Add support for TypeScript",
+		Slug:        "add-support-for-typescript",
+		Description: "TypeScript is great, please add support for it",
+		User:        mock.AryaStark,
+		Status:      enum.PostDeleted,
+		Response: &entity.PostResponse{
+			RespondedAt: time.Now(),
+			Text:        "Invalid post!",
+			User:        mock.JonSnow,
+		},
+	}
+
+	task := tasks.NotifyAboutDeletedPost(post, false)
+
+	err := worker.
+		OnTenant(mock.DemoTenant).
+		AsUser(mock.JonSnow).
+		WithBaseURL("http://domain.com").
+		Execute(task)
+
+	Expect(err).IsNil()
+	Expect(emailmock.MessageHistory).HasLen(0)
+
+	Expect(addNewNotification).IsNil()
+
+	Expect(triggerWebhooks).IsNotNil()
+	Expect(triggerWebhooks.Type).Equals(enum.WebhookDeletePost)
+	Expect(triggerWebhooks.Props).ContainsProps(webhook.Props{
+		"post_id":                    post.ID,
+		"post_number":                post.Number,
+		"post_title":                 post.Title,
+		"post_slug":                  post.Slug,
+		"post_description":           post.Description,
+		"post_status":                post.Status.Name(),
+		"post_url":                   "http://domain.com/posts/1/add-support-for-typescript",
+		"post_author_id":             mock.AryaStark.ID,
+		"post_author_name":           mock.AryaStark.Name,
+		"post_author_email":          mock.AryaStark.Email,
+		"post_author_role":           mock.AryaStark.Role.String(),
+		"post_response":              true,
+		"post_response_text":         post.Response.Text,
+		"post_response_responded_at": post.Response.RespondedAt,
+		"post_response_author_id":    mock.JonSnow.ID,
+		"post_response_author_name":  mock.JonSnow.Name,
+		"post_response_author_email": mock.JonSnow.Email,
+		"post_response_author_role":  mock.JonSnow.Role.String(),
+		"author_id":                  mock.JonSnow.ID,
+		"author_name":                mock.JonSnow.Name,
+		"author_email":               mock.JonSnow.Email,
+		"author_role":                mock.JonSnow.Role.String(),
+		"tenant_id":                  mock.DemoTenant.ID,
+		"tenant_name":                mock.DemoTenant.Name,
+		"tenant_subdomain":           mock.DemoTenant.Subdomain,
+		"tenant_status":              mock.DemoTenant.Status.String(),
+		"tenant_url":                 "http://domain.com",
+	})
+}