From 6c396fcab49f5de8b3f73a8353748d3606687966 Mon Sep 17 00:00:00 2001 From: ilja Date: Mon, 6 Mar 2023 12:04:35 +0100 Subject: [PATCH] Remove "default" image description When no image description is filled in, Pleroma allowed fallbacks. Those were (based on a setting) either the filename, or a fixed description. Neither are good options for image descriptions imo, so here we remove this. Note that there's two tests removed who supposedly tested something else. But examining closer, they didn't seem to test what they claimed to test, so I removed them rather than try to "fix" them. --- CHANGELOG.md | 1 + config/config.exs | 1 - config/test.exs | 3 +-- docs/docs/configuration/cheatsheet.md | 1 - lib/pleroma/upload.ex | 11 +--------- test/pleroma/upload_test.exs | 17 ++------------ .../web/activity_pub/activity_pub_test.exs | 22 ------------------- 7 files changed, 5 insertions(+), 51 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 859a09e7d..b451f297f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ### Removed - Possibility of using the `style` parameter on `span` elements. This will break certain MFM parameters. +- Option for "default" image description. ## 2023.02 diff --git a/config/config.exs b/config/config.exs index 5eaa8ce76..4d8fd52c4 100644 --- a/config/config.exs +++ b/config/config.exs @@ -65,7 +65,6 @@ config :pleroma, Pleroma.Upload, link_name: false, proxy_remote: false, filename_display_max_length: 30, - default_description: nil, base_url: nil config :pleroma, Pleroma.Uploaders.Local, uploads: "uploads" diff --git a/config/test.exs b/config/test.exs index 3056dbd03..4448eeb73 100644 --- a/config/test.exs +++ b/config/test.exs @@ -23,8 +23,7 @@ config :pleroma, :auth, oauth_consumer_strategies: [] config :pleroma, Pleroma.Upload, filters: [], - link_name: false, - default_description: :filename + link_name: false config :pleroma, Pleroma.Uploaders.Local, uploads: "test/uploads" diff --git a/docs/docs/configuration/cheatsheet.md b/docs/docs/configuration/cheatsheet.md index 4e84b9a44..1c4d9ec5d 100644 --- a/docs/docs/configuration/cheatsheet.md +++ b/docs/docs/configuration/cheatsheet.md @@ -562,7 +562,6 @@ the source code is here: [kocaptcha](https://github.com/koto-bank/kocaptcha). Th * `proxy_remote`: If you're using a remote uploader, Akkoma will proxy media requests instead of redirecting to it. * `proxy_opts`: Proxy options, see `Pleroma.ReverseProxy` documentation. * `filename_display_max_length`: Set max length of a filename to display. 0 = no limit. Default: 30. -* `default_description`: Sets which default description an image has if none is set explicitly. Options: nil (default) - Don't set a default, :filename - use the filename of the file, a string (e.g. "attachment") - Use this string !!! warning `strip_exif` has been replaced by `Pleroma.Upload.Filter.Mogrify`. diff --git a/lib/pleroma/upload.ex b/lib/pleroma/upload.ex index 3b5419db7..2f65540be 100644 --- a/lib/pleroma/upload.ex +++ b/lib/pleroma/upload.ex @@ -65,15 +65,6 @@ defmodule Pleroma.Upload do } defstruct [:id, :name, :tempfile, :content_type, :width, :height, :blurhash, :path] - defp get_description(opts, upload) do - case {opts[:description], Pleroma.Config.get([Pleroma.Upload, :default_description])} do - {description, _} when is_binary(description) -> description - {_, :filename} -> upload.name - {_, str} when is_binary(str) -> str - _ -> "" - end - end - @spec store(source, options :: [option()]) :: {:ok, Map.t()} | {:error, any()} @doc "Store a file. If using a `Plug.Upload{}` as the source, be sure to use `Majic.Plug` to ensure its content_type and filename is correct." def store(upload, opts \\ []) do @@ -82,7 +73,7 @@ defmodule Pleroma.Upload do with {:ok, upload} <- prepare_upload(upload, opts), upload = %__MODULE__{upload | path: upload.path || "#{upload.id}/#{upload.name}"}, {:ok, upload} <- Pleroma.Upload.Filter.filter(opts.filters, upload), - description = get_description(opts, upload), + description = Map.get(opts, :description) || "", {_, true} <- {:description_limit, String.length(description) <= Pleroma.Config.get([:instance, :description_limit])}, diff --git a/test/pleroma/upload_test.exs b/test/pleroma/upload_test.exs index 8f242630f..ad6065b43 100644 --- a/test/pleroma/upload_test.exs +++ b/test/pleroma/upload_test.exs @@ -54,7 +54,7 @@ defmodule Pleroma.UploadTest do assert result == %{ "id" => result["id"], - "name" => "image.jpg", + "name" => "", "type" => "Document", "mediaType" => "image/jpeg", "url" => [ @@ -154,19 +154,6 @@ defmodule Pleroma.UploadTest do "e30397b58d226d6583ab5b8b3c5defb0c682bda5c31ef07a9f57c1c4986e3781.jpg" end - test "copies the file to the configured folder without deduping" do - File.cp!("test/fixtures/image.jpg", "test/fixtures/image_tmp.jpg") - - file = %Plug.Upload{ - content_type: "image/jpeg", - path: Path.absname("test/fixtures/image_tmp.jpg"), - filename: "an [image.jpg" - } - - {:ok, data} = Upload.store(file) - assert data["name"] == "an [image.jpg" - end - test "fixes incorrect content type when base64 is given" do params = %{ img: "data:image/png;base64,#{Base.encode64(File.read!("test/fixtures/image.jpg"))}" @@ -184,7 +171,7 @@ defmodule Pleroma.UploadTest do } {:ok, data} = Upload.store(params) - assert String.ends_with?(data["name"], ".jpg") + assert String.ends_with?(List.first(data["url"])["href"], ".jpg") end test "copies the file to the configured folder with anonymizing filename" do diff --git a/test/pleroma/web/activity_pub/activity_pub_test.exs b/test/pleroma/web/activity_pub/activity_pub_test.exs index 17c52fc91..20435d149 100644 --- a/test/pleroma/web/activity_pub/activity_pub_test.exs +++ b/test/pleroma/web/activity_pub/activity_pub_test.exs @@ -1308,28 +1308,6 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubTest do assert object.data["name"] == "a cool file" end - test "it sets the default description depending on the configuration", %{test_file: file} do - clear_config([Pleroma.Upload, :default_description]) - - clear_config([Pleroma.Upload, :default_description], nil) - {:ok, %Object{} = object} = ActivityPub.upload(file) - assert object.data["name"] == "" - - clear_config([Pleroma.Upload, :default_description], :filename) - {:ok, %Object{} = object} = ActivityPub.upload(file) - assert object.data["name"] == "an_image.jpg" - - clear_config([Pleroma.Upload, :default_description], "unnamed attachment") - {:ok, %Object{} = object} = ActivityPub.upload(file) - assert object.data["name"] == "unnamed attachment" - end - - test "copies the file to the configured folder", %{test_file: file} do - clear_config([Pleroma.Upload, :default_description], :filename) - {:ok, %Object{} = object} = ActivityPub.upload(file) - assert object.data["name"] == "an_image.jpg" - end - test "works with base64 encoded images" do file = %{ img: data_uri()