woocommerce / sensei-certificates

Hi, I'm the Certificates extension for Sensei.
GNU General Public License v2.0
34 stars 24 forks source link

Delete certificates on user deletion #297

Open jom opened 2 years ago

jom commented 2 years ago

Steps to Reproduce

  1. As a student, complete a course in order to generate a certificate.
  2. As an admin, delete that student.

What I Expected

The deleted user's certificates to be removed.

BONUS: Provide a tool to clean up old certificates.

What Happened Instead

The certificates were left behind.

PHP / WordPress / Sensei Certificates / Sensei LMS version

PHP 7.4 / WP 5.9.3 / Sensei LMS Certificates 2.2.1 / Sensei LMS 4.3.0

Browser / OS version

n/a

Screenshot / Video

Screen Shot 2022-04-29 at 5 22 27 PM

Context / Source

5168737-zd-woothemes

jinnypark commented 2 years ago

@jom - the user in 5168737-zd-woothemes noted that they did not delete the student.

jom commented 2 years ago

@jinnypark Somehow the student's user account was deleted (not by Sensei). The WordPress user record has been deleted. What was left behind was Sensei's record that references the now deleted WordPress user.

muitsun commented 2 years ago

Hi all. I can confirm that it's a setting in WooCommerce that's deleting user accounts after a specified period of inactivity!! I have now changed this to not delete inactive users. So, the only bug is that a deleted user's certificates should be removed.

muitsun commented 2 years ago

In the meantime, is there a way to clean up the orphan certificate entries?

m1r0 commented 2 years ago

Hey @muitsun, here's a quick script that might help:

add_action( 'deleted_user', 'my_cleanup_orphaned_sensei_certificates' );
function my_cleanup_orphaned_sensei_certificates() {
    $where_author_does_not_exist = function( $clauses ) {
        global $wpdb;

        $clauses['join']  .= " LEFT JOIN {$wpdb->users} ON {$wpdb->posts}.post_author = {$wpdb->users}.ID";
        $clauses['where'] .= " AND {$wpdb->users}.ID IS NULL";

        return $clauses;
    };

    add_filter( 'posts_clauses', $where_author_does_not_exist );

    $orphaned_certificates = new WP_Query(
        [
            'posts_per_page' => -1,
            'post_type'      => 'certificate',
            'post_status'    => 'any',
            'fields'         => 'ids',
        ]
    );

    remove_filter( 'posts_clauses', $where_author_does_not_exist );

    foreach ( $orphaned_certificates->posts as $certificate_id ) {
        wp_delete_post( $certificate_id );
    }
}

The script will clean up all the orphaned certificates each time a user gets deleted.

❗ Please keep in mind this is a destructive action, so please make a backup before trying it.

mui-tsun commented 2 years ago

Sorry for the delay in replying. Would this script be incorporated into a future update? If so, I'd rather wait for the update. If not, I'll add this myself for now.

m1r0 commented 2 years ago

Hey @mui-tsun, no problem.

Would this script be incorporated into a future update?

I've added it to the queue, but it's hard to say when it will go in. Right now, the focus is on the main plugin, but hopefully, we will give some love to certificates soon. cc @burtrw

mui-tsun commented 2 years ago

@m1r0 Totally understand. I'll add the script myself for now. Thanks for your help.

mui-tsun commented 2 years ago

@m1r0 Forgot to ask... would the script also clear up orphan certificates that have been left behind? Or would it only remove certificates for newly deleted users? Thanks.

m1r0 commented 2 years ago

@mui-tsun It should clear all orphan certificates.

mui-tsun commented 2 years ago

@m1r0 Fantastic!

mui-tsun commented 2 years ago

@m1r0 Hi there again. I've just got round to installing the script. I created a test user, then deleted the same user. However, all the orphan certificates are still there. Any idea? Does the test user have to have a certificate first?

mui-tsun commented 2 years ago

@m1r0 Just to confirm... I added a test user to a course and completed the course for this user. At this point the user has a certificate. I then deleted this user and his certificate is removed, as expected. But there are still 4 orphan certificates left behind from somewhere. Since there are only 4, I could easily delete them manually but I thought I'd let you know in case it's something you would like to investigate.

m1r0 commented 2 years ago

Thanks for the feedback @mui-tsun! Did the script delete any certificates for the other users at all? Do you happen to remember the post status (published, draft, trashed, etc.) of the 4 reminding certificate posts?

mui-tsun commented 2 years ago

@m1r0 Yes, many orphan certificates were deleted. Of the remaining 4, the post status is "Published". Looks just like all the other valid certificates. Thanks.

m1r0 commented 2 years ago

Yes, many orphan certificates were deleted.

Good to hear!

Of the remaining 4, the post status is "Published". Looks just like all the other valid certificates.

I wasn't able to reproduce the issue locally, but this info might be useful if we add this feature to the plugin, so thanks again!