diff --git a/app/handlers/apiv1/post.go b/app/handlers/apiv1/post.go index 9fb086be3..bf33153ef 100644 --- a/app/handlers/apiv1/post.go +++ b/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{}) } diff --git a/app/tasks/delete_post.go b/app/tasks/delete_post.go index fece47789..b5b31ec4e 100644 --- a/app/tasks/delete_post.go +++ b/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 }) } diff --git a/app/tasks/delete_post_test.go b/app/tasks/delete_post_test.go index aa90002d7..04d45c92c 100644 --- a/app/tasks/delete_post_test.go +++ b/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", + }) +}