FEATURE: Consolidate link notifications (#26567)

Just like we have for consolidating likes this adds similar
functionality for consolidating links.
This commit is contained in:
Blake Erickson 2024-04-09 11:53:37 -06:00 committed by GitHub
parent 7f802e9c42
commit 72ac675e4e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
12 changed files with 170 additions and 3 deletions

View File

@ -41,6 +41,7 @@ export const REPLACEMENTS = {
"notification.invitee_accepted": "user",
"notification.moved_post": "sign-out-alt",
"notification.linked": "link",
"notification.linked_consolidated": "link",
"notification.granted_badge": "certificate",
"notification.topic_reminder": "far-clock",
"notification.watching_first_post": "discourse-bell-one",

View File

@ -9,6 +9,7 @@ import GroupMessageSummary from "discourse/lib/notification-types/group-message-
import InviteeAccepted from "discourse/lib/notification-types/invitee-accepted";
import Liked from "discourse/lib/notification-types/liked";
import LikedConsolidated from "discourse/lib/notification-types/liked-consolidated";
import LinkedConsolidated from "discourse/lib/notification-types/linked-consolidated";
import MembershipRequestAccepted from "discourse/lib/notification-types/membership-request-accepted";
import MembershipRequestConsolidated from "discourse/lib/notification-types/membership-request-consolidated";
import MovedPost from "discourse/lib/notification-types/moved-post";
@ -25,6 +26,7 @@ const CLASS_FOR_TYPE = {
invitee_accepted: InviteeAccepted,
liked: Liked,
liked_consolidated: LikedConsolidated,
linked_consolidated: LinkedConsolidated,
membership_request_accepted: MembershipRequestAccepted,
membership_request_consolidated: MembershipRequestConsolidated,
moved_post: MovedPost,

View File

@ -0,0 +1,16 @@
import NotificationTypeBase from "discourse/lib/notification-types/base";
import { userPath } from "discourse/lib/url";
import I18n from "discourse-i18n";
export default class extends NotificationTypeBase {
get linkHref() {
// Linking here for now until we have a proper new page for "linked" in the profile
return userPath(`${this.currentUser.username}/notifications`);
}
get description() {
return I18n.t("notifications.linked_consolidated_description", {
count: this.notification.data.count,
});
}
}

View File

@ -40,6 +40,8 @@ export const NOTIFICATION_TYPES = {
question_answer_user_commented: 35,
watching_category_or_tag: 36,
new_features: 37,
admin_problems: 38,
linked_consolidated: 39,
following: 800,
following_created_topic: 801,
following_replied: 802,

View File

@ -0,0 +1,64 @@
import { setupTest } from "ember-qunit";
import { module, test } from "qunit";
import Notification from "discourse/models/notification";
import { NOTIFICATION_TYPES } from "discourse/tests/fixtures/concerns/notification-types";
import { createRenderDirector } from "discourse/tests/helpers/notification-types-helper";
import { deepMerge } from "discourse-common/lib/object";
import I18n from "discourse-i18n";
function getNotification(overrides = {}) {
return Notification.create(
deepMerge(
{
id: 11,
user_id: 1,
notification_type: NOTIFICATION_TYPES.linked_consolidated,
read: false,
high_priority: false,
created_at: "2022-07-01T06:00:32.173Z",
data: {
topic_title: "this is some topic and it's irrelevant",
original_post_id: 3294,
original_post_type: 1,
original_username: "liker439",
display_username: "liker439",
username: "liker439",
count: 44,
},
},
overrides
)
);
}
module("Unit | Notification Types | linked-consolidated", function (hooks) {
setupTest(hooks);
test("linkHref", function (assert) {
const notification = getNotification();
const director = createRenderDirector(
notification,
"linked_consolidated",
this.siteSettings
);
assert.strictEqual(
director.linkHref,
"/u/eviltrout/notifications",
"links to the notifications page of the user"
);
});
test("description", function (assert) {
const notification = getNotification();
const director = createRenderDirector(
notification,
"linked_consolidated",
this.siteSettings
);
assert.strictEqual(
director.description,
I18n.t("notifications.linked_consolidated_description", { count: 44 }),
"displays the right content"
);
});
});

View File

@ -159,6 +159,7 @@ class Notification < ActiveRecord::Base
watching_category_or_tag: 36,
new_features: 37,
admin_problems: 38,
linked_consolidated: 39,
following: 800, # Used by https://github.com/discourse/discourse-follow
following_created_topic: 801, # Used by https://github.com/discourse/discourse-follow
following_replied: 802, # Used by https://github.com/discourse/discourse-follow

View File

@ -15,6 +15,7 @@ module Notifications
consolidation_plans = [
liked_by_two_users,
liked,
linked,
group_message_summary,
group_membership,
new_features_notification,
@ -98,6 +99,26 @@ module Notifications
)
end
def linked
ConsolidateNotifications
.new(
from: Notification.types[:linked],
to: Notification.types[:linked_consolidated],
threshold: -> { SiteSetting.notification_consolidation_threshold },
consolidation_window: SiteSetting.linked_notification_consolidation_window_mins.minutes,
unconsolidated_query_blk: filtered_by_data_attribute("display_username"),
consolidated_query_blk: filtered_by_data_attribute("display_username"),
)
.set_mutations(
set_data_blk:
Proc.new do |notification|
data = notification.data_hash
data.merge(username: data[:display_username])
end,
)
.set_precondition(precondition_blk: Proc.new { |data| data[:username2].blank? })
end
def group_membership
ConsolidateNotifications
.new(

View File

@ -2667,6 +2667,10 @@ en:
one: "liked %{count} of your posts"
other: "liked %{count} of your posts"
liked_consolidated: "<span>%{username}</span> %{description}"
linked_consolidated_description:
one: "linked %{count} of your posts"
other: "linked %{count} of your posts"
linked_consolidated: "<span>%{username}</span> %{description}"
private_message: "<span>%{username}</span> %{description}"
invited_to_private_message: "<p><span>%{username}</span> %{description}"
invited_to_topic: "<span>%{username}</span> %{description}"
@ -2740,6 +2744,7 @@ en:
watching_first_post: "new topic"
topic_reminder: "topic reminder"
liked_consolidated: "new likes"
linked_consolidated: "new links"
post_approved: "post approved"
membership_request_consolidated: "new membership requests"
reaction: "new reaction"

View File

@ -2384,6 +2384,8 @@ en:
likes_notification_consolidation_window_mins: "Duration in minutes where liked notifications are consolidated into a single notification once the threshold has been reached. The threshold can be configured via `SiteSetting.notification_consolidation_threshold`."
linked_notification_consolidation_window_mins: "Duration in minutes where linked notifications are consolidated into a single notification once the threshold has been reached. The threshold can be configured via `SiteSetting.notification_consolidation_threshold`."
automatically_unpin_topics: "Automatically unpin topics when the user reaches the bottom."
read_time_word_count: "Word count per minute for calculating estimated reading time."

View File

@ -2749,6 +2749,10 @@ uncategorized:
default: 120
min: 1
linked_notification_consolidation_window_mins:
default: 120
min: 1
delete_drafts_older_than_n_days:
default: 180
max: 36500

View File

@ -74,6 +74,9 @@
"liked_consolidated": {
"type": "integer"
},
"linked_consolidated": {
"type": "integer"
},
"post_approved": {
"type": "integer"
},
@ -156,6 +159,7 @@
"watching_first_post",
"topic_reminder",
"liked_consolidated",
"linked_consolidated",
"post_approved",
"code_review_commit_approved",
"membership_request_accepted",

View File

@ -7,10 +7,11 @@ RSpec.describe Notifications::ConsolidationPlanner do
let(:threshold) { 1 }
fab!(:user)
let(:like_user) { "user1" }
let(:link_user) { "user2" }
before { SiteSetting.notification_consolidation_threshold = threshold }
it "does nothing it haven't passed the consolidation threshold yet" do
it "does nothing when it hasn't passed the consolidation threshold yet for likes" do
notification = build_notification(:liked, { display_username: like_user })
saved_like = planner.consolidate_or_save!(notification)
@ -19,7 +20,16 @@ RSpec.describe Notifications::ConsolidationPlanner do
expect(saved_like.notification_type).to eq(Notification.types[:liked])
end
it "consolidates multiple notifications into a new one" do
it "does nothing when it hasn't passed the consolidation threshold yet for links" do
notification = build_notification(:linked, { display_username: link_user })
saved_link = planner.consolidate_or_save!(notification)
expect(saved_link.id).to be_present
expect(saved_link.notification_type).to eq(Notification.types[:linked])
end
it "consolidates multiple like notifications into a new one" do
first_notification =
Fabricate(
:notification,
@ -37,7 +47,25 @@ RSpec.describe Notifications::ConsolidationPlanner do
expect(data["count"]).to eq(threshold + 1)
end
it "updates the notification if we already consolidated it" do
it "consolidates multiple link notifications into a new one" do
first_notification =
Fabricate(
:notification,
user: user,
notification_type: Notification.types[:linked],
data: { display_username: link_user }.to_json,
)
notification = build_notification(:linked, { display_username: link_user })
consolidated_link = planner.consolidate_or_save!(notification)
expect(consolidated_link.id).not_to eq(first_notification.id)
expect(consolidated_link.notification_type).to eq(Notification.types[:linked_consolidated])
data = JSON.parse(consolidated_link.data)
expect(data["count"]).to eq(threshold + 1)
end
it "updates the like notification if we already consolidated it" do
count = 5
Fabricate(
:notification,
@ -53,6 +81,23 @@ RSpec.describe Notifications::ConsolidationPlanner do
data = JSON.parse(updated.data)
expect(data["count"]).to eq(count + 1)
end
it "updates the link notification if we already consolidated it" do
count = 5
Fabricate(
:notification,
user: user,
notification_type: Notification.types[:linked_consolidated],
data: { count: count, display_username: link_user }.to_json,
)
notification = build_notification(:linked, { display_username: link_user })
updated = planner.consolidate_or_save!(notification)
expect { notification.reload }.to raise_error(ActiveRecord::RecordNotFound)
data = JSON.parse(updated.data)
expect(data["count"]).to eq(count + 1)
end
end
def build_notification(type_sym, data)