web(activity_pub/utils): temporarily deal with nil status ID when dealing with report #1082

Yonle · 2026-03-11T07:38:41Z

Yonle commented

2026-03-11 07:38:41 +00:00

temporarily addresses #712
EDIT: does not address #599 and #366

temporarily addresses #712 **EDIT:** does not address `#599 and #366`

Yonle added 1 commit

2026-03-11 07:38:42 +00:00

web(activity_pub/utils): deal with deleted activity on strip_report_status_data

ci/woodpecker/pr/test/1 Pipeline is pending approval

Details

ci/woodpecker/pr/test/2 Pipeline is pending approval

Details

814d7ddd25

Oneric commented

2026-03-15 14:54:15 +00:00

As noted in #712, the real fix here is to stop mangling and deleting the post references for fetched or deleted posts. But given this still didn’t come to be and a nil reject like this will still be needed for already messed up reports (or a db migration to fully remove existing nils), i guess it makes sense to merge something like this as a interim bandaid to at least perevent reportsfrom getting stuck, yeah

However, your commit message and comment are incorrect. This does not only occur for deleted posts, but (as discussed in #712) also all posts initially discovered via fetch. This also does nothing for #599 and #366 which you linked. They did not fail with a 500 Internal Server error as you’d expect from the kind of clause mismatch fixed here, but because user lookup or instance rules blocked progression. Please do not claim fixes for only vaguely similar issues without proper confirmation.

As noted in #712, the real fix here is to stop mangling and deleting the post references for fetched or deleted posts. But given this still didn’t come to be and a `nil` reject like this will still be needed for already messed up reports *(or a db migration to fully remove existing `nil`s)*, i guess it makes sense to merge something like this as a interim bandaid to at least perevent reportsfrom getting stuck, yeah However, your commit message and comment are incorrect. This does not only occur for deleted posts, but (as discussed in #712) also all posts initially discovered via fetch. This also does nothing for #599 and #366 which you linked. They did not fail with a `500 Internal Server` error as you’d expect from the kind of clause mismatch fixed here, but because user lookup or instance rules blocked progression. Please do not claim fixes for only vaguely similar issues without proper confirmation.

Oneric commented

2026-03-15 15:06:33 +00:00

i was about to adjust the points brought up above (+ the commit title style, should be sth like: ``) and merge since the PR is set to be editable by maintainers, but then noticed you sign your commits.
Often when someone signs their commits they’ll want to keep the signature and edit commits themself. On the other hand this PR is set to be editable by maintainers, but perhaps you just missed the toggle to disable this. So, I’ll wait for you to come back to this and let me know what you prefer and/or edit it yourself.

i was about to adjust the points brought up above (+ the commit title style, should be sth like: ``) and merge since the PR is set to be editable by maintainers, but then noticed you sign your commits. Often when someone signs their commits they’ll want to keep the signature and edit commits themself. On the other hand this PR is set to be editable by maintainers, but perhaps you just missed the toggle to disable this. So, I’ll wait for you to come back to this and let me know what you prefer and/or edit it yourself.

Yonle commented

2026-03-15 20:22:45 +00:00

@Oneric wrote in #1082 (comment):

As noted in #712, the real fix here is to stop mangling and deleting the post references for fetched or deleted posts. But given this still didn’t come to be and a nil reject like this will still be needed for already messed up reports (or a db migration to fully remove existing nils), i guess it makes sense to merge something like this as a interim bandaid to at least perevent reportsfrom getting stuck, yeah

I am quite aware of that. For now, it's better to be an bandaid as of now before admin open issue about similar issue.

However, your commit message and comment are incorrect.

My bad. At the time i was creating a tempfix, I haven't really read much on the existing issue, so here goes my first assumption (but the thing is, the post exist on my server, but the id being null here is a strange quirk).

This is a strange one. I will alter the commit message probably the next morning.

@Oneric wrote in https://akkoma.dev/AkkomaGang/akkoma/pulls/1082#issuecomment-16295: > As noted in #712, the real fix here is to stop mangling and deleting the post references for fetched or deleted posts. But given this still didn’t come to be and a `nil` reject like this will still be needed for already messed up reports _(or a db migration to fully remove existing `nil`s)_, i guess it makes sense to merge something like this as a interim bandaid to at least perevent reportsfrom getting stuck, yeah I am quite aware of that. For now, it's better to be an bandaid as of now before admin open issue about similar issue. > However, your commit message and comment are incorrect. My bad. At the time i was creating a tempfix, I haven't really read much on the existing issue, so here goes my first assumption (but the thing is, the post exist on my server, but the `id` being null here is a strange quirk). This is a strange one. I will alter the commit message probably the next morning.

Yonle changed title from ~~web(activity_pub/utils): deal with deleted activity on strip_report_status_data~~ to web(activity_pub/utils): temporarily deal with nil activity ID when dealing with report

2026-03-16 08:15:04 +00:00

Yonle changed title from ~~web(activity_pub/utils): temporarily deal with nil activity ID when dealing with report~~ to web(activity_pub/utils): temporarily deal with nil status ID when dealing with report

2026-03-16 08:16:06 +00:00

Yonle force-pushed fixreport from 814d7ddd25

ci/woodpecker/pr/test/1 Pipeline is pending approval

Details

ci/woodpecker/pr/test/2 Pipeline is pending approval

Details

to 2a8a7bf55d

ci/woodpecker/pr/test/1 Pipeline is pending approval

Details

ci/woodpecker/pr/test/2 Pipeline is pending approval

Details

2026-03-16 08:19:38 +00:00

Compare

Oneric reviewed

2026-03-16 13:41:15 +00:00

lib/pleroma/web/activity_pub/utils.ex Outdated

					
				@ -801,2 +801,4 @@

				      |> Enum.map(fn

				        act when is_map(act) -> act["id"]

				        act when is_binary(act) -> act

				        _ -> nil # the reported post has been deleted

the comment here is still misleading/wrong

(and the commit title ideally consistently uses / for identifying the section, not a mix of / and parentheses)

the comment here is still misleading/wrong (and the commit title ideally consistently uses `/` for identifying the section, not a mix of `/` and parentheses)

Yonle marked this conversation as resolved

Yonle force-pushed fixreport from 2a8a7bf55d

ci/woodpecker/pr/test/1 Pipeline is pending approval

Details

ci/woodpecker/pr/test/2 Pipeline is pending approval

Details

to ca647214a2

ci/woodpecker/pr/test/2 Pipeline failed

Details

ci/woodpecker/pr/test/1 Pipeline was successful

Details

2026-03-16 14:16:33 +00:00

Compare

Oneric commented

2026-03-16 15:32:26 +00:00

The linter isn’t happy:

/woodpecker/src/akkoma.dev/AkkomaGang/akkoma/lib/pleroma/web/activity_pub/utils.ex

         |
802 802  |        act when is_map(act) -> act["id"]
803 803  |        act when is_binary(act) -> act
804     -|        _ -> nil # Status ID is null sometimes.
    804 +|        # Status ID is null sometimes.
    805 +|        _ -> nil
805 806  |      end)
806 807  |      |> Enum.reject(&is_nil/1)
         |

Make sure to run mix format (but beware if you’re uing elixir <1.19 results may differ from CI!) and mix test || mix test --failed || mix test --failed locally

The linter isn’t happy: ``` /woodpecker/src/akkoma.dev/AkkomaGang/akkoma/lib/pleroma/web/activity_pub/utils.ex | 802 802 | act when is_map(act) -> act["id"] 803 803 | act when is_binary(act) -> act 804 -| _ -> nil # Status ID is null sometimes. 804 +| # Status ID is null sometimes. 805 +| _ -> nil 805 806 | end) 806 807 | |> Enum.reject(&is_nil/1) | ``` Make sure to run `mix format` *(but beware if you’re uing elixir <1.19 results may differ from CI!)* and `mix test || mix test --failed || mix test --failed` locally

Yonle added 1 commit

2026-03-16 18:21:10 +00:00

f o r m a t

ci/woodpecker/pr/test/2 Pipeline was successful

Details

ci/woodpecker/pr/test/1 Pipeline was successful

Details

a0c59c96e6

Oneric approved these changes

2026-03-16 19:30:06 +00:00

Oneric commented

2026-03-16 19:30:13 +00:00