From: Alexander Strizhakov <alex.strizhakov@gmail.com>
Date: Thu, 6 Feb 2020 15:01:12 +0000 (+0300)
Subject: return data only for updated emoji
X-Git-Url: https://git.squeep.com/?a=commitdiff_plain;h=5839e67eb86d6d14b21222247ce8e113c3b26637;p=akkoma

return data only for updated emoji
---

diff --git a/CHANGELOG.md b/CHANGELOG.md
index c0f1bcf57..a220c14f6 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -125,13 +125,14 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 <details>
   <summary>API Changes</summary>
 
-- **Breaking** EmojiReactions: Change endpoints and responses to align with Mastodon
-- **Breaking** Admin API: `PATCH /api/pleroma/admin/users/:nickname/force_password_reset` is now `PATCH /api/pleroma/admin/users/force_password_reset` (accepts `nicknames` array in the request body)
+- **Breaking:** EmojiReactions: Change endpoints and responses to align with Mastodon
+- **Breaking:** Admin API: `PATCH /api/pleroma/admin/users/:nickname/force_password_reset` is now `PATCH /api/pleroma/admin/users/force_password_reset` (accepts `nicknames` array in the request body)
 - **Breaking:** Admin API: Return link alongside with token on password reset
 - **Breaking:** Admin API: `PUT /api/pleroma/admin/reports/:id` is now `PATCH /api/pleroma/admin/reports`, see admin_api.md for details
 - **Breaking:** `/api/pleroma/admin/users/invite_token` now uses `POST`, changed accepted params and returns full invite in json instead of only token string.
-- **Breaking** replying to reports is now "report notes", enpoint changed from `POST /api/pleroma/admin/reports/:id/respond` to `POST /api/pleroma/admin/reports/:id/notes`
+- **Breaking:** replying to reports is now "report notes", endpoint changed from `POST /api/pleroma/admin/reports/:id/respond` to `POST /api/pleroma/admin/reports/:id/notes`
 - Mastodon API: stopped sanitizing display names, field names and subject fields since they are supposed to be treated as plaintext
+- **Breaking:** Pleroma API: `/api/pleroma/emoji/packs/:name/update_file` endpoint returns only updated emoji data.
 - Admin API: Return `total` when querying for reports
 - Mastodon API: Return `pleroma.direct_conversation_id` when creating a direct message (`POST /api/v1/statuses`)
 - Admin API: Return link alongside with token on password reset
diff --git a/docs/API/pleroma_api.md b/docs/API/pleroma_api.md
index 90c43c356..a7c7731ce 100644
--- a/docs/API/pleroma_api.md
+++ b/docs/API/pleroma_api.md
@@ -357,7 +357,7 @@ The status posting endpoint takes an additional parameter, `in_reply_to_conversa
     * if the `action` is `update`, changes emoji shortcode
       (from `shortcode` to `new_shortcode` or moves the file (from the current filename to `new_filename`)
     * if the `action` is `remove`, removes the emoji named `shortcode` and it's associated file
-* Response: JSON, updated "files" section of the pack and 200 status, 409 if the trying to use a shortcode
+* Response: JSON, emoji shortcode with filename which was added/updated/deleted and 200 status, 409 if the trying to use a shortcode
   that is already taken, 400 if there was an error with the shortcode, filename or file (additional info
   in the "error" part of the response JSON)
 
diff --git a/lib/pleroma/web/pleroma_api/controllers/emoji_api_controller.ex b/lib/pleroma/web/pleroma_api/controllers/emoji_api_controller.ex
index e01825b48..981bac4fa 100644
--- a/lib/pleroma/web/pleroma_api/controllers/emoji_api_controller.ex
+++ b/lib/pleroma/web/pleroma_api/controllers/emoji_api_controller.ex
@@ -385,23 +385,35 @@ keeping it in cache for #{div(cache_ms, 1000)}s")
     json(conn, new_data)
   end
 
-  defp get_filename(%{"filename" => filename}), do: filename
+  defp get_filename(%Plug.Upload{filename: filename}), do: filename
+  defp get_filename(url) when is_binary(url), do: Path.basename(url)
 
-  defp get_filename(%{"file" => file}) do
-    case file do
-      %Plug.Upload{filename: filename} -> filename
-      url when is_binary(url) -> Path.basename(url)
+  defp empty?(str), do: String.trim(str) == ""
+
+  defp update_pack_file(updated_full_pack, pack_file_p) do
+    content = Jason.encode!(updated_full_pack, pretty: true)
+
+    File.write!(pack_file_p, content)
+  end
+
+  defp create_subdirs(file_path) do
+    if String.contains?(file_path, "/") do
+      file_path
+      |> Path.dirname()
+      |> File.mkdir_p!()
     end
   end
 
-  defp empty?(str), do: String.trim(str) == ""
+  defp pack_info(pack_name) do
+    dir = Path.join(emoji_dir_path(), pack_name)
+    json_path = Path.join(dir, "pack.json")
 
-  defp update_file_and_send(conn, updated_full_pack, pack_file_p) do
-    # Write the emoji pack file
-    File.write!(pack_file_p, Jason.encode!(updated_full_pack, pretty: true))
+    json =
+      json_path
+      |> File.read!()
+      |> Jason.decode!()
 
-    # Return the modified file list
-    json(conn, updated_full_pack["files"])
+    {dir, json_path, json}
   end
 
   @doc """
@@ -422,23 +434,25 @@ keeping it in cache for #{div(cache_ms, 1000)}s")
   # Add
   def update_file(
         conn,
-        %{"pack_name" => pack_name, "action" => "add", "shortcode" => shortcode} = params
+        %{"pack_name" => pack_name, "action" => "add"} = params
       ) do
-    pack_dir = Path.join(emoji_dir_path(), pack_name)
-    pack_file_p = Path.join(pack_dir, "pack.json")
+    shortcode =
+      if params["shortcode"] do
+        params["shortcode"]
+      else
+        filename = get_filename(params["file"])
+        Path.basename(filename, Path.extname(filename))
+      end
 
-    full_pack = Jason.decode!(File.read!(pack_file_p))
+    {pack_dir, pack_file_p, full_pack} = pack_info(pack_name)
 
     with {_, false} <- {:has_shortcode, Map.has_key?(full_pack["files"], shortcode)},
-         filename <- get_filename(params),
+         filename <- params["filename"] || get_filename(params["file"]),
          false <- empty?(shortcode),
-         false <- empty?(filename) do
-      file_path = Path.join(pack_dir, filename)
-
+         false <- empty?(filename),
+         file_path <- Path.join(pack_dir, filename) do
       # If the name contains directories, create them
-      if String.contains?(file_path, "/") do
-        File.mkdir_p!(Path.dirname(file_path))
-      end
+      create_subdirs(file_path)
 
       case params["file"] do
         %Plug.Upload{path: upload_path} ->
@@ -451,8 +465,11 @@ keeping it in cache for #{div(cache_ms, 1000)}s")
           File.write!(file_path, file_contents)
       end
 
-      updated_full_pack = put_in(full_pack, ["files", shortcode], filename)
-      update_file_and_send(conn, updated_full_pack, pack_file_p)
+      full_pack
+      |> put_in(["files", shortcode], filename)
+      |> update_pack_file(pack_file_p)
+
+      json(conn, %{shortcode => filename})
     else
       {:has_shortcode, _} ->
         conn
@@ -472,10 +489,7 @@ keeping it in cache for #{div(cache_ms, 1000)}s")
         "action" => "remove",
         "shortcode" => shortcode
       }) do
-    pack_dir = Path.join(emoji_dir_path(), pack_name)
-    pack_file_p = Path.join(pack_dir, "pack.json")
-
-    full_pack = Jason.decode!(File.read!(pack_file_p))
+    {pack_dir, pack_file_p, full_pack} = pack_info(pack_name)
 
     if Map.has_key?(full_pack["files"], shortcode) do
       {emoji_file_path, updated_full_pack} = pop_in(full_pack, ["files", shortcode])
@@ -494,7 +508,8 @@ keeping it in cache for #{div(cache_ms, 1000)}s")
         end
       end
 
-      update_file_and_send(conn, updated_full_pack, pack_file_p)
+      update_pack_file(updated_full_pack, pack_file_p)
+      json(conn, %{shortcode => full_pack["files"][shortcode]})
     else
       conn
       |> put_status(:bad_request)
@@ -507,10 +522,7 @@ keeping it in cache for #{div(cache_ms, 1000)}s")
         conn,
         %{"pack_name" => pack_name, "action" => "update", "shortcode" => shortcode} = params
       ) do
-    pack_dir = Path.join(emoji_dir_path(), pack_name)
-    pack_file_p = Path.join(pack_dir, "pack.json")
-
-    full_pack = Jason.decode!(File.read!(pack_file_p))
+    {pack_dir, pack_file_p, full_pack} = pack_info(pack_name)
 
     with {_, true} <- {:has_shortcode, Map.has_key?(full_pack["files"], shortcode)},
          %{"new_shortcode" => new_shortcode, "new_filename" => new_filename} <- params,
@@ -522,9 +534,7 @@ keeping it in cache for #{div(cache_ms, 1000)}s")
       new_emoji_file_path = Path.join(pack_dir, new_filename)
 
       # If the name contains directories, create them
-      if String.contains?(new_emoji_file_path, "/") do
-        File.mkdir_p!(Path.dirname(new_emoji_file_path))
-      end
+      create_subdirs(new_emoji_file_path)
 
       # Move/Rename the old filename to a new filename
       # These are probably on the same filesystem, so just rename should work
@@ -540,8 +550,11 @@ keeping it in cache for #{div(cache_ms, 1000)}s")
       end
 
       # Then, put in the new shortcode with the new path
-      updated_full_pack = put_in(updated_full_pack, ["files", new_shortcode], new_filename)
-      update_file_and_send(conn, updated_full_pack, pack_file_p)
+      updated_full_pack
+      |> put_in(["files", new_shortcode], new_filename)
+      |> update_pack_file(pack_file_p)
+
+      json(conn, %{new_shortcode => new_filename})
     else
       {:has_shortcode, _} ->
         conn
diff --git a/test/instance_static/add/shortcode.png b/test/instance_static/add/shortcode.png
new file mode 100644
index 000000000..8f50fa023
Binary files /dev/null and b/test/instance_static/add/shortcode.png differ
diff --git a/test/web/pleroma_api/controllers/emoji_api_controller_test.exs b/test/web/pleroma_api/controllers/emoji_api_controller_test.exs
index 4246eb400..6844601d7 100644
--- a/test/web/pleroma_api/controllers/emoji_api_controller_test.exs
+++ b/test/web/pleroma_api/controllers/emoji_api_controller_test.exs
@@ -295,96 +295,116 @@ defmodule Pleroma.Web.PleromaAPI.EmojiAPIControllerTest do
     end
   end
 
-  test "updating pack files" do
-    pack_file = "#{@emoji_dir_path}/test_pack/pack.json"
-    original_content = File.read!(pack_file)
+  describe "update_file/2" do
+    setup do
+      pack_file = "#{@emoji_dir_path}/test_pack/pack.json"
+      original_content = File.read!(pack_file)
 
-    on_exit(fn ->
-      File.write!(pack_file, original_content)
+      on_exit(fn ->
+        File.write!(pack_file, original_content)
+      end)
 
-      File.rm_rf!("#{@emoji_dir_path}/test_pack/blank_url.png")
-      File.rm_rf!("#{@emoji_dir_path}/test_pack/dir")
-      File.rm_rf!("#{@emoji_dir_path}/test_pack/dir_2")
-    end)
+      admin = insert(:user, is_admin: true)
+      %{conn: conn} = oauth_access(["admin:write"], user: admin)
+      {:ok, conn: conn}
+    end
 
-    admin = insert(:user, is_admin: true)
-    %{conn: conn} = oauth_access(["admin:write"], user: admin)
+    test "update file without shortcode", %{conn: conn} do
+      on_exit(fn -> File.rm_rf!("#{@emoji_dir_path}/test_pack/shortcode.png") end)
+
+      assert conn
+             |> post("/api/pleroma/emoji/packs/test_pack/update_file", %{
+               "action" => "add",
+               "file" => %Plug.Upload{
+                 filename: "shortcode.png",
+                 path: "#{Pleroma.Config.get([:instance, :static_dir])}/add/shortcode.png"
+               }
+             })
+             |> json_response(200) == %{"shortcode" => "shortcode.png"}
+    end
+
+    test "updating pack files", %{conn: conn} do
+      on_exit(fn ->
+        File.rm_rf!("#{@emoji_dir_path}/test_pack/blank_url.png")
+        File.rm_rf!("#{@emoji_dir_path}/test_pack/dir")
+        File.rm_rf!("#{@emoji_dir_path}/test_pack/dir_2")
+      end)
 
-    same_name = %{
-      "action" => "add",
-      "shortcode" => "blank",
-      "filename" => "dir/blank.png",
-      "file" => %Plug.Upload{
-        filename: "blank.png",
-        path: "#{@emoji_dir_path}/test_pack/blank.png"
+      same_name = %{
+        "action" => "add",
+        "shortcode" => "blank",
+        "filename" => "dir/blank.png",
+        "file" => %Plug.Upload{
+          filename: "blank.png",
+          path: "#{@emoji_dir_path}/test_pack/blank.png"
+        }
       }
-    }
 
-    different_name = %{same_name | "shortcode" => "blank_2"}
+      different_name = %{same_name | "shortcode" => "blank_2"}
 
-    assert (conn
-            |> post(emoji_api_path(conn, :update_file, "test_pack"), same_name)
-            |> json_response(:conflict))["error"] =~ "already exists"
+      assert (conn
+              |> post(emoji_api_path(conn, :update_file, "test_pack"), same_name)
+              |> json_response(:conflict))["error"] =~ "already exists"
 
-    assert conn
-           |> post(emoji_api_path(conn, :update_file, "test_pack"), different_name)
-           |> json_response(200) == %{"blank" => "blank.png", "blank_2" => "dir/blank.png"}
+      assert conn
+             |> post(emoji_api_path(conn, :update_file, "test_pack"), different_name)
+             |> json_response(200) == %{"blank_2" => "dir/blank.png"}
 
-    assert File.exists?("#{@emoji_dir_path}/test_pack/dir/blank.png")
+      assert File.exists?("#{@emoji_dir_path}/test_pack/dir/blank.png")
 
-    assert conn
-           |> post(emoji_api_path(conn, :update_file, "test_pack"), %{
-             "action" => "update",
-             "shortcode" => "blank_2",
-             "new_shortcode" => "blank_3",
-             "new_filename" => "dir_2/blank_3.png"
-           })
-           |> json_response(200) == %{"blank" => "blank.png", "blank_3" => "dir_2/blank_3.png"}
+      assert conn
+             |> post(emoji_api_path(conn, :update_file, "test_pack"), %{
+               "action" => "update",
+               "shortcode" => "blank_2",
+               "new_shortcode" => "blank_3",
+               "new_filename" => "dir_2/blank_3.png"
+             })
+             |> json_response(200) == %{"blank_3" => "dir_2/blank_3.png"}
 
-    refute File.exists?("#{@emoji_dir_path}/test_pack/dir/")
-    assert File.exists?("#{@emoji_dir_path}/test_pack/dir_2/blank_3.png")
+      refute File.exists?("#{@emoji_dir_path}/test_pack/dir/")
+      assert File.exists?("#{@emoji_dir_path}/test_pack/dir_2/blank_3.png")
 
-    assert conn
-           |> post(emoji_api_path(conn, :update_file, "test_pack"), %{
-             "action" => "remove",
-             "shortcode" => "blank_3"
-           })
-           |> json_response(200) == %{"blank" => "blank.png"}
+      assert conn
+             |> post(emoji_api_path(conn, :update_file, "test_pack"), %{
+               "action" => "remove",
+               "shortcode" => "blank_3"
+             })
+             |> json_response(200) == %{"blank_3" => "dir_2/blank_3.png"}
 
-    refute File.exists?("#{@emoji_dir_path}/test_pack/dir_2/")
+      refute File.exists?("#{@emoji_dir_path}/test_pack/dir_2/")
 
-    mock(fn
-      %{
-        method: :get,
-        url: "https://test-blank/blank_url.png"
-      } ->
-        text(File.read!("#{@emoji_dir_path}/test_pack/blank.png"))
-    end)
+      mock(fn
+        %{
+          method: :get,
+          url: "https://test-blank/blank_url.png"
+        } ->
+          text(File.read!("#{@emoji_dir_path}/test_pack/blank.png"))
+      end)
 
-    # The name should be inferred from the URL ending
-    from_url = %{
-      "action" => "add",
-      "shortcode" => "blank_url",
-      "file" => "https://test-blank/blank_url.png"
-    }
+      # The name should be inferred from the URL ending
+      from_url = %{
+        "action" => "add",
+        "shortcode" => "blank_url",
+        "file" => "https://test-blank/blank_url.png"
+      }
 
-    assert conn
-           |> post(emoji_api_path(conn, :update_file, "test_pack"), from_url)
-           |> json_response(200) == %{
-             "blank" => "blank.png",
-             "blank_url" => "blank_url.png"
-           }
+      assert conn
+             |> post(emoji_api_path(conn, :update_file, "test_pack"), from_url)
+             |> json_response(200) == %{
+               "blank_url" => "blank_url.png"
+             }
 
-    assert File.exists?("#{@emoji_dir_path}/test_pack/blank_url.png")
+      assert File.exists?("#{@emoji_dir_path}/test_pack/blank_url.png")
 
-    assert conn
-           |> post(emoji_api_path(conn, :update_file, "test_pack"), %{
-             "action" => "remove",
-             "shortcode" => "blank_url"
-           })
-           |> json_response(200) == %{"blank" => "blank.png"}
+      assert conn
+             |> post(emoji_api_path(conn, :update_file, "test_pack"), %{
+               "action" => "remove",
+               "shortcode" => "blank_url"
+             })
+             |> json_response(200) == %{"blank_url" => "blank_url.png"}
 
-    refute File.exists?("#{@emoji_dir_path}/test_pack/blank_url.png")
+      refute File.exists?("#{@emoji_dir_path}/test_pack/blank_url.png")
+    end
   end
 
   test "creating and deleting a pack" do