Merge branch 'proper_error_messages' into 'develop'

MastodonController: Return 404 errors correctly.

See merge request pleroma/pleroma!2270
This commit is contained in:
lain 2020-03-05 11:49:51 +00:00
commit 47604907c9
4 changed files with 51 additions and 20 deletions

View File

@ -70,20 +70,21 @@ def reject_follow_request(follower, followed) do
end end
def delete(activity_id, user) do def delete(activity_id, user) do
with %Activity{data: %{"object" => _}} = activity <- with {_, %Activity{data: %{"object" => _}} = activity} <-
Activity.get_by_id_with_object(activity_id), {:find_activity, Activity.get_by_id_with_object(activity_id)},
%Object{} = object <- Object.normalize(activity), %Object{} = object <- Object.normalize(activity),
true <- User.superuser?(user) || user.ap_id == object.data["actor"], true <- User.superuser?(user) || user.ap_id == object.data["actor"],
{:ok, _} <- unpin(activity_id, user), {:ok, _} <- unpin(activity_id, user),
{:ok, delete} <- ActivityPub.delete(object) do {:ok, delete} <- ActivityPub.delete(object) do
{:ok, delete} {:ok, delete}
else else
{:find_activity, _} -> {:error, :not_found}
_ -> {:error, dgettext("errors", "Could not delete")} _ -> {:error, dgettext("errors", "Could not delete")}
end end
end end
def repeat(id_or_ap_id, user, params \\ %{}) do def repeat(id_or_ap_id, user, params \\ %{}) do
with %Activity{} = activity <- get_by_id_or_ap_id(id_or_ap_id), with {_, %Activity{} = activity} <- {:find_activity, get_by_id_or_ap_id(id_or_ap_id)},
object <- Object.normalize(activity), object <- Object.normalize(activity),
announce_activity <- Utils.get_existing_announce(user.ap_id, object), announce_activity <- Utils.get_existing_announce(user.ap_id, object),
public <- public_announce?(object, params) do public <- public_announce?(object, params) do
@ -93,21 +94,23 @@ def repeat(id_or_ap_id, user, params \\ %{}) do
ActivityPub.announce(user, object, nil, true, public) ActivityPub.announce(user, object, nil, true, public)
end end
else else
{:find_activity, _} -> {:error, :not_found}
_ -> {:error, dgettext("errors", "Could not repeat")} _ -> {:error, dgettext("errors", "Could not repeat")}
end end
end end
def unrepeat(id_or_ap_id, user) do def unrepeat(id_or_ap_id, user) do
with %Activity{} = activity <- get_by_id_or_ap_id(id_or_ap_id) do with {_, %Activity{} = activity} <- {:find_activity, get_by_id_or_ap_id(id_or_ap_id)} do
object = Object.normalize(activity) object = Object.normalize(activity)
ActivityPub.unannounce(user, object) ActivityPub.unannounce(user, object)
else else
{:find_activity, _} -> {:error, :not_found}
_ -> {:error, dgettext("errors", "Could not unrepeat")} _ -> {:error, dgettext("errors", "Could not unrepeat")}
end end
end end
def favorite(id_or_ap_id, user) do def favorite(id_or_ap_id, user) do
with %Activity{} = activity <- get_by_id_or_ap_id(id_or_ap_id), with {_, %Activity{} = activity} <- {:find_activity, get_by_id_or_ap_id(id_or_ap_id)},
object <- Object.normalize(activity), object <- Object.normalize(activity),
like_activity <- Utils.get_existing_like(user.ap_id, object) do like_activity <- Utils.get_existing_like(user.ap_id, object) do
if like_activity do if like_activity do
@ -116,15 +119,17 @@ def favorite(id_or_ap_id, user) do
ActivityPub.like(user, object) ActivityPub.like(user, object)
end end
else else
{:find_activity, _} -> {:error, :not_found}
_ -> {:error, dgettext("errors", "Could not favorite")} _ -> {:error, dgettext("errors", "Could not favorite")}
end end
end end
def unfavorite(id_or_ap_id, user) do def unfavorite(id_or_ap_id, user) do
with %Activity{} = activity <- get_by_id_or_ap_id(id_or_ap_id) do with {_, %Activity{} = activity} <- {:find_activity, get_by_id_or_ap_id(id_or_ap_id)} do
object = Object.normalize(activity) object = Object.normalize(activity)
ActivityPub.unlike(user, object) ActivityPub.unlike(user, object)
else else
{:find_activity, _} -> {:error, :not_found}
_ -> {:error, dgettext("errors", "Could not unfavorite")} _ -> {:error, dgettext("errors", "Could not unfavorite")}
end end
end end

View File

@ -175,6 +175,8 @@ def show(%{assigns: %{user: user}} = conn, %{"id" => id}) do
for: user, for: user,
with_direct_conversation_id: true with_direct_conversation_id: true
) )
else
_ -> {:error, :not_found}
end end
end end
@ -183,6 +185,7 @@ def delete(%{assigns: %{user: user}} = conn, %{"id" => id}) do
with {:ok, %Activity{}} <- CommonAPI.delete(id, user) do with {:ok, %Activity{}} <- CommonAPI.delete(id, user) do
json(conn, %{}) json(conn, %{})
else else
{:error, :not_found} = e -> e
_e -> render_error(conn, :forbidden, "Can't delete this post") _e -> render_error(conn, :forbidden, "Can't delete this post")
end end
end end

View File

@ -1880,10 +1880,10 @@ test "deletes status", %{conn: conn, id: id, admin: admin} do
"@#{admin.nickname} deleted status ##{id}" "@#{admin.nickname} deleted status ##{id}"
end end
test "returns error when status is not exist", %{conn: conn} do test "returns 404 when the status does not exist", %{conn: conn} do
conn = delete(conn, "/api/pleroma/admin/statuses/test") conn = delete(conn, "/api/pleroma/admin/statuses/test")
assert json_response(conn, :bad_request) == "Could not delete" assert json_response(conn, :not_found) == "Not found"
end end
end end

View File

@ -476,6 +476,15 @@ test "get a status" do
assert id == to_string(activity.id) assert id == to_string(activity.id)
end end
test "getting a status that doesn't exist returns 404" do
%{conn: conn} = oauth_access(["read:statuses"])
activity = insert(:note_activity)
conn = get(conn, "/api/v1/statuses/#{String.downcase(activity.id)}")
assert json_response(conn, 404) == %{"error" => "Record not found"}
end
test "get a direct status" do test "get a direct status" do
%{user: user, conn: conn} = oauth_access(["read:statuses"]) %{user: user, conn: conn} = oauth_access(["read:statuses"])
other_user = insert(:user) other_user = insert(:user)
@ -520,6 +529,18 @@ test "when you created it" do
refute Activity.get_by_id(activity.id) refute Activity.get_by_id(activity.id)
end end
test "when it doesn't exist" do
%{user: author, conn: conn} = oauth_access(["write:statuses"])
activity = insert(:note_activity, user: author)
conn =
conn
|> assign(:user, author)
|> delete("/api/v1/statuses/#{String.downcase(activity.id)}")
assert %{"error" => "Record not found"} == json_response(conn, 404)
end
test "when you didn't create it" do test "when you didn't create it" do
%{conn: conn} = oauth_access(["write:statuses"]) %{conn: conn} = oauth_access(["write:statuses"])
activity = insert(:note_activity) activity = insert(:note_activity)
@ -574,6 +595,14 @@ test "reblogs and returns the reblogged status", %{conn: conn} do
assert to_string(activity.id) == id assert to_string(activity.id) == id
end end
test "returns 404 if the reblogged status doesn't exist", %{conn: conn} do
activity = insert(:note_activity)
conn = post(conn, "/api/v1/statuses/#{String.downcase(activity.id)}/reblog")
assert %{"error" => "Record not found"} = json_response(conn, 404)
end
test "reblogs privately and returns the reblogged status", %{conn: conn} do test "reblogs privately and returns the reblogged status", %{conn: conn} do
activity = insert(:note_activity) activity = insert(:note_activity)
@ -626,12 +655,6 @@ test "reblogged status for another user" do
assert to_string(activity.id) == id assert to_string(activity.id) == id
end end
test "returns 400 error when activity is not exist", %{conn: conn} do
conn = post(conn, "/api/v1/statuses/foo/reblog")
assert json_response(conn, 400) == %{"error" => "Could not repeat"}
end
end end
describe "unreblogging" do describe "unreblogging" do
@ -649,10 +672,10 @@ test "unreblogs and returns the unreblogged status", %{user: user, conn: conn} d
assert to_string(activity.id) == id assert to_string(activity.id) == id
end end
test "returns 400 error when activity is not exist", %{conn: conn} do test "returns 404 error when activity does not exist", %{conn: conn} do
conn = post(conn, "/api/v1/statuses/foo/unreblog") conn = post(conn, "/api/v1/statuses/foo/unreblog")
assert json_response(conn, 400) == %{"error" => "Could not unrepeat"} assert json_response(conn, 404) == %{"error" => "Record not found"}
end end
end end
@ -677,10 +700,10 @@ test "favoriting twice will just return 200", %{conn: conn} do
assert post(conn, "/api/v1/statuses/#{activity.id}/favourite") |> json_response(200) assert post(conn, "/api/v1/statuses/#{activity.id}/favourite") |> json_response(200)
end end
test "returns 400 error for a wrong id", %{conn: conn} do test "returns 404 error for a wrong id", %{conn: conn} do
conn = post(conn, "/api/v1/statuses/1/favourite") conn = post(conn, "/api/v1/statuses/1/favourite")
assert json_response(conn, 400) == %{"error" => "Could not favorite"} assert json_response(conn, 404) == %{"error" => "Record not found"}
end end
end end
@ -700,10 +723,10 @@ test "unfavorites a status and returns it", %{user: user, conn: conn} do
assert to_string(activity.id) == id assert to_string(activity.id) == id
end end
test "returns 400 error for a wrong id", %{conn: conn} do test "returns 404 error for a wrong id", %{conn: conn} do
conn = post(conn, "/api/v1/statuses/1/unfavourite") conn = post(conn, "/api/v1/statuses/1/unfavourite")
assert json_response(conn, 400) == %{"error" => "Could not unfavorite"} assert json_response(conn, 404) == %{"error" => "Record not found"}
end end
end end