Remove "default" image description #493

Merged
floatingghost merged 1 commits from ilja/akkoma:remove_default_image_description into develop 2023-04-14 16:27:42 +00:00
7 changed files with 5 additions and 51 deletions
Showing only changes of commit 6c396fcab4 - Show all commits

View File

@ -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

View File

@ -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"

View File

@ -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"

View File

@ -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`.

View File

@ -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) || "",
floatingghost marked this conversation as resolved
Review

might be cleaner is use Map.get/3 here instead

might be cleaner is use `Map.get/3` here instead
Review

Map.get/3 doesn't work in this case. The key can contain nil. So in that case the description would be nil instead of an empty string (and the rest of the code expects a string).

iex(1)> Map.get(%{description: nil}, :description, "")
nil
iex(2)> Map.get(%{description: nil}, :description) || ""
""
`Map.get/3` doesn't work in this case. The key can contain `nil`. So in that case the description would be `nil` instead of an empty string (and the rest of the code expects a string). ```iex iex(1)> Map.get(%{description: nil}, :description, "") nil iex(2)> Map.get(%{description: nil}, :description) || "" "" ```
Review

ah yeah , forgot that it could be explicitly nil , I'm stupid

ah yeah , forgot that it could be explicitly nil , I'm stupid
{_, true} <-
{:description_limit,
String.length(description) <= Pleroma.Config.get([:instance, :description_limit])},

View File

@ -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
Outdated
Review

Note: This test broke with removing the filename from this field. But this is not what it tests, and as afaict it doesn't even actually tests what it's supposed to do. Therefore I decided to just remove it instead of leaving it in what seems to be a broken state.

Note: This test broke with removing the filename from this field. But this is not what it tests, and as afaict it doesn't even actually tests what it's supposed to do. Therefore I decided to just remove it instead of leaving it in what seems to be a broken state.
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

View File

@ -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()