From 3d69807118d523e9ba8b15ebb6b5091037bd4fe9 Mon Sep 17 00:00:00 2001 From: Liam Date: Mon, 29 Jul 2024 19:13:08 -0400 Subject: [PATCH 1/4] Avoid creating notifications for user performing the action --- lib/philomena/comments.ex | 12 ++++------ lib/philomena/notifications.ex | 27 ++++++++++++++-------- lib/philomena/notifications/creator.ex | 32 ++++++++++++++++++-------- lib/philomena/posts.ex | 12 ++++------ lib/philomena/topics.ex | 7 ++++-- 5 files changed, 56 insertions(+), 34 deletions(-) diff --git a/lib/philomena/comments.ex b/lib/philomena/comments.ex index 281de6c0a..4498dca87 100644 --- a/lib/philomena/comments.ex +++ b/lib/philomena/comments.ex @@ -72,14 +72,12 @@ defmodule Philomena.Comments do end def perform_notify(comment_id) do - comment = get_comment!(comment_id) - - image = - comment - |> Repo.preload(:image) - |> Map.fetch!(:image) + comment = + comment_id + |> get_comment!() + |> Repo.preload([:user, :image]) - Notifications.create_image_comment_notification(image, comment) + Notifications.create_image_comment_notification(comment.user, comment.image, comment) end @doc """ diff --git a/lib/philomena/notifications.ex b/lib/philomena/notifications.ex index cbed74134..f441c24f8 100644 --- a/lib/philomena/notifications.ex +++ b/lib/philomena/notifications.ex @@ -78,7 +78,7 @@ defmodule Philomena.Notifications do """ def create_channel_live_notification(channel) do - Creator.create_single(ChannelSubscription, ChannelLiveNotification, :channel_id, channel) + Creator.create_single(ChannelSubscription, ChannelLiveNotification, nil, :channel_id, channel) end @doc """ @@ -86,14 +86,15 @@ defmodule Philomena.Notifications do ## Examples - iex> create_forum_post_notification(topic, post) + iex> create_forum_post_notification(user, topic, post) {:ok, 2} """ - def create_forum_post_notification(topic, post) do + def create_forum_post_notification(user, topic, post) do Creator.create_double( TopicSubscription, ForumPostNotification, + user, :topic_id, topic, :post_id, @@ -106,12 +107,12 @@ defmodule Philomena.Notifications do ## Examples - iex> create_forum_topic_notification(topic) + iex> create_forum_topic_notification(user, topic) {:ok, 2} """ - def create_forum_topic_notification(topic) do - Creator.create_single(ForumSubscription, ForumTopicNotification, :topic_id, topic) + def create_forum_topic_notification(user, topic) do + Creator.create_single(ForumSubscription, ForumTopicNotification, user, :topic_id, topic) end @doc """ @@ -124,7 +125,13 @@ defmodule Philomena.Notifications do """ def create_gallery_image_notification(gallery) do - Creator.create_single(GallerySubscription, GalleryImageNotification, :gallery_id, gallery) + Creator.create_single( + GallerySubscription, + GalleryImageNotification, + nil, + :gallery_id, + gallery + ) end @doc """ @@ -132,14 +139,15 @@ defmodule Philomena.Notifications do ## Examples - iex> create_image_comment_notification(image, comment) + iex> create_image_comment_notification(user, image, comment) {:ok, 2} """ - def create_image_comment_notification(image, comment) do + def create_image_comment_notification(user, image, comment) do Creator.create_double( ImageSubscription, ImageCommentNotification, + user, :image_id, image, :comment_id, @@ -160,6 +168,7 @@ defmodule Philomena.Notifications do Creator.create_double( ImageSubscription, ImageMergeNotification, + nil, :target_id, target, :source_id, diff --git a/lib/philomena/notifications/creator.ex b/lib/philomena/notifications/creator.ex index 5a04b7240..95dd25cd0 100644 --- a/lib/philomena/notifications/creator.ex +++ b/lib/philomena/notifications/creator.ex @@ -22,13 +22,13 @@ defmodule Philomena.Notifications.Creator do ## Example - iex> create_single(GallerySubscription, GalleryImageNotification, :gallery_id, gallery) + iex> create_single(GallerySubscription, GalleryImageNotification, nil, :gallery_id, gallery) {:ok, 2} """ - def create_single(subscription, notification, name, object) do + def create_single(subscription, notification, user, name, object) do subscription - |> create_notification_query(name, object) + |> create_notification_query(user, name, object) |> create_notification(notification, name) end @@ -45,6 +45,7 @@ defmodule Philomena.Notifications.Creator do iex> create_double( ...> ImageSubscription, ...> ImageCommentNotification, + ...> user, ...> :image_id, ...> image, ...> :comment_id, @@ -53,9 +54,9 @@ defmodule Philomena.Notifications.Creator do {:ok, 2} """ - def create_double(subscription, notification, name1, object1, name2, object2) do + def create_double(subscription, notification, user, name1, object1, name2, object2) do subscription - |> create_notification_query(name1, object1, name2, object2) + |> create_notification_query(user, name1, object1, name2, object2) |> create_notification(notification, name1) end @@ -80,10 +81,10 @@ defmodule Philomena.Notifications.Creator do # TODO: the following cannot be accomplished with a single query expression # due to this Ecto bug: https://github.com/elixir-ecto/ecto/issues/4430 - defp create_notification_query(subscription, name, object) do + defp create_notification_query(subscription, user, name, object) do now = DateTime.utc_now(:second) - from s in subscription, + from s in subscription_query(subscription, user), where: field(s, ^name) == ^object.id, select: %{ ^name => type(^object.id, :integer), @@ -94,10 +95,10 @@ defmodule Philomena.Notifications.Creator do } end - defp create_notification_query(subscription, name1, object1, name2, object2) do + defp create_notification_query(subscription, user, name1, object1, name2, object2) do now = DateTime.utc_now(:second) - from s in subscription, + from s in subscription_query(subscription, user), where: field(s, ^name1) == ^object1.id, select: %{ ^name1 => type(^object1.id, :integer), @@ -109,6 +110,19 @@ defmodule Philomena.Notifications.Creator do } end + defp subscription_query(subscription, user) do + case user do + %{id: user_id} -> + # Avoid sending notifications to the user which performed the action. + from s in subscription, + where: s.user_id != ^user_id + + _ -> + # When not created by a user, send notifications to all subscribers. + subscription + end + end + defp create_notification(query, notification, name) do {count, nil} = Repo.insert_all( diff --git a/lib/philomena/posts.ex b/lib/philomena/posts.ex index 2de6cfbbb..af29c2e31 100644 --- a/lib/philomena/posts.ex +++ b/lib/philomena/posts.ex @@ -121,14 +121,12 @@ defmodule Philomena.Posts do end def perform_notify(post_id) do - post = get_post!(post_id) + post = + post_id + |> get_post!() + |> Repo.preload([:user, :topic]) - topic = - post - |> Repo.preload(:topic) - |> Map.fetch!(:topic) - - Notifications.create_forum_post_notification(topic, post) + Notifications.create_forum_post_notification(post.user, post.topic, post) end @doc """ diff --git a/lib/philomena/topics.ex b/lib/philomena/topics.ex index 3ff4aba57..02524ea3b 100644 --- a/lib/philomena/topics.ex +++ b/lib/philomena/topics.ex @@ -91,9 +91,12 @@ defmodule Philomena.Topics do end def perform_notify([topic_id, _post_id]) do - topic = get_topic!(topic_id) + topic = + topic_id + |> get_topic!() + |> Repo.preload(:user) - Notifications.create_forum_topic_notification(topic) + Notifications.create_forum_topic_notification(topic.user, topic) end @doc """ From 3f832e89f610d252e3a144e62200e2e69120b4a2 Mon Sep 17 00:00:00 2001 From: Liam Date: Mon, 29 Jul 2024 19:27:14 -0400 Subject: [PATCH 2/4] Fix topic creation notifications --- lib/philomena/notifications.ex | 24 ++++++++++++++++++------ lib/philomena/notifications/creator.ex | 12 ++++++++---- 2 files changed, 26 insertions(+), 10 deletions(-) diff --git a/lib/philomena/notifications.ex b/lib/philomena/notifications.ex index f441c24f8..baf2f8beb 100644 --- a/lib/philomena/notifications.ex +++ b/lib/philomena/notifications.ex @@ -78,7 +78,13 @@ defmodule Philomena.Notifications do """ def create_channel_live_notification(channel) do - Creator.create_single(ChannelSubscription, ChannelLiveNotification, nil, :channel_id, channel) + Creator.create_single( + where(ChannelSubscription, channel_id: ^channel.id), + ChannelLiveNotification, + nil, + :channel_id, + channel + ) end @doc """ @@ -92,7 +98,7 @@ defmodule Philomena.Notifications do """ def create_forum_post_notification(user, topic, post) do Creator.create_double( - TopicSubscription, + where(TopicSubscription, topic_id: ^topic.id), ForumPostNotification, user, :topic_id, @@ -112,7 +118,13 @@ defmodule Philomena.Notifications do """ def create_forum_topic_notification(user, topic) do - Creator.create_single(ForumSubscription, ForumTopicNotification, user, :topic_id, topic) + Creator.create_single( + where(ForumSubscription, forum_id: ^topic.forum_id), + ForumTopicNotification, + user, + :topic_id, + topic + ) end @doc """ @@ -126,7 +138,7 @@ defmodule Philomena.Notifications do """ def create_gallery_image_notification(gallery) do Creator.create_single( - GallerySubscription, + where(GallerySubscription, gallery_id: ^gallery.id), GalleryImageNotification, nil, :gallery_id, @@ -145,7 +157,7 @@ defmodule Philomena.Notifications do """ def create_image_comment_notification(user, image, comment) do Creator.create_double( - ImageSubscription, + where(ImageSubscription, image_id: ^image.id), ImageCommentNotification, user, :image_id, @@ -166,7 +178,7 @@ defmodule Philomena.Notifications do """ def create_image_merge_notification(target, source) do Creator.create_double( - ImageSubscription, + where(ImageSubscription, image_id: ^target.id), ImageMergeNotification, nil, :target_id, diff --git a/lib/philomena/notifications/creator.ex b/lib/philomena/notifications/creator.ex index 95dd25cd0..ee4fec6a1 100644 --- a/lib/philomena/notifications/creator.ex +++ b/lib/philomena/notifications/creator.ex @@ -22,7 +22,13 @@ defmodule Philomena.Notifications.Creator do ## Example - iex> create_single(GallerySubscription, GalleryImageNotification, nil, :gallery_id, gallery) + iex> create_single( + ...> where(GallerySubscription, gallery_id: ^gallery.id), + ...> GalleryImageNotification, + ...> nil, + ...> :gallery_id, + ...> gallery + ...> ) {:ok, 2} """ @@ -43,7 +49,7 @@ defmodule Philomena.Notifications.Creator do ## Example iex> create_double( - ...> ImageSubscription, + ...> where(ImageSubscription, image_id: ^image.id), ...> ImageCommentNotification, ...> user, ...> :image_id, @@ -85,7 +91,6 @@ defmodule Philomena.Notifications.Creator do now = DateTime.utc_now(:second) from s in subscription_query(subscription, user), - where: field(s, ^name) == ^object.id, select: %{ ^name => type(^object.id, :integer), user_id: s.user_id, @@ -99,7 +104,6 @@ defmodule Philomena.Notifications.Creator do now = DateTime.utc_now(:second) from s in subscription_query(subscription, user), - where: field(s, ^name1) == ^object1.id, select: %{ ^name1 => type(^object1.id, :integer), ^name2 => type(^object2.id, :integer), From c30406bcca1a3905d0da7cecf4ec4bd4df61433c Mon Sep 17 00:00:00 2001 From: Liam Date: Mon, 29 Jul 2024 19:51:25 -0400 Subject: [PATCH 3/4] Fix notification dismissal --- assets/js/boorujs.js | 7 +++++++ .../templates/notification/_channel.html.slime | 4 ++-- .../templates/notification/_comment.html.slime | 4 ++-- .../templates/notification/_gallery.html.slime | 4 ++-- lib/philomena_web/templates/notification/_image.html.slime | 4 ++-- lib/philomena_web/templates/notification/_post.html.slime | 4 ++-- lib/philomena_web/templates/notification/_topic.html.slime | 4 ++-- 7 files changed, 19 insertions(+), 12 deletions(-) diff --git a/assets/js/boorujs.js b/assets/js/boorujs.js index 9d8a71c6b..86d9901bb 100644 --- a/assets/js/boorujs.js +++ b/assets/js/boorujs.js @@ -52,6 +52,13 @@ const actions = { addTag(document.querySelector(data.el.closest('[data-target]').dataset.target), data.el.dataset.tagName); }, + hideParent(data) { + const base = data.el.closest(data.value); + if (base) { + base.classList.add('hidden'); + } + }, + tab(data) { const block = data.el.parentNode.parentNode, newTab = $(`.block__tab[data-tab="${data.value}"]`), diff --git a/lib/philomena_web/templates/notification/_channel.html.slime b/lib/philomena_web/templates/notification/_channel.html.slime index 0c4c5b739..c22299db5 100644 --- a/lib/philomena_web/templates/notification/_channel.html.slime +++ b/lib/philomena_web/templates/notification/_channel.html.slime @@ -7,8 +7,8 @@ => pretty_time @notification.updated_at .flex.flex--centered.flex--no-wrap - a.button.button--separate-right title="Delete" href=~p"/channels/#{@notification.channel}/read" data-method="post" data-remote="true" + a.button.button--separate-right title="Delete" href=~p"/channels/#{@notification.channel}/read" data-method="post" data-remote="true" data-click-hideparent=".notification" i.fa.fa-trash - a.button title="Unsubscribe" href=~p"/channels/#{@notification.channel}/subscription" data-method="delete" data-remote="true" + a.button title="Unsubscribe" href=~p"/channels/#{@notification.channel}/subscription" data-method="delete" data-remote="true" data-click-hideparent=".notification" i.fa.fa-bell-slash diff --git a/lib/philomena_web/templates/notification/_comment.html.slime b/lib/philomena_web/templates/notification/_comment.html.slime index 4e9efeb6f..076ccb344 100644 --- a/lib/philomena_web/templates/notification/_comment.html.slime +++ b/lib/philomena_web/templates/notification/_comment.html.slime @@ -15,8 +15,8 @@ => pretty_time @notification.updated_at .flex.flex--centered.flex--no-wrap - a.button.button--separate-right title="Delete" href=~p"/images/#{image}/read" data-method="post" data-remote="true" + a.button.button--separate-right title="Delete" href=~p"/images/#{image}/read" data-method="post" data-remote="true" data-click-hideparent=".notification" i.fa.fa-trash - a.button title="Unsubscribe" href=~p"/images/#{image}/subscription" data-method="delete" data-remote="true" + a.button title="Unsubscribe" href=~p"/images/#{image}/subscription" data-method="delete" data-remote="true" data-click-hideparent=".notification" i.fa.fa-bell-slash diff --git a/lib/philomena_web/templates/notification/_gallery.html.slime b/lib/philomena_web/templates/notification/_gallery.html.slime index 0192b4495..8ef024d06 100644 --- a/lib/philomena_web/templates/notification/_gallery.html.slime +++ b/lib/philomena_web/templates/notification/_gallery.html.slime @@ -11,8 +11,8 @@ => pretty_time @notification.updated_at .flex.flex--centered.flex--no-wrap - a.button.button--separate-right title="Delete" href=~p"/galleries/#{gallery}/read" data-method="post" data-remote="true" + a.button.button--separate-right title="Delete" href=~p"/galleries/#{gallery}/read" data-method="post" data-remote="true" data-click-hideparent=".notification" i.fa.fa-trash - a.button title="Unsubscribe" href=~p"/galleries/#{gallery}/subscription" data-method="delete" data-remote="true" + a.button title="Unsubscribe" href=~p"/galleries/#{gallery}/subscription" data-method="delete" data-remote="true" data-click-hideparent=".notification" i.fa.fa-bell-slash diff --git a/lib/philomena_web/templates/notification/_image.html.slime b/lib/philomena_web/templates/notification/_image.html.slime index 01f72dd90..dcfad4ebf 100644 --- a/lib/philomena_web/templates/notification/_image.html.slime +++ b/lib/philomena_web/templates/notification/_image.html.slime @@ -17,8 +17,8 @@ => pretty_time @notification.updated_at .flex.flex--centered.flex--no-wrap - a.button.button--separate-right title="Delete" href=~p"/images/#{target}/read" data-method="post" data-remote="true" + a.button.button--separate-right title="Delete" href=~p"/images/#{target}/read" data-method="post" data-remote="true" data-click-hideparent=".notification" i.fa.fa-trash - a.button title="Unsubscribe" href=~p"/images/#{target}/subscription" data-method="delete" data-remote="true" + a.button title="Unsubscribe" href=~p"/images/#{target}/subscription" data-method="delete" data-remote="true" data-click-hideparent=".notification" i.fa.fa-bell-slash diff --git a/lib/philomena_web/templates/notification/_post.html.slime b/lib/philomena_web/templates/notification/_post.html.slime index bac4acb9f..f0dc0b660 100644 --- a/lib/philomena_web/templates/notification/_post.html.slime +++ b/lib/philomena_web/templates/notification/_post.html.slime @@ -12,8 +12,8 @@ => pretty_time @notification.updated_at .flex.flex--centered.flex--no-wrap - a.button.button--separate-right title="Delete" href=~p"/forums/#{topic.forum}/topics/#{topic}/read" data-method="post" data-remote="true" + a.button.button--separate-right title="Delete" href=~p"/forums/#{topic.forum}/topics/#{topic}/read" data-method="post" data-remote="true" data-click-hideparent=".notification" i.fa.fa-trash - a.button title="Unsubscribe" href=~p"/forums/#{topic.forum}/topics/#{topic}/subscription" data-method="delete" data-remote="true" + a.button title="Unsubscribe" href=~p"/forums/#{topic.forum}/topics/#{topic}/subscription" data-method="delete" data-remote="true" data-click-hideparent=".notification" i.fa.fa-bell-slash diff --git a/lib/philomena_web/templates/notification/_topic.html.slime b/lib/philomena_web/templates/notification/_topic.html.slime index cf2bd5df9..ff84ca3ea 100644 --- a/lib/philomena_web/templates/notification/_topic.html.slime +++ b/lib/philomena_web/templates/notification/_topic.html.slime @@ -16,8 +16,8 @@ => pretty_time @notification.updated_at .flex.flex--centered.flex--no-wrap - a.button.button--separate-right title="Delete" href=~p"/forums/#{forum}/topics/#{topic}/read" data-method="post" data-remote="true" + a.button.button--separate-right title="Delete" href=~p"/forums/#{forum}/topics/#{topic}/read" data-method="post" data-remote="true" data-click-hideparent=".notification" i.fa.fa-trash - a.button title="Unsubscribe" href=~p"/forums/#{forum}/subscription" data-method="delete" data-remote="true" + a.button title="Unsubscribe" href=~p"/forums/#{forum}/subscription" data-method="delete" data-remote="true" data-click-hideparent=".notification" i.fa.fa-bell-slash From c7f618d9dd5546ce0f36ddca7f5193173a096fd5 Mon Sep 17 00:00:00 2001 From: Liam Date: Mon, 29 Jul 2024 20:03:49 -0400 Subject: [PATCH 4/4] Clear notifications when subscription is removed --- lib/philomena/channels.ex | 1 + lib/philomena/galleries.ex | 1 + lib/philomena/images.ex | 1 + lib/philomena/subscriptions.ex | 14 ++++++++++++++ lib/philomena/topics.ex | 1 + .../templates/notification/_topic.html.slime | 3 --- 6 files changed, 18 insertions(+), 3 deletions(-) diff --git a/lib/philomena/channels.ex b/lib/philomena/channels.ex index 2ffd4ce46..efa6d47c8 100644 --- a/lib/philomena/channels.ex +++ b/lib/philomena/channels.ex @@ -12,6 +12,7 @@ defmodule Philomena.Channels do alias Philomena.Tags use Philomena.Subscriptions, + on_delete: :clear_channel_notification, id_name: :channel_id @doc """ diff --git a/lib/philomena/galleries.ex b/lib/philomena/galleries.ex index 4c76df354..c553ad3ef 100644 --- a/lib/philomena/galleries.ex +++ b/lib/philomena/galleries.ex @@ -19,6 +19,7 @@ defmodule Philomena.Galleries do alias Philomena.Images use Philomena.Subscriptions, + on_delete: :clear_gallery_notification, id_name: :gallery_id @doc """ diff --git a/lib/philomena/images.ex b/lib/philomena/images.ex index 92c02155a..4ebc215ab 100644 --- a/lib/philomena/images.ex +++ b/lib/philomena/images.ex @@ -39,6 +39,7 @@ defmodule Philomena.Images do alias Philomena.Users.User use Philomena.Subscriptions, + on_delete: :clear_image_notification, id_name: :image_id @doc """ diff --git a/lib/philomena/subscriptions.ex b/lib/philomena/subscriptions.ex index 8e67183f4..d0688107a 100644 --- a/lib/philomena/subscriptions.ex +++ b/lib/philomena/subscriptions.ex @@ -25,6 +25,18 @@ defmodule Philomena.Subscriptions do # For Philomena.Images, this yields :image_id field_name = Keyword.fetch!(opts, :id_name) + # Deletion callback + on_delete = + case Keyword.get(opts, :on_delete) do + nil -> + [] + + callback when is_atom(callback) -> + quote do + apply(__MODULE__, unquote(callback), [object, user]) + end + end + # For Philomena.Images, this yields Philomena.Images.Subscription subscription_module = Module.concat(__CALLER__.module, Subscription) @@ -100,6 +112,8 @@ defmodule Philomena.Subscriptions do """ def delete_subscription(object, user) do + unquote(on_delete) + Philomena.Subscriptions.delete_subscription( unquote(subscription_module), unquote(field_name), diff --git a/lib/philomena/topics.ex b/lib/philomena/topics.ex index 02524ea3b..e1e5af295 100644 --- a/lib/philomena/topics.ex +++ b/lib/philomena/topics.ex @@ -14,6 +14,7 @@ defmodule Philomena.Topics do alias Philomena.NotificationWorker use Philomena.Subscriptions, + on_delete: :clear_topic_notification, id_name: :topic_id @doc """ diff --git a/lib/philomena_web/templates/notification/_topic.html.slime b/lib/philomena_web/templates/notification/_topic.html.slime index ff84ca3ea..37f6786dc 100644 --- a/lib/philomena_web/templates/notification/_topic.html.slime +++ b/lib/philomena_web/templates/notification/_topic.html.slime @@ -18,6 +18,3 @@ .flex.flex--centered.flex--no-wrap a.button.button--separate-right title="Delete" href=~p"/forums/#{forum}/topics/#{topic}/read" data-method="post" data-remote="true" data-click-hideparent=".notification" i.fa.fa-trash - - a.button title="Unsubscribe" href=~p"/forums/#{forum}/subscription" data-method="delete" data-remote="true" data-click-hideparent=".notification" - i.fa.fa-bell-slash