Make WordPress Core

Opened 6 years ago

Last modified 4 years ago

#44179 new enhancement

Use wp_update_comment instead of $wpdb->update $wpdb->comments when anonymizing comments

Reported by: allendav's profile allendav Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 4.9.6
Component: Privacy Keywords: has-patch needs-testing
Focuses: Cc:

Description

During the course of the development on #43442 we switched from wp_update_comment to a direct $wpdb->update( $wpdb->comments, $anonymized_comment, $args );

Unfortunately, plugins that do things with comments when they change are more likely to hook wp_update_comment - so plugins like that are unaware when comments are anonymized

Let's switch back to wp_update_comment (as it was in earlier diffs on that ticket)

cc @azaozz

Attachments (2)

44179.diff (1.3 KB) - added by allendav 6 years ago.
44179.2.diff (785 bytes) - added by nielsdeblaauw 5 years ago.

Download all attachments as: .zip

Change History (19)

This ticket was mentioned in Slack in #gdpr-compliance by allendav. View the logs.


6 years ago

@allendav
6 years ago

#2 @allendav
6 years ago

To test:

  • Apply the patch
  • Generate lots of comments (e.g. 500) for some email address (bonus: I put a plugin here to make that easier: https://github.com/allendav/commentariat )
  • Use wp-admin > Tools > Erase Personal Data to anonymize that email address's comments

Note: During my testing I saw the old method take < 1 second to directly update the table for 500 comments and < 2 seconds to use this hookable method. Yes, it is slower but not horribly so and it allows plugins that hook comment updates (like Jetpack) to stay involved and aware of the now anonymized comment.

#3 @allendav
6 years ago

  • Keywords has-patch needs-testing added; needs-patch removed

#4 @desrosj
6 years ago

  • Version changed from trunk to 4.9.6

This looks good to me, @allendav! The global $wpdb declaration can be also removed at the top of the function. I did not get to look into the unit tests that we have for this function, but there may be some additional assertions we could add.

#5 @desrosj
6 years ago

  • Milestone changed from Awaiting Review to 4.9.7

This ticket was mentioned in Slack in #core-privacy by desrosj. View the logs.


6 years ago

#7 @azaozz
6 years ago

Thinking that this has the potential to slow things considerably, especially when plugins perform some (remote) actions on updating a comment.

How about we add another hook, perhaps do_action( 'comment_anonymized', $comment_id, $anonymized_comment ) (props @desrosj for the name)? This way plugins will be able to hook into this "special case" of updating a comment.

#8 @ocean90
6 years ago

  • Milestone changed from 4.9.7 to 4.9.8

4.9.7 has been released, moving to next milestone.

This ticket was mentioned in Slack in #core by jon_bossenger. View the logs.


6 years ago

#10 @desrosj
6 years ago

  • Keywords gdpr removed

Removing the GDPR keyword. This has been replaced by the new Privacy component and privacy focuses in Trac.

This ticket was mentioned in Slack in #core by joshuawold. View the logs.


6 years ago

#12 @desrosj
6 years ago

  • Keywords changed from has-patch, needs-testing to has-patch needs-testing
  • Milestone changed from 4.9.8 to 4.9.9

4.9.8 RC is out. Punting to 4.9.9 as this still needs work.

#13 @pento
6 years ago

  • Milestone changed from 4.9.9 to Future Release

This ticket was mentioned in Slack in #core-privacy by webdevlaw. View the logs.


6 years ago

#15 @garrett-eclipse
6 years ago

  • Keywords needs-refresh added

#16 @nielsdeblaauw
5 years ago

  • Focuses privacy added
  • Keywords needs-refresh removed

Refreshed with a do_action that allows plugins to react to anonymized comments.

#17 @garrett-eclipse
4 years ago

  • Focuses privacy removed

Dropping privacy focus as it's already in the Privacy component.

Note: See TracTickets for help on using tickets.