-
Notifications
You must be signed in to change notification settings - Fork 82
Refine handling of local content visibility for posts #2236
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: trunk
Are you sure you want to change the base?
Conversation
Improves logic for 'activitypub_content_visibility' set to 'local', ensuring posts are only disabled if not already federated. Updates transformer logic to set correct recipients for local posts, prevents in-reply-to for local visibility, and adds/updates related unit tests to cover these scenarios.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR improves the handling of posts switched to 'local' visibility by sending Update activities instead of Delete activities when a previously federated post becomes local. This allows posts to be reactivated by changing visibility again rather than requiring deletion and recreation.
- Modified logic to send Update activities for previously federated posts set to local visibility
- Updated transformer to set correct audience for local posts (author as both
to
andcc
) - Prevented
in_reply_to
field population for local visibility posts
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
includes/functions.php | Enhanced is_post_disabled() logic to handle local visibility posts based on federation status |
includes/transformer/class-base.php | Added audience handling for local visibility posts |
includes/transformer/class-post.php | Prevented in_reply_to population for local posts |
tests/includes/transformer/class-test-post.php | Updated test to verify local posts have author in to and cc fields |
tests/includes/collection/class-test-replies.php | Added status metadata cleanup in test |
tests/includes/class-test-query.php | Enhanced test coverage for local visibility scenarios |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Konstantin Obenland <[email protected]>
Corrected the indentation of a comment and assignment in the is_post_disabled function for better code readability.
Updates the Base transformer to clear 'to' and 'cc' fields for local content visibility instead of adding the actor's ID. Removes early return in Post transformer for local content visibility in get_in_reply_to().
For activities with local visibility, set the actor's ID as both 'to' and 'cc' recipients instead of empty arrays. This ensures proper addressing for local-only content.
After some feedback from Mastodon contributors: This is actually not possible. We would have to send a The risk here: A |
@obenland the question is if we want to send a delete!? |
Maybe we could add a notice into the visibility selector in the editor, warning that it's a permanent change? Other than that, it feels like a specific-enough action to me, to assume that it doesn't happen accidentally. I'd be fine with sending the delete even without a warning. |
In the case of Misskey, there is an option for delete-and-edit. For WordPress federation, this could be implemented by actually deleting the original post and publishing a duplicate as a new post with a new ID. Of course, any existing comments or interactions would be lost. |
By the way, couldn’t this approach also be applied to actors? I have a hunch that threads.net might be using a similar method to implement federation opt-out. |
This PR aims to send an
Update
activity when a previously federated post is later set to local, reflecting changes to the audience (to
,cc
fields) andin_reply_to
.The benefit of this change (if it works as intended) over sending a
Delete
is that the post can be reactivated by simply changing its visibility.Improves logic for 'activitypub_content_visibility' set to 'local', ensuring posts are only disabled if not already federated. Updates transformer logic to set correct recipients for local posts, prevents in-reply-to for local visibility, and adds/updates related unit tests to cover these scenarios.
For reference https://wordpress.org/support/topic/how-to-delete-federated-posts/
Proposed changes:
local
afterwardsin_reply_to
(not sure if that is needed)to
andcc
(not if that is needed or if empty arrays would also do the job)Other information:
Testing instructions:
Publish a
public
postChange visibility to
local
Check Outbox if
Update
is properly createdWrite new post and set to
local
before publishingCheck that no
Update
/Create
Activities were createdChangelog entry
Changelog Entry Details
Significance
Type
Message
Improved handling of posts switched to 'local' visibility so they update correctly and can be re-enabled by changing visibility again.