From 9538575c975277c4c8d3b515951e1fb1a467ae3a Mon Sep 17 00:00:00 2001 From: Liam Date: Tue, 30 Jul 2024 13:19:23 -0400 Subject: [PATCH] Streamline notification broadcasts --- lib/philomena/notifications.ex | 106 ++++++++++++----------- lib/philomena/notifications/creator.ex | 115 +++++++++---------------- 2 files changed, 97 insertions(+), 124 deletions(-) diff --git a/lib/philomena/notifications.ex b/lib/philomena/notifications.ex index baf2f8be..1adae26e 100644 --- a/lib/philomena/notifications.ex +++ b/lib/philomena/notifications.ex @@ -4,6 +4,7 @@ defmodule Philomena.Notifications do """ import Ecto.Query, warn: false + alias Philomena.Repo alias Philomena.Channels.Subscription, as: ChannelSubscription alias Philomena.Forums.Subscription, as: ForumSubscription @@ -78,12 +79,11 @@ defmodule Philomena.Notifications do """ def create_channel_live_notification(channel) do - Creator.create_single( - where(ChannelSubscription, channel_id: ^channel.id), - ChannelLiveNotification, - nil, - :channel_id, - channel + Creator.broadcast_notification( + from: {ChannelSubscription, channel_id: channel.id}, + into: ChannelLiveNotification, + select: [channel_id: channel.id], + unique_key: :channel_id ) end @@ -97,14 +97,12 @@ defmodule Philomena.Notifications do """ def create_forum_post_notification(user, topic, post) do - Creator.create_double( - where(TopicSubscription, topic_id: ^topic.id), - ForumPostNotification, - user, - :topic_id, - topic, - :post_id, - post + Creator.broadcast_notification( + notification_author: user, + from: {TopicSubscription, topic_id: topic.id}, + into: ForumPostNotification, + select: [topic_id: topic.id, post_id: post.id], + unique_key: :topic_id ) end @@ -118,12 +116,12 @@ defmodule Philomena.Notifications do """ def create_forum_topic_notification(user, topic) do - Creator.create_single( - where(ForumSubscription, forum_id: ^topic.forum_id), - ForumTopicNotification, - user, - :topic_id, - topic + Creator.broadcast_notification( + notification_author: user, + from: {ForumSubscription, forum_id: topic.forum_id}, + into: ForumTopicNotification, + select: [topic_id: topic.id], + unique_key: :topic_id ) end @@ -137,12 +135,11 @@ defmodule Philomena.Notifications do """ def create_gallery_image_notification(gallery) do - Creator.create_single( - where(GallerySubscription, gallery_id: ^gallery.id), - GalleryImageNotification, - nil, - :gallery_id, - gallery + Creator.broadcast_notification( + from: {GallerySubscription, gallery_id: gallery.id}, + into: GalleryImageNotification, + select: [gallery_id: gallery.id], + unique_key: :gallery_id ) end @@ -156,14 +153,12 @@ defmodule Philomena.Notifications do """ def create_image_comment_notification(user, image, comment) do - Creator.create_double( - where(ImageSubscription, image_id: ^image.id), - ImageCommentNotification, - user, - :image_id, - image, - :comment_id, - comment + Creator.broadcast_notification( + notification_author: user, + from: {ImageSubscription, image_id: image.id}, + into: ImageCommentNotification, + select: [image_id: image.id, comment_id: comment.id], + unique_key: :image_id ) end @@ -177,14 +172,11 @@ defmodule Philomena.Notifications do """ def create_image_merge_notification(target, source) do - Creator.create_double( - where(ImageSubscription, image_id: ^target.id), - ImageMergeNotification, - nil, - :target_id, - target, - :source_id, - source + Creator.broadcast_notification( + from: {ImageSubscription, image_id: target.id}, + into: ImageMergeNotification, + select: [target_id: target.id, source_id: source.id], + unique_key: :target_id ) end @@ -201,7 +193,7 @@ defmodule Philomena.Notifications do def clear_channel_live_notification(channel, user) do ChannelLiveNotification |> where(channel_id: ^channel.id) - |> Creator.clear(user) + |> delete_all_for_user(user) end @doc """ @@ -217,7 +209,7 @@ defmodule Philomena.Notifications do def clear_forum_post_notification(topic, user) do ForumPostNotification |> where(topic_id: ^topic.id) - |> Creator.clear(user) + |> delete_all_for_user(user) end @doc """ @@ -233,7 +225,7 @@ defmodule Philomena.Notifications do def clear_forum_topic_notification(topic, user) do ForumTopicNotification |> where(topic_id: ^topic.id) - |> Creator.clear(user) + |> delete_all_for_user(user) end @doc """ @@ -249,7 +241,7 @@ defmodule Philomena.Notifications do def clear_gallery_image_notification(gallery, user) do GalleryImageNotification |> where(gallery_id: ^gallery.id) - |> Creator.clear(user) + |> delete_all_for_user(user) end @doc """ @@ -265,7 +257,7 @@ defmodule Philomena.Notifications do def clear_image_comment_notification(image, user) do ImageCommentNotification |> where(image_id: ^image.id) - |> Creator.clear(user) + |> delete_all_for_user(user) end @doc """ @@ -281,6 +273,24 @@ defmodule Philomena.Notifications do def clear_image_merge_notification(image, user) do ImageMergeNotification |> where(target_id: ^image.id) - |> Creator.clear(user) + |> delete_all_for_user(user) + end + + # + # Clear all unread notifications using the given query. + # + # Returns `{:ok, count}`, where `count` is the number of affected rows. + # + defp delete_all_for_user(query, user) do + if user do + {count, nil} = + query + |> where(user_id: ^user.id) + |> Repo.delete_all() + + {:ok, count} + else + {:ok, 0} + end end end diff --git a/lib/philomena/notifications/creator.ex b/lib/philomena/notifications/creator.ex index ee4fec6a..1ceec14f 100644 --- a/lib/philomena/notifications/creator.ex +++ b/lib/philomena/notifications/creator.ex @@ -1,98 +1,61 @@ defmodule Philomena.Notifications.Creator do @moduledoc """ Internal notifications creation logic. - - Supports two formats for notification creation: - - Key-only (`create_single/4`): The object's id is the only other component inserted. - - Non-key (`create_double/6`): The object's id plus another object's id are inserted. - - See the respective documentation for each function for more details. """ import Ecto.Query, warn: false alias Philomena.Repo @doc """ - Propagate notifications for a notification table type containing a single reference column. - - The single reference column (`name`, `object`) is also part of the unique key for the table, - and is used to select which object to act on. + Propagate notifications for a notification table type. Returns `{:ok, count}`, where `count` is the number of affected rows. - ## Example + ## Examples - iex> create_single( - ...> where(GallerySubscription, gallery_id: ^gallery.id), - ...> GalleryImageNotification, - ...> nil, - ...> :gallery_id, - ...> gallery + iex> broadcast_notification( + ...> from: {GallerySubscription, gallery_id: gallery.id}, + ...> into: GalleryImageNotification, + ...> select: [gallery_id: gallery.id], + ...> unique_key: :gallery_id + ...> ) + {:ok, 2} + + iex> broadcast_notification( + ...> notification_author: user, + ...> from: {ImageSubscription, image_id: image.id}, + ...> into: ImageCommentNotification, + ...> select: [image_id: image.id, comment_id: comment.id], + ...> unique_key: :image_id ...> ) {:ok, 2} """ - def create_single(subscription, notification, user, name, object) do - subscription - |> create_notification_query(user, name, object) - |> create_notification(notification, name) - end + def broadcast_notification(opts) do + opts = Keyword.validate!(opts, [:notification_author, :from, :into, :select, :unique_key]) - @doc """ - Propagate notifications for a notification table type containing two reference columns. + notification_author = Keyword.get(opts, :notification_author, nil) + {subscription_schema, filters} = Keyword.fetch!(opts, :from) + notification_schema = Keyword.fetch!(opts, :into) + select_keywords = Keyword.fetch!(opts, :select) + unique_key = Keyword.fetch!(opts, :unique_key) - The first reference column (`name1`, `object1`) is also part of the unique key for the table, - and is used to select which object to act on. - - Returns `{:ok, count}`, where `count` is the number of affected rows. - - ## Example - - iex> create_double( - ...> where(ImageSubscription, image_id: ^image.id), - ...> ImageCommentNotification, - ...> user, - ...> :image_id, - ...> image, - ...> :comment_id, - ...> comment - ...> ) - {:ok, 2} - - """ - def create_double(subscription, notification, user, name1, object1, name2, object2) do - subscription - |> create_notification_query(user, name1, object1, name2, object2) - |> create_notification(notification, name1) - end - - @doc """ - Clear all unread notifications using the given query. - - Returns `{:ok, count}`, where `count` is the number of affected rows. - """ - def clear(query, user) do - if user do - {count, nil} = - query - |> where(user_id: ^user.id) - |> Repo.delete_all() - - {:ok, count} - else - {:ok, 0} - end + subscription_schema + |> subscription_query(notification_author) + |> where(^filters) + |> convert_to_notification(select_keywords) + |> insert_notifications(notification_schema, unique_key) end # 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, user, name, object) do + defp convert_to_notification(subscription, [{name, object_id}]) do now = DateTime.utc_now(:second) - from s in subscription_query(subscription, user), + from s in subscription, select: %{ - ^name => type(^object.id, :integer), + ^name => type(^object_id, :integer), user_id: s.user_id, created_at: ^now, updated_at: ^now, @@ -100,13 +63,13 @@ defmodule Philomena.Notifications.Creator do } end - defp create_notification_query(subscription, user, name1, object1, name2, object2) do + defp convert_to_notification(subscription, [{name1, object_id1}, {name2, object_id2}]) do now = DateTime.utc_now(:second) - from s in subscription_query(subscription, user), + from s in subscription, select: %{ - ^name1 => type(^object1.id, :integer), - ^name2 => type(^object2.id, :integer), + ^name1 => type(^object_id1, :integer), + ^name2 => type(^object_id2, :integer), user_id: s.user_id, created_at: ^now, updated_at: ^now, @@ -114,8 +77,8 @@ defmodule Philomena.Notifications.Creator do } end - defp subscription_query(subscription, user) do - case user do + defp subscription_query(subscription, notification_author) do + case notification_author do %{id: user_id} -> # Avoid sending notifications to the user which performed the action. from s in subscription, @@ -127,13 +90,13 @@ defmodule Philomena.Notifications.Creator do end end - defp create_notification(query, notification, name) do + defp insert_notifications(query, notification, unique_key) do {count, nil} = Repo.insert_all( notification, query, on_conflict: {:replace_all_except, [:created_at]}, - conflict_target: [name, :user_id] + conflict_target: [unique_key, :user_id] ) {:ok, count}