Merge branch 'bugfix/filter-id' into 'develop'

Ensure filters have a filter_id

See merge request pleroma/pleroma!412

lib/pleroma/filter.ex

@@ -36,6 +36,34 @@ defmodule Pleroma.Filter do
     Repo.all(query)
   end
 
+  def create(%Pleroma.Filter{user_id: user_id, filter_id: nil} = filter) do
+    # If filter_id wasn't given, use the max filter_id for this user plus 1.
+    # XXX This could result in a race condition if a user tries to add two 
+    # different filters for their account from two different clients at the 
+    # same time, but that should be unlikely. 
+
+    max_id_query =
+      from(
+        f in Pleroma.Filter,
+        where: f.user_id == ^user_id,
+        select: max(f.filter_id)
+      )
+
+    filter_id =
+      case Repo.one(max_id_query) do
+        # Start allocating from 1
+        nil ->
+          1
+
+        max_id ->
+          max_id + 1
+      end
+
+    filter
+    |> Map.put(:filter_id, filter_id)
+    |> Repo.insert()
+  end
+
   def create(%Pleroma.Filter{} = filter) do
     Repo.insert(filter)
   end

test/filter_test.exs

@@ -5,19 +5,88 @@ defmodule Pleroma.FilterTest do
   import Pleroma.Factory
   import Ecto.Query
 
-  test "creating a filter" do
-    user = insert(:user)
+  describe "creating filters" do
+    test "creating one filter" do
+      user = insert(:user)
 
-    query = %Pleroma.Filter{
-      user_id: user.id,
-      filter_id: 42,
-      phrase: "knights",
-      context: ["home"]
-    }
+      query = %Pleroma.Filter{
+        user_id: user.id,
+        filter_id: 42,
+        phrase: "knights",
+        context: ["home"]
+      }
 
-    {:ok, %Pleroma.Filter{} = filter} = Pleroma.Filter.create(query)
-    result = Pleroma.Filter.get(filter.filter_id, user)
-    assert query.phrase == result.phrase
+      {:ok, %Pleroma.Filter{} = filter} = Pleroma.Filter.create(query)
+      result = Pleroma.Filter.get(filter.filter_id, user)
+      assert query.phrase == result.phrase
+    end
+
+    test "creating one filter without a pre-defined filter_id" do
+      user = insert(:user)
+
+      query = %Pleroma.Filter{
+        user_id: user.id,
+        phrase: "knights",
+        context: ["home"]
+      }
+
+      {:ok, %Pleroma.Filter{} = filter} = Pleroma.Filter.create(query)
+      # Should start at 1
+      assert filter.filter_id == 1
+    end
+
+    test "creating additional filters uses previous highest filter_id + 1" do
+      user = insert(:user)
+
+      query_one = %Pleroma.Filter{
+        user_id: user.id,
+        filter_id: 42,
+        phrase: "knights",
+        context: ["home"]
+      }
+
+      {:ok, %Pleroma.Filter{} = filter_one} = Pleroma.Filter.create(query_one)
+
+      query_two = %Pleroma.Filter{
+        user_id: user.id,
+        # No filter_id
+        phrase: "who",
+        context: ["home"]
+      }
+
+      {:ok, %Pleroma.Filter{} = filter_two} = Pleroma.Filter.create(query_two)
+      assert filter_two.filter_id == filter_one.filter_id + 1
+    end
+
+    test "filter_id is unique per user" do
+      user_one = insert(:user)
+      user_two = insert(:user)
+
+      query_one = %Pleroma.Filter{
+        user_id: user_one.id,
+        phrase: "knights",
+        context: ["home"]
+      }
+
+      {:ok, %Pleroma.Filter{} = filter_one} = Pleroma.Filter.create(query_one)
+
+      query_two = %Pleroma.Filter{
+        user_id: user_two.id,
+        phrase: "who",
+        context: ["home"]
+      }
+
+      {:ok, %Pleroma.Filter{} = filter_two} = Pleroma.Filter.create(query_two)
+
+      assert filter_one.filter_id == 1
+      assert filter_two.filter_id == 1
+
+      result_one = Pleroma.Filter.get(filter_one.filter_id, user_one)
+      assert result_one.phrase == filter_one.phrase
+
+      result_two = Pleroma.Filter.get(filter_two.filter_id, user_two)
+      assert result_two.phrase == filter_two.phrase
+    end
   end
 
   test "deleting a filter" do

test/web/mastodon_api/mastodon_api_controller_test.exs

@@ -280,6 +280,8 @@ defmodule Pleroma.Web.MastodonAPI.MastodonAPIControllerTest do
       assert response = json_response(conn, 200)
       assert response["phrase"] == filter.phrase
       assert response["context"] == filter.context
+      assert response["id"] != nil
+      assert response["id"] != ""
     end
 
     test "fetching a list of filters", %{conn: conn} do