FIX: Generate webhook payloads before destroy events (PR #6325)

GitHub

You’ve signed the CLA, vinothkannans. Thank you! This pull request is ready for review.

@tgxworld can you review?

@vinothkannans I think we should use around_destroy here instead - https://stackoverflow.com/questions/4998553/rails-around-callbacks. The documentation around this is quite poor but it basically allows us to do something like this.

def around_destroy
  payload = generate_webhook_payload
  yield
  queue_webhook(payload)
end 

This can just be WebHook.enqueue_hooks(type, "#{type}_destroyed".to_sym,

Why not just do this?

def email
  primary_email&.email
end

I think primary_email can’t be nil. But only in the above commented spec url we making it as nil by removing user_emails table. I feel like it’s incorrect. So I need to ask @tgxworld whether it’s possible to keep using primary_email.email by fixing the old spec.

Sure, I will remove those extra lines before merging this :wink:

Have you tried setting up a user webhook and deleting a user? Pretty sure the email records are deleted before the webhook triggers :wink:

Yes, preventing that is the job for this PR.

I think the name here should be enqueue_destroyed_web_hook

Maybe HasDestroyedWebHook? I feel like HasWebHooks is too generic here.

So I need to ask @tgxworld whether it’s possible to keep using primary_email.email by fixing the old spec.

Yea I think we should fix the old spec here.

:+1:

Since Post and Topic are only soft destroyed, perhaps we can only generate the payload after it has been trashed?

After a post is soft deleted by author we changing the raw with (post withdrawn by author... text. I think it’s good to keep it like all other “destroyed” webhooks.

@tgxworld done. I hope it’s mergable now.

LGTM :+1: