-
Notifications
You must be signed in to change notification settings - Fork 83
Handle quote comment deletion with Reject activity #2460
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?
Changes from all commits
3afe849
a6b77d4
4b3b44d
f3a587c
2a8353b
960dfe1
f3e6cf9
5553a2a
d01494f
22be295
c011031
3893740
ad6d41a
007d4db
33839fe
3ff06ac
a1ebc93
79dcadd
f96fe1c
2eb2486
cfa6544
5d4187f
2e58dbe
9b20c10
7d4d27a
818ada9
34c58ed
adc9bc1
b646fcb
d4b193c
e8b42e8
62be0dc
b9a94b1
9213e0c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -83,6 +83,17 @@ public static function add( $activity, $recipients ) { | |
| $title = self::get_object_title( $activity->get_object() ); | ||
| $visibility = is_activity_public( $activity ) ? ACTIVITYPUB_CONTENT_VISIBILITY_PUBLIC : ACTIVITYPUB_CONTENT_VISIBILITY_PRIVATE; | ||
|
|
||
| /* | ||
| * For activities with an 'instrument' property (e.g., QuoteRequest), we store | ||
| * the instrument URL as the object_id. This allows efficient querying by instrument. | ||
| * For all other activities, we store the object URL as before. | ||
| */ | ||
| if ( $activity->has( 'instrument' ) && $activity->get_instrument() ) { | ||
| $object_id = object_to_uri( $activity->get_instrument() ); | ||
| } else { | ||
| $object_id = object_to_uri( $activity->get_object() ); | ||
| } | ||
|
Comment on lines
+91
to
+95
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this a situation where
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe... I was not sure if we can simply always prioritize
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like at least Friendica has a different interpretation of what |
||
|
|
||
| $inbox_item = array( | ||
| 'post_type' => self::POST_TYPE, | ||
| 'post_title' => sprintf( | ||
|
|
@@ -96,7 +107,7 @@ public static function add( $activity, $recipients ) { | |
| 'post_status' => 'publish', | ||
| 'guid' => $activity->get_id(), | ||
| 'meta_input' => array( | ||
| '_activitypub_object_id' => object_to_uri( $activity->get_object() ), | ||
| '_activitypub_object_id' => $object_id, | ||
| '_activitypub_activity_type' => $activity->get_type(), | ||
| '_activitypub_activity_remote_actor' => object_to_uri( $activity->get_actor() ), | ||
| 'activitypub_content_visibility' => $visibility, | ||
|
|
@@ -369,6 +380,50 @@ public static function get_by_guid_and_recipient( $guid, $user_id ) { | |
| return $post; | ||
| } | ||
|
|
||
| /** | ||
| * Get an inbox item by activity type and object ID. | ||
| * | ||
| * This is useful for finding specific activity types (like QuoteRequest) | ||
| * by their object identifier. For QuoteRequest activities, the object_id | ||
| * is the instrument URL (the quote post). | ||
| * | ||
| * @param string $activity_type The activity type (e.g., 'QuoteRequest'). | ||
| * @param string $object_id The object identifier to search for. | ||
| * | ||
| * @return \WP_Post|\WP_Error The inbox item or WP_Error if not found. | ||
|
Comment on lines
+383
to
+393
|
||
| */ | ||
| public static function get_by_type_and_object( $activity_type, $object_id ) { | ||
| global $wpdb; | ||
|
|
||
| // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching | ||
| $post_id = $wpdb->get_var( | ||
| $wpdb->prepare( | ||
| "SELECT p.ID | ||
| FROM {$wpdb->posts} p | ||
| INNER JOIN {$wpdb->postmeta} pm1 ON p.ID = pm1.post_id AND pm1.meta_key = '_activitypub_activity_type' | ||
| INNER JOIN {$wpdb->postmeta} pm2 ON p.ID = pm2.post_id AND pm2.meta_key = '_activitypub_object_id' | ||
| WHERE p.post_type = %s | ||
| AND pm1.meta_value = %s | ||
| AND pm2.meta_value = %s | ||
| ORDER BY p.ID DESC | ||
| LIMIT 1", | ||
| self::POST_TYPE, | ||
| $activity_type, | ||
| $object_id | ||
| ) | ||
| ); | ||
|
Comment on lines
+399
to
+414
|
||
|
|
||
| if ( ! $post_id ) { | ||
| return new \WP_Error( | ||
| 'activitypub_inbox_item_not_found', | ||
| \__( 'Inbox item not found', 'activitypub' ), | ||
| array( 'status' => 404 ) | ||
| ); | ||
| } | ||
|
|
||
| return \get_post( $post_id ); | ||
| } | ||
|
|
||
| /** | ||
| * Deduplicate inbox items with the same GUID. | ||
| * | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -10,6 +10,7 @@ | |||||||||||||||||||||
| use Activitypub\Activity\Activity; | ||||||||||||||||||||||
| use Activitypub\Collection\Actors; | ||||||||||||||||||||||
| use Activitypub\Collection\Followers; | ||||||||||||||||||||||
| use Activitypub\Collection\Inbox; | ||||||||||||||||||||||
| use Activitypub\Collection\Remote_Actors; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| use function Activitypub\add_to_outbox; | ||||||||||||||||||||||
|
|
@@ -27,6 +28,7 @@ class Quote_Request { | |||||||||||||||||||||
| public static function init() { | ||||||||||||||||||||||
| \add_action( 'activitypub_inbox_quote_request', array( self::class, 'handle_quote_request' ), 10, 2 ); | ||||||||||||||||||||||
| \add_action( 'activitypub_rest_inbox_disallowed', array( self::class, 'handle_blocked_request' ), 10, 3 ); | ||||||||||||||||||||||
| \add_action( 'delete_comment', array( self::class, 'handle_quote_delete' ), 10, 2 ); | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| \add_filter( 'activitypub_validate_object', array( self::class, 'validate_object' ), 10, 3 ); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
@@ -100,6 +102,85 @@ public static function handle_blocked_request( $activity, $user_ids, $type ) { | |||||||||||||||||||||
| self::queue_reject( $activity, $user_id ); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| /** | ||||||||||||||||||||||
| * Handle deletion of a quote comment. | ||||||||||||||||||||||
| * | ||||||||||||||||||||||
| * When a local quote comment is deleted, send a Reject activity to revoke | ||||||||||||||||||||||
| * the previously accepted QuoteRequest. | ||||||||||||||||||||||
| * | ||||||||||||||||||||||
| * @param int $comment_id The comment ID being deleted. | ||||||||||||||||||||||
| * @param \WP_Comment $comment The comment object. | ||||||||||||||||||||||
| */ | ||||||||||||||||||||||
| public static function handle_quote_delete( $comment_id, $comment ) { | ||||||||||||||||||||||
| // Only handle quote comments. | ||||||||||||||||||||||
| if ( 'quote' !== $comment->comment_type ) { | ||||||||||||||||||||||
| return; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| // Get the post being quoted. | ||||||||||||||||||||||
| $post_id = $comment->comment_post_ID; | ||||||||||||||||||||||
| if ( ! $post_id ) { | ||||||||||||||||||||||
| return; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| // Get the instrument URL (the quote post URL) from comment meta. | ||||||||||||||||||||||
| $instrument_url = \get_comment_meta( $comment_id, 'source_url', true ); | ||||||||||||||||||||||
| if ( ! $instrument_url ) { | ||||||||||||||||||||||
| $instrument_url = \get_comment_meta( $comment_id, 'source_id', true ); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| if ( ! $instrument_url ) { | ||||||||||||||||||||||
| return; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| // Get the post author (who accepted the quote). | ||||||||||||||||||||||
| $post = \get_post( $post_id ); | ||||||||||||||||||||||
| if ( ! $post || ! $post->post_author ) { | ||||||||||||||||||||||
| return; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
Comment on lines
+138
to
+140
|
||||||||||||||||||||||
| if ( ! $post || ! $post->post_author ) { | |
| return; | |
| } | |
| if ( ! $post || ! isset( $post->post_author ) ) { | |
| return; | |
| } | |
| $actor = Actors::get_by_id( $post->post_author ); | |
| if ( ! $actor ) { | |
| return; | |
| } |
Copilot
AI
Nov 13, 2025
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.
The assignment $activity_object = null; is unnecessary since it's immediately used in a condition at line 150 that checks for null/empty. This line can be removed as $inbox_item result will be checked, and $activity_object is only assigned if the condition passes. If the condition fails, the fallback block at line 155 assigns a value to $activity_object, so the initialization to null serves no purpose.
| $activity_object = null; |
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.
| if ( ! \is_wp_error( $inbox_item ) && $inbox_item instanceof \WP_Post ) { | |
| if ( $inbox_item instanceof \WP_Post ) { |
Copilot
AI
Nov 13, 2025
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.
Missing error handling for JSON decode failure. If $inbox_item->post_content contains invalid JSON, json_decode() will return null, causing the fallback path to be used silently. Consider checking for JSON decode errors explicitly:
$activity_object = \json_decode( $inbox_item->post_content, true );
if ( JSON_ERROR_NONE !== \json_last_error() ) {
$activity_object = null;
}This makes it clear when the fallback is used due to corrupted data versus a missing inbox item.
| $activity_object = \json_decode( $inbox_item->post_content, true ); | |
| $activity_object = \json_decode( $inbox_item->post_content, true ); | |
| if ( JSON_ERROR_NONE !== \json_last_error() ) { | |
| $activity_object = null; | |
| } |
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.
Needs action docs. Do we even need this action?
Copilot
AI
Nov 13, 2025
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.
Consider including 'published' field in the fallback reconstruction for consistency with the test expectations. The test test_delete_uses_inbox_item creates a QuoteRequest with a 'published' field (line 646), and the comment at line 714 states "verify the reject object is a QuoteRequest with proper structure". While not required for the Reject to function, including all original fields in the fallback would maintain better parity with the retrieved inbox version and make the fallback more complete.
| 'instrument' => $instrument_url, | |
| 'instrument' => $instrument_url, | |
| 'published' => gmdate( 'c' ), |
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.
Consider documenting the side effect of this change on existing code that may query
_activitypub_object_idmeta. For QuoteRequest activities, the_activitypub_object_idnow stores the instrument URL instead of the object URL. This is a breaking change in the meta structure. While this is necessary for the new feature, adding a note in the comment about backwards compatibility or migration considerations would be helpful. For instance, any existing code that queries by object URL for QuoteRequest activities will no longer find them.