Skip to content
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

Delete certificates on user deletion #297

Open
jom opened this issue Apr 29, 2022 · 16 comments
Open

Delete certificates on user deletion #297

jom opened this issue Apr 29, 2022 · 16 comments

Comments

@jom
Copy link
Contributor

jom commented Apr 29, 2022

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

@jom jom added the [Type] Bug label Apr 29, 2022
@jinnypark
Copy link

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

@jom
Copy link
Contributor Author

jom commented May 3, 2022

@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
Copy link

muitsun commented May 8, 2022

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
Copy link

muitsun commented May 8, 2022

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

@m1r0
Copy link
Member

m1r0 commented May 25, 2022

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
Copy link

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 m1r0 added the [Status] Queued In the queue of issues to work on next. label Jun 16, 2022
@m1r0
Copy link
Member

m1r0 commented Jun 16, 2022

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
Copy link

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

@mui-tsun
Copy link

@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
Copy link
Member

m1r0 commented Jun 17, 2022

@mui-tsun It should clear all orphan certificates.

@mui-tsun
Copy link

@m1r0 Fantastic!

@mui-tsun
Copy link

mui-tsun commented Jul 1, 2022

@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
Copy link

mui-tsun commented Jul 1, 2022

@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
Copy link
Member

m1r0 commented Jul 4, 2022

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
Copy link

mui-tsun commented Jul 5, 2022

@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
Copy link
Member

m1r0 commented Jul 6, 2022

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!

@gikaragia gikaragia removed the [Status] Queued In the queue of issues to work on next. label Dec 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants