From ce80f0600eb19bfb1d89527c99523052b150368e Mon Sep 17 00:00:00 2001 From: Shadowfacts Date: Thu, 26 Aug 2021 19:20:27 -0400 Subject: [PATCH] Make notification handling marginally less painful --- lib/clacks/activity.ex | 15 +++-- lib/clacks/inbox.ex | 4 +- lib/clacks/notification.ex | 37 +++++++---- lib/clacks/object.ex | 2 +- lib/clacks/timeline.ex | 62 ++++++++++++------- .../templates/frontend/notifications.html.eex | 16 ++--- .../20210826220029_more_belongs_to.exs | 14 +++++ 7 files changed, 102 insertions(+), 48 deletions(-) create mode 100644 priv/repo/migrations/20210826220029_more_belongs_to.exs diff --git a/lib/clacks/activity.ex b/lib/clacks/activity.ex index 7a63798..4873a6f 100644 --- a/lib/clacks/activity.ex +++ b/lib/clacks/activity.ex @@ -12,8 +12,8 @@ defmodule Clacks.Activity do schema "activities" do field :data, :map field :local, :boolean - field :actor, :string + belongs_to :actor, Clacks.Actor, foreign_key: :actor_ap_id, references: :ap_id, type: :string has_one :object, Clacks.Object, on_delete: :nothing, foreign_key: :id timestamps() @@ -21,8 +21,8 @@ defmodule Clacks.Activity do def changeset(%__MODULE__{} = schema, attrs) do schema - |> cast(attrs, [:data, :local, :actor]) - |> validate_required([:data, :local, :actor]) + |> cast(attrs, [:data, :local, :actor_ap_id]) + |> validate_required([:data, :local, :actor_ap_id]) end @spec changeset_for_creating(activity :: map(), local :: boolean()) :: Ecto.Changeset.t() @@ -30,7 +30,7 @@ defmodule Clacks.Activity do changeset(%__MODULE__{}, %{ data: activity, local: local, - actor: activity["actor"] + actor_ap_id: activity["actor"] }) end @@ -69,6 +69,8 @@ defmodule Clacks.Activity do fragment("COALESCE(?->'object'->>'id', ?->>'object')", a.data, a.data) == ^object_id ) |> preload_object() + # todo: may not need to preload actor by default + |> preload_actor() |> Repo.one() end @@ -91,6 +93,11 @@ defmodule Clacks.Activity do |> preload([_a, object: object], object: object) end + @spec preload_actor(Ecto.Queryable.t()) :: Ecto.Query.t() + def preload_actor(query) do + preload(query, :actor) + end + @spec fetch(ap_id :: String.t()) :: t() | nil def fetch(ap_id) do case Clacks.ActivityPub.Fetcher.fetch_activity(ap_id) do diff --git a/lib/clacks/inbox.ex b/lib/clacks/inbox.ex index dbcb3e6..30efc77 100644 --- a/lib/clacks/inbox.ex +++ b/lib/clacks/inbox.ex @@ -4,9 +4,11 @@ defmodule Clacks.Inbox do defp store_activity(%{"actor" => actor, "id" => ap_id} = activity, local \\ false) when is_binary(actor) do + # remove the embedded object (if there is one) from the activity activity_without_embedded_object = case Map.get(activity, "object") do %{"id" => object_id} -> + # todo: this assumes we already have stored the object Map.put(activity, "object", object_id) _ -> @@ -17,7 +19,7 @@ defmodule Clacks.Inbox do Activity.changeset(Activity.get_cached_by_ap_id(ap_id) || %Activity{}, %{ data: activity_without_embedded_object, local: local, - actor: actor + actor_ap_id: actor }) case Repo.insert_or_update(changeset) do diff --git a/lib/clacks/notification.ex b/lib/clacks/notification.ex index 0991c8f..18204d7 100644 --- a/lib/clacks/notification.ex +++ b/lib/clacks/notification.ex @@ -12,30 +12,37 @@ defmodule Clacks.Notification do belongs_to :user, Clacks.User belongs_to :activity, Clacks.Activity, type: FlakeId.Ecto.Type + belongs_to :referenced_activity, Clacks.Activity, type: FlakeId.Ecto.Type timestamps() end def changeset(%__MODULE__{} = schema, attrs) do schema - |> cast(attrs, [:type, :user_id, :activity_id]) - |> validate_required([:type, :user_id]) + |> cast(attrs, [:type, :user_id, :activity_id, :referenced_activity_id]) + |> validate_required([:type, :user_id, :activity_id]) |> validate_inclusion(:type, @valid_types) end - @spec create(type :: String.t(), activity :: Activity.t(), actor :: Actor.t()) :: + @spec create( + type :: String.t(), + activity :: Activity.t(), + actor :: Actor.t(), + referenced_activity :: Activity.t() | nil + ) :: {:ok, Notification.t()} | {:error, any()} - def create(type, _, _) when not (type in @valid_types) do + def create(type, _, _, _) when not (type in @valid_types) do {:error, "invalid notification type '#{type}'"} end - def create(type, activity, actor) do + def create(type, activity, actor, referenced_activity) do changeset = changeset(%__MODULE__{}, %{ type: type, user_id: actor.user_id, - activity_id: activity.id + activity_id: activity.id, + referenced_activity_id: if(referenced_activity, do: referenced_activity.id, else: nil) }) Repo.insert(changeset) @@ -48,7 +55,7 @@ defmodule Clacks.Notification do ) do case Actor.get_cached_by_ap_id(followee_ap_id) do %Actor{local: true} = followee -> - create("follow", activity, followee) + create("follow", activity, followee, nil) _ -> :ok @@ -62,7 +69,7 @@ defmodule Clacks.Notification do Enum.each(tags, fn %{"href" => mentioned_actor_id} -> case Actor.get_cached_by_ap_id(mentioned_actor_id) do %Actor{local: true} = mentioned_actor -> - create("mention", activity, mentioned_actor) + create("mention", activity, mentioned_actor, nil) _ -> :ok @@ -74,11 +81,15 @@ defmodule Clacks.Notification do %Activity{data: %{"type" => type, "object" => original_object_ap_id}} = activity ) when type in ["Announce", "Like"] do - with %Activity{local: true, actor: local_actor_id} <- - Activity.get_by_object_ap_id(original_object_ap_id, "Create"), - %Actor{local: true} = original_activity_actor <- - Actor.get_cached_by_ap_id(local_actor_id) do - create(String.downcase(type), activity, original_activity_actor) + with %Activity{local: true, actor: %Actor{local: true} = original_activity_actor} = + original_activity <- + Activity.get_by_object_ap_id(original_object_ap_id, "Create") do + create( + String.downcase(type), + activity, + original_activity_actor, + original_activity + ) else _ -> :ok diff --git a/lib/clacks/object.ex b/lib/clacks/object.ex index ce27a00..47c26fc 100644 --- a/lib/clacks/object.ex +++ b/lib/clacks/object.ex @@ -55,7 +55,7 @@ defmodule Clacks.Object do @spec fetch(url :: String.t(), synthesize_create :: boolean(), return :: :object | :activity) :: t() | Activity.t() | nil - def fetch(url, synthesize_create \\ true, return \\ :object) do + def fetch(url, synthesize_create \\ true, return \\ :object) when is_binary(url) do case Clacks.ActivityPub.Fetcher.fetch_object(url) do nil -> nil diff --git a/lib/clacks/timeline.ex b/lib/clacks/timeline.ex index abef945..8d8f518 100644 --- a/lib/clacks/timeline.ex +++ b/lib/clacks/timeline.ex @@ -1,5 +1,5 @@ defmodule Clacks.Timeline do - alias Clacks.{Repo, Actor, Activity, User, Notification} + alias Clacks.{Repo, Actor, Activity, Object, User, Notification} import Clacks.Paginator import Ecto.Query @@ -82,34 +82,54 @@ defmodule Clacks.Timeline do |> Repo.all() end - @spec notifications(actor :: Actor.t(), params :: map()) :: [ - {:follow, activity :: Activity.t(), actor :: Actor.t()} - | {:mention, activity :: Activity.t(), actor :: Actor.t()} - | {:announce, announce :: Activity.t(), announce_actor :: Actor.t(), - activity :: Activity.t(), actor :: Actor.t()} - | {:like, like :: Activity.t(), like_actor :: Actor.t(), activity :: Activity.t(), - actor :: Actor.t()} - ] + @spec notifications(Actor.t(), map()) :: [Notification.t()] def notifications(actor, params) do Notification |> where([n], n.user_id == ^actor.user_id) - |> join(:inner, [n], activity in Activity, on: activity.id == n.activity_id) - |> join(:inner, [n, activity], actor in Actor, on: activity.actor == actor.ap_id) - |> join(:left, [n, activity, actor], other in Activity, - on: - n.type in ["announce", "like"] and fragment("?->>'type'", other.data) == "Create" and - fragment("?->>'object'", activity.data) == fragment("?->'object'->>'id'", other.data) - ) |> paginate(params) |> limit(^Map.get(params, "limit", 20)) + |> join(:left, [n], a in Activity, + as: :activity, + on: a.id == n.activity_id + ) + |> join(:left, [n], a in Activity, + as: :referenced_activity, + on: a.id == n.referenced_activity_id + ) + |> join(:left, [n, activity: a], o in Object, + as: :object, + on: + fragment("?->>'id'", o.data) == + fragment("COALESCE(?->'object'->>'id', ?->>'object')", a.data, a.data) + ) + |> join(:left, [n, referenced_activity: a], o in Object, + as: :referenced_object, + on: + fragment("?->>'id'", o.data) == + fragment("COALESCE(?->'object'->>'id', ?->>'object')", a.data, a.data) + ) + |> join(:left, [n, activity: activity], actor in Actor, + as: :activity_actor, + on: activity.actor_ap_id == actor.ap_id + ) + # note: we shouldn't need to load the actor for the referenced_activity here, + # because notifications for a given actor always reference activities from that same actor |> select( - [notification, activity, actor, original_activity], - {notification, activity, actor, original_activity} + [ + n, + activity: activity, + referenced_activity: referenced_activity, + object: object, + referenced_object: referenced_object, + activity_actor: activity_actor + ], + %Notification{ + n + | activity: %Activity{activity | object: object, actor: activity_actor}, + referenced_activity: %Activity{referenced_activity | object: referenced_object} + } ) |> Repo.all() - |> Enum.map(fn {notification, activity, actor, original_activity} -> - {String.to_existing_atom(notification.type), activity, actor, original_activity} - end) end defp restrict_to_actor(query, actor_id) do diff --git a/lib/clacks_web/templates/frontend/notifications.html.eex b/lib/clacks_web/templates/frontend/notifications.html.eex index 331d1e1..b0ebd5a 100644 --- a/lib/clacks_web/templates/frontend/notifications.html.eex +++ b/lib/clacks_web/templates/frontend/notifications.html.eex @@ -11,17 +11,17 @@ <%= for notification <- @notifications do %>
  • <%= case notification do %> - <% {:like, like_activity, actor, original_activity} -> %> - <%= render "_action_status.html", class: "notification", conn: @conn, action: :like, action_activity: like_activity, action_actor: actor, original_activity: original_activity, original_note: original_activity.data["object"], original_actor: @current_user.actor %> + <% %{type: "like", activity: %{actor: like_actor} = like_activity, referenced_activity: original_activity} -> %> + <%= render "_action_status.html", class: "notification", conn: @conn, action: :like, action_activity: like_activity, action_actor: like_actor, original_activity: original_activity, original_note: original_activity.object.data, original_actor: @current_user.actor %> - <% {:announce, announce_activity, actor, original_activity} -> %> - <%= render "_action_status.html", class: "notification", conn: @conn, action: :announce, action_activity: announce_activity, action_actor: actor, original_activity: original_activity, original_note: original_activity.data["object"], original_actor: @current_user.actor %> + <% %{type: "announce", activity: %{actor: announce_actor} = announce_activity, referenced_activity: original_activity} -> %> + <%= render "_action_status.html", class: "notification", conn: @conn, action: :announce, action_activity: announce_activity, action_actor: announce_actor, original_activity: original_activity, original_note: original_activity.object.data, original_actor: @current_user.actor %> - <% {:mention, mention_activity, actor, _} -> %> - <%= render "_status.html", class: "notification", conn: @conn, author: actor, status: mention_activity, note: mention_activity.data["object"] %> + <% %{type: "mention", activity: mention_activity} -> %> + <%= render "_status.html", class: "notification", conn: @conn, author: mention_activity.actor, status: mention_activity, note: mention_activity.object.data %> - <% {:follow, follow_activity, actor, _} -> %> - <%= render "_follow_notification.html", activity: follow_activity, actor: actor %> + <% %{type: "follow", activity: follow_activity} -> %> + <%= render "_follow_notification.html", activity: follow_activity, actor: follow_activity.actor %> <% end %>
    • <% end %> diff --git a/priv/repo/migrations/20210826220029_more_belongs_to.exs b/priv/repo/migrations/20210826220029_more_belongs_to.exs new file mode 100644 index 0000000..19b6d56 --- /dev/null +++ b/priv/repo/migrations/20210826220029_more_belongs_to.exs @@ -0,0 +1,14 @@ +defmodule Clacks.Repo.Migrations.MoreBelongsTo do + use Ecto.Migration + + def change do + alter table(:notifications) do + add :referenced_activity_id, references(:activities, type: :uuid) + end + + alter table(:activities) do + remove :actor + add :actor_ap_id, references(:actors, column: :ap_id, type: :string) + end + end +end