From 4beb3ce5c57e6caf03365d32078ea58fd0c93434 Mon Sep 17 00:00:00 2001 From: Maksim Pechnikov Date: Mon, 18 Nov 2019 09:44:08 +0300 Subject: [PATCH 1/7] /api/v1/favourites: added sorting for activites by adds to favorites --- lib/pleroma/web/activity_pub/activity_pub.ex | 39 +++++++++++++++++++ .../controllers/status_controller.ex | 10 +---- 2 files changed, 40 insertions(+), 9 deletions(-) diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index d0c014e9d..95d97615c 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -1055,6 +1055,45 @@ def fetch_activities(recipients, opts \\ %{}, pagination \\ :keyset) do |> maybe_update_cc(list_memberships, opts["user"]) end + @doc """ + Fetch favorites activities of user with order by sort adds to favorites + """ + @spec fetch_favourites(list(String.t()), User.t(), map(), atom()) :: list(Activity.t()) + def fetch_favourites(recipients, user, params \\ %{}, pagination \\ :keyset) do + opts = + %{ + "type" => "Create", + "favorited_by" => user.ap_id, + "blocking_user" => user + } + |> Map.merge(params) + + recipients + |> fetch_activities_query(opts) + |> order_by_favourites(user) + |> Pagination.fetch_paginated(opts, pagination) + end + + # sorts by adds to favorites + # + @spec order_by_favourites(Ecto.Query.t(), User.t()) :: Ecto.Query.t() + defp order_by_favourites(query, user) do + join(query, :inner, [activity, object], a1 in Activity, + on: + fragment( + "(?->>'id') = COALESCE(?->'object'->>'id', ?->>'object') AND (?->>'type' = 'Like') AND (?.actor = ?)", + object.data, + a1.data, + a1.data, + a1.data, + a1, + ^user.ap_id + ), + as: :like_activity + ) + |> order_by([_, _, like_activity], desc: like_activity.updated_at) + end + defp maybe_update_cc(activities, list_memberships, %User{ap_id: user_ap_id}) when is_list(list_memberships) and length(list_memberships) > 0 do Enum.map(activities, fn diff --git a/lib/pleroma/web/mastodon_api/controllers/status_controller.ex b/lib/pleroma/web/mastodon_api/controllers/status_controller.ex index 74b223cf4..cdc4cec9c 100644 --- a/lib/pleroma/web/mastodon_api/controllers/status_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/status_controller.ex @@ -346,15 +346,7 @@ def context(%{assigns: %{user: user}} = conn, %{"id" => id}) do @doc "GET /api/v1/favourites" def favourites(%{assigns: %{user: user}} = conn, params) do - params = - params - |> Map.put("type", "Create") - |> Map.put("favorited_by", user.ap_id) - |> Map.put("blocking_user", user) - - activities = - ActivityPub.fetch_activities([], params) - |> Enum.reverse() + activities = ActivityPub.fetch_favourites([], user, Map.take(params, ["limit"])) conn |> add_link_headers(activities) From 9da4c88b49b4ccdc1eac90e04e9d8948fd943f37 Mon Sep 17 00:00:00 2001 From: Maksim Pechnikov Date: Mon, 18 Nov 2019 10:00:48 +0300 Subject: [PATCH 2/7] fix test --- lib/pleroma/pagination.ex | 3 +++ .../web/mastodon_api/controllers/status_controller.ex | 7 ++++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/lib/pleroma/pagination.ex b/lib/pleroma/pagination.ex index 9d279fba7..c77ba78bb 100644 --- a/lib/pleroma/pagination.ex +++ b/lib/pleroma/pagination.ex @@ -13,6 +13,9 @@ defmodule Pleroma.Pagination do alias Pleroma.Repo @default_limit 20 + @page_keys ["max_id", "min_id", "limit", "since_id", "order"] + + def page_keys, do: @page_keys def fetch_paginated(query, params, type \\ :keyset) diff --git a/lib/pleroma/web/mastodon_api/controllers/status_controller.ex b/lib/pleroma/web/mastodon_api/controllers/status_controller.ex index cdc4cec9c..e11bee383 100644 --- a/lib/pleroma/web/mastodon_api/controllers/status_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/status_controller.ex @@ -346,7 +346,12 @@ def context(%{assigns: %{user: user}} = conn, %{"id" => id}) do @doc "GET /api/v1/favourites" def favourites(%{assigns: %{user: user}} = conn, params) do - activities = ActivityPub.fetch_favourites([], user, Map.take(params, ["limit"])) + activities = + ActivityPub.fetch_favourites( + [], + user, + Map.take(params, Pleroma.Pagination.page_keys()) + ) conn |> add_link_headers(activities) From 0937895182f167966872a3347098f78efe23dde9 Mon Sep 17 00:00:00 2001 From: Maksim Pechnikov Date: Mon, 18 Nov 2019 16:56:25 +0300 Subject: [PATCH 3/7] updated fetch_favorites --- lib/pleroma/object.ex | 17 ++++ lib/pleroma/pagination.ex | 80 +++++++++++-------- lib/pleroma/web/activity_pub/activity_pub.ex | 44 +++------- .../controllers/status_controller.ex | 1 - 4 files changed, 74 insertions(+), 68 deletions(-) diff --git a/lib/pleroma/object.ex b/lib/pleroma/object.ex index d9b41d710..91cb9941f 100644 --- a/lib/pleroma/object.ex +++ b/lib/pleroma/object.ex @@ -23,6 +23,23 @@ defmodule Pleroma.Object do timestamps() end + def with_joined_activity(query, activity_type \\ "Create", join_type \\ :inner) do + object_position = Map.get(query.aliases, :object, 0) + + join(query, join_type, [{object, object_position}], a in Activity, + on: + fragment( + "COALESCE(?->'object'->>'id', ?->>'object') = (? ->> 'id') AND (?->>'type' = ?) ", + a.data, + a.data, + object.data, + a.data, + ^activity_type + ), + as: :object_activity + ) + end + def create(data) do Object.change(%Object{}, %{data: data}) |> Repo.insert() diff --git a/lib/pleroma/pagination.ex b/lib/pleroma/pagination.ex index c77ba78bb..243f1a329 100644 --- a/lib/pleroma/pagination.ex +++ b/lib/pleroma/pagination.ex @@ -17,59 +17,59 @@ defmodule Pleroma.Pagination do def page_keys, do: @page_keys - def fetch_paginated(query, params, type \\ :keyset) + def fetch_paginated(query, params, type \\ :keyset, table_binding \\ nil) - def fetch_paginated(query, %{"total" => true} = params, :keyset) do + def fetch_paginated(query, %{"total" => true} = params, :keyset, table_binding) do total = Repo.aggregate(query, :count, :id) %{ total: total, - items: fetch_paginated(query, Map.drop(params, ["total"]), :keyset) + items: fetch_paginated(query, Map.drop(params, ["total"]), :keyset, table_binding) } end - def fetch_paginated(query, params, :keyset) do + def fetch_paginated(query, params, :keyset, table_binding) do options = cast_params(params) query - |> paginate(options, :keyset) + |> paginate(options, :keyset, table_binding) |> Repo.all() |> enforce_order(options) end - def fetch_paginated(query, %{"total" => true} = params, :offset) do + def fetch_paginated(query, %{"total" => true} = params, :offset, table_binding) do total = Repo.aggregate(query, :count, :id) %{ total: total, - items: fetch_paginated(query, Map.drop(params, ["total"]), :offset) + items: fetch_paginated(query, Map.drop(params, ["total"]), :offset, table_binding) } end - def fetch_paginated(query, params, :offset) do + def fetch_paginated(query, params, :offset, table_binding) do options = cast_params(params) query - |> paginate(options, :offset) + |> paginate(options, :offset, table_binding) |> Repo.all() end - def paginate(query, options, method \\ :keyset) + def paginate(query, options, method \\ :keyset, table_binding \\ nil) - def paginate(query, options, :keyset) do + def paginate(query, options, :keyset, table_binding) do query - |> restrict(:min_id, options) - |> restrict(:since_id, options) - |> restrict(:max_id, options) - |> restrict(:order, options) - |> restrict(:limit, options) + |> restrict(:min_id, options, table_binding) + |> restrict(:since_id, options, table_binding) + |> restrict(:max_id, options, table_binding) + |> restrict(:order, options, table_binding) + |> restrict(:limit, options, table_binding) end - def paginate(query, options, :offset) do + def paginate(query, options, :offset, table_binding) do query - |> restrict(:order, options) - |> restrict(:offset, options) - |> restrict(:limit, options) + |> restrict(:order, options, table_binding) + |> restrict(:offset, options, table_binding) + |> restrict(:limit, options, table_binding) end defp cast_params(params) do @@ -91,38 +91,46 @@ defp cast_params(params) do changeset.changes end - defp restrict(query, :min_id, %{min_id: min_id}) do - where(query, [q], q.id > ^min_id) + defp restrict(query, :min_id, %{min_id: min_id}, table_binding) do + where(query, [{q, table_position(query, table_binding)}], q.id > ^min_id) end - defp restrict(query, :since_id, %{since_id: since_id}) do - where(query, [q], q.id > ^since_id) + defp restrict(query, :since_id, %{since_id: since_id}, table_binding) do + where(query, [{q, table_position(query, table_binding)}], q.id > ^since_id) end - defp restrict(query, :max_id, %{max_id: max_id}) do - where(query, [q], q.id < ^max_id) + defp restrict(query, :max_id, %{max_id: max_id}, table_binding) do + where(query, [{q, table_position(query, table_binding)}], q.id < ^max_id) end - defp restrict(query, :order, %{min_id: _}) do - order_by(query, [u], fragment("? asc nulls last", u.id)) + defp restrict(query, :order, %{min_id: _}, table_binding) do + order_by( + query, + [{u, table_position(query, table_binding)}], + fragment("? asc nulls last", u.id) + ) end - defp restrict(query, :order, _options) do - order_by(query, [u], fragment("? desc nulls last", u.id)) + defp restrict(query, :order, _options, table_binding) do + order_by( + query, + [{u, table_position(query, table_binding)}], + fragment("? desc nulls last", u.id) + ) end - defp restrict(query, :offset, %{offset: offset}) do + defp restrict(query, :offset, %{offset: offset}, _table_binding) do offset(query, ^offset) end - defp restrict(query, :limit, options) do + defp restrict(query, :limit, options, _table_binding) do limit = Map.get(options, :limit, @default_limit) query |> limit(^limit) end - defp restrict(query, _, _), do: query + defp restrict(query, _, _, _), do: query defp enforce_order(result, %{min_id: _}) do result @@ -130,4 +138,10 @@ defp enforce_order(result, %{min_id: _}) do end defp enforce_order(result, _), do: result + + defp table_position(%Ecto.Query{} = query, binding_name) do + Map.get(query.aliases, binding_name, 0) + end + + defp table_position(_, _), do: 0 end diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index 95d97615c..e452278f0 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -1058,40 +1058,16 @@ def fetch_activities(recipients, opts \\ %{}, pagination \\ :keyset) do @doc """ Fetch favorites activities of user with order by sort adds to favorites """ - @spec fetch_favourites(list(String.t()), User.t(), map(), atom()) :: list(Activity.t()) - def fetch_favourites(recipients, user, params \\ %{}, pagination \\ :keyset) do - opts = - %{ - "type" => "Create", - "favorited_by" => user.ap_id, - "blocking_user" => user - } - |> Map.merge(params) - - recipients - |> fetch_activities_query(opts) - |> order_by_favourites(user) - |> Pagination.fetch_paginated(opts, pagination) - end - - # sorts by adds to favorites - # - @spec order_by_favourites(Ecto.Query.t(), User.t()) :: Ecto.Query.t() - defp order_by_favourites(query, user) do - join(query, :inner, [activity, object], a1 in Activity, - on: - fragment( - "(?->>'id') = COALESCE(?->'object'->>'id', ?->>'object') AND (?->>'type' = 'Like') AND (?.actor = ?)", - object.data, - a1.data, - a1.data, - a1.data, - a1, - ^user.ap_id - ), - as: :like_activity - ) - |> order_by([_, _, like_activity], desc: like_activity.updated_at) + @spec fetch_favourites(User.t(), map(), atom()) :: list(Activity.t()) + def fetch_favourites(user, params \\ %{}, pagination \\ :keyset) do + user.ap_id + |> Activity.Queries.by_actor() + |> Activity.Queries.by_type("Like") + |> Activity.with_joined_object() + |> Object.with_joined_activity() + |> select([_like, object, activity], %{activity | object: object}) + |> order_by([like, _, _], desc: like.updated_at) + |> Pagination.fetch_paginated(params, pagination, :object_activity) end defp maybe_update_cc(activities, list_memberships, %User{ap_id: user_ap_id}) diff --git a/lib/pleroma/web/mastodon_api/controllers/status_controller.ex b/lib/pleroma/web/mastodon_api/controllers/status_controller.ex index e11bee383..1149fb469 100644 --- a/lib/pleroma/web/mastodon_api/controllers/status_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/status_controller.ex @@ -348,7 +348,6 @@ def context(%{assigns: %{user: user}} = conn, %{"id" => id}) do def favourites(%{assigns: %{user: user}} = conn, params) do activities = ActivityPub.fetch_favourites( - [], user, Map.take(params, Pleroma.Pagination.page_keys()) ) From 5cee51fac5b5feca7e85420861bf512169e499c7 Mon Sep 17 00:00:00 2001 From: Maksim Pechnikov Date: Mon, 18 Nov 2019 21:34:54 +0300 Subject: [PATCH 4/7] fix `order by` for fetch_favorites --- lib/pleroma/pagination.ex | 5 ++++- lib/pleroma/web/activity_pub/activity_pub.ex | 6 +++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/lib/pleroma/pagination.ex b/lib/pleroma/pagination.ex index 243f1a329..6321c2600 100644 --- a/lib/pleroma/pagination.ex +++ b/lib/pleroma/pagination.ex @@ -78,7 +78,8 @@ defp cast_params(params) do since_id: :string, max_id: :string, offset: :integer, - limit: :integer + limit: :integer, + skip_order: :boolean } params = @@ -103,6 +104,8 @@ defp restrict(query, :max_id, %{max_id: max_id}, table_binding) do where(query, [{q, table_position(query, table_binding)}], q.id < ^max_id) end + defp restrict(query, :order, %{skip_order: true}, _), do: query + defp restrict(query, :order, %{min_id: _}, table_binding) do order_by( query, diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index e452278f0..f1ff0ee0d 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -1067,7 +1067,11 @@ def fetch_favourites(user, params \\ %{}, pagination \\ :keyset) do |> Object.with_joined_activity() |> select([_like, object, activity], %{activity | object: object}) |> order_by([like, _, _], desc: like.updated_at) - |> Pagination.fetch_paginated(params, pagination, :object_activity) + |> Pagination.fetch_paginated( + Map.merge(params, %{"skip_order" => true}), + pagination, + :object_activity + ) end defp maybe_update_cc(activities, list_memberships, %User{ap_id: user_ap_id}) From 7d727dbfecad8596c00e70c4bb22d1fcf8814710 Mon Sep 17 00:00:00 2001 From: Maksim Pechnikov Date: Mon, 18 Nov 2019 22:32:43 +0300 Subject: [PATCH 5/7] added test --- test/web/activity_pub/activity_pub_test.exs | 28 +++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/test/web/activity_pub/activity_pub_test.exs b/test/web/activity_pub/activity_pub_test.exs index d437ad456..3322f00fe 100644 --- a/test/web/activity_pub/activity_pub_test.exs +++ b/test/web/activity_pub/activity_pub_test.exs @@ -1555,4 +1555,32 @@ test "detects hidden follows" do assert follow_info.hide_follows == true end end + + describe "fetch_favourites/3" do + test "returns a favourite activities sorted by adds to favorite" do + user = insert(:user) + user1 = insert(:user) + user2 = insert(:user) + {:ok, a1} = CommonAPI.post(user1, %{"status" => "bla"}) + {:ok, _a2} = CommonAPI.post(user2, %{"status" => "traps are happy"}) + {:ok, a3} = CommonAPI.post(user2, %{"status" => "Trees Are "}) + {:ok, a4} = CommonAPI.post(user2, %{"status" => "Agent Smith "}) + {:ok, a5} = CommonAPI.post(user1, %{"status" => "Red or Blue "}) + + {:ok, _, _} = CommonAPI.favorite(a4.id, user) + Process.sleep(1000) + {:ok, _, _} = CommonAPI.favorite(a3.id, user) + Process.sleep(1000) + {:ok, _, _} = CommonAPI.favorite(a5.id, user) + Process.sleep(1000) + {:ok, _, _} = CommonAPI.favorite(a1.id, user) + + result = ActivityPub.fetch_favourites(user) + + assert Enum.map(result, & &1.id) == [a1.id, a5.id, a3.id, a4.id] + + result = ActivityPub.fetch_favourites(user, %{"limit" => 2}) + assert Enum.map(result, & &1.id) == [a1.id, a5.id] + end + end end From 708fd234bdff5423ca6d8003232eca0df231bbc2 Mon Sep 17 00:00:00 2001 From: Maksim Pechnikov Date: Tue, 19 Nov 2019 20:19:41 +0300 Subject: [PATCH 6/7] fix order favorites activites --- lib/pleroma/web/activity_pub/activity_pub.ex | 2 +- test/web/activity_pub/activity_pub_test.exs | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index f1ff0ee0d..6eb69e183 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -1066,7 +1066,7 @@ def fetch_favourites(user, params \\ %{}, pagination \\ :keyset) do |> Activity.with_joined_object() |> Object.with_joined_activity() |> select([_like, object, activity], %{activity | object: object}) - |> order_by([like, _, _], desc: like.updated_at) + |> order_by([like, _, _], desc: like.id) |> Pagination.fetch_paginated( Map.merge(params, %{"skip_order" => true}), pagination, diff --git a/test/web/activity_pub/activity_pub_test.exs b/test/web/activity_pub/activity_pub_test.exs index 3322f00fe..4f2d2d093 100644 --- a/test/web/activity_pub/activity_pub_test.exs +++ b/test/web/activity_pub/activity_pub_test.exs @@ -1559,6 +1559,7 @@ test "detects hidden follows" do describe "fetch_favourites/3" do test "returns a favourite activities sorted by adds to favorite" do user = insert(:user) + other_user = insert(:user) user1 = insert(:user) user2 = insert(:user) {:ok, a1} = CommonAPI.post(user1, %{"status" => "bla"}) @@ -1568,13 +1569,16 @@ test "returns a favourite activities sorted by adds to favorite" do {:ok, a5} = CommonAPI.post(user1, %{"status" => "Red or Blue "}) {:ok, _, _} = CommonAPI.favorite(a4.id, user) + {:ok, _, _} = CommonAPI.favorite(a3.id, other_user) Process.sleep(1000) {:ok, _, _} = CommonAPI.favorite(a3.id, user) + {:ok, _, _} = CommonAPI.favorite(a5.id, other_user) Process.sleep(1000) {:ok, _, _} = CommonAPI.favorite(a5.id, user) + {:ok, _, _} = CommonAPI.favorite(a4.id, other_user) Process.sleep(1000) {:ok, _, _} = CommonAPI.favorite(a1.id, user) - + {:ok, _, _} = CommonAPI.favorite(a1.id, other_user) result = ActivityPub.fetch_favourites(user) assert Enum.map(result, & &1.id) == [a1.id, a5.id, a3.id, a4.id] From 086706444145cf008e3413c8ec22c92381865252 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Wed, 11 Dec 2019 15:03:54 -0600 Subject: [PATCH 7/7] Document the favorites timeline order change --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 56b6371d5..213742545 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -91,6 +91,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - MRF: `Delete` activities being exempt from MRF policies - OTP releases: Not being able to configure OAuth expired token cleanup interval - OTP releases: Not being able to configure HTML sanitization policy +- Favorites timeline now ordered by favorite date instead of post date
API Changes