🐛 Fix: use correct deletion function for inactive users

[ygerlach]

I have seen the code deleting inactive users in the AccountDeletionController. It misses the parts of deleting a user as seen in UserController::deleteUserAccount (ProfilePicture and DatabaseNotification).

Im not a laravel expert, but i believe this might be a bug keeping ProfilePictures and Notifications for deleted users. If this problem is real, we may need to create a migration that checks the Database for non deleted profile pictures and notification objects, as this might be a privacy concern.

[MrKrisKrisu]

but i believe this might be a bug keeping ProfilePictures and Notifications for deleted users. If this problem is real, we may need to create a migration that checks the Database for non deleted profile pictures and notification objects, as this might be a privacy concern.

Notifications are automatically deleted after a certain amount of time. As for profile pictures, we have a script that periodically deletes any that are not associated with an account. So this should not be an acute problem.

traewelling/app/Console/Commands/CleanUpProfilePictures.php

Lines 15 to 25 in 3b6d547

	public function handle(): void {
	$usedPictures = User::select('avatar')->distinct()->get()->pluck('avatar')->filter()->toArray();
	$profilePictures = new \FilesystemIterator(public_path("uploads/avatars"));
	foreach ($profilePictures as $profilePicture) {
	if (!in_array(basename($profilePicture), $usedPictures)) {
	File::delete($profilePicture);
	$this->info('Deleted profile picture ' . basename($profilePicture));
	}
	}
	}

[ygerlach]

ok, so the data should be gone anyways. But i guess deleting it directly wouldn't hurt?

[MrKrisKrisu]

ok, so the data should be gone anyways. But i guess deleting it directly wouldn't hurt?

Nope, should be done like this. Can you please have a look at the tests, as currently the code is broken and I cannot merge it.

Also - did you test it?

[ygerlach]

Also - did you test it?

No. As i didn't manage to get a working dev setup yet.

I was just posting this MR as a discussion question, i didn't expect such a fast response.

[MrKrisKrisu]

Also - did you test it?

No.

Works on my machine - thanks. :)