Implemented [Bugfix] Banner files deletion on user deletion

Daegaladh

Member
I've noticed that when you delete a user, some banner files still remain, that's because the if-elseif structure, since the user can upload a new banner with a different extension, those files are never deleted. Also premium banners are never deleted either.

That's why I propose this fix:

in sources/admin/delete.php
Replace:
PHP:
    if(file_exists("banners/{$username}.jpg"))
    {
        unlink("banners/{$username}.jpg");
    }
    else if(file_exists("banners/{$username}.jpeg"))
    {
        unlink("banners/{$username}.jpeg");
    }
    else if(file_exists("banners/{$username}.png"))
    {
        unlink("banners/{$username}.png");
    }
    else if(file_exists("banners/{$username}.gif"))
    {
        unlink("banners/{$username}.gif");
With:
PHP:
    foreach (glob("banners/{$username}.{jpg,jpeg,png,gif}", GLOB_BRACE) as $banner)
        unlink($banner);
    foreach (glob("banners/premium_{$username}.{jpg,jpeg,png,gif}", GLOB_BRACE) as $premium_banner)
        unlink($premium_banner);
 
Last edited:

Mark

Administrator
Staff member
moved to core feature requests :)

much better solution! this will be added to 1.6. Thanks VERY much for your contribution.
 

Basti

Administrator
Staff member
That stuff now changed in 1.6, though not in the way you suggested, ty for the suggestion though :) Was definitely better compared to the old.
With the addition of gif to mp4 convert, we take whatever user has set in the database.
One can have a different banner path now via plugins so hardcoded value isn't good anymore.
Banner/mp4 paths now also include a timestamp to avoid cached images when a user reuploads a fresh image, so again checking {$username} will not work anymore, hence we take the database value


Info why we decided against glob for anyone interested
- As a sidenode, foreach isn't needed, as we only deal with a single file
- On a low image count in the folder, glob seems slightly faster, but as image count increases into 1k+ file_exists seems to win by a huge margin. So we rather take the quicker method when many files are involved
 
Last edited:
Top