-
Notifications
You must be signed in to change notification settings - Fork 84
Comment: Send announce for all new comments #1562
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
…tomattic/wordpress-activitypub into add/boost-incoming-activities
Replaces incorrect 'Comment_Utils' with 'Comment_Util' in schedule_comment_delete_activity to ensure the correct utility class is used for checking if a comment was sent.
Replaces all instances of 'Comment_Util' with 'Comment_Utils' in class-comment.php to match the correct class name and prevent potential errors.
Adds a default value for the 'type' field in handle_undo to prevent undefined index errors. Updates object attribute validation to only check required fields if 'object' is an array, allowing string/URI objects. Adds a test case to ensure URI objects pass validation.
This reverts commit 11e098c.
Adds stricter validation for ActivityPub activity data before creating Announce activities when comments are approved. Updates tests to cover cases where Announce should not be created, including missing, malformed, or non-received activity data and incorrect actor modes.
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 implements automatic Announce activity generation when comments are received from other ActivityPub instances and approved. The system now stores the original activity data in comment metadata and uses it to create an Announce when the comment transitions to approved status, ensuring other instances are notified about the interaction.
Key Changes
- Stores incoming ActivityPub activity data in comment metadata for later use
- Hooks into the comment approval workflow to generate Announce activities for received comments
- Adds retry behavior for HTTP 400 errors in the dispatcher
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| includes/collection/class-interactions.php | Stores the incoming activity data in comment metadata |
| includes/scheduler/class-comment.php | Implements the announce interaction logic and integrates it into the comment status transition workflow |
| includes/class-dispatcher.php | Adds HTTP 400 to the list of retryable error codes |
| tests/phpunit/tests/includes/scheduler/class-test-comment.php | Adds comprehensive test coverage for the announce interaction feature |
| .github/changelog/1562-from-description | Documents the feature addition in the changelog |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Eliminated unnecessary set_to and set_cc calls for the announce activity in Comment scheduler. The recipients are now handled by add_to_outbox, simplifying the code.
Replaces retrieval of the blog actor object with direct use of Actors::BLOG_USER_ID when setting the actor for Announce activities. This streamlines the code and removes unnecessary error handling for actor retrieval.
| } | ||
|
|
||
| // Get activity from comment meta. | ||
| $activity = \get_comment_meta( $comment->comment_ID, '_activitypub_activity', true ); |
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.
Could we get this from the Inbox, instead of saving it in comment meta?
Alternatively, could we use the source_id meta as the object instead of offering the full object?
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.
We can postpone this a bit until we have the inbox activated by default. PR incoming.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
Some thoughts:
As you know, I don’t think a blog profile is suitable as a group actor, since it isn’t a forum-type entity. After all, Lemmy’s |
|
By the way, has Lemmy’s WebFinger ambiguity issue been resolved?
Wouldn’t a conflict occur in WordPress if the blog profile handle and the user profile handle are identical? |
Supersedes #1515 and fixes #1001
Proposed changes:
approveprocess, to late federate theAnnounceOther information:
Testing instructions:
Changelog entry
Changelog Entry Details
Significance
Type
Message
Incoming interactions create an Announce activity so other instances get notified about it.