WIP: FIX: missing edit notification after like (PR #9622)

In this commit https://github.com/discourse/discourse/commit/e90f9e5cc47 I tried to fix bug mentioned here https://meta.discourse.org/t/reply-then-edit-to-add-quote-notification-redundancy/138358

The idea was that when a user replies to a post and then edit and for example add mention or quote, we should not create another notification.

image

However, that solution was broken and created new bug mentioned here. https://meta.discourse.org/t/edits-not-being-stored-in-user-actions-table/145025

So if for example admin first like the post of the user, and then edit, we already got like notification, but it doesn’t mean that edit one should be skipped.

So I reverted original solution and now we are allowing to skip notification if we have notification of the exact same type or type of edited_group (replied, mentioned, quoted, edited)

image

GitHub

why was this removed?

I added it here: https://github.com/discourse/discourse/commit/e90f9e5cc47 so basically it is a revert. I only left spec from original commit to ensure I didn’t break that edit and add quote thingy

This is still somewhat … not right imo.

  • Sam created post X
  • Jane edits post X
  • Jane edits post X after 1 minute
  • Jane edits post X after 20 minutes
  • Jill edits post X

How should notifications look? In my ideal world.

Notification #1: Jane edits post X Notification #1 deleted (since it happened 20 minutes ago which is beyond grace): new notification created #2 for Jane edits post X Notification #3 created for Jill edits post X

What happens now?

also not sure how edit is grouped with like. this is very confusing to me

Yeah we are only getting 1 edit notification ever? this is a bit confusing.

@SamSaffron Your scenario will be fine because inside should_notify_previous? we got that check

  def should_notify_edit?(notification, post, opts)
    notification.data_hash["display_username"] != (opts[:display_username].presence || post.user.username)
  end

So second “editor” notification will be created, but let me run that scenario to ensure

The title of this pull request changed from “FIX: missing edit notification after like” to "WIP: FIX: missing edit notification after like

https://github.com/discourse/discourse/blob/57fcea7709f235ef40898abeddb2dcafb273e4da/app/services/post_alerter.rb#L324-L329

This is a fundamental problem here.

We are heavily suppressing edit notifications, you need like 10 like notifications to unlock an edit notification now. Let me pull this discussion to dev.